{"thread":{"id":"58381","subject":"[PATCH 0/3] Output fixes for --remerge-diff","startedAt":"2022-08-31T06:21:56Z","lastAt":"2022-09-02T03:53:56Z","messageCount":23,"participants":["Elijah Newren via GitGitGadget","Junio C Hamano","Elijah Newren","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"462251","messageId":"pull.1342.git.1661926908.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":null,"subject":"[PATCH 0/3] Output fixes for --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-31T06:21:45Z","receivedAt":"2022-08-31T06:21:56Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Philippe Blain found and reported a couple issues with the output of\n--remerge-diff[1]. After digging in, I think one of them actually counts as\ntwo separate issues, so here's a series with three patches to fix these\nissues. Each includes testcases to keep us from regressing.\n\nNote: The issue fixed by the third commit for --remerge-diff is also an\nissue exhibited by 'git log --cc $FILTER_RULES $COMMIT' (or by -c instead of\n--cc). However, as far as I can tell the causes are different and come from\nseparate codepaths; this series focuses on --remerge-diff and hence makes no\nattempt to fix independent (even if similar) --cc or -c issues.\n\n[1]\nhttps://lore.kernel.org/git/43cf2a1d-058a-fd79-befe-7d9bc62581ed@gmail.com/\n\nElijah Newren (3):\n diff: have submodule_format logic avoid additional diff headers\n diff: fix filtering of additional headers under --remerge-diff\n diff: fix filtering of merge commits under --remerge-diff\n\n diff.c | 9 +++++++--\n t/t4069-remerge-diff.sh | 30 +++++++++++++++++++++++++++++-\n 2 files changed, 36 insertions(+), 3 deletions(-)\n\n\nbase-commit: 6c8e4ee870332d11e4bba84901654b355a9ff016\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1342%2Fnewren%2Fremerge-diff-output-fixes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1342/newren/remerge-diff-output-fixes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1342\n-- \ngitgitgadget\n"},{"id":"462252","messageId":"e4414cf630da284bdb11f5613ca0ce6413c6a443.1661926908.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.git.1661926908.gitgitgadget@gmail.com","subject":"[PATCH 1/3] diff: have submodule_format logic avoid additional diff headers","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-31T06:21:46Z","receivedAt":"2022-08-31T06:22:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers,\ncreated in create_filepairs_for_header_only_notifications(). These are\nrepresented by inserting additional pairs in diff_queued_diff which\nalways have a mode of 0 and a null_oid. When these were added, one\ncode path was noted to assume that at least one of the diff_filespecs\nin the pair were valid, and that codepath was corrected.\n\nThe submodule_format handling is another codepath with the same issue;\nit would operate on these additional headers and attempt to display them\nas submodule changes. Prevent that by explicitly checking for both\nmodes being 0.\n\nReported-by: Philippe Blain \nSigned-off-by: Elijah Newren \n---\n diff.c | 6 ++++--\n t/t4069-remerge-diff.sh | 8 ++++++++\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 974626a6211..be23f660570 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3429,14 +3429,16 @@ static void builtin_diff(const char *name_a,\n \n \tif (o->submodule_format == DIFF_SUBMODULE_LOG &&\n \t (!one->mode || S_ISGITLINK(one->mode)) &&\n-\t (!two->mode || S_ISGITLINK(two->mode))) {\n+\t (!two->mode || S_ISGITLINK(two->mode)) &&\n+\t (one->mode || two->mode)) {\n \t\tshow_submodule_diff_summary(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n \t\treturn;\n \t} else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&\n \t\t (!one->mode || S_ISGITLINK(one->mode)) &&\n-\t\t (!two->mode || S_ISGITLINK(two->mode))) {\n+\t\t (!two->mode || S_ISGITLINK(two->mode)) &&\n+\t\t (one->mode || two->mode)) {\n \t\tshow_submodule_inline_diff(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex 9e7cac68b1c..e3e6fbd97b2 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -185,6 +185,14 @@ test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'submodule formatting ignores additional headers' '\n+\t# Reuses \"expect\" from last testcase\n+\n+\tgit show --oneline --remerge-diff --diff-filter=U --submodule=log >tmp &&\n+\tsed -e \"s/[0-9a-f]\\{7,\\}/HASH/g\" tmp >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'remerge-diff w/ diff-filter=R: relevant file + conflict header' '\n \tgit log -1 --oneline resolution >tmp &&\n \tcat <<-EOF >>tmp &&\n-- \ngitgitgadget\n\n"},{"id":"462253","messageId":"feac97494600e522125b7bb202f4dc5ca020ca99.1661926908.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.git.1661926908.gitgitgadget@gmail.com","subject":"[PATCH 2/3] diff: fix filtering of additional headers under --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-31T06:21:47Z","receivedAt":"2022-08-31T06:22:02Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers.\nBecause there could be conflicts with no content differences (e.g. a\nmodify/delete conflict resolved in favor of taking the modified file\nas-is), that commit also modified the diff_queue_is_empty() and\ndiff_flush_patch() logic to ensure these headers were included even if\nthere was no associated content diff.\n\nHowever, when the pickaxe is active, we really only want the remerge\nconflict headers to be shown when there is an associated content diff.\nAdjust the logic in these two functions accordingly.\n\nThis also removes the TEST_PASSES_SANITIZE_LEAK=true declaration from\nt4069, as there is apparently some kind of memory leak with the pickaxe\ncode.\n\nSigned-off-by: Elijah Newren \n---\n diff.c | 2 ++\n t/t4069-remerge-diff.sh | 17 ++++++++++++++++-\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex be23f660570..9535e755c73 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5854,6 +5854,7 @@ static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)\n {\n \tint include_conflict_headers =\n \t (additional_headers(o, p->one->path) &&\n+\t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \n \t/*\n@@ -5909,6 +5910,7 @@ int diff_queue_is_empty(struct diff_options *o)\n \tint i;\n \tint include_conflict_headers =\n \t (o->additional_path_headers &&\n+\t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \n \tif (include_conflict_headers)\ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex e3e6fbd97b2..95a16d19aec 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -2,7 +2,6 @@\n \n test_description='remerge-diff handling'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # This test is ort-specific\n@@ -90,6 +89,22 @@ test_expect_success 'remerge-diff with both a resolved conflict and an unrelated\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'pickaxe still includes additional headers for relevant changes' '\n+\t# reuses \"expect\" from the previous testcase\n+\n+\tgit log --oneline --remerge-diff -Sacht ab_resolution >tmp &&\n+\tsed -e \"s/[0-9a-f]\\{7,\\}/HASH/g\" tmp >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'can filter out additional headers with pickaxe' '\n+\tgit show --remerge-diff --submodule=log --find-object=HEAD ab_resolution >actual &&\n+\ttest_must_be_empty actual &&\n+\n+\tgit show --remerge-diff -S\"not present\" --all >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'setup non-content conflicts' '\n \tgit switch --orphan base &&\n \n-- \ngitgitgadget\n\n"},{"id":"462254","messageId":"46ea0d7dd6555dbb9939940207fa2c8044d0ab56.1661926908.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.git.1661926908.gitgitgadget@gmail.com","subject":"[PATCH 3/3] diff: fix filtering of merge commits under --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-31T06:21:48Z","receivedAt":"2022-08-31T06:22:04Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers.\nBecause there could be conflicts with no content differences (e.g. a\nmodify/delete conflict resolved in favor of taking the modified file\nas-is), that commit also modified the diff_queue_is_empty() and\ndiff_flush_patch() logic to ensure these headers were included even if\nthere was no associated content diff.\n\nHowever, the added logic was a bit inconsistent between these two\nfunctions. diff_queue_is_empty() overlooked the fact that the\nadditional headers strmap could be non-NULL and empty, which would cause\nit to display commits that should have been filtered out.\n\nFix the diff_queue_is_empty() logic to also account for\nadditional_path_headers being empty.\n\nReported-by: Philippe Blain \nSigned-off-by: Elijah Newren \n---\n diff.c | 1 +\n t/t4069-remerge-diff.sh | 5 +++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex 9535e755c73..f5c0e0599c9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5910,6 +5910,7 @@ int diff_queue_is_empty(struct diff_options *o)\n \tint i;\n \tint include_conflict_headers =\n \t (o->additional_path_headers &&\n+\t strmap_get_size(o->additional_path_headers) &&\n \t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex 95a16d19aec..07323ebafe0 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -56,6 +56,11 @@ test_expect_success 'remerge-diff on a clean merge' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'remerge-diff on a clean merge with a filter' '\n+\tgit show --oneline --remerge-diff --diff-filter=U bc_resolution >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'remerge-diff with both a resolved conflict and an unrelated change' '\n \tgit log -1 --oneline ab_resolution >tmp &&\n \tcat <<-EOF >>tmp &&\n-- \ngitgitgadget\n"},{"id":"462334","messageId":"xmqqedwwdph3.fsf@gitster.g","threadId":"58381","inReplyTo":"e4414cf630da284bdb11f5613ca0ce6413c6a443.1661926908.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] diff: have submodule_format logic avoid additional diff headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-31T22:20:56Z","receivedAt":"2022-08-31T22:21:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> From: Elijah Newren \n>\n> Commit 95433eeed9 (\"diff: add ability to insert additional headers for\n> paths\", 2022-02-02) introduced the possibility of additional headers,\n> created in create_filepairs_for_header_only_notifications(). These are\n> represented by inserting additional pairs in diff_queued_diff which\n> always have a mode of 0 and a null_oid. When these were added, one\n> code path was noted to assume that at least one of the diff_filespecs\n> in the pair were valid, and that codepath was corrected.\n>\n> The submodule_format handling is another codepath with the same issue;\n> it would operate on these additional headers and attempt to display them\n> as submodule changes. Prevent that by explicitly checking for both\n> modes being 0.\n\nIt may make sense to give a concrete name for the condition these\nnew codepaths check, which presumably exists in the part that was\ntouched in 95433eeed9 when \"that codepath was corrected\". I think\nyou want to treat a diffpair with at least one side with non-zero\nmode as a \"real\" thing (as opposed to the phony \"additional headers\"\nhack), so perhaps\n\n\tint diff_filepair_is_phoney(struct diff_filespec *one, \n \t\t\t struct diff_filespec *two)\n\t{\n\t\treturn !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);\n\t}\n\nor something like that. The use of the FILE_VALID macro here is\nvery much deliberate, and tries to match the more recent hack after\nthis hunk that says:\n\n\tif (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {\n\t\t/*\n\t\t * We should only reach this point for pairs from\n\t\t * create_filepairs_for_header_only_notifications(). For\n\t\t * these, we should avoid the \"/dev/null\" special casing\n\t\t * above, meaning we avoid showing such pairs as either\n\t\t * \"new file\" or \"deleted file\" below.\n\t\t */\n\t\tlbl[0] = a_one;\n\t\tlbl[1] = b_two;\n\t}\n\nWe shouldn't expect readers to understand (one->mode || two->mode)\nis about the same hack as the other one.\n\n\n"},{"id":"462335","messageId":"xmqq7d2odp8m.fsf@gitster.g","threadId":"58381","inReplyTo":"feac97494600e522125b7bb202f4dc5ca020ca99.1661926908.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] diff: fix filtering of additional headers under --remerge-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-31T22:26:01Z","receivedAt":"2022-08-31T22:26:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> From: Elijah Newren \n>\n> Commit 95433eeed9 (\"diff: add ability to insert additional headers for\n> paths\", 2022-02-02) introduced the possibility of additional headers.\n> Because there could be conflicts with no content differences (e.g. a\n> modify/delete conflict resolved in favor of taking the modified file\n> as-is), that commit also modified the diff_queue_is_empty() and\n> diff_flush_patch() logic to ensure these headers were included even if\n> there was no associated content diff.\n\nIn the longer term, I think we may have to redo the way additional\nheaders are inserted to the diff_queue. All the diff code I am\nfamiliar with (read: written before this hack was introduced) trusts\nthat diff_queue.nr is the number of paths that are returned by the\ndiff frontend, and unless there is diffcore_break involved, there\nwill be at most one diff_filepair that is about a path.\n\nWhy do these need to be separate entries in the queue, not a new\nattribute added to an existing filepair? Are we inserting pieces of\ninformation that are not about any paths that are involved in the\ndiff?\n\n"},{"id":"462407","messageId":"xmqqv8q7dhh4.fsf@gitster.g","threadId":"58381","inReplyTo":"pull.1342.git.1661926908.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Output fixes for --remerge-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-01T01:13:43Z","receivedAt":"2022-09-01T01:13:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> Philippe Blain found and reported a couple issues with the output of\n> --remerge-diff[1]. After digging in, I think one of them actually counts as\n> two separate issues, so here's a series with three patches to fix these\n> issues. Each includes testcases to keep us from regressing.\n\nIncluding this to 'seen' seems to break the leaks-check CI job X-<.\n\nhttps://github.com/git/git/runs/8124648321?check_suite_focus=true\n\n"},{"id":"462411","messageId":"CABPp-BGo6JST9uO-dUEXw1sx6oxCWjBzq9MftLqjcoiFhATMuQ@mail.gmail.com","threadId":"58381","inReplyTo":"xmqq7d2odp8m.fsf@gitster.g","subject":"Re: [PATCH 2/3] diff: fix filtering of additional headers under --remerge-diff","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-09-01T03:38:00Z","receivedAt":"2022-09-01T03:38:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Aug 31, 2022 at 3:26 PM Junio C Hamano wrote:\n>\n> \"Elijah Newren via GitGitGadget\" writes:\n>\n> > From: Elijah Newren \n> >\n> > Commit 95433eeed9 (\"diff: add ability to insert additional headers for\n> > paths\", 2022-02-02) introduced the possibility of additional headers.\n> > Because there could be conflicts with no content differences (e.g. a\n> > modify/delete conflict resolved in favor of taking the modified file\n> > as-is), that commit also modified the diff_queue_is_empty() and\n> > diff_flush_patch() logic to ensure these headers were included even if\n> > there was no associated content diff.\n>\n> In the longer term, I think we may have to redo the way additional\n> headers are inserted to the diff_queue. All the diff code I am\n> familiar with (read: written before this hack was introduced) trusts\n> that diff_queue.nr is the number of paths that are returned by the\n> diff frontend, and unless there is diffcore_break involved, there\n> will be at most one diff_filepair that is about a path.\n>\n> Why do these need to be separate entries in the queue, not a new\n> attribute added to an existing filepair? Are we inserting pieces of\n> information that are not about any paths that are involved in the\n> diff?\n\nThey usually are added to an existing filepair, but as your last\nquestion alludes to, there isn't always one that exists.\n\nOne example where this can happen is with a modify/delete conflict.\nIf someone got one of those and decided to keep the modified file\nas-is when resolving the conflict, then there'd be no content diff to\nshow for that file when investigating the resulting merge commit with\n--remerge-diff. But since the point of --remerge-diff is to show\nwhatever work the user did to resolve conflicts, and that file was\nconflicted, we want to show _something_ for that path. Thus the\nreason for creating a new \"phoney\" filepair just for attaching the\n\"remerge CONFLICT (modify/delete) ...\" notice.\n"},{"id":"462412","messageId":"CABPp-BEyb9hOWuuG24wp+qwF0Ni2wVvxe4ma4Rsph_eOeWFV8A@mail.gmail.com","threadId":"58381","inReplyTo":"xmqqedwwdph3.fsf@gitster.g","subject":"Re: [PATCH 1/3] diff: have submodule_format logic avoid additional diff headers","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-09-01T03:44:50Z","receivedAt":"2022-09-01T03:45:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Aug 31, 2022 at 3:20 PM Junio C Hamano wrote:\n>\n> \"Elijah Newren via GitGitGadget\" writes:\n>\n> > From: Elijah Newren \n> >\n> > Commit 95433eeed9 (\"diff: add ability to insert additional headers for\n> > paths\", 2022-02-02) introduced the possibility of additional headers,\n> > created in create_filepairs_for_header_only_notifications(). These are\n> > represented by inserting additional pairs in diff_queued_diff which\n> > always have a mode of 0 and a null_oid. When these were added, one\n> > code path was noted to assume that at least one of the diff_filespecs\n> > in the pair were valid, and that codepath was corrected.\n> >\n> > The submodule_format handling is another codepath with the same issue;\n> > it would operate on these additional headers and attempt to display them\n> > as submodule changes. Prevent that by explicitly checking for both\n> > modes being 0.\n>\n> It may make sense to give a concrete name for the condition these\n> new codepaths check, which presumably exists in the part that was\n> touched in 95433eeed9 when \"that codepath was corrected\". I think\n> you want to treat a diffpair with at least one side with non-zero\n> mode as a \"real\" thing (as opposed to the phony \"additional headers\"\n> hack), so perhaps\n>\n> int diff_filepair_is_phoney(struct diff_filespec *one,\n> struct diff_filespec *two)\n> {\n> return !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);\n> }\n>\n> or something like that. The use of the FILE_VALID macro here is\n> very much deliberate, and tries to match the more recent hack after\n> this hunk that says:\n>\n> if (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {\n> /*\n> * We should only reach this point for pairs from\n> * create_filepairs_for_header_only_notifications(). For\n> * these, we should avoid the \"/dev/null\" special casing\n> * above, meaning we avoid showing such pairs as either\n> * \"new file\" or \"deleted file\" below.\n> */\n> lbl[0] = a_one;\n> lbl[1] = b_two;\n> }\n>\n> We shouldn't expect readers to understand (one->mode || two->mode)\n> is about the same hack as the other one.\n\nI like that; I'll make that change.\n"},{"id":"462413","messageId":"CABPp-BHULBGAbmY1r9fpRr+MrjqOp7j-devOgkfA25jpYBGY9g@mail.gmail.com","threadId":"58381","inReplyTo":"xmqqv8q7dhh4.fsf@gitster.g","subject":"Re: [PATCH 0/3] Output fixes for --remerge-diff","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-09-01T03:47:58Z","receivedAt":"2022-09-01T03:48:16Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Aug 31, 2022 at 6:13 PM Junio C Hamano wrote:\n>\n> \"Elijah Newren via GitGitGadget\" writes:\n>\n> > Philippe Blain found and reported a couple issues with the output of\n> > --remerge-diff[1]. After digging in, I think one of them actually counts as\n> > two separate issues, so here's a series with three patches to fix these\n> > issues. Each includes testcases to keep us from regressing.\n>\n> Including this to 'seen' seems to break the leaks-check CI job X-<.\n>\n> https://github.com/git/git/runs/8124648321?check_suite_focus=true\n\nThat's...surprising. Any chance of a mis-merge?\n\nI ask for two reasons:\n * This series, built on main, passed the leaks-check job.\n * The link you provide points to t4069 as the test failing, but the\nsecond patch of this series removes the TEST_PASSES_SANITIZE_LEAK=true\nline from t4069, which should make that test a no-op for the\nleaks-check job.\n"},{"id":"462414","messageId":"CABPp-BFJ3-MxJOuU+Rd-1AtVeC0SH3NJnpN=L0511vhkLchiJA@mail.gmail.com","threadId":"58381","inReplyTo":"CABPp-BHULBGAbmY1r9fpRr+MrjqOp7j-devOgkfA25jpYBGY9g@mail.gmail.com","subject":"Re: [PATCH 0/3] Output fixes for --remerge-diff","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-09-01T04:01:14Z","receivedAt":"2022-09-01T04:01:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Aug 31, 2022 at 8:47 PM Elijah Newren wrote:\n>\n> On Wed, Aug 31, 2022 at 6:13 PM Junio C Hamano wrote:\n> >\n> > \"Elijah Newren via GitGitGadget\" writes:\n> >\n> > > Philippe Blain found and reported a couple issues with the output of\n> > > --remerge-diff[1]. After digging in, I think one of them actually counts as\n> > > two separate issues, so here's a series with three patches to fix these\n> > > issues. Each includes testcases to keep us from regressing.\n> >\n> > Including this to 'seen' seems to break the leaks-check CI job X-<.\n> >\n> > https://github.com/git/git/runs/8124648321?check_suite_focus=true\n>\n> That's...surprising. Any chance of a mis-merge?\n>\n> I ask for two reasons:\n> * This series, built on main, passed the leaks-check job.\n> * The link you provide points to t4069 as the test failing, but the\n> second patch of this series removes the TEST_PASSES_SANITIZE_LEAK=true\n> line from t4069, which should make that test a no-op for the\n> leaks-check job.\n\nActually, looks not like a mis-merge, but some kind of faulty `git am`\napplication. The merge in question isn't available for me to fetch,\nbut clicking through the UI from the link you provide eventually leads\nme to:\n\n https://github.com/git/git/commit/81f120208d02afee71543d4f588b471950f156f2\n\nwhich does NOT match what I submitted:\n\n https://lore.kernel.org/git/feac97494600e522125b7bb202f4dc5ca020ca99.1661926908.git.gitgitgadget@gmail.com/\n\nIt's close, but despite still including this part of my commit message:\n\n\"\"\"\nThis also removes the TEST_PASSES_SANITIZE_LEAK=true declaration from\nt4069, as there is apparently some kind of memory leak with the pickaxe\ncode.\n\"\"\"\n\nit's missing this part of the diff:\n\ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex e3e6fbd97b2..95a16d19aec 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -2,7 +2,6 @@\n\n test_description='remerge-diff handling'\n\n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n\n # This test is ort-specific\n\n\nThat part of the diff is important. I did not add any leaks to the\ncode (I did run the leaks-checking job and looked through the output\nto verify that none of them involved any codepath I added or\nmodified), but I did add some test code which exercises pre-existing\nmemory leaks, and testing those codepaths is critical to verify I got\nthe appropriate fixes. Any idea what happened here?\n\nEither way, I'm going to resubmit the series due to your other\nsuggestion. So long as the unfortunate munging doesn't occur again,\nthings should be fine if you just take the new series.\n"},{"id":"462420","messageId":"pull.1342.v2.git.1662016087.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.git.1661926908.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Output fixes for --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-01T07:08:04Z","receivedAt":"2022-09-01T07:09:17Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Philippe Blain found and reported a couple issues with the output of\n--remerge-diff[1]. After digging in, I think one of them actually counts as\ntwo separate issues, so here's a series with three patches to fix these\nissues. Each includes testcases to keep us from regressing.\n\nChanges since v1:\n\n * Added a new diff_filepair_is_phoney() function to make the code more\n self-documenting, as suggested by Junio.\n * Note: Patch 2, as called out in its commit message, disables\n TEST_PASSES_SANITIZE_LEAK for t4069. This is not because I introduce any\n memory leaks, but because I add new testcases invoking additional parts\n of the code (pickaxe stuff) which already had pre-existing leaks. This is\n not a change since v1, but this somehow accidentally got munged out of\n Junio's application of my v1.\n\nNote: The issue fixed by the third commit for --remerge-diff is also an\nissue exhibited by 'git log --cc $FILTER_RULES $COMMIT' (or by -c instead of\n--cc). However, as far as I can tell the causes are different and come from\nseparate codepaths; this series focuses on --remerge-diff and hence makes no\nattempt to fix independent (even if similar) --cc or -c issues.\n\n[1]\nhttps://lore.kernel.org/git/43cf2a1d-058a-fd79-befe-7d9bc62581ed@gmail.com/\n\nElijah Newren (3):\n diff: have submodule_format logic avoid additional diff headers\n diff: fix filtering of additional headers under --remerge-diff\n diff: fix filtering of merge commits under --remerge-diff\n\n diff.c | 17 ++++++++++++++---\n t/t4069-remerge-diff.sh | 30 +++++++++++++++++++++++++++++-\n 2 files changed, 43 insertions(+), 4 deletions(-)\n\n\nbase-commit: 6c8e4ee870332d11e4bba84901654b355a9ff016\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1342%2Fnewren%2Fremerge-diff-output-fixes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1342/newren/remerge-diff-output-fixes-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1342\n\nRange-diff vs v1:\n\n 1: e4414cf630d ! 1: aea0bbc1c6a diff: have submodule_format logic avoid additional diff headers\n @@ Commit message\n Signed-off-by: Elijah Newren \n \n ## diff.c ##\n +@@ diff.c: static void add_formatted_headers(struct strbuf *msg,\n + \t\t\t\t line_prefix, meta, reset);\n + }\n + \n ++static int diff_filepair_is_phoney(struct diff_filespec *one,\n ++\t\t\t\t struct diff_filespec *two)\n ++{\n ++\treturn !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);\n ++}\n ++\n + static void builtin_diff(const char *name_a,\n + \t\t\t const char *name_b,\n + \t\t\t struct diff_filespec *one,\n @@ diff.c: static void builtin_diff(const char *name_a,\n \n \tif (o->submodule_format == DIFF_SUBMODULE_LOG &&\n \t (!one->mode || S_ISGITLINK(one->mode)) &&\n -\t (!two->mode || S_ISGITLINK(two->mode))) {\n +\t (!two->mode || S_ISGITLINK(two->mode)) &&\n -+\t (one->mode || two->mode)) {\n ++\t (!diff_filepair_is_phoney(one, two))) {\n \t\tshow_submodule_diff_summary(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n @@ diff.c: static void builtin_diff(const char *name_a,\n \t\t (!one->mode || S_ISGITLINK(one->mode)) &&\n -\t\t (!two->mode || S_ISGITLINK(two->mode))) {\n +\t\t (!two->mode || S_ISGITLINK(two->mode)) &&\n -+\t\t (one->mode || two->mode)) {\n ++\t\t (!diff_filepair_is_phoney(one, two))) {\n \t\tshow_submodule_inline_diff(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n +@@ diff.c: static void builtin_diff(const char *name_a,\n + \tb_two = quote_two(b_prefix, name_b + (*name_b == '/'));\n + \tlbl[0] = DIFF_FILE_VALID(one) ? a_one : \"/dev/null\";\n + \tlbl[1] = DIFF_FILE_VALID(two) ? b_two : \"/dev/null\";\n +-\tif (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {\n ++\tif (diff_filepair_is_phoney(one, two)) {\n + \t\t/*\n + \t\t * We should only reach this point for pairs from\n + \t\t * create_filepairs_for_header_only_notifications(). For\n \n ## t/t4069-remerge-diff.sh ##\n @@ t/t4069-remerge-diff.sh: test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif\n 2: feac9749460 = 2: 3bd622d5b45 diff: fix filtering of additional headers under --remerge-diff\n 3: 46ea0d7dd65 = 3: 7903f8380dc diff: fix filtering of merge commits under --remerge-diff\n\n-- \ngitgitgadget\n"},{"id":"462421","messageId":"aea0bbc1c6a33d62f6b3313d56664d53df8f620a.1662016087.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.v2.git.1662016087.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] diff: have submodule_format logic avoid additional diff headers","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-01T07:08:05Z","receivedAt":"2022-09-01T07:09:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers,\ncreated in create_filepairs_for_header_only_notifications(). These are\nrepresented by inserting additional pairs in diff_queued_diff which\nalways have a mode of 0 and a null_oid. When these were added, one\ncode path was noted to assume that at least one of the diff_filespecs\nin the pair were valid, and that codepath was corrected.\n\nThe submodule_format handling is another codepath with the same issue;\nit would operate on these additional headers and attempt to display them\nas submodule changes. Prevent that by explicitly checking for both\nmodes being 0.\n\nReported-by: Philippe Blain \nSigned-off-by: Elijah Newren \n---\n diff.c | 14 +++++++++++---\n t/t4069-remerge-diff.sh | 8 ++++++++\n 2 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 974626a6211..d672efed7ab 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3398,6 +3398,12 @@ static void add_formatted_headers(struct strbuf *msg,\n \t\t\t\t line_prefix, meta, reset);\n }\n \n+static int diff_filepair_is_phoney(struct diff_filespec *one,\n+\t\t\t\t struct diff_filespec *two)\n+{\n+\treturn !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);\n+}\n+\n static void builtin_diff(const char *name_a,\n \t\t\t const char *name_b,\n \t\t\t struct diff_filespec *one,\n@@ -3429,14 +3435,16 @@ static void builtin_diff(const char *name_a,\n \n \tif (o->submodule_format == DIFF_SUBMODULE_LOG &&\n \t (!one->mode || S_ISGITLINK(one->mode)) &&\n-\t (!two->mode || S_ISGITLINK(two->mode))) {\n+\t (!two->mode || S_ISGITLINK(two->mode)) &&\n+\t (!diff_filepair_is_phoney(one, two))) {\n \t\tshow_submodule_diff_summary(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n \t\treturn;\n \t} else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&\n \t\t (!one->mode || S_ISGITLINK(one->mode)) &&\n-\t\t (!two->mode || S_ISGITLINK(two->mode))) {\n+\t\t (!two->mode || S_ISGITLINK(two->mode)) &&\n+\t\t (!diff_filepair_is_phoney(one, two))) {\n \t\tshow_submodule_inline_diff(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n@@ -3456,7 +3464,7 @@ static void builtin_diff(const char *name_a,\n \tb_two = quote_two(b_prefix, name_b + (*name_b == '/'));\n \tlbl[0] = DIFF_FILE_VALID(one) ? a_one : \"/dev/null\";\n \tlbl[1] = DIFF_FILE_VALID(two) ? b_two : \"/dev/null\";\n-\tif (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {\n+\tif (diff_filepair_is_phoney(one, two)) {\n \t\t/*\n \t\t * We should only reach this point for pairs from\n \t\t * create_filepairs_for_header_only_notifications(). For\ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex 9e7cac68b1c..e3e6fbd97b2 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -185,6 +185,14 @@ test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'submodule formatting ignores additional headers' '\n+\t# Reuses \"expect\" from last testcase\n+\n+\tgit show --oneline --remerge-diff --diff-filter=U --submodule=log >tmp &&\n+\tsed -e \"s/[0-9a-f]\\{7,\\}/HASH/g\" tmp >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'remerge-diff w/ diff-filter=R: relevant file + conflict header' '\n \tgit log -1 --oneline resolution >tmp &&\n \tcat <<-EOF >>tmp &&\n-- \ngitgitgadget\n\n"},{"id":"462422","messageId":"3bd622d5b45d05b89de73536ae948ea4e24f9505.1662016088.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.v2.git.1662016087.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] diff: fix filtering of additional headers under --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-01T07:08:06Z","receivedAt":"2022-09-01T07:09:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers.\nBecause there could be conflicts with no content differences (e.g. a\nmodify/delete conflict resolved in favor of taking the modified file\nas-is), that commit also modified the diff_queue_is_empty() and\ndiff_flush_patch() logic to ensure these headers were included even if\nthere was no associated content diff.\n\nHowever, when the pickaxe is active, we really only want the remerge\nconflict headers to be shown when there is an associated content diff.\nAdjust the logic in these two functions accordingly.\n\nThis also removes the TEST_PASSES_SANITIZE_LEAK=true declaration from\nt4069, as there is apparently some kind of memory leak with the pickaxe\ncode.\n\nSigned-off-by: Elijah Newren \n---\n diff.c | 2 ++\n t/t4069-remerge-diff.sh | 17 ++++++++++++++++-\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex d672efed7ab..85905261ce4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5860,6 +5860,7 @@ static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)\n {\n \tint include_conflict_headers =\n \t (additional_headers(o, p->one->path) &&\n+\t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \n \t/*\n@@ -5915,6 +5916,7 @@ int diff_queue_is_empty(struct diff_options *o)\n \tint i;\n \tint include_conflict_headers =\n \t (o->additional_path_headers &&\n+\t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \n \tif (include_conflict_headers)\ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex e3e6fbd97b2..95a16d19aec 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -2,7 +2,6 @@\n \n test_description='remerge-diff handling'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # This test is ort-specific\n@@ -90,6 +89,22 @@ test_expect_success 'remerge-diff with both a resolved conflict and an unrelated\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'pickaxe still includes additional headers for relevant changes' '\n+\t# reuses \"expect\" from the previous testcase\n+\n+\tgit log --oneline --remerge-diff -Sacht ab_resolution >tmp &&\n+\tsed -e \"s/[0-9a-f]\\{7,\\}/HASH/g\" tmp >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'can filter out additional headers with pickaxe' '\n+\tgit show --remerge-diff --submodule=log --find-object=HEAD ab_resolution >actual &&\n+\ttest_must_be_empty actual &&\n+\n+\tgit show --remerge-diff -S\"not present\" --all >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'setup non-content conflicts' '\n \tgit switch --orphan base &&\n \n-- \ngitgitgadget\n\n"},{"id":"462423","messageId":"7903f8380dc512ba827209ba3b152d0784df7b60.1662016088.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.v2.git.1662016087.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] diff: fix filtering of merge commits under --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-01T07:08:07Z","receivedAt":"2022-09-01T07:09:24Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers.\nBecause there could be conflicts with no content differences (e.g. a\nmodify/delete conflict resolved in favor of taking the modified file\nas-is), that commit also modified the diff_queue_is_empty() and\ndiff_flush_patch() logic to ensure these headers were included even if\nthere was no associated content diff.\n\nHowever, the added logic was a bit inconsistent between these two\nfunctions. diff_queue_is_empty() overlooked the fact that the\nadditional headers strmap could be non-NULL and empty, which would cause\nit to display commits that should have been filtered out.\n\nFix the diff_queue_is_empty() logic to also account for\nadditional_path_headers being empty.\n\nReported-by: Philippe Blain \nSigned-off-by: Elijah Newren \n---\n diff.c | 1 +\n t/t4069-remerge-diff.sh | 5 +++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex 85905261ce4..5ea9071ff07 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5916,6 +5916,7 @@ int diff_queue_is_empty(struct diff_options *o)\n \tint i;\n \tint include_conflict_headers =\n \t (o->additional_path_headers &&\n+\t strmap_get_size(o->additional_path_headers) &&\n \t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex 95a16d19aec..07323ebafe0 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -56,6 +56,11 @@ test_expect_success 'remerge-diff on a clean merge' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'remerge-diff on a clean merge with a filter' '\n+\tgit show --oneline --remerge-diff --diff-filter=U bc_resolution >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'remerge-diff with both a resolved conflict and an unrelated change' '\n \tgit log -1 --oneline ab_resolution >tmp &&\n \tcat <<-EOF >>tmp &&\n-- \ngitgitgadget\n"},{"id":"462452","messageId":"xmqqo7vzce39.fsf@gitster.g","threadId":"58381","inReplyTo":"CABPp-BHULBGAbmY1r9fpRr+MrjqOp7j-devOgkfA25jpYBGY9g@mail.gmail.com","subject":"Re: [PATCH 0/3] Output fixes for --remerge-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-01T15:24:26Z","receivedAt":"2022-09-01T15:24:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> On Wed, Aug 31, 2022 at 6:13 PM Junio C Hamano wrote:\n>>\n>> \"Elijah Newren via GitGitGadget\" writes:\n>>\n>> > Philippe Blain found and reported a couple issues with the output of\n>> > --remerge-diff[1]. After digging in, I think one of them actually counts as\n>> > two separate issues, so here's a series with three patches to fix these\n>> > issues. Each includes testcases to keep us from regressing.\n>>\n>> Including this to 'seen' seems to break the leaks-check CI job X-<.\n>>\n>> https://github.com/git/git/runs/8124648321?check_suite_focus=true\n>\n> That's...surprising. Any chance of a mis-merge?\n>\n> I ask for two reasons:\n> * This series, built on main, passed the leaks-check job.\n\nAh, that.\n\nYes, I did rebase it to 'maint' to be nice to our users as this is\nnot a new feature development but a bugfix or two.\n\nThis is why I hate the leak-check CI job (yes, I do help maintain\nall parts of the tree, but it does not mean I have to love every bit\nof the codebase, and this is one of the things I love to hate).\n\nInstead of saying \"subcommand X with feature Y? It ought to be clean\nso complain if leak checker find something. subcommand Z? It is\nknown to be unclean, so do not bother\", it says \"In this test in\nentirety, we currently happen to use only the ones that are clean\"\nand penalizes developers who wants to use an unclean tool merely for\nchecking. The approach is fundamentally flawed and does not play\nwell with multiple integration branches, just like we saw here.\n"},{"id":"462468","messageId":"xmqqedwvawhj.fsf@gitster.g","threadId":"58381","inReplyTo":"aea0bbc1c6a33d62f6b3313d56664d53df8f620a.1662016087.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] diff: have submodule_format logic avoid additional diff headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-01T16:30:00Z","receivedAt":"2022-09-01T16:30:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n\nAs there is no guarantee that we'd never add another hack to use a\ndifferent kind of \"phoney\" filepair to diff_queue for another\npurpose, either the name of the helper function, or a comment in\nfront of it, should say something that is sufficient to help readers\nunderstand ...\n\n> +static int diff_filepair_is_phoney(struct diff_filespec *one,\n> +\t\t\t\t struct diff_filespec *two)\n> +{\n> +\treturn !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);\n> +}\n> ...\n> -\tif (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {\n> +\tif (diff_filepair_is_phoney(one, two)) {\n> \t\t/*\n> \t\t * We should only reach this point for pairs from\n> \t\t * create_filepairs_for_header_only_notifications(). For\n\n... some of what this comment said, i.e. the pair that was added by\ncreate_filepairs_for_header_only_notifications() is what the helper\ndetects. \n\nOther than that, the whole series looks good.\n"},{"id":"462482","messageId":"220901.86tu5rhqtg.gmgdl@evledraar.gmail.com","threadId":"58381","inReplyTo":"xmqqo7vzce39.fsf@gitster.g","subject":"Re: [PATCH 0/3] Output fixes for --remerge-diff","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-09-01T18:46:44Z","receivedAt":"2022-09-01T18:50:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Sep 01 2022, Junio C Hamano wrote:\n\n> Elijah Newren writes:\n>\n>> On Wed, Aug 31, 2022 at 6:13 PM Junio C Hamano wrote:\n>>>\n>>> \"Elijah Newren via GitGitGadget\" writes:\n>>>\n>>> > Philippe Blain found and reported a couple issues with the output of\n>>> > --remerge-diff[1]. After digging in, I think one of them actually counts as\n>>> > two separate issues, so here's a series with three patches to fix these\n>>> > issues. Each includes testcases to keep us from regressing.\n>>>\n>>> Including this to 'seen' seems to break the leaks-check CI job X-<.\n>>>\n>>> https://github.com/git/git/runs/8124648321?check_suite_focus=true\n>>\n>> That's...surprising. Any chance of a mis-merge?\n>>\n>> I ask for two reasons:\n>> * This series, built on main, passed the leaks-check job.\n>\n> Ah, that.\n>\n> Yes, I did rebase it to 'maint' to be nice to our users as this is\n> not a new feature development but a bugfix or two.\n>\n> This is why I hate the leak-check CI job (yes, I do help maintain\n> all parts of the tree, but it does not mean I have to love every bit\n> of the codebase, and this is one of the things I love to hate).\n>\n> Instead of saying \"subcommand X with feature Y? It ought to be clean\n> so complain if leak checker find something. subcommand Z? It is\n> known to be unclean, so do not bother\", it says \"In this test in\n> entirety, we currently happen to use only the ones that are clean\"\n> and penalizes developers who wants to use an unclean tool merely for\n> checking. The approach is fundamentally flawed and does not play\n> well with multiple integration branches, just like we saw here.\n\nWe've discussed doing it that way before. I wouldn't be fundamentally\nopposed, but I do think we're far enough along the way to being\nleak-free that we'd want to mark more than just a \"top-level\" command as\nleak-free.\n\nIt's just also not the case that we even could do that in all but the\nmost trivial cases. Most commands still leak somewhere in some obscure\ncases, but we have entire tests now where the code they run in those\ncommon cases doesn't leak.\n\nHowever, in this case this seems to just be a case that Elijah tested\nhis code on base X, and you applied it on base Y.\n\nI don't really see how the approach you're suggesting would be any more\nlikely to be resilient in the face of that. Then we'd presumably use\nsome command that's leak-free on \"master\", but that command wouldn't be\nleak-free on \"maint\".\n"},{"id":"462485","messageId":"xmqqa67ian0z.fsf@gitster.g","threadId":"58381","inReplyTo":"220901.86tu5rhqtg.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/3] Output fixes for --remerge-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-01T19:54:20Z","receivedAt":"2022-09-01T19:54:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> We've discussed doing it that way before. I wouldn't be fundamentally\n> opposed, but I do think we're far enough along the way to being\n> leak-free that we'd want to mark more than just a \"top-level\" command as\n> leak-free.\n\nTwo things. Seeing the leak-check breakage quite often, I doubt how\nmuch trust I can place in your \"far enough along the way\" statement.\nAlso, I do not think we thought the alternative was to mark only the\ntop-level. The test could inspect the crash after the fact, and say\n\"ah, allocation made by xcalloc() called from this and that functions\nare still known to be leaky, so do not stop and mark the CI job a\nfailure for this one\", for example?\n"},{"id":"462503","messageId":"ed28acaed973238b314d1ffc8d56ce61efbb5376.1662090810.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.v3.git.1662090810.gitgitgadget@gmail.com","subject":"[PATCH v3 1/3] diff: have submodule_format logic avoid additional diff headers","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-02T03:53:28Z","receivedAt":"2022-09-02T03:53:47Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers,\ncreated in create_filepairs_for_header_only_notifications(). These are\nrepresented by inserting additional pairs in diff_queued_diff which\nalways have a mode of 0 and a null_oid. When these were added, one\ncode path was noted to assume that at least one of the diff_filespecs\nin the pair were valid, and that codepath was corrected.\n\nThe submodule_format handling is another codepath with the same issue;\nit would operate on these additional headers and attempt to display them\nas submodule changes. Prevent that by explicitly checking for \"phoney\"\nfilepairs (i.e. filepairs with both modes being 0).\n\nReported-by: Philippe Blain \nSigned-off-by: Elijah Newren \n---\n diff.c | 29 +++++++++++++++++++++++------\n t/t4069-remerge-diff.sh | 8 ++++++++\n 2 files changed, 31 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 974626a6211..7f8637f3006 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3398,6 +3398,21 @@ static void add_formatted_headers(struct strbuf *msg,\n \t\t\t\t line_prefix, meta, reset);\n }\n \n+static int diff_filepair_is_phoney(struct diff_filespec *one,\n+\t\t\t\t struct diff_filespec *two)\n+{\n+\t/*\n+\t * This function specifically looks for pairs injected by\n+\t * create_filepairs_for_header_only_notifications(). Such\n+\t * pairs are \"phoney\" in that they do not represent any\n+\t * content or even mode difference, but were inserted because\n+\t * diff_queued_diff previously had no pair associated with\n+\t * that path but we needed some pair to avoid losing the\n+\t * \"remerge CONFLICT\" header associated with the path.\n+\t */\n+\treturn !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);\n+}\n+\n static void builtin_diff(const char *name_a,\n \t\t\t const char *name_b,\n \t\t\t struct diff_filespec *one,\n@@ -3429,14 +3444,16 @@ static void builtin_diff(const char *name_a,\n \n \tif (o->submodule_format == DIFF_SUBMODULE_LOG &&\n \t (!one->mode || S_ISGITLINK(one->mode)) &&\n-\t (!two->mode || S_ISGITLINK(two->mode))) {\n+\t (!two->mode || S_ISGITLINK(two->mode)) &&\n+\t (!diff_filepair_is_phoney(one, two))) {\n \t\tshow_submodule_diff_summary(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n \t\treturn;\n \t} else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&\n \t\t (!one->mode || S_ISGITLINK(one->mode)) &&\n-\t\t (!two->mode || S_ISGITLINK(two->mode))) {\n+\t\t (!two->mode || S_ISGITLINK(two->mode)) &&\n+\t\t (!diff_filepair_is_phoney(one, two))) {\n \t\tshow_submodule_inline_diff(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n@@ -3456,12 +3473,12 @@ static void builtin_diff(const char *name_a,\n \tb_two = quote_two(b_prefix, name_b + (*name_b == '/'));\n \tlbl[0] = DIFF_FILE_VALID(one) ? a_one : \"/dev/null\";\n \tlbl[1] = DIFF_FILE_VALID(two) ? b_two : \"/dev/null\";\n-\tif (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {\n+\tif (diff_filepair_is_phoney(one, two)) {\n \t\t/*\n-\t\t * We should only reach this point for pairs from\n+\t\t * We should only reach this point for pairs generated from\n \t\t * create_filepairs_for_header_only_notifications(). For\n-\t\t * these, we should avoid the \"/dev/null\" special casing\n-\t\t * above, meaning we avoid showing such pairs as either\n+\t\t * these, we want to avoid the \"/dev/null\" special casing\n+\t\t * above, because we do not want such pairs shown as either\n \t\t * \"new file\" or \"deleted file\" below.\n \t\t */\n \t\tlbl[0] = a_one;\ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex 9e7cac68b1c..e3e6fbd97b2 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -185,6 +185,14 @@ test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'submodule formatting ignores additional headers' '\n+\t# Reuses \"expect\" from last testcase\n+\n+\tgit show --oneline --remerge-diff --diff-filter=U --submodule=log >tmp &&\n+\tsed -e \"s/[0-9a-f]\\{7,\\}/HASH/g\" tmp >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'remerge-diff w/ diff-filter=R: relevant file + conflict header' '\n \tgit log -1 --oneline resolution >tmp &&\n \tcat <<-EOF >>tmp &&\n-- \ngitgitgadget\n\n"},{"id":"462504","messageId":"084a037756d79d569c19bd85f30bae97b15ec26f.1662090810.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.v3.git.1662090810.gitgitgadget@gmail.com","subject":"[PATCH v3 3/3] diff: fix filtering of merge commits under --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-02T03:53:30Z","receivedAt":"2022-09-02T03:53:51Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers.\nBecause there could be conflicts with no content differences (e.g. a\nmodify/delete conflict resolved in favor of taking the modified file\nas-is), that commit also modified the diff_queue_is_empty() and\ndiff_flush_patch() logic to ensure these headers were included even if\nthere was no associated content diff.\n\nHowever, the added logic was a bit inconsistent between these two\nfunctions. diff_queue_is_empty() overlooked the fact that the\nadditional headers strmap could be non-NULL and empty, which would cause\nit to display commits that should have been filtered out.\n\nFix the diff_queue_is_empty() logic to also account for\nadditional_path_headers being empty.\n\nReported-by: Philippe Blain \nSigned-off-by: Elijah Newren \n---\n diff.c | 1 +\n t/t4069-remerge-diff.sh | 5 +++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex 2439310ae1f..e1f9cef2f38 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5925,6 +5925,7 @@ int diff_queue_is_empty(struct diff_options *o)\n \tint i;\n \tint include_conflict_headers =\n \t (o->additional_path_headers &&\n+\t strmap_get_size(o->additional_path_headers) &&\n \t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex 95a16d19aec..07323ebafe0 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -56,6 +56,11 @@ test_expect_success 'remerge-diff on a clean merge' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'remerge-diff on a clean merge with a filter' '\n+\tgit show --oneline --remerge-diff --diff-filter=U bc_resolution >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'remerge-diff with both a resolved conflict and an unrelated change' '\n \tgit log -1 --oneline ab_resolution >tmp &&\n \tcat <<-EOF >>tmp &&\n-- \ngitgitgadget\n"},{"id":"462505","messageId":"f91bea2bbc3744e2789b421dfd4fdc4498a790c7.1662090810.git.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.v3.git.1662090810.gitgitgadget@gmail.com","subject":"[PATCH v3 2/3] diff: fix filtering of additional headers under --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-02T03:53:29Z","receivedAt":"2022-09-02T03:53:52Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 95433eeed9 (\"diff: add ability to insert additional headers for\npaths\", 2022-02-02) introduced the possibility of additional headers.\nBecause there could be conflicts with no content differences (e.g. a\nmodify/delete conflict resolved in favor of taking the modified file\nas-is), that commit also modified the diff_queue_is_empty() and\ndiff_flush_patch() logic to ensure these headers were included even if\nthere was no associated content diff.\n\nHowever, when the pickaxe is active, we really only want the remerge\nconflict headers to be shown when there is an associated content diff.\nAdjust the logic in these two functions accordingly.\n\nThis also removes the TEST_PASSES_SANITIZE_LEAK=true declaration from\nt4069, as there is apparently some kind of memory leak with the pickaxe\ncode.\n\nSigned-off-by: Elijah Newren \n---\n diff.c | 2 ++\n t/t4069-remerge-diff.sh | 17 ++++++++++++++++-\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 7f8637f3006..2439310ae1f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5869,6 +5869,7 @@ static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)\n {\n \tint include_conflict_headers =\n \t (additional_headers(o, p->one->path) &&\n+\t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \n \t/*\n@@ -5924,6 +5925,7 @@ int diff_queue_is_empty(struct diff_options *o)\n \tint i;\n \tint include_conflict_headers =\n \t (o->additional_path_headers &&\n+\t !o->pickaxe_opts &&\n \t (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));\n \n \tif (include_conflict_headers)\ndiff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh\nindex e3e6fbd97b2..95a16d19aec 100755\n--- a/t/t4069-remerge-diff.sh\n+++ b/t/t4069-remerge-diff.sh\n@@ -2,7 +2,6 @@\n \n test_description='remerge-diff handling'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # This test is ort-specific\n@@ -90,6 +89,22 @@ test_expect_success 'remerge-diff with both a resolved conflict and an unrelated\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'pickaxe still includes additional headers for relevant changes' '\n+\t# reuses \"expect\" from the previous testcase\n+\n+\tgit log --oneline --remerge-diff -Sacht ab_resolution >tmp &&\n+\tsed -e \"s/[0-9a-f]\\{7,\\}/HASH/g\" tmp >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'can filter out additional headers with pickaxe' '\n+\tgit show --remerge-diff --submodule=log --find-object=HEAD ab_resolution >actual &&\n+\ttest_must_be_empty actual &&\n+\n+\tgit show --remerge-diff -S\"not present\" --all >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'setup non-content conflicts' '\n \tgit switch --orphan base &&\n \n-- \ngitgitgadget\n\n"},{"id":"462506","messageId":"pull.1342.v3.git.1662090810.gitgitgadget@gmail.com","threadId":"58381","inReplyTo":"pull.1342.v2.git.1662016087.gitgitgadget@gmail.com","subject":"[PATCH v3 0/3] Output fixes for --remerge-diff","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-02T03:53:27Z","receivedAt":"2022-09-02T03:53:56Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Philippe Blain found and reported a couple issues with the output of\n--remerge-diff[1]. After digging in, I think one of them actually counts as\ntwo separate issues, so here's a series with three patches to fix these\nissues. Each includes testcases to keep us from regressing.\n\nChanges since v2:\n\n * Added a comment describing the rationale for the new\n diff_filepair_is_phoney() function.\n\nChanges since v1:\n\n * Added a new diff_filepair_is_phoney() function to make the code more\n self-documenting, as suggested by Junio.\n * Note: Patch 2, as called out in its commit message, disables\n TEST_PASSES_SANITIZE_LEAK for t4069. This is not because I introduce any\n memory leaks, but because I add new testcases invoking additional parts\n of the code (pickaxe stuff) which already had pre-existing leaks. This is\n not a change since v1, but this somehow accidentally got munged out of\n Junio's application of my v1.\n\nNote: The issue fixed by the third commit for --remerge-diff is also an\nissue exhibited by 'git log --cc $FILTER_RULES $COMMIT' (or by -c instead of\n--cc). However, as far as I can tell the causes are different and come from\nseparate codepaths; this series focuses on --remerge-diff and hence makes no\nattempt to fix independent (even if similar) --cc or -c issues.\n\n[1]\nhttps://lore.kernel.org/git/43cf2a1d-058a-fd79-befe-7d9bc62581ed@gmail.com/\n\nElijah Newren (3):\n diff: have submodule_format logic avoid additional diff headers\n diff: fix filtering of additional headers under --remerge-diff\n diff: fix filtering of merge commits under --remerge-diff\n\n diff.c | 32 ++++++++++++++++++++++++++------\n t/t4069-remerge-diff.sh | 30 +++++++++++++++++++++++++++++-\n 2 files changed, 55 insertions(+), 7 deletions(-)\n\n\nbase-commit: 6c8e4ee870332d11e4bba84901654b355a9ff016\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1342%2Fnewren%2Fremerge-diff-output-fixes-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1342/newren/remerge-diff-output-fixes-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1342\n\nRange-diff vs v2:\n\n 1: aea0bbc1c6a ! 1: ed28acaed97 diff: have submodule_format logic avoid additional diff headers\n @@ Commit message\n \n The submodule_format handling is another codepath with the same issue;\n it would operate on these additional headers and attempt to display them\n - as submodule changes. Prevent that by explicitly checking for both\n - modes being 0.\n + as submodule changes. Prevent that by explicitly checking for \"phoney\"\n + filepairs (i.e. filepairs with both modes being 0).\n \n Reported-by: Philippe Blain \n Signed-off-by: Elijah Newren \n @@ diff.c: static void add_formatted_headers(struct strbuf *msg,\n +static int diff_filepair_is_phoney(struct diff_filespec *one,\n +\t\t\t\t struct diff_filespec *two)\n +{\n ++\t/*\n ++\t * This function specifically looks for pairs injected by\n ++\t * create_filepairs_for_header_only_notifications(). Such\n ++\t * pairs are \"phoney\" in that they do not represent any\n ++\t * content or even mode difference, but were inserted because\n ++\t * diff_queued_diff previously had no pair associated with\n ++\t * that path but we needed some pair to avoid losing the\n ++\t * \"remerge CONFLICT\" header associated with the path.\n ++\t */\n +\treturn !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);\n +}\n +\n @@ diff.c: static void builtin_diff(const char *name_a,\n -\tif (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {\n +\tif (diff_filepair_is_phoney(one, two)) {\n \t\t/*\n - \t\t * We should only reach this point for pairs from\n +-\t\t * We should only reach this point for pairs from\n ++\t\t * We should only reach this point for pairs generated from\n \t\t * create_filepairs_for_header_only_notifications(). For\n +-\t\t * these, we should avoid the \"/dev/null\" special casing\n +-\t\t * above, meaning we avoid showing such pairs as either\n ++\t\t * these, we want to avoid the \"/dev/null\" special casing\n ++\t\t * above, because we do not want such pairs shown as either\n + \t\t * \"new file\" or \"deleted file\" below.\n + \t\t */\n + \t\tlbl[0] = a_one;\n \n ## t/t4069-remerge-diff.sh ##\n @@ t/t4069-remerge-diff.sh: test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif\n 2: 3bd622d5b45 = 2: f91bea2bbc3 diff: fix filtering of additional headers under --remerge-diff\n 3: 7903f8380dc = 3: 084a037756d diff: fix filtering of merge commits under --remerge-diff\n\n-- \ngitgitgadget\n"}]}