The output from 'git status' and 'git diff-index --cached -M' is broken when there are unmerged files as well as new files similar to the unmerged file (in stage 1). When two copies have been created the output is: $ git status -s C original -> exact-copy R original -> modified-copy U original There are several errors here: 1) "original" has invalid status " U", 2) "modified-copy" is listed as a rename, even though its source ("original") still exists, and 3) "exact-copy" is detected as a copy, even though 'git status' is only supposed to show renames In the verbose 'git status' output, the unmerged path is completely missing. 'git diff-index --cached -M' show similar output. Fix these problems by making diffcore-rename not consider unmerged paths as source for rename detection. For simplicty, don't consider the path independent of the type of merge conflict, even if the path is deleted on at least one side. Still consider unmerged paths as source for copy detection. While at it, update the users of diff_options to use DIFF_DETECT_RENAME instead of directly using the value 1 to enable rename detection. Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@xxxxxxxxx> --- This is the first time I look at the diff code, so please review carefully. I think the changes make sense, but I really don't know the code enough to be sure. Also not sure about the "while at it" stuff... The test cases assume that the paths will be printed in a certain order. Can I rely on that? builtin/commit.c | 2 +- diffcore-rename.c | 7 +++- t/t7510-status-merge-rename-copy.sh | 68 +++++++++++++++++++++++++++++++++++ wt-status.c | 6 ++-- 4 files changed, 77 insertions(+), 6 deletions(-) create mode 100755 t/t7510-status-merge-rename-copy.sh diff --git a/builtin/commit.c b/builtin/commit.c index 82092e5..1f9f314 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1284,7 +1284,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1) rev.show_root_diff = 1; get_commit_format(format.buf, &rev); rev.always_show_header = 0; - rev.diffopt.detect_rename = 1; + rev.diffopt.detect_rename = DIFF_DETECT_RENAME; rev.diffopt.rename_limit = 100; rev.diffopt.break_opt = 0; diff_setup_done(&rev.diffopt); diff --git a/diffcore-rename.c b/diffcore-rename.c index 0cd4c13..c53ca36 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -464,7 +464,7 @@ void diffcore_rename(struct diff_options *options) else locate_rename_dst(p->two, 1); } - else if (!DIFF_FILE_VALID(p->two)) { + else if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) { /* * If the source is a broken "delete", and * they did not really want to get broken, @@ -575,7 +575,10 @@ void diffcore_rename(struct diff_options *options) struct diff_filepair *p = q->queue[i]; struct diff_filepair *pair_to_free = NULL; - if (!DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) { + if (DIFF_PAIR_UNMERGED(p)) { + diff_q(&outq, p); + } + else if (!DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) { /* * Creation * diff --git a/t/t7510-status-merge-rename-copy.sh b/t/t7510-status-merge-rename-copy.sh new file mode 100755 index 0000000..8c7a4d2 --- /dev/null +++ b/t/t7510-status-merge-rename-copy.sh @@ -0,0 +1,68 @@ +#!/bin/sh +# +# Copyright (c) 2011 Martin von Zweigbergk +# + +test_description='git status during merge with copies' + +. ./test-lib.sh + +test_expect_success 'setup' ' + echo a b c | xargs -n1 >original && + git add original && + git commit -m initial && + + cp original exact-copy && + cp original modified-copy && + echo x >>original && + echo y >>modified-copy && + git add original exact-copy modified-copy && + git commit -m "create copies and modify original on master" && + + git checkout -b topic HEAD^ && + echo z >>original && + git add original && + git commit -m "modify original on topic" && + test_must_fail git merge master +' + +cat >expected <<\EOF +A exact-copy +A modified-copy +UU original +EOF + +test_expect_success 'git status -s shows 2 added + 1 unmerged' ' + git status -s -uno >actual && + test_cmp expected actual +' + +cat >expected <<\EOF +A exact-copy +A modified-copy +U original +EOF + +test_expect_success 'git diff-index --cached shows 2 added + 1 unmerged' ' + git diff-index --cached --name-status HEAD >actual && + test_cmp expected actual +' + +test_expect_success 'git diff-index --cached -M shows 2 added + 1 unmerged' ' + git diff-index --cached --name-status HEAD >actual && + test_cmp expected actual +' + +cat >expected <<\EOF +C original exact-copy +C original modified-copy +U original +EOF + +test_expect_success 'git diff-index --cached -C shows 2 copies + 1 unmerged' ' + git diff-index --cached -C --name-status HEAD | + sed "s/^C[0-9]*/C/g" >actual && + test_cmp expected actual +' + +test_done diff --git a/wt-status.c b/wt-status.c index 4daa8bb..532bbcc 100644 --- a/wt-status.c +++ b/wt-status.c @@ -320,7 +320,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s) if (s->ignore_submodule_arg) { DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG); handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg); - } + } rev.diffopt.format_callback = wt_status_collect_changed_cb; rev.diffopt.format_callback_data = s; init_pathspec(&rev.prune_data, s->pathspec); @@ -345,7 +345,7 @@ static void wt_status_collect_changes_index(struct wt_status *s) rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK; rev.diffopt.format_callback = wt_status_collect_updated_cb; rev.diffopt.format_callback_data = s; - rev.diffopt.detect_rename = 1; + rev.diffopt.detect_rename = DIFF_DETECT_RENAME; rev.diffopt.rename_limit = 200; rev.diffopt.break_opt = 0; init_pathspec(&rev.prune_data, s->pathspec); @@ -597,7 +597,7 @@ static void wt_status_print_verbose(struct wt_status *s) setup_revisions(0, NULL, &rev, &opt); rev.diffopt.output_format |= DIFF_FORMAT_PATCH; - rev.diffopt.detect_rename = 1; + rev.diffopt.detect_rename = DIFF_DETECT_RENAME; rev.diffopt.file = s->fp; rev.diffopt.close_file = 0; /* -- 1.7.4.79.gcbe20 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html