git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 0/3] Output fixes for --remerge-diff

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 2, 2022, 03:53 UTC
Message-ID
<pull.1342.v3.git.1662090810.gitgitgadget@gmail.com>
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
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 20 of 23 in “Output fixes for --remerge-diff”
  1. 0/3 Output fixes for --remerge-diffElijah Newren via GitGitGadget, Aug 31, 2022
  2. 1/3 diff: have submodule_format logic avoid additional diff headersElijah Newren via GitGitGadget, Aug 31, 2022
  3. Junio C HamanoAug 31, 2022
  4. Elijah NewrenSep 1, 2022
  5. 2/3 diff: fix filtering of additional headers under --remerge-diffElijah Newren via GitGitGadget, Aug 31, 2022
  6. Junio C HamanoAug 31, 2022
  7. Elijah NewrenSep 1, 2022
  8. 3/3 diff: fix filtering of merge commits under --remerge-diffElijah Newren via GitGitGadget, Aug 31, 2022
  9. Junio C HamanoSep 1, 2022
  10. Elijah NewrenSep 1, 2022
  11. Elijah NewrenSep 1, 2022
  12. Junio C HamanoSep 1, 2022
  13. Ævar Arnfjörð BjarmasonSep 1, 2022
  14. Junio C HamanoSep 1, 2022
  15. 0/3 Output fixes for --remerge-diffElijah Newren via GitGitGadget, Sep 1, 2022
  16. 1/3 diff: have submodule_format logic avoid additional diff headersElijah Newren via GitGitGadget, Sep 1, 2022
  17. Junio C HamanoSep 1, 2022
  18. 2/3 diff: fix filtering of additional headers under --remerge-diffElijah Newren via GitGitGadget, Sep 1, 2022
  19. 3/3 diff: fix filtering of merge commits under --remerge-diffElijah Newren via GitGitGadget, Sep 1, 2022
  20. 0/3 Output fixes for --remerge-diffElijah Newren via GitGitGadget, Sep 2, 2022
  21. 1/3 diff: have submodule_format logic avoid additional diff headersElijah Newren via GitGitGadget, Sep 2, 2022
  22. 3/3 diff: fix filtering of merge commits under --remerge-diffElijah Newren via GitGitGadget, Sep 2, 2022
  23. 2/3 diff: fix filtering of additional headers under --remerge-diffElijah Newren via GitGitGadget, Sep 2, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.