From ea4b52a86f6b6b8e5aef8e47fb557f37422512bf Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 7 Apr 2007 05:42:01 -0700 Subject: t1000: fix case table. Case #10 is not handled with unpack-trees.c:threeway_merge() internally, unless under the agressive rule, and it is not a bug. As the test expects, ND (one side did not do anything, other side deleted) case was meant to be handled by the caller's policy (e.g. git-merge-one-file or git-merge-recursive). Signed-off-by: Junio C Hamano diff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh index e26a36c..de4e5eb 100755 --- a/t/t1000-read-tree-m-3way.sh +++ b/t/t1000-read-tree-m-3way.sh @@ -184,7 +184,7 @@ checked. 9 exists O!=A missing no merge must match A and be up-to-date, if exists. ------------------------------------------------------------------ - 10 exists O==A missing remove ditto + 10 exists O==A missing no merge must match A ------------------------------------------------------------------ 11 exists O!=A O!=B no merge must match A and be A!=B up-to-date, if exists. diff --git a/unpack-trees.c b/unpack-trees.c index a0b6769..2a58b7f 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -786,7 +786,7 @@ int threeway_merge(struct cache_entry **stages, o->nontrivial_merge = 1; - /* #2, #3, #4, #6, #7, #9, #11. */ + /* #2, #3, #4, #6, #7, #9, #10, #11. */ count = 0; if (!head_match || !remote_match) { for (i = 1; i < o->head_idx; i++) { -- cgit v0.10.2-6-g49f6 From 4c4caafc9cef1031beb46babe9adfdcc03f3cd52 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 7 Apr 2007 05:49:19 -0700 Subject: Treat D/F conflict entry more carefully in unpack-trees.c::threeway_merge() This fixes three buglets in threeway_merge() regarding D/F conflict entries. * After finishing with path D and handling path D/F, some stages have D/F conflict entry which are obviously non-NULL. For the purpose of determining if the path D/F is missing in the ancestor, they should not be taken into account. * D/F conflict entry is a marker to say "this stage does _not_ have the path", so do not send them to keep_entry(). Signed-off-by: Junio C Hamano diff --git a/unpack-trees.c b/unpack-trees.c index 2a58b7f..5139481 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -665,7 +665,6 @@ int threeway_merge(struct cache_entry **stages, int count; int head_match = 0; int remote_match = 0; - const char *path = NULL; int df_conflict_head = 0; int df_conflict_remote = 0; @@ -675,13 +674,10 @@ int threeway_merge(struct cache_entry **stages, int i; for (i = 1; i < o->head_idx; i++) { - if (!stages[i]) + if (!stages[i] || stages[i] == o->df_conflict_entry) any_anc_missing = 1; - else { - if (!path) - path = stages[i]->name; + else no_anc_exists = 0; - } } index = stages[0]; @@ -697,13 +693,6 @@ int threeway_merge(struct cache_entry **stages, remote = NULL; } - if (!path && index) - path = index->name; - if (!path && head) - path = head->name; - if (!path && remote) - path = remote->name; - /* First, if there's a #16 situation, note that to prevent #13 * and #14. */ @@ -755,6 +744,23 @@ int threeway_merge(struct cache_entry **stages, if (o->aggressive) { int head_deleted = !head && !df_conflict_head; int remote_deleted = !remote && !df_conflict_remote; + const char *path = NULL; + + if (index) + path = index->name; + else if (head) + path = head->name; + else if (remote) + path = remote->name; + else { + for (i = 1; i < o->head_idx; i++) { + if (stages[i] && stages[i] != o->df_conflict_entry) { + path = stages[i]->name; + break; + } + } + } + /* * Deleted in both. * Deleted in one and unchanged in the other. @@ -790,7 +796,7 @@ int threeway_merge(struct cache_entry **stages, count = 0; if (!head_match || !remote_match) { for (i = 1; i < o->head_idx; i++) { - if (stages[i]) { + if (stages[i] && stages[i] != o->df_conflict_entry) { keep_entry(stages[i], o); count++; break; -- cgit v0.10.2-6-g49f6 From ac7f0f436e4f45d616ca509f5163fddab104516b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 7 Apr 2007 05:52:57 -0700 Subject: merge-recursive: do not barf on "to be removed" entries. When update-trees::threeway_merge() decides that a path that exists in the current index (and HEAD) is to be removed, it leaves a stage 0 entry whose mode bits are set to 0. The code mistook it as "this stage wants the blob here", and proceeded to call update_file_flags() which ended up trying to put the mode=0 entry in the index, got very confused, and ended up barfing with "do not know what to do with 000000". Since threeway_merge() does not handle case #10 (one side removes while the other side does not do anything), this is not a problem while we refuse to merge branches that have D/F conflicts, but when we start resolving them, we would need to be able to remove cache entries, and at that point it starts to matter. Signed-off-by: Junio C Hamano diff --git a/merge-recursive.c b/merge-recursive.c index 3096594..0e25956 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1018,9 +1018,9 @@ static int process_renames(struct path_list *a_renames, return clean_merge; } -static unsigned char *has_sha(const unsigned char *sha) +static unsigned char *stage_sha(const unsigned char *sha, unsigned mode) { - return is_null_sha1(sha) ? NULL: (unsigned char *)sha; + return (is_null_sha1(sha) || mode == 0) ? NULL: (unsigned char *)sha; } /* Per entry merge function */ @@ -1033,12 +1033,12 @@ static int process_entry(const char *path, struct stage_data *entry, print_index_entry("\tpath: ", entry); */ int clean_merge = 1; - unsigned char *o_sha = has_sha(entry->stages[1].sha); - unsigned char *a_sha = has_sha(entry->stages[2].sha); - unsigned char *b_sha = has_sha(entry->stages[3].sha); unsigned o_mode = entry->stages[1].mode; unsigned a_mode = entry->stages[2].mode; unsigned b_mode = entry->stages[3].mode; + unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode); + unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode); + unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode); if (o_sha && (!a_sha || !b_sha)) { /* Case A: Deleted in one */ @@ -1139,6 +1139,12 @@ static int process_entry(const char *path, struct stage_data *entry, update_file_flags(mfi.sha, mfi.mode, path, 0 /* update_cache */, 1 /* update_working_directory */); } + } else if (!o_sha && !a_sha && !b_sha) { + /* + * this entry was deleted altogether. a_mode == 0 means + * we had that path and want to actively remove it. + */ + remove_file(1, path, !a_mode); } else die("Fatal merge failure, shouldn't happen."); -- cgit v0.10.2-6-g49f6 From 4d50895a390658bf1b9a9385dbc0b9f90b48c803 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 7 Apr 2007 06:41:13 -0700 Subject: merge-recursive: handle D/F conflict case more carefully. When a path D that originally was blob in the ancestor was modified on our branch while it was removed on the other branch, we keep stages 1 and 2, and leave our version in the working tree. If the other branch created a path D/F, however, that path can cleanly be resolved in the index (after all, the ancestor nor we do not have it and only the other side added), but it cannot be checked out. The issue is the same when the other branch had D and we had renamed it to D/F, or the ancestor had D/F instead of D (so there are four combinations). Do not stop the merge, but leave both D and D/F paths in the index so that the user can clear things up. Signed-off-by: Junio C Hamano diff --git a/merge-recursive.c b/merge-recursive.c index 0e25956..595b022 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -596,9 +596,31 @@ static void update_file_flags(const unsigned char *sha, if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) { int fd; - if (mkdir_p(path, 0777)) - die("failed to create path %s: %s", path, strerror(errno)); - unlink(path); + int status; + const char *msg = "failed to create path '%s'%s"; + + status = mkdir_p(path, 0777); + if (status) { + if (status == -3) { + /* something else exists */ + error(msg, path, ": perhaps a D/F conflict?"); + update_wd = 0; + goto update_index; + } + die(msg, path, ""); + } + if (unlink(path)) { + if (errno == EISDIR) { + /* something else exists */ + error(msg, path, ": perhaps a D/F conflict?"); + update_wd = 0; + goto update_index; + } + if (errno != ENOENT) + die("failed to unlink %s " + "in preparation to update: %s", + path, strerror(errno)); + } if (mode & 0100) mode = 0777; else @@ -620,6 +642,7 @@ static void update_file_flags(const unsigned char *sha, die("do not know what to do with %06o %s '%s'", mode, sha1_to_hex(sha), path); } + update_index: if (update_cache) add_cacheinfo(mode, sha, path, 0, update_wd, ADD_CACHE_OK_TO_ADD); } -- cgit v0.10.2-6-g49f6 From 885b98107547fe3f6d17ca0af0578e040f7600d0 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 7 Apr 2007 07:17:35 -0700 Subject: t3030: merge-recursive backend test. We have fairly extensive coverage of read-tree 3-way machinery, and many Porcelain-ish tests use git-merge front-end tests, but we did not have good basic test for merge-recursive, which made it very hard to hack on it. I used this during the recent work to teach D/F conflicts to merge-recursive. Signed-off-by: Junio C Hamano diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh new file mode 100755 index 0000000..aef92b9 --- /dev/null +++ b/t/t3030-merge-recursive.sh @@ -0,0 +1,528 @@ +#!/bin/sh + +test_description='merge-recursive backend test' + +. ./test-lib.sh + +test_expect_success 'setup 1' ' + + echo hello >a && + o0=$(git hash-object a) && + cp a b && + cp a A && + mkdir d && + cp a d/e && + + test_tick && + git add a b A d/e && + git commit -m initial && + c0=$(git rev-parse --verify HEAD) && + git branch side && + git branch df-1 && + git branch df-2 && + git branch df-3 && + git branch remove && + + echo hello >>a && + cp a d/e && + o1=$(git hash-object a) && + + git add a d/e && + + test_tick && + git commit -m "master modifies a and d/e" && + c1=$(git rev-parse --verify HEAD) && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o1 a" + echo "100644 blob $o0 b" + echo "100644 blob $o1 d/e" + echo "100644 $o0 0 A" + echo "100644 $o1 0 a" + echo "100644 $o0 0 b" + echo "100644 $o1 0 d/e" + ) >expected && + git diff -u expected actual +' + +test_expect_success 'setup 2' ' + + rm -rf [Aabd] && + git checkout side && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o0 a" + echo "100644 blob $o0 b" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o0 0 a" + echo "100644 $o0 0 b" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual && + + echo goodbye >>a && + o2=$(git hash-object a) && + + git add a && + + test_tick && + git commit -m "side modifies a" && + c2=$(git rev-parse --verify HEAD) && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o2 a" + echo "100644 blob $o0 b" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o2 0 a" + echo "100644 $o0 0 b" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual +' + +test_expect_success 'setup 3' ' + + rm -rf [Aabd] && + git checkout df-1 && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o0 a" + echo "100644 blob $o0 b" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o0 0 a" + echo "100644 $o0 0 b" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual && + + rm -f b && mkdir b && echo df-1 >b/c && git add b/c && + o3=$(git hash-object b/c) && + + test_tick && + git commit -m "df-1 makes b/c" && + c3=$(git rev-parse --verify HEAD) && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o0 a" + echo "100644 blob $o3 b/c" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o0 0 a" + echo "100644 $o3 0 b/c" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual +' + +test_expect_success 'setup 4' ' + + rm -rf [Aabd] && + git checkout df-2 && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o0 a" + echo "100644 blob $o0 b" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o0 0 a" + echo "100644 $o0 0 b" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual && + + rm -f a && mkdir a && echo df-2 >a/c && git add a/c && + o4=$(git hash-object a/c) && + + test_tick && + git commit -m "df-2 makes a/c" && + c4=$(git rev-parse --verify HEAD) && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o4 a/c" + echo "100644 blob $o0 b" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o4 0 a/c" + echo "100644 $o0 0 b" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual +' + +test_expect_success 'setup 5' ' + + rm -rf [Aabd] && + git checkout remove && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o0 a" + echo "100644 blob $o0 b" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o0 0 a" + echo "100644 $o0 0 b" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual && + + rm -f b && + echo remove-conflict >a && + + git add a && + git rm b && + o5=$(git hash-object a) && + + test_tick && + git commit -m "remove removes b and modifies a" && + c5=$(git rev-parse --verify HEAD) && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o5 a" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o5 0 a" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual + +' + +test_expect_success 'setup 6' ' + + rm -rf [Aabd] && + git checkout df-3 && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o0 a" + echo "100644 blob $o0 b" + echo "100644 blob $o0 d/e" + echo "100644 $o0 0 A" + echo "100644 $o0 0 a" + echo "100644 $o0 0 b" + echo "100644 $o0 0 d/e" + ) >expected && + git diff -u expected actual && + + rm -fr d && echo df-3 >d && git add d && + o6=$(git hash-object d) && + + test_tick && + git commit -m "df-3 makes d" && + c6=$(git rev-parse --verify HEAD) && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "100644 blob $o0 A" + echo "100644 blob $o0 a" + echo "100644 blob $o0 b" + echo "100644 blob $o6 d" + echo "100644 $o0 0 A" + echo "100644 $o0 0 a" + echo "100644 $o0 0 b" + echo "100644 $o6 0 d" + ) >expected && + git diff -u expected actual +' + +test_expect_success 'merge-recursive simple' ' + + rm -fr [Aabd] && + git checkout -f "$c2" && + + git-merge-recursive "$c0" -- "$c2" "$c1" + status=$? + case "$status" in + 1) + : happy + ;; + *) + echo >&2 "why status $status!!!" + false + ;; + esac +' + +test_expect_success 'merge-recursive result' ' + + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o0 1 a" + echo "100644 $o2 2 a" + echo "100644 $o1 3 a" + echo "100644 $o0 0 b" + echo "100644 $o1 0 d/e" + ) >expected && + git diff -u expected actual + +' + +test_expect_success 'merge-recursive remove conflict' ' + + rm -fr [Aabd] && + git checkout -f "$c1" && + + git-merge-recursive "$c0" -- "$c1" "$c5" + status=$? + case "$status" in + 1) + : happy + ;; + *) + echo >&2 "why status $status!!!" + false + ;; + esac +' + +test_expect_success 'merge-recursive remove conflict' ' + + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o0 1 a" + echo "100644 $o1 2 a" + echo "100644 $o5 3 a" + echo "100644 $o1 0 d/e" + ) >expected && + git diff -u expected actual + +' + +test_expect_success 'merge-recursive d/f simple' ' + rm -fr [Aabd] && + git reset --hard && + git checkout -f "$c1" && + + git-merge-recursive "$c0" -- "$c1" "$c3" +' + +test_expect_success 'merge-recursive result' ' + + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o1 0 a" + echo "100644 $o3 0 b/c" + echo "100644 $o1 0 d/e" + ) >expected && + git diff -u expected actual + +' + +test_expect_success 'merge-recursive d/f conflict' ' + + rm -fr [Aabd] && + git reset --hard && + git checkout -f "$c1" && + + git-merge-recursive "$c0" -- "$c1" "$c4" + status=$? + case "$status" in + 1) + : happy + ;; + *) + echo >&2 "why status $status!!!" + false + ;; + esac +' + +test_expect_success 'merge-recursive d/f conflict result' ' + + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o0 1 a" + echo "100644 $o1 2 a" + echo "100644 $o4 0 a/c" + echo "100644 $o0 0 b" + echo "100644 $o1 0 d/e" + ) >expected && + git diff -u expected actual + +' + +test_expect_success 'merge-recursive d/f conflict the other way' ' + + rm -fr [Aabd] && + git reset --hard && + git checkout -f "$c4" && + + git-merge-recursive "$c0" -- "$c4" "$c1" + status=$? + case "$status" in + 1) + : happy + ;; + *) + echo >&2 "why status $status!!!" + false + ;; + esac +' + +test_expect_success 'merge-recursive d/f conflict result the other way' ' + + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o0 1 a" + echo "100644 $o1 3 a" + echo "100644 $o4 0 a/c" + echo "100644 $o0 0 b" + echo "100644 $o1 0 d/e" + ) >expected && + git diff -u expected actual + +' + +test_expect_success 'merge-recursive d/f conflict' ' + + rm -fr [Aabd] && + git reset --hard && + git checkout -f "$c1" && + + git-merge-recursive "$c0" -- "$c1" "$c6" + status=$? + case "$status" in + 1) + : happy + ;; + *) + echo >&2 "why status $status!!!" + false + ;; + esac +' + +test_expect_success 'merge-recursive d/f conflict result' ' + + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o1 0 a" + echo "100644 $o0 0 b" + echo "100644 $o6 3 d" + echo "100644 $o0 1 d/e" + echo "100644 $o1 2 d/e" + ) >expected && + git diff -u expected actual + +' + +test_expect_success 'merge-recursive d/f conflict' ' + + rm -fr [Aabd] && + git reset --hard && + git checkout -f "$c6" && + + git-merge-recursive "$c0" -- "$c6" "$c1" + status=$? + case "$status" in + 1) + : happy + ;; + *) + echo >&2 "why status $status!!!" + false + ;; + esac +' + +test_expect_success 'merge-recursive d/f conflict result' ' + + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o1 0 a" + echo "100644 $o0 0 b" + echo "100644 $o6 2 d" + echo "100644 $o0 1 d/e" + echo "100644 $o1 3 d/e" + ) >expected && + git diff -u expected actual + +' + +test_expect_success 'reset and 3-way merge' ' + + git reset --hard "$c2" && + git read-tree -m "$c0" "$c2" "$c1" + +' + +test_expect_success 'reset and bind merge' ' + + git reset --hard master && + git read-tree --prefix=M/ master && + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o0 0 M/A" + echo "100644 $o1 0 M/a" + echo "100644 $o0 0 M/b" + echo "100644 $o1 0 M/d/e" + echo "100644 $o1 0 a" + echo "100644 $o0 0 b" + echo "100644 $o1 0 d/e" + ) >expected && + git diff -u expected actual && + + git read-tree --prefix=a1/ master && + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o0 0 M/A" + echo "100644 $o1 0 M/a" + echo "100644 $o0 0 M/b" + echo "100644 $o1 0 M/d/e" + echo "100644 $o1 0 a" + echo "100644 $o0 0 a1/A" + echo "100644 $o1 0 a1/a" + echo "100644 $o0 0 a1/b" + echo "100644 $o1 0 a1/d/e" + echo "100644 $o0 0 b" + echo "100644 $o1 0 d/e" + ) >expected && + git diff -u expected actual + + git read-tree --prefix=z/ master && + git ls-files -s >actual && + ( + echo "100644 $o0 0 A" + echo "100644 $o0 0 M/A" + echo "100644 $o1 0 M/a" + echo "100644 $o0 0 M/b" + echo "100644 $o1 0 M/d/e" + echo "100644 $o1 0 a" + echo "100644 $o0 0 a1/A" + echo "100644 $o1 0 a1/a" + echo "100644 $o0 0 a1/b" + echo "100644 $o1 0 a1/d/e" + echo "100644 $o0 0 b" + echo "100644 $o1 0 d/e" + echo "100644 $o0 0 z/A" + echo "100644 $o1 0 z/a" + echo "100644 $o0 0 z/b" + echo "100644 $o1 0 z/d/e" + ) >expected && + git diff -u expected actual + +' + +test_done + -- cgit v0.10.2-6-g49f6