git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Philippe Blain <levraiphilippeblain@gmail.com>,
	Elijah Newren <newren@gmail.com>,
	Elijah Newren <newren@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH v2 2/3] diff: fix filtering of additional headers under --remerge-diff
Date: Thu, 01 Sep 2022 07:08:06 +0000	[thread overview]
Message-ID: <3bd622d5b45d05b89de73536ae948ea4e24f9505.1662016088.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1342.v2.git.1662016087.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

Commit 95433eeed9 ("diff: add ability to insert additional headers for
paths", 2022-02-02) introduced the possibility of additional headers.
Because there could be conflicts with no content differences (e.g. a
modify/delete conflict resolved in favor of taking the modified file
as-is), that commit also modified the diff_queue_is_empty() and
diff_flush_patch() logic to ensure these headers were included even if
there was no associated content diff.

However, when the pickaxe is active, we really only want the remerge
conflict headers to be shown when there is an associated content diff.
Adjust the logic in these two functions accordingly.

This also removes the TEST_PASSES_SANITIZE_LEAK=true declaration from
t4069, as there is apparently some kind of memory leak with the pickaxe
code.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 diff.c                  |  2 ++
 t/t4069-remerge-diff.sh | 17 ++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index d672efed7ab..85905261ce4 100644
--- a/diff.c
+++ b/diff.c
@@ -5860,6 +5860,7 @@ static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)
 {
 	int include_conflict_headers =
 	    (additional_headers(o, p->one->path) &&
+	     !o->pickaxe_opts &&
 	     (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));
 
 	/*
@@ -5915,6 +5916,7 @@ int diff_queue_is_empty(struct diff_options *o)
 	int i;
 	int include_conflict_headers =
 	    (o->additional_path_headers &&
+	     !o->pickaxe_opts &&
 	     (!o->filter || filter_bit_tst(DIFF_STATUS_UNMERGED, o)));
 
 	if (include_conflict_headers)
diff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh
index e3e6fbd97b2..95a16d19aec 100755
--- a/t/t4069-remerge-diff.sh
+++ b/t/t4069-remerge-diff.sh
@@ -2,7 +2,6 @@
 
 test_description='remerge-diff handling'
 
-TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # This test is ort-specific
@@ -90,6 +89,22 @@ test_expect_success 'remerge-diff with both a resolved conflict and an unrelated
 	test_cmp expect actual
 '
 
+test_expect_success 'pickaxe still includes additional headers for relevant changes' '
+	# reuses "expect" from the previous testcase
+
+	git log --oneline --remerge-diff -Sacht ab_resolution >tmp &&
+	sed -e "s/[0-9a-f]\{7,\}/HASH/g" tmp >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'can filter out additional headers with pickaxe' '
+	git show --remerge-diff --submodule=log --find-object=HEAD ab_resolution >actual &&
+	test_must_be_empty actual &&
+
+	git show --remerge-diff -S"not present" --all >actual &&
+	test_must_be_empty actual
+'
+
 test_expect_success 'setup non-content conflicts' '
 	git switch --orphan base &&
 
-- 
gitgitgadget


  parent reply	other threads:[~2022-09-01  7:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-31  6:21 [PATCH 0/3] Output fixes for --remerge-diff Elijah Newren via GitGitGadget
2022-08-31  6:21 ` [PATCH 1/3] diff: have submodule_format logic avoid additional diff headers Elijah Newren via GitGitGadget
2022-08-31 22:20   ` Junio C Hamano
2022-09-01  3:44     ` Elijah Newren
2022-08-31  6:21 ` [PATCH 2/3] diff: fix filtering of additional headers under --remerge-diff Elijah Newren via GitGitGadget
2022-08-31 22:26   ` Junio C Hamano
2022-09-01  3:38     ` Elijah Newren
2022-08-31  6:21 ` [PATCH 3/3] diff: fix filtering of merge commits " Elijah Newren via GitGitGadget
2022-09-01  1:13 ` [PATCH 0/3] Output fixes for --remerge-diff Junio C Hamano
2022-09-01  3:47   ` Elijah Newren
2022-09-01  4:01     ` Elijah Newren
2022-09-01 15:24     ` Junio C Hamano
2022-09-01 18:46       ` Ævar Arnfjörð Bjarmason
2022-09-01 19:54         ` Junio C Hamano
2022-09-01  7:08 ` [PATCH v2 " Elijah Newren via GitGitGadget
2022-09-01  7:08   ` [PATCH v2 1/3] diff: have submodule_format logic avoid additional diff headers Elijah Newren via GitGitGadget
2022-09-01 16:30     ` Junio C Hamano
2022-09-01  7:08   ` Elijah Newren via GitGitGadget [this message]
2022-09-01  7:08   ` [PATCH v2 3/3] diff: fix filtering of merge commits under --remerge-diff Elijah Newren via GitGitGadget
2022-09-02  3:53   ` [PATCH v3 0/3] Output fixes for --remerge-diff Elijah Newren via GitGitGadget
2022-09-02  3:53     ` [PATCH v3 1/3] diff: have submodule_format logic avoid additional diff headers Elijah Newren via GitGitGadget
2022-09-02  3:53     ` [PATCH v3 2/3] diff: fix filtering of additional headers under --remerge-diff Elijah Newren via GitGitGadget
2022-09-02  3:53     ` [PATCH v3 3/3] diff: fix filtering of merge commits " Elijah Newren via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3bd622d5b45d05b89de73536ae948ea4e24f9505.1662016088.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).