From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Philippe Blain" <levraiphilippeblain@gmail.com>,
"Elijah Newren" <newren@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Elijah Newren" <newren@gmail.com>
Subject: [PATCH v3 0/3] Output fixes for --remerge-diff
Date: Fri, 02 Sep 2022 03:53:27 +0000 [thread overview]
Message-ID: <pull.1342.v3.git.1662090810.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1342.v2.git.1662016087.gitgitgadget@gmail.com>
Philippe Blain found and reported a couple issues with the output of
--remerge-diff[1]. After digging in, I think one of them actually counts as
two separate issues, so here's a series with three patches to fix these
issues. Each includes testcases to keep us from regressing.
Changes since v2:
* Added a comment describing the rationale for the new
diff_filepair_is_phoney() function.
Changes since v1:
* Added a new diff_filepair_is_phoney() function to make the code more
self-documenting, as suggested by Junio.
* Note: Patch 2, as called out in its commit message, disables
TEST_PASSES_SANITIZE_LEAK for t4069. This is not because I introduce any
memory leaks, but because I add new testcases invoking additional parts
of the code (pickaxe stuff) which already had pre-existing leaks. This is
not a change since v1, but this somehow accidentally got munged out of
Junio's application of my v1.
Note: The issue fixed by the third commit for --remerge-diff is also an
issue exhibited by 'git log --cc $FILTER_RULES $COMMIT' (or by -c instead of
--cc). However, as far as I can tell the causes are different and come from
separate codepaths; this series focuses on --remerge-diff and hence makes no
attempt to fix independent (even if similar) --cc or -c issues.
[1]
https://lore.kernel.org/git/43cf2a1d-058a-fd79-befe-7d9bc62581ed@gmail.com/
Elijah Newren (3):
diff: have submodule_format logic avoid additional diff headers
diff: fix filtering of additional headers under --remerge-diff
diff: fix filtering of merge commits under --remerge-diff
diff.c | 32 ++++++++++++++++++++++++++------
t/t4069-remerge-diff.sh | 30 +++++++++++++++++++++++++++++-
2 files changed, 55 insertions(+), 7 deletions(-)
base-commit: 6c8e4ee870332d11e4bba84901654b355a9ff016
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1342%2Fnewren%2Fremerge-diff-output-fixes-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1342/newren/remerge-diff-output-fixes-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1342
Range-diff vs v2:
1: aea0bbc1c6a ! 1: ed28acaed97 diff: have submodule_format logic avoid additional diff headers
@@ Commit message
The submodule_format handling is another codepath with the same issue;
it would operate on these additional headers and attempt to display them
- as submodule changes. Prevent that by explicitly checking for both
- modes being 0.
+ as submodule changes. Prevent that by explicitly checking for "phoney"
+ filepairs (i.e. filepairs with both modes being 0).
Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
@@ diff.c: static void add_formatted_headers(struct strbuf *msg,
+static int diff_filepair_is_phoney(struct diff_filespec *one,
+ struct diff_filespec *two)
+{
++ /*
++ * This function specifically looks for pairs injected by
++ * create_filepairs_for_header_only_notifications(). Such
++ * pairs are "phoney" in that they do not represent any
++ * content or even mode difference, but were inserted because
++ * diff_queued_diff previously had no pair associated with
++ * that path but we needed some pair to avoid losing the
++ * "remerge CONFLICT" header associated with the path.
++ */
+ return !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);
+}
+
@@ diff.c: static void builtin_diff(const char *name_a,
- if (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {
+ if (diff_filepair_is_phoney(one, two)) {
/*
- * We should only reach this point for pairs from
+- * We should only reach this point for pairs from
++ * We should only reach this point for pairs generated from
* create_filepairs_for_header_only_notifications(). For
+- * these, we should avoid the "/dev/null" special casing
+- * above, meaning we avoid showing such pairs as either
++ * these, we want to avoid the "/dev/null" special casing
++ * above, because we do not want such pairs shown as either
+ * "new file" or "deleted file" below.
+ */
+ lbl[0] = a_one;
## t/t4069-remerge-diff.sh ##
@@ t/t4069-remerge-diff.sh: test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif
2: 3bd622d5b45 = 2: f91bea2bbc3 diff: fix filtering of additional headers under --remerge-diff
3: 7903f8380dc = 3: 084a037756d diff: fix filtering of merge commits under --remerge-diff
--
gitgitgadget
next prev parent reply other threads:[~2022-09-02 3:55 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 ` [PATCH v2 2/3] diff: fix filtering of additional headers under --remerge-diff Elijah Newren via GitGitGadget
2022-09-01 7:08 ` [PATCH v2 3/3] diff: fix filtering of merge commits " Elijah Newren via GitGitGadget
2022-09-02 3:53 ` Elijah Newren via GitGitGadget [this message]
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1342.v3.git.1662090810.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=avarab@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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).