{"thread":{"id":"38984","subject":"[PATCH 2/4] combine-diff.c: refactor: extract insert_path()","startedAt":"2015-04-02T20:34:09Z","lastAt":"2015-04-14T04:22:52Z","messageCount":25,"participants":["Max Kirillov","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"258886","messageId":"1428006853-21212-1-git-send-email-max@max630.net","threadId":"38984","inReplyTo":null,"subject":"[PATCH 0/4] diff --cc: relax path filtering","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-02T20:34:09Z","receivedAt":"2015-04-02T20:34:09Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"For diff --cc, paths fitering used to select only paths which have\nchanged in all parents, while diffing itself output hunks which are\nchanged in as few as 2 parents.\n\nFix intersect_paths() to add paths which have at least 2 changed\nparents.\n\nIntersects with branch 'bc/object-id' which is not yet in master. This is\nrebased on top of it.\n\nMax Kirillov (4):\n Add test for showing discarded changes with diff --cc\n combine-diff.c: refactor: extract insert_path()\n diff --cc: relax too strict paths picking\n t4059: rewrite to be adaptive to hunk filtering\n\n combine-diff.c | 95 ++++++++++++++++++-----------\n t/t4059-diff-merge-with-base.sh | 132 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 193 insertions(+), 34 deletions(-)\n create mode 100755 t/t4059-diff-merge-with-base.sh\n\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258888","messageId":"1428006853-21212-2-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428006853-21212-1-git-send-email-max@max630.net","subject":"[PATCH 1/4] Add test for showing discarded changes with diff --cc","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-02T20:34:10Z","receivedAt":"2015-04-02T20:34:10Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"If a hunk has been changed in both branches and conflict resolution\nfully takes one of branches, discarding all changes that are in the\nother, then the merge is not shown in 3-way diff which git uses by\ndefault.\n\nThe advice is to use flag --cc and and explicitly add the mergebase was\ngiven in $gmane/191557. It was discovered though that it does not always\nwork. If the whole file has not changed at all compared to one of\nbranches then no difference is shown for this file.\n\nThis looks inconsistent and for particular scenario of viewing discarded\nchanges is undesirable.\n\nAdd the test which demonstrates the issue.\n\nSigned-off-by: Max Kirillov \n---\n t/t4059-diff-merge-with-base.sh | 100 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 100 insertions(+)\n create mode 100755 t/t4059-diff-merge-with-base.sh\n\ndiff --git a/t/t4059-diff-merge-with-base.sh b/t/t4059-diff-merge-with-base.sh\nnew file mode 100755\nindex 0000000..e650a33\n--- /dev/null\n+++ b/t/t4059-diff-merge-with-base.sh\n@@ -0,0 +1,100 @@\n+#!/bin/sh\n+\n+test_description='Diff aware of merge base'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tmkdir short &&\n+\tmkdir long &&\n+\tfor fn in win1 win2 merge delete base only1 only2; do\n+\t\ttest_seq 3 >short/$fn\n+\t\tgit add short/$fn &&\n+\t\ttest_seq 11 >long/$fn &&\n+\t\tgit add long/$fn\n+\tdone &&\n+\tgit commit -m mergebase &&\n+\tgit branch mergebase &&\n+\n+\tfor fn in win1 win2 merge delete base only1; do\n+\t\tfor dir in short long; do\n+\t\t\tsed -e \"s/^2/2change1/\" -e \"s/^7/7change1/\" $dir/$fn >sed.new &&\n+\t\t\tmv sed.new $dir/$fn &&\n+\t\t\tgit add $dir/$fn\n+\t\tdone\n+\tdone &&\n+\tsed -e \"s/^7/7change1/\" long/only2 >sed.new &&\n+\tmv sed.new long/only2 &&\n+\tgit add long/only2 &&\n+\tgit commit -m branch1 &&\n+\tgit branch branch1 &&\n+\n+\tgit reset --hard mergebase &&\n+\tfor fn in win1 win2 merge delete base only2; do\n+\t\tfor dir in short long; do\n+\t\t\tsed -e \"s/^2/2change2/\" -e \"s/^11/11change2/\" $dir/$fn >sed.new &&\n+\t\t\tmv sed.new $dir/$fn &&\n+\t\t\tgit add $dir/$fn\n+\t\tdone\n+\tdone &&\n+\tsed -e \"s/^11/11change2/\" long/only1 >sed.new &&\n+\tmv sed.new long/only1 &&\n+\tgit add long/only1 &&\n+\tgit commit -m branch2 &&\n+\tgit branch branch2 &&\n+\n+\ttest_must_fail git merge branch1 &&\n+\tgit checkout mergebase -- . &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/base &&\n+\tgit add long/base &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change1/\" >long/win1 &&\n+\tgit add long/win1 &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change2/\" >long/win2 &&\n+\tgit add long/win2 &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2merged/\" >long/merge &&\n+\tgit add long/merge &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"/^2/d\" >long/delete &&\n+\tgit add long/delete &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change1/\" >long/only1 &&\n+\tgit add long/only1 &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change2/\" >long/only2 &&\n+\tgit add long/only2 &&\n+\ttest_seq 3 >short/base &&\n+\tgit add short/base &&\n+\ttest_seq 3 | sed -e \"s/^2/2change1/\" >short/win1 &&\n+\tgit add short/win1 &&\n+\ttest_seq 3 | sed -e \"s/^2/2change2/\" >short/win2 &&\n+\tgit add short/win2 &&\n+\ttest_seq 3 | sed -e \"s/^2/2merged/\" >short/merge &&\n+\tgit add short/merge &&\n+\ttest_seq 3 | sed -e \"/^2/d\" >short/delete &&\n+\tgit add short/delete &&\n+\ttest_seq 3 | sed -e \"s/^2/2change1/\" >short/only1 &&\n+\tgit add short/only1 &&\n+\ttest_seq 3 | sed -e \"s/^2/2change2/\" >short/only2 &&\n+\tgit add short/only2 &&\n+\tgit commit -m merge &&\n+\tgit branch merge\n+'\n+\n+test_expect_success 'diff with mergebase shows discarded change from parent 2 in merged file' '\n+\tgit diff --cc merge branch1 branch2 mergebase -- long/win1 >actual &&\n+\ttest -s actual\n+'\n+\n+test_expect_success 'diff with mergebase shows discarded change from parent 1 in merged file' '\n+\tgit diff --cc merge branch1 branch2 mergebase -- long/win2 >actual &&\n+\ttest -s actual\n+'\n+\n+test_expect_failure 'diff with mergebase shows fully discarded file from parent 2' '\n+\tgit diff --cc merge branch1 branch2 mergebase -- short/win1 >actual &&\n+\ttest -s actual\n+'\n+\n+test_expect_failure 'diff with mergebase shows fully discarded file from parent 1' '\n+\tgit diff --cc merge branch1 branch2 mergebase -- short/win2 >actual &&\n+\ttest -s actual\n+'\n+\n+test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258885","messageId":"1428006853-21212-3-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428006853-21212-1-git-send-email-max@max630.net","subject":"[PATCH 2/4] combine-diff.c: refactor: extract insert_path()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-02T20:34:11Z","receivedAt":"2015-04-02T20:34:11Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Signed-off-by: Max Kirillov \n---\n combine-diff.c | 43 ++++++++++++++++++++++++-------------------\n 1 file changed, 24 insertions(+), 19 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 8eb7278..a236bb5 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -22,6 +22,28 @@ static int compare_paths(const struct combine_diff_path *one,\n \t\t\t\t two->path, strlen(two->path), two->mode);\n }\n \n+static void insert_path(struct combine_diff_path **pos, const char* path, int n, int num_parent, struct diff_filepair *queue_item)\n+{\n+\tint len;\n+\tstruct combine_diff_path *p;\n+\n+\tlen = strlen(path);\n+\tp = xmalloc(combine_diff_path_size(num_parent, len));\n+\tp->path = (char *) &(p->parent[num_parent]);\n+\tmemcpy(p->path, path, len);\n+\tp->path[len] = 0;\n+\tp->next = *pos;\n+\tmemset(p->parent, 0,\n+\t sizeof(p->parent[0]) * num_parent);\n+\n+\thashcpy(p->oid.hash, queue_item->two->sha1);\n+\tp->mode = queue_item->two->mode;\n+\thashcpy(p->parent[n].oid.hash, queue_item->one->sha1);\n+\tp->parent[n].mode = queue_item->one->mode;\n+\tp->parent[n].status = queue_item->status;\n+\t*pos = p;\n+}\n+\n static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n@@ -30,27 +52,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \n \tif (!n) {\n \t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tint len;\n-\t\t\tconst char *path;\n \t\t\tif (diff_unmodified_pair(q->queue[i]))\n \t\t\t\tcontinue;\n-\t\t\tpath = q->queue[i]->two->path;\n-\t\t\tlen = strlen(path);\n-\t\t\tp = xmalloc(combine_diff_path_size(num_parent, len));\n-\t\t\tp->path = (char *) &(p->parent[num_parent]);\n-\t\t\tmemcpy(p->path, path, len);\n-\t\t\tp->path[len] = 0;\n-\t\t\tp->next = NULL;\n-\t\t\tmemset(p->parent, 0,\n-\t\t\t sizeof(p->parent[0]) * num_parent);\n-\n-\t\t\thashcpy(p->oid.hash, q->queue[i]->two->sha1);\n-\t\t\tp->mode = q->queue[i]->two->mode;\n-\t\t\thashcpy(p->parent[n].oid.hash, q->queue[i]->one->sha1);\n-\t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n-\t\t\tp->parent[n].status = q->queue[i]->status;\n-\t\t\t*tail = p;\n-\t\t\ttail = &p->next;\n+\t\t\tinsert_path(tail, q->queue[i]->two->path, n, num_parent, q->queue[i]);\n+\t\t\ttail = &(*tail)->next;\n \t\t}\n \t\treturn curr;\n \t}\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258887","messageId":"1428006853-21212-4-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428006853-21212-1-git-send-email-max@max630.net","subject":"[PATCH 3/4] diff --cc: relax too strict paths picking","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-02T20:34:12Z","receivedAt":"2015-04-02T20:34:12Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"For diff --cc, paths fitering used to select only paths which have\nchanged in all parents, while diffing itself output hunks which are\nchanged in as few as 2 parents.\n\nFix intersect_paths() to add paths which have at least 2 changed\nparents. In this new functionality does not require special treatment of\nfirst pass, because path can be added from any parent, not just the\nfirst one.\n\nSigned-off-by: Max Kirillov \n---\n combine-diff.c | 56 ++++++++++++++++++++++++++++-------------\n t/t4059-diff-merge-with-base.sh | 4 +--\n 2 files changed, 41 insertions(+), 19 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex a236bb5..fd752e7 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -25,6 +25,7 @@ static int compare_paths(const struct combine_diff_path *one,\n static void insert_path(struct combine_diff_path **pos, const char* path, int n, int num_parent, struct diff_filepair *queue_item)\n {\n \tint len;\n+\tint parent_idx;\n \tstruct combine_diff_path *p;\n \n \tlen = strlen(path);\n@@ -41,43 +42,64 @@ static void insert_path(struct combine_diff_path **pos, const char* path, int n,\n \thashcpy(p->parent[n].oid.hash, queue_item->one->sha1);\n \tp->parent[n].mode = queue_item->one->mode;\n \tp->parent[n].status = queue_item->status;\n+\tfor (parent_idx = 0; parent_idx < n; ++parent_idx) {\n+\t\thashcpy(p->parent[parent_idx].oid.hash, p->oid.hash);\n+\t\tp->parent[parent_idx].mode = p->mode;\n+\t\tp->parent[parent_idx].status = ' ';\n+\t}\n \t*pos = p;\n }\n \n+static int changed_parents(struct combine_diff_path *p, int n)\n+{\n+\tint parent_idx;\n+\tint result = 0;\n+\n+\tfor (parent_idx = 0; parent_idx < n; ++parent_idx) {\n+\t\tif (p->parent[parent_idx].status != ' ')\n+\t\t\t++result;\n+\t}\n+\n+\treturn result;\n+}\n+\n static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n \tint i, cmp;\n \n-\tif (!n) {\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tif (diff_unmodified_pair(q->queue[i]))\n-\t\t\t\tcontinue;\n-\t\t\tinsert_path(tail, q->queue[i]->two->path, n, num_parent, q->queue[i]);\n-\t\t\ttail = &(*tail)->next;\n-\t\t}\n-\t\treturn curr;\n-\t}\n-\n \t/*\n \t * paths in curr (linked list) and q->queue[] (array) are\n \t * both sorted in the tree order.\n \t */\n \ti = 0;\n-\twhile ((p = *tail) != NULL) {\n-\t\tcmp = ((i >= q->nr)\n-\t\t ? -1 : compare_paths(p, q->queue[i]->two));\n+\twhile ((p = *tail) != NULL || i < q->nr) {\n+\t\tcmp = (i >= q->nr) ? -1\n+\t\t : (p == NULL) ? 1\n+\t\t : compare_paths(p, q->queue[i]->two);\n \n \t\tif (cmp < 0) {\n-\t\t\t/* p->path not in q->queue[]; drop it */\n-\t\t\t*tail = p->next;\n-\t\t\tfree(p);\n+\t\t\t/* p->path not in q->queue[] */\n+\t\t\tif (num_parent > 2 && 2 - changed_parents(p, n) <= num_parent - n - 1) {\n+\t\t\t\t/* still can get 2 changed parents */\n+\t\t\t\thashcpy(p->parent[n].oid.hash, p->oid.hash);\n+\t\t\t\tp->parent[n].mode = p->mode;\n+\t\t\t\tp->parent[n].status = ' ';\n+\t\t\t\ttail = &p->next;\n+\t\t\t} else {\n+\t\t\t\t*tail = p->next;\n+\t\t\t\tfree(p);\n+\t\t\t}\n \t\t\tcontinue;\n \t\t}\n \n \t\tif (cmp > 0) {\n-\t\t\t/* q->queue[i] not in p->path; skip it */\n+\t\t\t/* q->queue[i] not in p->path */\n+\t\t\tif (1 <= num_parent - n - 1) {\n+\t\t\t\tinsert_path(tail, q->queue[i]->two->path, n, num_parent, q->queue[i]);\n+\t\t\t\ttail = &(*tail)->next;\n+\t\t\t}\n \t\t\ti++;\n \t\t\tcontinue;\n \t\t}\ndiff --git a/t/t4059-diff-merge-with-base.sh b/t/t4059-diff-merge-with-base.sh\nindex e650a33..6341f7c 100755\n--- a/t/t4059-diff-merge-with-base.sh\n+++ b/t/t4059-diff-merge-with-base.sh\n@@ -87,12 +87,12 @@ test_expect_success 'diff with mergebase shows discarded change from parent 1 in\n \ttest -s actual\n '\n \n-test_expect_failure 'diff with mergebase shows fully discarded file from parent 2' '\n+test_expect_success 'diff with mergebase shows fully discarded file from parent 2' '\n \tgit diff --cc merge branch1 branch2 mergebase -- short/win1 >actual &&\n \ttest -s actual\n '\n \n-test_expect_failure 'diff with mergebase shows fully discarded file from parent 1' '\n+test_expect_success 'diff with mergebase shows fully discarded file from parent 1' '\n \tgit diff --cc merge branch1 branch2 mergebase -- short/win2 >actual &&\n \ttest -s actual\n '\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258889","messageId":"1428006853-21212-5-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428006853-21212-1-git-send-email-max@max630.net","subject":"[PATCH 4/4] t4059: rewrite to be adaptive to hunk filtering","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-02T20:34:13Z","receivedAt":"2015-04-02T20:34:13Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Looks like there is no exact specification what `diff -c` and\n`diff --cc` should output. Considering this it is not reasonable to\nhardcode any specific output in test. Rather, it should verify that file\nselection behaves the same as hunk selection.\n\nRewrite test so that it makes sure that a \"short\" file is shown if and\nonly if the corresponding \"long\" file's contains the changed hunk.\n\nSigned-off-by: Max Kirillov \n---\n t/t4059-diff-merge-with-base.sh | 84 ++++++++++++++++++++++++++++-------------\n 1 file changed, 58 insertions(+), 26 deletions(-)\n\ndiff --git a/t/t4059-diff-merge-with-base.sh b/t/t4059-diff-merge-with-base.sh\nindex 6341f7c..75a3820 100755\n--- a/t/t4059-diff-merge-with-base.sh\n+++ b/t/t4059-diff-merge-with-base.sh\n@@ -1,13 +1,22 @@\n #!/bin/sh\n \n-test_description='Diff aware of merge base'\n+test_description='combined diff filtering is not affected by preliminary path filtering'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/diff-lib.sh\n \n+# history is:\n+# (mergebase) --> (branch1) --\\\n+# | V\n+# \\ --> (branch2) ----------(merge)\n+# there are files in 2 subdirectories, \"long\" and \"short\"\n+# each file in \"long\" subdirecty has exactly same history as same file in \"short\" one,\n+# but it has added lines with changes in both branches which merge without conflict\n+# so the long files are always selected at path filtering\n test_expect_success setup '\n \tmkdir short &&\n \tmkdir long &&\n-\tfor fn in win1 win2 merge delete base only1 only2; do\n+\tfor fn in win1 win2 merge delete base only1 only2 only1discard only2discard; do\n \t\ttest_seq 3 >short/$fn\n \t\tgit add short/$fn &&\n \t\ttest_seq 11 >long/$fn &&\n@@ -23,9 +32,11 @@ test_expect_success setup '\n \t\t\tgit add $dir/$fn\n \t\tdone\n \tdone &&\n-\tsed -e \"s/^7/7change1/\" long/only2 >sed.new &&\n-\tmv sed.new long/only2 &&\n-\tgit add long/only2 &&\n+\tfor fn in only2 only2discard; do\n+\t sed -e \"s/^7/7change1/\" long/$fn >sed.new &&\n+\t mv sed.new long/$fn &&\n+\t git add long/$fn\n+\tdone &&\n \tgit commit -m branch1 &&\n \tgit branch branch1 &&\n \n@@ -37,9 +48,11 @@ test_expect_success setup '\n \t\t\tgit add $dir/$fn\n \t\tdone\n \tdone &&\n-\tsed -e \"s/^11/11change2/\" long/only1 >sed.new &&\n-\tmv sed.new long/only1 &&\n-\tgit add long/only1 &&\n+\tfor fn in only1 only1discard; do\n+\t sed -e \"s/^11/11change2/\" long/$fn >sed.new &&\n+\t mv sed.new long/$fn &&\n+\t git add long/$fn\n+\tdone &&\n \tgit commit -m branch2 &&\n \tgit branch branch2 &&\n \n@@ -47,6 +60,10 @@ test_expect_success setup '\n \tgit checkout mergebase -- . &&\n \ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/base &&\n \tgit add long/base &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/only1discard &&\n+\tgit add long/only1discard &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/only2discard &&\n+\tgit add long/only2discard &&\n \ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change1/\" >long/win1 &&\n \tgit add long/win1 &&\n \ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change2/\" >long/win2 &&\n@@ -61,6 +78,10 @@ test_expect_success setup '\n \tgit add long/only2 &&\n \ttest_seq 3 >short/base &&\n \tgit add short/base &&\n+\ttest_seq 3 >short/only1discard &&\n+\tgit add short/only1discard &&\n+\ttest_seq 3 >short/only2discard &&\n+\tgit add short/only2discard &&\n \ttest_seq 3 | sed -e \"s/^2/2change1/\" >short/win1 &&\n \tgit add short/win1 &&\n \ttest_seq 3 | sed -e \"s/^2/2change2/\" >short/win2 &&\n@@ -77,24 +98,35 @@ test_expect_success setup '\n \tgit branch merge\n '\n \n-test_expect_success 'diff with mergebase shows discarded change from parent 2 in merged file' '\n-\tgit diff --cc merge branch1 branch2 mergebase -- long/win1 >actual &&\n-\ttest -s actual\n-'\n-\n-test_expect_success 'diff with mergebase shows discarded change from parent 1 in merged file' '\n-\tgit diff --cc merge branch1 branch2 mergebase -- long/win2 >actual &&\n-\ttest -s actual\n-'\n+# the difference in short file must be returned if and only if it is shown in long file\n+for fn in win1 win2 merge delete base only1 only2 only1discard only2discard; do\n+\tif git diff --cc merge branch1 branch2 mergebase -- long/$fn | grep -q '^[ +-]\\{3\\}2\\(change[12]|merge\\)\\?$'\n+\tthen\n+\t\ttest_expect_success \"diff --cc contains short/$fn\" '\n+\t\t\tgit diff --cc merge branch1 branch2 mergebase -- short/'\"$fn\"' >actual &&\n+\t\t\ttest -s actual\n+\t\t'\n+\telse\n+\t\ttest_expect_success \"diff --cc does not contain short/$fn\" '\n+\t\t\tgit diff --cc merge branch1 branch2 mergebase -- short/'\"$fn\"' >actual &&\n+\t\t\t! test -s actual\n+\t\t'\n+\tfi\n+done\n \n-test_expect_success 'diff with mergebase shows fully discarded file from parent 2' '\n-\tgit diff --cc merge branch1 branch2 mergebase -- short/win1 >actual &&\n-\ttest -s actual\n-'\n-\n-test_expect_success 'diff with mergebase shows fully discarded file from parent 1' '\n-\tgit diff --cc merge branch1 branch2 mergebase -- short/win2 >actual &&\n-\ttest -s actual\n-'\n+for fn in win1 win2 merge delete base only1 only2 only1discard only2discard; do\n+\tif git diff -c merge branch1 branch2 mergebase -- long/$fn | grep -q '^[ +-]\\{3\\}2\\(change[12]|merge\\)\\?$'\n+\tthen\n+\t\ttest_expect_success \"diff -c contains short/$fn\" '\n+\t\t\tgit diff -c merge branch1 branch2 mergebase -- short/'\"$fn\"' >actual &&\n+\t\t\ttest -s actual\n+\t\t'\n+\telse\n+\t\ttest_expect_success \"diff -c does not contain short/$fn\" '\n+\t\t\tgit diff -c merge branch1 branch2 mergebase -- short/'\"$fn\"' >actual &&\n+\t\t\t! test -s actual\n+\t\t'\n+\tfi\n+done\n \n test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258891","messageId":"xmqqy4mai71d.fsf@gitster.dls.corp.google.com","threadId":"38984","inReplyTo":"1428006853-21212-2-git-send-email-max@max630.net","subject":"Re: [PATCH 1/4] Add test for showing discarded changes with diff --cc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-02T20:55:58Z","receivedAt":"2015-04-02T20:55:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> If a hunk has been changed in both branches and conflict resolution\n> fully takes one of branches, discarding all changes that are in the\n> other, then the merge is not shown in 3-way diff which git uses by\n> default.\n>\n> The advice is to use flag --cc and and explicitly add the mergebase was\n> given in $gmane/191557. It was discovered though that it does not always\n> work. If the whole file has not changed at all compared to one of\n> branches then no difference is shown for this file.\n>\n> This looks inconsistent and for particular scenario of viewing discarded\n> changes is undesirable.\n>\n> Add the test which demonstrates the issue.\n>\n> Signed-off-by: Max Kirillov \n\nThanks. I will not have time to review this properly at the moment\nwhile preparing 2.4-rc1, and I do not want to spend time figuring\nout if the parent culling you are chaning was done deliberately (in\nwhich case this patch may be breaking things) or not.\n\nSo I'll give a preliminary nitpicks first ;-)\n\n>> Subject: Re: [PATCH 1/4] Add test for showing discarded changes with diff --cc\n\nSubject: t4059: test 'diff --cc' with a change from only few parents\n\nor something? \": \" without capitalization of\nthe beginning of what you did and the full-stop at the end.\n\n> ---\n> t/t4059-diff-merge-with-base.sh | 100 ++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 100 insertions(+)\n> create mode 100755 t/t4059-diff-merge-with-base.sh\n>\n> diff --git a/t/t4059-diff-merge-with-base.sh b/t/t4059-diff-merge-with-base.sh\n> new file mode 100755\n> index 0000000..e650a33\n> --- /dev/null\n> +++ b/t/t4059-diff-merge-with-base.sh\n> @@ -0,0 +1,100 @@\n> +#!/bin/sh\n> +\n> +test_description='Diff aware of merge base'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success setup '\n> +\tmkdir short &&\n> +\tmkdir long &&\n> +\tfor fn in win1 win2 merge delete base only1 only2; do\n> +\t\ttest_seq 3 >short/$fn\n> +\t\tgit add short/$fn &&\n> +\t\ttest_seq 11 >long/$fn &&\n> +\t\tgit add long/$fn\n> +\tdone &&\n\nTwo nits:\n\n - Style: have \"do\" on its own line. A good rule of thumb is that\n you shouldn't have to type ';' when you are writing a shell\n script, unless you are terminating a case arm with ';;'.\n\n\tfor fn in ...\n do\n \t...\n\tdone\n\n - Correctness: aside from missing && after \"test_seq 3\", if the\n last step \"git add long/$fn\" failed in an earlier iteration, you\n would not notice any breakage. Either\n\n\t(\n for fn in ...\n do\n ... &&\n ... &&\n ... || exit $?\n done\n\t)\n\n or\n\n\tfor fn in ...\n do\n \t... &&\n ... &&\n ... || return $?\n\tdone\n\n The latter is only valid in our test scripts, where the test\n framework gives you an artificial \"caller\" of the \"subroutine\"\n you are writing as the test body.\n\n> +\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/base &&\n> +\tgit add long/base &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2change1/\" >long/win1 &&\n> +\tgit add long/win1 &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2change2/\" >long/win2 &&\n> +\tgit add long/win2 &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2merged/\" >long/merge &&\n> +\tgit add long/merge &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"/^2/d\" >long/delete &&\n> +\tgit add long/delete &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2change1/\" >long/only1 &&\n> +\tgit add long/only1 &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2change2/\" >long/only2 &&\n> +\tgit add long/only2 &&\n\nPatch is line-wrapped around here?\n"},{"id":"258892","messageId":"xmqqtwwyi6uz.fsf@gitster.dls.corp.google.com","threadId":"38984","inReplyTo":"1428006853-21212-4-git-send-email-max@max630.net","subject":"Re: [PATCH 3/4] diff --cc: relax too strict paths picking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-02T20:59:48Z","receivedAt":"2015-04-02T20:59:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\nAgain, just surface nitpicks.\n\n[side: Kirill Smelkov removed from CC, as the\nddress just bounced for me]\n\n> +static int changed_parents(struct combine_diff_path *p, int n)\n> +{\n> +\tint parent_idx;\n> +\tint result = 0;\n> +\n> +\tfor (parent_idx = 0; parent_idx < n; ++parent_idx) {\n> +\t\tif (p->parent[parent_idx].status != ' ')\n> +\t\t\t++result;\n\nWe write C (not C++) and favor post_increment++ over ++pre_increment\nwhen there is no valid reason to do otherwise (e.g. the result of\nincrement getting used in a larger expression).\n\nThanks.\n"},{"id":"258895","messageId":"20150402211310.GA22988@peff.net","threadId":"38984","inReplyTo":"1428006853-21212-1-git-send-email-max@max630.net","subject":"Re: [PATCH 0/4] diff --cc: relax path filtering","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-04-02T21:13:10Z","receivedAt":"2015-04-02T21:13:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 02, 2015 at 11:34:09PM +0300, Max Kirillov wrote:\n\n> For diff --cc, paths fitering used to select only paths which have\n> changed in all parents, while diffing itself output hunks which are\n> changed in as few as 2 parents.\n\nI'm confused about \"used to\" here. Is this a regression due to the\ncombine-diff rewriting that happened in 2.0, or do you just mean \"before\nthis patch series, we used to do this other thing\".\n\n> Fix intersect_paths() to add paths which have at least 2 changed\n> parents.\n\nI'd worry a little that this is increasing the cost to do \"log --cc\", as\nit means we will have to open and look at extra files, and we may find\nin many cases that there aren't any interesting hunks. Which would imply\nwe might want to put it behind a flag, rather than as the default\n(\"--cc-me-harder\").\n\nBut if I'm understanding the issue correctly, this should only matter\nfor octopus merges. That is, the old rule for looking at a path was \"is\nthere at least one parent whose content we took verbatim\", but the new\none is \"are there are at least 2 parents whose content we did not take\nverbatim\". With only two parents, those would be the same thing, I\nthink.\n\n-Peff\n"},{"id":"258927","messageId":"1428076716-4449-1-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428006853-21212-1-git-send-email-max@max630.net","subject":"[PATCH v2 0/4] diff --cc: relax path filtering","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-03T15:58:32Z","receivedAt":"2015-04-03T15:58:32Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Fixes:\n* test renamed and commit message rewritten, so that\n it mentions 3-parent merge rather than other uses\n* test: fix && chaining and do at new line\n* use postincrement in C code\n\nMax Kirillov (4):\n t4059: test 'diff --cc' with a change from only few parents\n combine-diff.c: refactor: extract insert_path()\n diff --cc: relax too strict paths picking\n t4059: rewrite to be adaptive to hunk filtering\n\n combine-diff.c | 95 +++++++++------\n t/t4059-diff-cc-not-affected-by-path-filtering.sh | 142 ++++++++++++++++++++++\n 2 files changed, 203 insertions(+), 34 deletions(-)\n create mode 100755 t/t4059-diff-cc-not-affected-by-path-filtering.sh\n\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258928","messageId":"1428076716-4449-2-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428076716-4449-1-git-send-email-max@max630.net","subject":"[PATCH v2 1/4] t4059: test 'diff --cc' with a change from only few parents","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-03T15:58:33Z","receivedAt":"2015-04-03T15:58:33Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"If `git diff --cc` is used with 2 or more parents, then\nit shows all hunks which have changed compared to at least 2 parents.\nWhich is reasonable, because those places are likely places for\nconflicts, and it should be displayed how they were resolved.\nBut, preliminary path filtering passes a path only it was changed\ncompared to each parent. So, if a hunk which has not changed compared to\nsome of parents is shown if there are more changed hunks in the file,\nbut not shown if it is the only change.\n\nThis looks inconsistent and for some scenarios it is desirable to show\nsuch changes.\n\nAdd the test which demonstrates the issue.\n\nSigned-off-by: Max Kirillov \n---\n t/t4059-diff-cc-not-affected-by-path-filtering.sh | 108 ++++++++++++++++++++++\n 1 file changed, 108 insertions(+)\n create mode 100755 t/t4059-diff-cc-not-affected-by-path-filtering.sh\n\ndiff --git a/t/t4059-diff-cc-not-affected-by-path-filtering.sh b/t/t4059-diff-cc-not-affected-by-path-filtering.sh\nnew file mode 100755\nindex 0000000..3e6e59b\n--- /dev/null\n+++ b/t/t4059-diff-cc-not-affected-by-path-filtering.sh\n@@ -0,0 +1,108 @@\n+#!/bin/sh\n+\n+test_description='combined diff filtering is not affected by preliminary path filtering'\n+# Since diff --cc allows use not only real parents but any commits, use merge\n+# base here as the 3rd \"parent\". The trick was suggested in $gmane/191557 to\n+# spot changes which were discarded during conflict resolution.\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tmkdir short &&\n+\tmkdir long &&\n+\tfor fn in win1 win2 merge delete base only1 only2\n+\tdo\n+\t\ttest_seq 3 >short/$fn &&\n+\t\tgit add short/$fn &&\n+\t\ttest_seq 11 >long/$fn &&\n+\t\tgit add long/$fn || return $?\n+\tdone &&\n+\tgit commit -m mergebase &&\n+\tgit branch mergebase &&\n+\n+\tfor fn in win1 win2 merge delete base only1\n+\tdo\n+\t\tfor dir in short long\n+\t\tdo\n+\t\t\tsed -e \"s/^2/2change1/\" -e \"s/^7/7change1/\" $dir/$fn >sed.new &&\n+\t\t\tmv sed.new $dir/$fn &&\n+\t\t\tgit add $dir/$fn || return $?\n+\t\tdone || return $?\n+\tdone &&\n+\tsed -e \"s/^7/7change1/\" long/only2 >sed.new &&\n+\tmv sed.new long/only2 &&\n+\tgit add long/only2 &&\n+\tgit commit -m branch1 &&\n+\tgit branch branch1 &&\n+\n+\tgit reset --hard mergebase &&\n+\tfor fn in win1 win2 merge delete base only2\n+\tdo\n+\t\tfor dir in short long\n+\t\tdo\n+\t\t\tsed -e \"s/^2/2change2/\" -e \"s/^11/11change2/\" $dir/$fn >sed.new &&\n+\t\t\tmv sed.new $dir/$fn &&\n+\t\t\tgit add $dir/$fn || return $?\n+\t\tdone || return $?\n+\tdone &&\n+\tsed -e \"s/^11/11change2/\" long/only1 >sed.new &&\n+\tmv sed.new long/only1 &&\n+\tgit add long/only1 &&\n+\tgit commit -m branch2 &&\n+\tgit branch branch2 &&\n+\n+\ttest_must_fail git merge branch1 &&\n+\tgit checkout mergebase -- . &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/base &&\n+\tgit add long/base &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change1/\" >long/win1 &&\n+\tgit add long/win1 &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change2/\" >long/win2 &&\n+\tgit add long/win2 &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2merged/\" >long/merge &&\n+\tgit add long/merge &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"/^2/d\" >long/delete &&\n+\tgit add long/delete &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change1/\" >long/only1 &&\n+\tgit add long/only1 &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change2/\" >long/only2 &&\n+\tgit add long/only2 &&\n+\ttest_seq 3 >short/base &&\n+\tgit add short/base &&\n+\ttest_seq 3 | sed -e \"s/^2/2change1/\" >short/win1 &&\n+\tgit add short/win1 &&\n+\ttest_seq 3 | sed -e \"s/^2/2change2/\" >short/win2 &&\n+\tgit add short/win2 &&\n+\ttest_seq 3 | sed -e \"s/^2/2merged/\" >short/merge &&\n+\tgit add short/merge &&\n+\ttest_seq 3 | sed -e \"/^2/d\" >short/delete &&\n+\tgit add short/delete &&\n+\ttest_seq 3 | sed -e \"s/^2/2change1/\" >short/only1 &&\n+\tgit add short/only1 &&\n+\ttest_seq 3 | sed -e \"s/^2/2change2/\" >short/only2 &&\n+\tgit add short/only2 &&\n+\tgit commit -m merge &&\n+\tgit branch merge\n+'\n+\n+test_expect_success 'diff with mergebase shows discarded change from parent 2 in merged file' '\n+\tgit diff --cc merge branch1 branch2 mergebase -- long/win1 >actual &&\n+\ttest -s actual\n+'\n+\n+test_expect_success 'diff with mergebase shows discarded change from parent 1 in merged file' '\n+\tgit diff --cc merge branch1 branch2 mergebase -- long/win2 >actual &&\n+\ttest -s actual\n+'\n+\n+test_expect_failure 'diff with mergebase shows fully discarded file from parent 2' '\n+\tgit diff --cc merge branch1 branch2 mergebase -- short/win1 >actual &&\n+\ttest -s actual\n+'\n+\n+test_expect_failure 'diff with mergebase shows fully discarded file from parent 1' '\n+\tgit diff --cc merge branch1 branch2 mergebase -- short/win2 >actual &&\n+\ttest -s actual\n+'\n+\n+test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258926","messageId":"1428076716-4449-3-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428076716-4449-1-git-send-email-max@max630.net","subject":"[PATCH v2 2/4] combine-diff.c: refactor: extract insert_path()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-03T15:58:34Z","receivedAt":"2015-04-03T15:58:34Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Signed-off-by: Max Kirillov \n---\n combine-diff.c | 43 ++++++++++++++++++++++++-------------------\n 1 file changed, 24 insertions(+), 19 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 8eb7278..a236bb5 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -22,6 +22,28 @@ static int compare_paths(const struct combine_diff_path *one,\n \t\t\t\t two->path, strlen(two->path), two->mode);\n }\n \n+static void insert_path(struct combine_diff_path **pos, const char* path, int n, int num_parent, struct diff_filepair *queue_item)\n+{\n+\tint len;\n+\tstruct combine_diff_path *p;\n+\n+\tlen = strlen(path);\n+\tp = xmalloc(combine_diff_path_size(num_parent, len));\n+\tp->path = (char *) &(p->parent[num_parent]);\n+\tmemcpy(p->path, path, len);\n+\tp->path[len] = 0;\n+\tp->next = *pos;\n+\tmemset(p->parent, 0,\n+\t sizeof(p->parent[0]) * num_parent);\n+\n+\thashcpy(p->oid.hash, queue_item->two->sha1);\n+\tp->mode = queue_item->two->mode;\n+\thashcpy(p->parent[n].oid.hash, queue_item->one->sha1);\n+\tp->parent[n].mode = queue_item->one->mode;\n+\tp->parent[n].status = queue_item->status;\n+\t*pos = p;\n+}\n+\n static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n@@ -30,27 +52,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \n \tif (!n) {\n \t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tint len;\n-\t\t\tconst char *path;\n \t\t\tif (diff_unmodified_pair(q->queue[i]))\n \t\t\t\tcontinue;\n-\t\t\tpath = q->queue[i]->two->path;\n-\t\t\tlen = strlen(path);\n-\t\t\tp = xmalloc(combine_diff_path_size(num_parent, len));\n-\t\t\tp->path = (char *) &(p->parent[num_parent]);\n-\t\t\tmemcpy(p->path, path, len);\n-\t\t\tp->path[len] = 0;\n-\t\t\tp->next = NULL;\n-\t\t\tmemset(p->parent, 0,\n-\t\t\t sizeof(p->parent[0]) * num_parent);\n-\n-\t\t\thashcpy(p->oid.hash, q->queue[i]->two->sha1);\n-\t\t\tp->mode = q->queue[i]->two->mode;\n-\t\t\thashcpy(p->parent[n].oid.hash, q->queue[i]->one->sha1);\n-\t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n-\t\t\tp->parent[n].status = q->queue[i]->status;\n-\t\t\t*tail = p;\n-\t\t\ttail = &p->next;\n+\t\t\tinsert_path(tail, q->queue[i]->two->path, n, num_parent, q->queue[i]);\n+\t\t\ttail = &(*tail)->next;\n \t\t}\n \t\treturn curr;\n \t}\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258924","messageId":"1428076716-4449-4-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428076716-4449-1-git-send-email-max@max630.net","subject":"[PATCH v2 3/4] diff --cc: relax too strict paths picking","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-03T15:58:35Z","receivedAt":"2015-04-03T15:58:35Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"For diff --cc, paths fitering used to select only paths which have\nchanged in all parents, while diffing itself output hunks which are\nchanged in as few as 2 parents.\n\nFix intersect_paths() to add paths which have at least 2 changed\nparents. In this new functionality does not require special treatment of\nfirst pass, because path can be added from any parent, not just the\nfirst one.\n\nSigned-off-by: Max Kirillov \n---\n combine-diff.c | 56 ++++++++++++++++-------\n t/t4059-diff-cc-not-affected-by-path-filtering.sh | 4 +-\n 2 files changed, 41 insertions(+), 19 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex a236bb5..2285c7c 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -25,6 +25,7 @@ static int compare_paths(const struct combine_diff_path *one,\n static void insert_path(struct combine_diff_path **pos, const char* path, int n, int num_parent, struct diff_filepair *queue_item)\n {\n \tint len;\n+\tint parent_idx;\n \tstruct combine_diff_path *p;\n \n \tlen = strlen(path);\n@@ -41,43 +42,64 @@ static void insert_path(struct combine_diff_path **pos, const char* path, int n,\n \thashcpy(p->parent[n].oid.hash, queue_item->one->sha1);\n \tp->parent[n].mode = queue_item->one->mode;\n \tp->parent[n].status = queue_item->status;\n+\tfor (parent_idx = 0; parent_idx < n; parent_idx++) {\n+\t\thashcpy(p->parent[parent_idx].oid.hash, p->oid.hash);\n+\t\tp->parent[parent_idx].mode = p->mode;\n+\t\tp->parent[parent_idx].status = ' ';\n+\t}\n \t*pos = p;\n }\n \n+static int changed_parents(struct combine_diff_path *p, int n)\n+{\n+\tint parent_idx;\n+\tint result = 0;\n+\n+\tfor (parent_idx = 0; parent_idx < n; parent_idx++) {\n+\t\tif (p->parent[parent_idx].status != ' ')\n+\t\t\tresult++;\n+\t}\n+\n+\treturn result;\n+}\n+\n static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n \tint i, cmp;\n \n-\tif (!n) {\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tif (diff_unmodified_pair(q->queue[i]))\n-\t\t\t\tcontinue;\n-\t\t\tinsert_path(tail, q->queue[i]->two->path, n, num_parent, q->queue[i]);\n-\t\t\ttail = &(*tail)->next;\n-\t\t}\n-\t\treturn curr;\n-\t}\n-\n \t/*\n \t * paths in curr (linked list) and q->queue[] (array) are\n \t * both sorted in the tree order.\n \t */\n \ti = 0;\n-\twhile ((p = *tail) != NULL) {\n-\t\tcmp = ((i >= q->nr)\n-\t\t ? -1 : compare_paths(p, q->queue[i]->two));\n+\twhile ((p = *tail) != NULL || i < q->nr) {\n+\t\tcmp = (i >= q->nr) ? -1\n+\t\t : (p == NULL) ? 1\n+\t\t : compare_paths(p, q->queue[i]->two);\n \n \t\tif (cmp < 0) {\n-\t\t\t/* p->path not in q->queue[]; drop it */\n-\t\t\t*tail = p->next;\n-\t\t\tfree(p);\n+\t\t\t/* p->path not in q->queue[] */\n+\t\t\tif (num_parent > 2 && 2 - changed_parents(p, n) <= num_parent - n - 1) {\n+\t\t\t\t/* still can get 2 changed parents */\n+\t\t\t\thashcpy(p->parent[n].oid.hash, p->oid.hash);\n+\t\t\t\tp->parent[n].mode = p->mode;\n+\t\t\t\tp->parent[n].status = ' ';\n+\t\t\t\ttail = &p->next;\n+\t\t\t} else {\n+\t\t\t\t*tail = p->next;\n+\t\t\t\tfree(p);\n+\t\t\t}\n \t\t\tcontinue;\n \t\t}\n \n \t\tif (cmp > 0) {\n-\t\t\t/* q->queue[i] not in p->path; skip it */\n+\t\t\t/* q->queue[i] not in p->path */\n+\t\t\tif (1 <= num_parent - n - 1) {\n+\t\t\t\tinsert_path(tail, q->queue[i]->two->path, n, num_parent, q->queue[i]);\n+\t\t\t\ttail = &(*tail)->next;\n+\t\t\t}\n \t\t\ti++;\n \t\t\tcontinue;\n \t\t}\ndiff --git a/t/t4059-diff-cc-not-affected-by-path-filtering.sh b/t/t4059-diff-cc-not-affected-by-path-filtering.sh\nindex 3e6e59b..ab3dbd2 100755\n--- a/t/t4059-diff-cc-not-affected-by-path-filtering.sh\n+++ b/t/t4059-diff-cc-not-affected-by-path-filtering.sh\n@@ -95,12 +95,12 @@ test_expect_success 'diff with mergebase shows discarded change from parent 1 in\n \ttest -s actual\n '\n \n-test_expect_failure 'diff with mergebase shows fully discarded file from parent 2' '\n+test_expect_success 'diff with mergebase shows fully discarded file from parent 2' '\n \tgit diff --cc merge branch1 branch2 mergebase -- short/win1 >actual &&\n \ttest -s actual\n '\n \n-test_expect_failure 'diff with mergebase shows fully discarded file from parent 1' '\n+test_expect_success 'diff with mergebase shows fully discarded file from parent 1' '\n \tgit diff --cc merge branch1 branch2 mergebase -- short/win2 >actual &&\n \ttest -s actual\n '\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258925","messageId":"1428076716-4449-5-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"1428076716-4449-1-git-send-email-max@max630.net","subject":"[PATCH v2 4/4] t4059: rewrite to be adaptive to hunk filtering","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-03T15:58:36Z","receivedAt":"2015-04-03T15:58:36Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Looks like there is no exact specification what `diff -c` and\n`diff --cc` should output. Considering this it is not reasonable to\nhardcode any specific output in test. Rather, it should verify that file\nselection behaves the same as hunk selection.\n\nRewrite test so that it makes sure that a \"short\" file is shown if and\nonly if the corresponding \"long\" file's contains the changed hunk.\n\nSigned-off-by: Max Kirillov \n---\n t/t4059-diff-cc-not-affected-by-path-filtering.sh | 84 ++++++++++++++++-------\n 1 file changed, 59 insertions(+), 25 deletions(-)\n\ndiff --git a/t/t4059-diff-cc-not-affected-by-path-filtering.sh b/t/t4059-diff-cc-not-affected-by-path-filtering.sh\nindex ab3dbd2..f4232b0 100755\n--- a/t/t4059-diff-cc-not-affected-by-path-filtering.sh\n+++ b/t/t4059-diff-cc-not-affected-by-path-filtering.sh\n@@ -6,11 +6,20 @@ test_description='combined diff filtering is not affected by preliminary path fi\n # spot changes which were discarded during conflict resolution.\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/diff-lib.sh\n \n+# history is:\n+# (mergebase) --> (branch1) --\\\n+# | V\n+# \\ --> (branch2) ----------(merge)\n+# there are files in 2 subdirectories, \"long\" and \"short\"\n+# each file in \"long\" subdirecty has exactly same history as same file in \"short\" one,\n+# but it has added lines with changes in both branches which merge without conflict\n+# so the long files are always selected at path filtering\n test_expect_success setup '\n \tmkdir short &&\n \tmkdir long &&\n-\tfor fn in win1 win2 merge delete base only1 only2\n+\tfor fn in win1 win2 merge delete base only1 only2 only1discard only2discard\n \tdo\n \t\ttest_seq 3 >short/$fn &&\n \t\tgit add short/$fn &&\n@@ -29,9 +38,12 @@ test_expect_success setup '\n \t\t\tgit add $dir/$fn || return $?\n \t\tdone || return $?\n \tdone &&\n-\tsed -e \"s/^7/7change1/\" long/only2 >sed.new &&\n-\tmv sed.new long/only2 &&\n-\tgit add long/only2 &&\n+\tfor fn in only2 only2discard\n+\tdo\n+\t sed -e \"s/^7/7change1/\" long/$fn >sed.new &&\n+\t mv sed.new long/$fn &&\n+\t git add long/$fn || return $?\n+\tdone &&\n \tgit commit -m branch1 &&\n \tgit branch branch1 &&\n \n@@ -45,9 +57,12 @@ test_expect_success setup '\n \t\t\tgit add $dir/$fn || return $?\n \t\tdone || return $?\n \tdone &&\n-\tsed -e \"s/^11/11change2/\" long/only1 >sed.new &&\n-\tmv sed.new long/only1 &&\n-\tgit add long/only1 &&\n+\tfor fn in only1 only1discard\n+\tdo\n+\t sed -e \"s/^11/11change2/\" long/$fn >sed.new &&\n+\t mv sed.new long/$fn &&\n+\t git add long/$fn || return $?\n+\tdone &&\n \tgit commit -m branch2 &&\n \tgit branch branch2 &&\n \n@@ -55,6 +70,10 @@ test_expect_success setup '\n \tgit checkout mergebase -- . &&\n \ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/base &&\n \tgit add long/base &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/only1discard &&\n+\tgit add long/only1discard &&\n+\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/only2discard &&\n+\tgit add long/only2discard &&\n \ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change1/\" >long/win1 &&\n \tgit add long/win1 &&\n \ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e \"s/^2/2change2/\" >long/win2 &&\n@@ -69,6 +88,10 @@ test_expect_success setup '\n \tgit add long/only2 &&\n \ttest_seq 3 >short/base &&\n \tgit add short/base &&\n+\ttest_seq 3 >short/only1discard &&\n+\tgit add short/only1discard &&\n+\ttest_seq 3 >short/only2discard &&\n+\tgit add short/only2discard &&\n \ttest_seq 3 | sed -e \"s/^2/2change1/\" >short/win1 &&\n \tgit add short/win1 &&\n \ttest_seq 3 | sed -e \"s/^2/2change2/\" >short/win2 &&\n@@ -85,24 +108,35 @@ test_expect_success setup '\n \tgit branch merge\n '\n \n-test_expect_success 'diff with mergebase shows discarded change from parent 2 in merged file' '\n-\tgit diff --cc merge branch1 branch2 mergebase -- long/win1 >actual &&\n-\ttest -s actual\n-'\n-\n-test_expect_success 'diff with mergebase shows discarded change from parent 1 in merged file' '\n-\tgit diff --cc merge branch1 branch2 mergebase -- long/win2 >actual &&\n-\ttest -s actual\n-'\n+# the difference in short file must be returned if and only if it is shown in long file\n+for fn in win1 win2 merge delete base only1 only2 only1discard only2discard; do\n+\tif git diff --cc merge branch1 branch2 mergebase -- long/$fn | grep -q '^[ +-]\\{3\\}2\\(change[12]|merge\\)\\?$'\n+\tthen\n+\t\ttest_expect_success \"diff --cc contains short/$fn\" '\n+\t\t\tgit diff --cc merge branch1 branch2 mergebase -- short/'\"$fn\"' >actual &&\n+\t\t\ttest -s actual\n+\t\t'\n+\telse\n+\t\ttest_expect_success \"diff --cc does not contain short/$fn\" '\n+\t\t\tgit diff --cc merge branch1 branch2 mergebase -- short/'\"$fn\"' >actual &&\n+\t\t\t! test -s actual\n+\t\t'\n+\tfi\n+done\n \n-test_expect_success 'diff with mergebase shows fully discarded file from parent 2' '\n-\tgit diff --cc merge branch1 branch2 mergebase -- short/win1 >actual &&\n-\ttest -s actual\n-'\n-\n-test_expect_success 'diff with mergebase shows fully discarded file from parent 1' '\n-\tgit diff --cc merge branch1 branch2 mergebase -- short/win2 >actual &&\n-\ttest -s actual\n-'\n+for fn in win1 win2 merge delete base only1 only2 only1discard only2discard; do\n+\tif git diff -c merge branch1 branch2 mergebase -- long/$fn | grep -q '^[ +-]\\{3\\}2\\(change[12]|merge\\)\\?$'\n+\tthen\n+\t\ttest_expect_success \"diff -c contains short/$fn\" '\n+\t\t\tgit diff -c merge branch1 branch2 mergebase -- short/'\"$fn\"' >actual &&\n+\t\t\ttest -s actual\n+\t\t'\n+\telse\n+\t\ttest_expect_success \"diff -c does not contain short/$fn\" '\n+\t\t\tgit diff -c merge branch1 branch2 mergebase -- short/'\"$fn\"' >actual &&\n+\t\t\t! test -s actual\n+\t\t'\n+\tfi\n+done\n \n test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258929","messageId":"20150403160333.GD8771@wheezy.local","threadId":"38984","inReplyTo":"xmqqy4mai71d.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/4] Add test for showing discarded changes with diff --cc","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-03T16:03:33Z","receivedAt":"2015-04-03T16:03:33Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Thu, Apr 02, 2015 at 01:55:58PM -0700, Junio C Hamano wrote:\n> So I'll give a preliminary nitpicks first ;-)\n\nThank you; fixed the issues you mentioned.\n\n>> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n>> \"s/^2/2change2/\" >long/only2 &&\n>> +\tgit add long/only2 &&\n> \n> Patch is line-wrapped around here?\n\nI hope it was you email program which wrapped it (maybe at\nquoting). It looks ok in my mutt and in source, and I uses\nsame git format-patch+git send-email as usual.\n\n-- \nMax\n"},{"id":"258931","messageId":"20150403162945.GE8771@wheezy.local","threadId":"38984","inReplyTo":"20150402211310.GA22988@peff.net","subject":"Re: [PATCH 0/4] diff --cc: relax path filtering","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-03T16:29:45Z","receivedAt":"2015-04-03T16:29:45Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Thu, Apr 02, 2015 at 05:13:10PM -0400, Jeff King wrote:\n> On Thu, Apr 02, 2015 at 11:34:09PM +0300, Max Kirillov wrote:\n> \n>> For diff --cc, paths fitering used to select only paths which have\n>> changed in all parents, while diffing itself output hunks which are\n>> changed in as few as 2 parents.\n> \n> I'm confused about \"used to\" here. Is this a regression due to the\n> combine-diff rewriting that happened in 2.0, or do you just mean \"before\n> this patch series, we used to do this other thing\".\n\nAs far as I can see it was \"always\", at least since 1.8.0;\nthe test script did not work before that.\n\n>> Fix intersect_paths() to add paths which have at least 2 changed\n>> parents.\n> \n> I'd worry a little that this is increasing the cost to do \"log --cc\", as\n> it means we will have to open and look at extra files, and we may find\n> in many cases that there aren't any interesting hunks. Which would imply\n> we might want to put it behind a flag, rather than as the default\n> (\"--cc-me-harder\").\n> \n> But if I'm understanding the issue correctly, this should only matter\n> for octopus merges. That is, the old rule for looking at a path was \"is\n> there at least one parent whose content we took verbatim\", but the new\n> one is \"are there are at least 2 parents whose content we did not take\n> verbatim\". With only two parents, those would be the same thing, I\n> think.\n\nYes, I hope so. I tried to reproduce benchamrk which is in 8518ff8fabc\n(git log --raw --no-abbrev --no-renames (-c|--cc) v3.10..v3.11),\nand saw no difference. But my times was about 3 seconds, not\n20 as there, andI cannot say my computer is very fast, so\nprobably I've done something wrong.\n\n-- \nMax\n"},{"id":"259244","messageId":"xmqqmw2ea0t1.fsf@gitster.dls.corp.google.com","threadId":"38984","inReplyTo":"1428076716-4449-2-git-send-email-max@max630.net","subject":"Re: [PATCH v2 1/4] t4059: test 'diff --cc' with a change from only few parents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-11T20:04:58Z","receivedAt":"2015-04-11T20:04:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> If `git diff --cc` is used with 2 or more parents, then\n> it shows all hunks which have changed compared to at least 2 parents.\n> Which is reasonable, because those places are likely places for\n> conflicts, and it should be displayed how they were resolved.\n> But, preliminary path filtering passes a path only it was changed\n> compared to each parent. So, if a hunk which has not changed compared to\n> some of parents is shown if there are more changed hunks in the file,\n> but not shown if it is the only change.\n>\n> This looks inconsistent and for some scenarios it is desirable to show\n> such changes.\n>\n> Add the test which demonstrates the issue.\n>\n> Signed-off-by: Max Kirillov \n> ---\n> t/t4059-diff-cc-not-affected-by-path-filtering.sh | 108 ++++++++++++++++++++++\n> 1 file changed, 108 insertions(+)\n> create mode 100755 t/t4059-diff-cc-not-affected-by-path-filtering.sh\n>\n> diff --git a/t/t4059-diff-cc-not-affected-by-path-filtering.sh\n> b/t/t4059-diff-cc-not-affected-by-path-filtering.sh\n> new file mode 100755\n> index 0000000..3e6e59b\n> --- /dev/null\n> +++ b/t/t4059-diff-cc-not-affected-by-path-filtering.sh\n> @@ -0,0 +1,108 @@\n...\n> +\ttest_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" >long/base &&\n> +\tgit add long/base &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2change1/\" >long/win1 &&\n> +\tgit add long/win1 &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2change2/\" >long/win2 &&\n> +\tgit add long/win2 &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2merged/\" >long/merge &&\n> +\tgit add long/merge &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"/^2/d\" >long/delete &&\n> +\tgit add long/delete &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2change1/\" >long/only1 &&\n> +\tgit add long/only1 &&\n> + test_seq 11 | sed -e \"s/^7/7change1/\" -e \"s/^11/11change2/\" -e\n> \"s/^2/2change2/\" >long/only2 &&\n\nHmph. Is it gmane who is munging these lines?\n\nThe other copy of this message I received in my mbox (which I read\nin the same MUA) does not seem to have this corruption, and I do not\nexpect vger.kernel.org to do this kind of munging without getting\nyelled at by the kernel folks.\n\nAnyway, thanks; I'll take a deeper look once I got a chance to.\n"},{"id":"259245","messageId":"xmqqfv865snv.fsf@gitster.dls.corp.google.com","threadId":"38984","inReplyTo":"1428076716-4449-3-git-send-email-max@max630.net","subject":"Re: [PATCH v2 2/4] combine-diff.c: refactor: extract insert_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-11T20:14:28Z","receivedAt":"2015-04-11T20:14:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> Signed-off-by: Max Kirillov \n> ---\n> combine-diff.c | 43 ++++++++++++++++++++++++-------------------\n> 1 file changed, 24 insertions(+), 19 deletions(-)\n>\n> diff --git a/combine-diff.c b/combine-diff.c\n> index 8eb7278..a236bb5 100644\n> --- a/combine-diff.c\n> +++ b/combine-diff.c\n> @@ -22,6 +22,28 @@ static int compare_paths(const struct combine_diff_path *one,\n> \t\t\t\t two->path, strlen(two->path), two->mode);\n> }\n> \n> +static void insert_path(struct combine_diff_path **pos, const char* path, int n, int num_parent, struct diff_filepair *queue_item)\n\nThis is overlong (and I wonder why I am not seeing the linewrap\nproblem with this one). I'd suggest to split them perhaps in the\nsame way that compare_paths() match_string_spaces() and other\nexisting functions are defined?\n\nAlso the pointer-asterisk sticks to the variable name, not type,\ni.e.\n\n\tstatic void insert_path(struct combine_diff_path **pos, const char *path,\n\t\t\t\tint n, int num_parent,\n struct diff_filepair *queue_item)\n\n\n> +{\n"},{"id":"259246","messageId":"xmqqa8ye5q7m.fsf@gitster.dls.corp.google.com","threadId":"38984","inReplyTo":"1428076716-4449-2-git-send-email-max@max630.net","subject":"Re: [PATCH v2 1/4] t4059: test 'diff --cc' with a change from only few parents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-11T21:07:25Z","receivedAt":"2015-04-11T21:07:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> If `git diff --cc` is used with 2 or more parents, then it shows\n> all hunks which have changed compared to at least 2 parents.\n> Which is reasonable, because those places are likely places for\n> conflicts, and it should be displayed how they were resolved.\n\nOK.\n\n> But, preliminary path filtering passes a path only it was changed\n> compared to each parent.\n\nThat is true, but I am a bit confused by the above, especially the\nword \"But\" that begins the sentence. Are you talking about this\ncomment that describes what the caller wants to do?\n\n /* find set of paths that every parent touches */\n static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,\n const struct sha1_array *parents, struct diff_options *opt)\n\nWhen the result of a merge exactly matches one (or more) of the\nparent of the merge, we do not want to show it in the combined\nformat, so intersect_paths() does want to find paths that are\ndifferent from all parents. Isn't that a good thing?\n\n> So, if a hunk which has not changed compared to some of parents is\n> shown if there are more changed hunks in the file, but not shown\n> if it is the only change.\n>\n> This looks inconsistent and for some scenarios it is desirable to show\n> such changes.\n\nHmm, that may be true. So help me see if I understand your goal by\nchecking if I rephrased you correctly below:\n\n - We want to show a combined hunk when the number of parent\n variants for the hunk is more than 2 (i.e. interesting octopus)\n or the result does not match any of the parents (i.e. conflict\n resolution of a pairwise merge). We want to drop a hunk whose\n result came from only one set of parent and the other parents had\n the same original that is different from the result.\n\n - The current code filters out a path that matches one of the\n parents very early. This is OK for a two-way merge. If the\n result matches one of the parent's, then any hunk we might\n produce by not pre-filtering would have the result that came from\n one parent (i.e. the one identical to the result) and there is\n only one other parent, which cannot make it an interesting\n octopus by definition.\n\n But an octopus may merge three variants and pick the result from\n one of the parents as a whole. With the pre-filtering, no hunk\n from such a path is shown, even when the other two variants in\n \"discarded\" parents are not identical.\n\nThe original to refer to are two commits bf1c32bd (combine-diff:\nupdate --cc \"uninteresting hunks\" logic., 2006-02-02) and fd4b1d21\n(combine-diff: add safety check to --cc., 2006-02-02).\n\nEspecially, we need to pay close attention to the discussion that\ngerminated the current behaviour:\n\n http://thread.gmane.org/gmane.comp.version-control.git/15486/focus=15519\n\nI recall that the \"diff --cc\" before that change was not discarding\nuninteresting merges sufficiently and the two commits were a\ndeliberate attempt to reject what your series wants to show as\nuninteresting hunks. \n\nTwo suggestions.\n\n - This is primarily for 2/4, but can we make it more clear in the\n code that we do this \"include more\" change only on Octopus\n merges? This change should not make any difference for two-way\n merges and I'd prefer to avoid extra processing of finding\n matching hunks and combining, only to discard the result.\n\n - Can you run \"diff --cc\" with and without your patches to the\n \"merge from hell\" commit mentioned in the original thread and see\n if we show more hunks with your patches, and make sure what are\n shown additionally looked really \"interesting\"?\n\nThanks.\n"},{"id":"259247","messageId":"CAPc5daVkhEGQRhCjVpV4wh+cNyfz5yFrAaBrc9re0YM=rtSw+g@mail.gmail.com","threadId":"38984","inReplyTo":"xmqqa8ye5q7m.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/4] t4059: test 'diff --cc' with a change from only few parents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-11T21:20:03Z","receivedAt":"2015-04-11T21:20:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Sat, Apr 11, 2015 at 2:07 PM, Junio C Hamano wrote:\n> Especially, we need to pay close attention to the discussion that\n> germinated the current behaviour:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/15486/focus=15519\n>\n> I recall that the \"diff --cc\" before that change was not discarding\n> uninteresting merges sufficiently and the two commits were a\n> deliberate attempt to reject what your series wants to show as\n> uninteresting hunks.\n>\n> Two suggestions.\n>\n> - This is primarily for 2/4, but can we make it more clear in the\n\nEh, sorry, I obviously meant 3/4. And let me half-retract this suggestion\nand put it on hold; the code-flow has become sufficiently different that\nspecial-casing two-way merge may not be worth it after all.\n\n> code that we do this \"include more\" change only on Octopus\n> merges? This change should not make any difference for two-way\n> merges and I'd prefer to avoid extra processing of finding\n> matching hunks and combining, only to discard the result.\n>\n> - Can you run \"diff --cc\" with and without your patches to the\n> \"merge from hell\" commit mentioned in the original thread and see\n> if we show more hunks with your patches, and make sure what are\n> shown additionally looked really \"interesting\"?\n\nThis one still stands. We'd be interested to see what difference this\nmakes in the real world project history.\n\nThanks.\n"},{"id":"259249","messageId":"20150412054332.GA28555@wheezy.local","threadId":"38984","inReplyTo":"xmqqa8ye5q7m.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/4] t4059: test 'diff --cc' with a change from only few parents","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-12T05:43:32Z","receivedAt":"2015-04-12T05:43:32Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Sat, Apr 11, 2015 at 02:07:25PM -0700, Junio C Hamano wrote:\n> Max Kirillov writes:\n> \n>> If `git diff --cc` is used with 2 or more parents, then it shows\n>> all hunks which have changed compared to at least 2 parents.\n>> Which is reasonable, because those places are likely places for\n>> conflicts, and it should be displayed how they were resolved.\n> \n> OK.\n> \n>> But, preliminary path filtering passes a path only it was changed\n>> compared to each parent.\n> \n> That is true, but I am a bit confused by the above, especially the\n> word \"But\" that begins the sentence. Are you talking about this\n> comment that describes what the caller wants to do?\n> \n> /* find set of paths that every parent touches */\n> static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,\n> const struct sha1_array *parents, struct diff_options *opt)\n> \n> When the result of a merge exactly matches one (or more) of the\n> parent of the merge, we do not want to show it in the combined\n> format, so intersect_paths() does want to find paths that are\n> different from all parents. Isn't that a good thing?\n\nIf it is a good thing, then probably for file which has not\nbeen filtered out it is also a good thing, then diff --cc\nshould have been hiding hunks which fully preserve one of\nthe parent.\n\n>> So, if a hunk which has not changed compared to some of parents is\n>> shown if there are more changed hunks in the file, but not shown\n>> if it is the only change.\n>>\n>> This looks inconsistent and for some scenarios it is desirable to show\n>> such changes.\n> \n> Hmm, that may be true. So help me see if I understand your goal by\n> checking if I rephrased you correctly below:\n> \n> - We want to show a combined hunk when the number of parent\n> variants for the hunk is more than 2 (i.e. interesting octopus)\n> or the result does not match any of the parents (i.e. conflict\n> resolution of a pairwise merge). We want to drop a hunk whose\n> result came from only one set of parent and the other parents had\n> the same original that is different from the result.\n> \n> - The current code filters out a path that matches one of the\n> parents very early. This is OK for a two-way merge. If the\n> result matches one of the parent's, then any hunk we might\n> produce by not pre-filtering would have the result that came from\n> one parent (i.e. the one identical to the result) and there is\n> only one other parent, which cannot make it an interesting\n> octopus by definition.\n> \n> But an octopus may merge three variants and pick the result from\n> one of the parents as a whole. With the pre-filtering, no hunk\n> from such a path is shown, even when the other two variants in\n> \"discarded\" parents are not identical.\n\nYes, I meant that.\n\n> \n> The original to refer to are two commits bf1c32bd (combine-diff:\n> update --cc \"uninteresting hunks\" logic., 2006-02-02) and fd4b1d21\n> (combine-diff: add safety check to --cc., 2006-02-02).\n> \n> Especially, we need to pay close attention to the discussion that\n> germinated the current behaviour:\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/15486/focus=15519\n> \n> I recall that the \"diff --cc\" before that change was not discarding\n> uninteresting merges sufficiently and the two commits were a\n> deliberate attempt to reject what your series wants to show as\n> uninteresting hunks. \n> \n> Two suggestions.\n> \n> - This is primarily for 2/4, but can we make it more clear in the\n> code that we do this \"include more\" change only on Octopus\n> merges? This change should not make any difference for two-way\n> merges and I'd prefer to avoid extra processing of finding\n> matching hunks and combining, only to discard the result.\n> \n> - Can you run \"diff --cc\" with and without your patches to the\n> \"merge from hell\" commit mentioned in the original thread and see\n> if we show more hunks with your patches, and make sure what are\n> shown additionally looked really \"interesting\"?\n\n\nThe diffs:\nolder version (without my patch):\nhttps://gist.githubusercontent.com/max630/70080d38e8e9951b58a4/raw/diffcc_old.txt\nnewer version:\nhttps://gist.githubusercontent.com/max630/70080d38e8e9951b58a4/raw/diffcc_new.txt\n\nThe outcome is quite different, yes.\n\nFirst, I can see that file removal/deletion are passed\nwithout filtering, even if file was only added/deleted\ncompared to some parents and not changed compared to any, so\nit's in some way \"2 versions\" diff which should not be\nshown. This I think is something should be fixed in addition\nto this series.\n\nOther than that, all other changes look \"interesting\",\nmeaning they contain more than 2 versions. Sometimes it's not\neasy to spot because a long hunk is almost inherited from\none parent with all others being the same, and only in\ncouple of lines there when they differ. But I always find\nsome difference.\n\nIt was also quite slow, 11 seconds compared to some 0.4.\nBut for 2-way merges speed did not change.\n\n\n\nMy exact case was that there was a change in one branch\nwhich was overwritten during merge conflict resolution by\nfully acepting the other branch - in a 2-parent merge. I\nstarted looking for a way to visualize such cases. They\nare not visible in usual diff, because they look same as\naccepting change compared to the unchange branch. To\nrecognize them you should consider the mergebase. I googled\nto your suggestion in [1], and prepared a trivial testcase\nfor such a merge - in contained a base commit, change in\nboth branches and children which fully repeat one of the\nbranches. But then I discovered that such merge is not shown\nin \"diff --cc\" because of the path filtering. So here am I.\n\nActually I think this is the very true rule which I would\nlike to be used: if there is a parent which is changed\ncompared to the merge base and child commit differs from\nthat parent. Because this is where there had been a valuable\nchange, and it disappeared. Linus in [2] wrote that those\nchange was fixing the same problem and it's good that they\nare not shown, by it my case there were fixing different\nproblems, they just happened to be in the same place, and\nafter merge the bug reappeared.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/191553/focus=191557\n[2] http://thread.gmane.org/gmane.comp.version-control.git/15486/focus=15519\n"},{"id":"259250","messageId":"xmqq61916gnt.fsf@gitster.dls.corp.google.com","threadId":"38984","inReplyTo":"1428076716-4449-5-git-send-email-max@max630.net","subject":"Re: [PATCH v2 4/4] t4059: rewrite to be adaptive to hunk filtering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-12T05:48:22Z","receivedAt":"2015-04-12T05:48:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> Looks like there is no exact specification what `diff -c` and\n> `diff --cc` should output. Considering this it is not reasonable to\n> hardcode any specific output in test.\n\nOK.\n\n> Rather, it should verify that file\n> selection behaves the same as hunk selection.\n\nHrm, really? \"git diff --raw\" and \"git diff -p -w\" on two trees\nwould not show identical set of paths, when the blob object are\ndifferent byte-wise but are equivalent at the content level per\ngiven definition of equivalence (e.g. \"-w\"). Given that --cc is\nto look at the differences at hunk/content level to combine and omit\nuninteresting ones from the output, relative to -c output, I would\nimagine that the file selection and the hunk selection are expected\nto behave differently.\n\nSo, having said that I am a bit skeptical about the description of\nthe goals, there are a few nits on the implementation, too.\n\n> +# the difference in short file must be returned if and only if it is shown in long file\n> +for fn in win1 win2 merge delete base only1 only2 only1discard only2discard; do\n> +\tif git diff --cc merge branch1 branch2 mergebase -- long/$fn | grep -q '^[ +-]\\{3\\}2\\(change[12]|merge\\)\\?$'\n> +\tthen\n\nJust like the earlier parts of this patch, write a newline when you\ndo not have to write a semicolon, and split lines after pipe when\nyour pipeline gets long, i.e.\n\n\tfor fn in win2 win2 ...\n do\n\t\tif git diff --cc ... |\n\t\t\tgrep 'pattern'\n\t\tthen\n\nDo I smell some GNUism in your \"grep\" patterns?\n\nYou have\n\n '^[ +-]\\{3\\}2\\(change[12]|merge\\)\\?$'\n\nbut matching zero-or-one repetition with ? is not in BRE, and \\? to\nuse it in BRE is a GNU extension.\n\nAlso in BRE , '|' is not an alternation (and making it into such\nwith '\\|' in BRE is a GNU extension IIRC.\n\nWorse, you are not using backslash here to invoke GNU extension, so\nI suspect the grep invocations in the patch may not be working as\nyou expect.\n\nDoubly worse, what is shown by \"diff -c\" for line 2 seems to be\nthree [- +] columns, followed by one of\n\n 2\n 2change1\n 2change2\n 2merged\n\nfollowed by the end of line, so your \"|merge\" part may never match.\n\nWe encourage people to avoid \\{m,n\\}, because, it is not supported\nby some implementations even though it is specified in POSIX for\nBRE.\n\nPerhaps spelling out what you are expecting a bit more explicitly,\ne.g.\n\n\tgrep -e \"^[-+ ][-+ ][-+ ]2$\" \\\n -e \"^[-+ ][-+ ][-+ ]2change[12]$\" \\\n -e \"^[-+ ][-+ ][-+ ]2merged$\"\n\nmight be a reasonable way to write this in a more portable and\nreadable way. And turn that into a helper shell function to make it\nmore readable and maintainable for a bonus point.\n\nThanks.\n"},{"id":"259251","messageId":"xmqq1tjp6gj5.fsf@gitster.dls.corp.google.com","threadId":"38984","inReplyTo":"20150412054332.GA28555@wheezy.local","subject":"Re: [PATCH v2 1/4] t4059: test 'diff --cc' with a change from only few parents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-12T05:51:10Z","receivedAt":"2015-04-12T05:51:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> My exact case was that there was a change in one branch\n> which was overwritten during merge conflict resolution by\n> fully acepting the other branch - in a 2-parent merge. I\n> started looking for a way to visualize such cases. They\n> are not visible in usual diff, because they look same as\n> accepting change compared to the unchange branch.\n\nHmph, isn't that exactly why \"diff -c\" exists, not \"diff --cc\"\nthat omits (usually) uninteresting hunks?\n"},{"id":"259323","messageId":"1428984553-11363-1-git-send-email-max@max630.net","threadId":"38984","inReplyTo":"20150412054332.GA28555@wheezy.local","subject":"[PATCH/RFC] combine-diff.c: make intersect_paths() behave like hunk filtering","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-14T04:09:13Z","receivedAt":"2015-04-14T04:09:13Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"* for `diff --cc` 2 cases:\n * the path must be changed since at least 2 parents, which\n should have the path different. In other words, the child and its\n parents must contain at least 3 different versions of file.\n Non-existing in the commit path counts as one version.\n * All parents are the same, but child commit differs from them.\n* for `diff -c`: the path must be changed since at least 1 parent.\n\nSigned-off-by: Max Kirillov \n---\nThis what could be done to hide the added and removed files.\nIt also makes it work faster - diff --cc on the evil merge runs now 2.5 seconds\ninstead of 11 (or 0.4 without the series).\n\nThis also fails t4057, but I really don't see the logic of `diff -c`\nothwerwise. It should show all changes, with trivial merges also, and it\nis shown by long files in my t4059. But it uses same path filtering as\n`diff --cc`, which is even more restrictive.\n combine-diff.c | 91 ++++++++++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 66 insertions(+), 25 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 2285c7c..f44032a 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -50,20 +50,49 @@ static void insert_path(struct combine_diff_path **pos, const char* path, int n,\n \t*pos = p;\n }\n \n-static int changed_parents(struct combine_diff_path *p, int n)\n+static int path_not_interesting(struct combine_diff_path *p, int n,\n+\t\t\t\tstruct diff_filespec *new_parent)\n {\n \tint parent_idx;\n-\tint result = 0;\n+\tstruct object_id first_parent;\n+\tint found_first = 0;\n+\tint found_same_parent = 0;\n \n \tfor (parent_idx = 0; parent_idx < n; parent_idx++) {\n-\t\tif (p->parent[parent_idx].status != ' ')\n-\t\t\tresult++;\n+\t\tif (p->parent[parent_idx].status != ' ') {\n+\t\t\tif (found_first) {\n+\t\t\t\tif (hashcmp(p->parent[parent_idx].oid.hash, first_parent.hash)) {\n+\t\t\t\t\t/* found second different unique parent - non-trivial merge */\n+\t\t\t\t\treturn 0;\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\tfound_first = 1;\n+\t\t\t\thashcpy(first_parent.hash,\n+\t\t\t\t\tp->parent[parent_idx].oid.hash);\n+\t\t\t}\n+\t\t} else {\n+\t\t\t/* the new commit repeats some of parents */\n+\t\t\tfound_same_parent = 1;\n+\t\t}\n \t}\n \n-\treturn result;\n+\tif (new_parent) {\n+\t\tif (hashcmp(p->oid.hash, new_parent->sha1)) {\n+\t\t\tif (!found_same_parent || hashcmp(first_parent.hash, new_parent->sha1)) {\n+\t\t\t\treturn 0;\n+\t\t\t} else {\n+\t\t\t\treturn 1;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tfound_same_parent = 1;\n+\t\t}\n+\t}\n+\n+\treturn found_same_parent;\n }\n \n-static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n+static struct combine_diff_path *intersect_paths(\n+\tstruct combine_diff_path *curr, int n, int num_parent, int dense)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n@@ -81,35 +110,46 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \n \t\tif (cmp < 0) {\n \t\t\t/* p->path not in q->queue[] */\n-\t\t\tif (num_parent > 2 && 2 - changed_parents(p, n) <= num_parent - n - 1) {\n-\t\t\t\t/* still can get 2 changed parents */\n+\t\t\tif (dense &&\n+\t\t\t n == num_parent - 1 &&\n+\t\t\t path_not_interesting(p, n, NULL)) {\n+\t\t\t\t/* only 1 unique different parent\n+\t\t\t\t not interesting change */\n+\t\t\t\t*tail = p->next;\n+\t\t\t\tfree(p);\n+\t\t\t} else {\n+\t\t\t\t/* already has or still can get 2 changed parents */\n \t\t\t\thashcpy(p->parent[n].oid.hash, p->oid.hash);\n \t\t\t\tp->parent[n].mode = p->mode;\n \t\t\t\tp->parent[n].status = ' ';\n \t\t\t\ttail = &p->next;\n-\t\t\t} else {\n-\t\t\t\t*tail = p->next;\n-\t\t\t\tfree(p);\n \t\t\t}\n \t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (cmp > 0) {\n+\t\t} else if (cmp > 0) {\n \t\t\t/* q->queue[i] not in p->path */\n-\t\t\tif (1 <= num_parent - n - 1) {\n-\t\t\t\tinsert_path(tail, q->queue[i]->two->path, n, num_parent, q->queue[i]);\n+\t\t\tif (!dense || n < num_parent - 1) {\n+\t\t\t\tinsert_path(tail, q->queue[i]->two->path,\n+\t\t\t\t\t n, num_parent, q->queue[i]);\n \t\t\t\ttail = &(*tail)->next;\n \t\t\t}\n \t\t\ti++;\n \t\t\tcontinue;\n-\t\t}\n+\t\t} else {\n+\t\t\tif (dense &&\n+\t\t\t n == num_parent - 1 &&\n+\t\t\t path_not_interesting(p, n, q->queue[i]->one)) {\n+\t\t\t\t*tail = p->next;\n+\t\t\t\tfree(p);\n+\t\t\t} else {\n+\t\t\t\thashcpy(p->parent[n].oid.hash, q->queue[i]->one->sha1);\n+\t\t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n+\t\t\t\tp->parent[n].status = q->queue[i]->status;\n \n-\t\thashcpy(p->parent[n].oid.hash, q->queue[i]->one->sha1);\n-\t\tp->parent[n].mode = q->queue[i]->one->mode;\n-\t\tp->parent[n].status = q->queue[i]->status;\n+\t\t\t\ttail = &p->next;\n+\t\t\t}\n+\t\t\ti++;\n+\t\t}\n \n-\t\ttail = &p->next;\n-\t\ti++;\n \t}\n \treturn curr;\n }\n@@ -1341,7 +1381,8 @@ static const char *path_path(void *obj)\n \n /* find set of paths that every parent touches */\n static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,\n-\tconst struct sha1_array *parents, struct diff_options *opt)\n+\tconst struct sha1_array *parents, struct diff_options *opt,\n+\tint dense)\n {\n \tstruct combine_diff_path *paths = NULL;\n \tint i, num_parent = parents->nr;\n@@ -1367,7 +1408,7 @@ static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,\n \t\t\topt->output_format = DIFF_FORMAT_NO_OUTPUT;\n \t\tdiff_tree_sha1(parents->sha1[i], sha1, \"\", opt);\n \t\tdiffcore_std(opt);\n-\t\tpaths = intersect_paths(paths, i, num_parent);\n+\t\tpaths = intersect_paths(paths, i, num_parent, dense);\n \n \t\t/* if showing diff, show it in requested order */\n \t\tif (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n@@ -1477,7 +1518,7 @@ void diff_tree_combined(const unsigned char *sha1,\n \t\t * diff(sha1,parent_i) for all i to do the job, specifically\n \t\t * for parent0.\n \t\t */\n-\t\tpaths = find_paths_generic(sha1, parents, &diffopts);\n+\t\tpaths = find_paths_generic(sha1, parents, &diffopts, dense);\n \t}\n \telse {\n \t\tint stat_opt;\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"259324","messageId":"20150414041849.GB786@wheezy.local","threadId":"38984","inReplyTo":"xmqq61916gnt.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 4/4] t4059: rewrite to be adaptive to hunk filtering","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-14T04:18:49Z","receivedAt":"2015-04-14T04:18:49Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Sat, Apr 11, 2015 at 10:48:22PM -0700, Junio C Hamano wrote:\n> Max Kirillov writes:\n>> Rather, it should verify that file\n>> selection behaves the same as hunk selection.\n> \n> Hrm, really? \"git diff --raw\" and \"git diff -p -w\" on two trees\n> would not show identical set of paths, when the blob object are\n> different byte-wise but are equivalent at the content level per\n> given definition of equivalence (e.g. \"-w\"). Given that --cc is\n> to look at the differences at hunk/content level to combine and omit\n> uninteresting ones from the output, relative to -c output, I would\n> imagine that the file selection and the hunk selection are expected\n> to behave differently.\n\nI mean, that if a hunk is shown in some long file, which\nhave other changes, with some option (-c or --cc), it should\nalso be shown if it is the only change in file.\n\n> So, having said that I am a bit skeptical about the description of\n> the goals, there are a few nits on the implementation, too.\n> \n> > +# the difference in short file must be returned if and only if it is shown in long file\n> > +for fn in win1 win2 merge delete base only1 only2 only1discard only2discard; do\n> > +\tif git diff --cc merge branch1 branch2 mergebase -- long/$fn | grep -q '^[ +-]\\{3\\}2\\(change[12]|merge\\)\\?$'\n> > +\tthen\n> \n> Just like the earlier parts of this patch, write a newline when you\n> do not have to write a semicolon, and split lines after pipe when\n> your pipeline gets long, i.e.\n> \n> \tfor fn in win2 win2 ...\n> do\n> \t\tif git diff --cc ... |\n> \t\t\tgrep 'pattern'\n> \t\tthen\n> \n> Do I smell some GNUism in your \"grep\" patterns?\n> \n> You have\n> \n> '^[ +-]\\{3\\}2\\(change[12]|merge\\)\\?$'\n> \n> but matching zero-or-one repetition with ? is not in BRE, and \\? to\n> use it in BRE is a GNU extension.\n> \n> Also in BRE , '|' is not an alternation (and making it into such\n> with '\\|' in BRE is a GNU extension IIRC.\n> \n> Worse, you are not using backslash here to invoke GNU extension, so\n> I suspect the grep invocations in the patch may not be working as\n> you expect.\n\nThank you, I missed it. I suspect it was working because\nsingle \"2\" still matching. Will fix it and other issues.\n\n-- \nMax\n"},{"id":"259325","messageId":"20150414042252.GC786@wheezy.local","threadId":"38984","inReplyTo":"xmqq1tjp6gj5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/4] t4059: test 'diff --cc' with a change from only few parents","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-04-14T04:22:52Z","receivedAt":"2015-04-14T04:22:52Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Sat, Apr 11, 2015 at 10:51:10PM -0700, Junio C Hamano wrote:\n> Max Kirillov writes:\n> \n>> My exact case was that there was a change in one branch\n>> which was overwritten during merge conflict resolution by\n>> fully acepting the other branch - in a 2-parent merge. I\n>> started looking for a way to visualize such cases. They\n>> are not visible in usual diff, because they look same as\n>> accepting change compared to the unchange branch.\n> \n> Hmph, isn't that exactly why \"diff -c\" exists, not \"diff --cc\"\n> that omits (usually) uninteresting hunks?\n\nNo, this shows too many. If a change is done in one branch\nbut the other branch did not introduce any changes since\nmergebase and they merged cleanly the merge should not be\nshown, and `diff -c` seems to show them.\n"}]}