From ac49f5ca84d82e5b10bc1eb022dfdd9b0e8f7749 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 16 Feb 2011 05:47:44 -0500 Subject: [PATCH 1/2] rerere "remaining" After "rerere" resolves conflicts by reusing old resolution, there would be three kinds of paths with conflict in the index: * paths that have been resolved in the working tree by rerere; * paths that need further work whose resolution could be recorded; * paths that need resolving that rerere won't help. When the user wants a list of paths that need hand-resolving, output from "rerere status" does not help, as it shows only the second category, but the paths in the third category still needs work (rerere only makes sense for regular files that have both our side and their side, and does not help other kinds of conflicts, e.g. "we modified, they deleted"). The new subcommand "rerere remaining" can be used to show both. As opposed to "rerere status", this subcommand also skips printing paths that have been added to the index, since these paths are already resolved and are no longer "remaining". Initial patch provided by Junio. Refactored and modified to skip resolved paths by Martin. Commit message mostly by Junio. Helped-by: Junio C Hamano Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/rerere.c | 14 +++++++-- rerere.c | 78 +++++++++++++++++++++++++++++++++++++++++------- rerere.h | 8 +++++ 3 files changed, 88 insertions(+), 12 deletions(-) diff --git a/builtin/rerere.c b/builtin/rerere.c index 642bf3558..67cbfeb15 100644 --- a/builtin/rerere.c +++ b/builtin/rerere.c @@ -8,7 +8,7 @@ #include "xdiff-interface.h" static const char * const rerere_usage[] = { - "git rerere [clear | status | diff | gc]", + "git rerere [clear | status | remaining | diff | gc]", NULL, }; @@ -156,7 +156,17 @@ int cmd_rerere(int argc, const char **argv, const char *prefix) else if (!strcmp(argv[0], "status")) for (i = 0; i < merge_rr.nr; i++) printf("%s\n", merge_rr.items[i].string); - else if (!strcmp(argv[0], "diff")) + else if (!strcmp(argv[0], "remaining")) { + rerere_remaining(&merge_rr); + for (i = 0; i < merge_rr.nr; i++) { + if (merge_rr.items[i].util != RERERE_RESOLVED) + printf("%s\n", merge_rr.items[i].string); + else + /* prepare for later call to + * string_list_clear() */ + merge_rr.items[i].util = NULL; + } + } else if (!strcmp(argv[0], "diff")) for (i = 0; i < merge_rr.nr; i++) { const char *path = merge_rr.items[i].string; const char *name = (const char *)merge_rr.items[i].util; diff --git a/rerere.c b/rerere.c index d26084347..22996bd08 100644 --- a/rerere.c +++ b/rerere.c @@ -7,6 +7,11 @@ #include "ll-merge.h" #include "attr.h" +#define RESOLVED 0 +#define PUNTED 1 +#define THREE_STAGED 2 +void *RERERE_RESOLVED = &RERERE_RESOLVED; + /* if rerere_enabled == -1, fall back to detection of .git/rr-cache */ static int rerere_enabled = -1; @@ -345,21 +350,74 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu return hunk_no; } -static int find_conflict(struct string_list *conflict) +static int check_one_conflict(int i, int *type) { - int i; - if (read_cache() < 0) - return error("Could not read index"); - for (i = 0; i+1 < active_nr; i++) { + struct cache_entry *e = active_cache[i]; + + if (!ce_stage(e)) { + *type = RESOLVED; + return i + 1; + } + + *type = PUNTED; + if (ce_stage(e) == 1) { + if (active_nr <= ++i) + return i + 1; + } + + /* Only handle regular files with both stages #2 and #3 */ + if (i + 1 < active_nr) { struct cache_entry *e2 = active_cache[i]; - struct cache_entry *e3 = active_cache[i+1]; + struct cache_entry *e3 = active_cache[i + 1]; if (ce_stage(e2) == 2 && ce_stage(e3) == 3 && - ce_same_name(e2, e3) && + ce_same_name(e, e3) && S_ISREG(e2->ce_mode) && - S_ISREG(e3->ce_mode)) { - string_list_insert(conflict, (const char *)e2->name); - i++; /* skip over both #2 and #3 */ + S_ISREG(e3->ce_mode)) + *type = THREE_STAGED; + } + + /* Skip the entries with the same name */ + while (i < active_nr && ce_same_name(e, active_cache[i])) + i++; + return i; +} + +static int find_conflict(struct string_list *conflict) +{ + int i; + if (read_cache() < 0) + return error("Could not read index"); + + for (i = 0; i < active_nr;) { + int conflict_type; + struct cache_entry *e = active_cache[i]; + i = check_one_conflict(i, &conflict_type); + if (conflict_type == THREE_STAGED) + string_list_insert(conflict, (const char *)e->name); + } + return 0; +} + +int rerere_remaining(struct string_list *merge_rr) +{ + int i; + if (read_cache() < 0) + return error("Could not read index"); + + for (i = 0; i < active_nr;) { + int conflict_type; + struct cache_entry *e = active_cache[i]; + i = check_one_conflict(i, &conflict_type); + if (conflict_type == PUNTED) + string_list_insert(merge_rr, (const char *)e->name); + else if (conflict_type == RESOLVED) { + struct string_list_item *it; + it = string_list_lookup(merge_rr, (const char *)e->name); + if (it != NULL) { + free(it->util); + it->util = RERERE_RESOLVED; + } } } return 0; diff --git a/rerere.h b/rerere.h index eaa9004dc..595f49f70 100644 --- a/rerere.h +++ b/rerere.h @@ -6,11 +6,19 @@ #define RERERE_AUTOUPDATE 01 #define RERERE_NOAUTOUPDATE 02 +/* + * Marks paths that have been hand-resolved and added to the + * index. Set in the util field of such paths after calling + * rerere_remaining. + */ +extern void *RERERE_RESOLVED; + extern int setup_rerere(struct string_list *, int); extern int rerere(int); extern const char *rerere_path(const char *hex, const char *file); extern int has_rerere_resolution(const char *hex); extern int rerere_forget(const char **); +extern int rerere_remaining(struct string_list *); #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \ "update the index with reused conflict resolution if possible") From 2f59c947044d535e078941711e562d825eb055e3 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 16 Feb 2011 05:47:45 -0500 Subject: [PATCH 2/2] mergetool: don't skip modify/remove conflicts Since bb0a484 (mergetool: Skip autoresolved paths, 2010-08-17), mergetool uses different ways of figuring out the list of files with merge conflicts depending on whether rerere is active. If rerere is active, mergetool will use 'git rerere status' to list the files with remaining conflicts. However, the output from that command does not list conflicts of types that rerere does not handle, such as modify/remove conflicts. Another problem with solely relying on the output from 'git rerere status' is that, for new conflicts that are not yet known to rerere, the output from the command will list the files even after adding them to the index. This means that if the conflicts in some files have been resolved and 'git mergetool' is run again, it will ask the user something like the following for each of those files. file1: file does not need merging Continue merging other unresolved paths (y/n) ? Solve both of these problems by replacing the call to 'git rerere status' with a call to the new 'git rerere remaining' that was introduced in the previous commit. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- git-mergetool.sh | 2 +- t/t7610-mergetool.sh | 40 ++++++++++++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 2f8dc441c..bacbda2bb 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -269,7 +269,7 @@ rerere=false files_to_merge() { if test "$rerere" = true then - git rerere status + git rerere remaining else git ls-files -u | sed -e 's/^[^ ]* //' | sort -u fi diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index d78bdec33..dc838c93b 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -16,23 +16,33 @@ Testing basic merge tool invocation' test_expect_success 'setup' ' git config rerere.enabled true && echo master >file1 && + echo master file11 >file11 && + echo master file12 >file12 && + echo master file13 >file13 && + echo master file14 >file14 && mkdir subdir && echo master sub >subdir/file3 && - git add file1 subdir/file3 && - git commit -m "added file1" && + git add file1 file1[1-4] subdir/file3 && + git commit -m "add initial versions" && git checkout -b branch1 master && echo branch1 change >file1 && echo branch1 newfile >file2 && + echo branch1 change file11 >file11 && + echo branch1 change file13 >file13 && echo branch1 sub >subdir/file3 && - git add file1 file2 subdir/file3 && + git add file1 file11 file13 file2 subdir/file3 && + git rm file12 && git commit -m "branch1 changes" && git checkout master && echo master updated >file1 && echo master new >file2 && + echo master updated file12 >file12 && + echo master updated file14 >file14 && echo master new sub >subdir/file3 && - git add file1 file2 subdir/file3 && + git add file1 file12 file14 file2 subdir/file3 && + git rm file11 && git commit -m "master updates" && git config merge.tool mytool && @@ -46,6 +56,8 @@ test_expect_success 'custom mergetool' ' ( yes "" | git mergetool file1 >/dev/null 2>&1 ) && ( yes "" | git mergetool file2 >/dev/null 2>&1 ) && ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && test "$(cat file1)" = "master updated" && test "$(cat file2)" = "master new" && test "$(cat subdir/file3)" = "master new sub" && @@ -59,6 +71,8 @@ test_expect_success 'mergetool crlf' ' ( yes "" | git mergetool file1 >/dev/null 2>&1 ) && ( yes "" | git mergetool file2 >/dev/null 2>&1 ) && ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && test "$(printf x | cat file1 -)" = "$(printf "master updated\r\nx")" && test "$(printf x | cat file2 -)" = "$(printf "master new\r\nx")" && test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" && @@ -82,6 +96,8 @@ test_expect_success 'mergetool on file in parent dir' ' cd subdir && ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) && ( yes "" | git mergetool ../file2 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && git commit -m "branch1 resolved with mergetool - subdir" @@ -92,6 +108,8 @@ test_expect_success 'mergetool skips autoresolved' ' git checkout -b test4 branch1 && test_must_fail git merge master && test -n "$(git ls-files -u)" && + ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && git reset --hard @@ -102,13 +120,23 @@ test_expect_success 'mergetool merges all from subdir' ' cd subdir && git config rerere.enabled false && test_must_fail git merge master && - git mergetool --no-prompt && + ( yes "d" "d" | git mergetool --no-prompt ) && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat file3)" = "master new sub" && - git add ../file1 ../file2 file3 && git commit -m "branch2 resolved by mergetool from subdir" ) ' +test_expect_success 'mergetool skips resolved paths when rerere is active' ' + git config rerere.enabled true && + rm -rf .git/rr-cache && + git checkout -b test5 branch1 + test_must_fail git merge master >/dev/null 2>&1 && + ( yes "d" "d" | git mergetool --no-prompt >/dev/null 2>&1 ) && + output="$(yes "n" | git mergetool --no-prompt)" && + test "$output" = "No files need merging" && + git reset --hard +' + test_done