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

[PATCH v3 0/2] Support diff merges option in range diff

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
Dec 16, 2024, 14:11 UTC
Message-ID
<pull.1734.v3.git.1734358282.gitgitgadget@gmail.com>
In-Reply-To
<pull.1734.v2.git.1731073383564.gitgitgadget@gmail.com>

The git range-diff command does the same with merge commits as git rebase: It ignores them.

However, when comparing branch thickets it can be quite illuminating to watch out for inadvertent changes in merge commits, in particular when some "evil" merges have been replayed, i.e. merges that needed to introduce changes outside of the merge conflicts (e.g. when one branch changed a function's signature and another branch introduced a caller of said function), in case the replayed merge is no longer "evil" and therefore potentially incorrect.

Let's introduce support for the --diff-merges option that is passed through to those git log commands.

I had a need for this earlier this year and got it working, leaving the GitGitGadget PR in a draft mode. Phil Blain found it and kindly nerd-sniped me into readying it for submitting, so say thanks to Phil!

Changes since v2:
 * Rebased onto js/log-remerge-keep-ancestry, to fix --diff-merges=remerge
 * Now the documentation suggests remerge instead of first-parent mode
 * Added a follow-up commit to introduce the convenience option git
   range-diff --remerge-diff
Changes since v1:
 * Changed the documentation to recommend first-parent mode instead of
   vaguely talking about various modes' merits.
 * Disallowed the no-arg --diff-merges option (because --diff-merges
   requires an argument).
Johannes Schindelin (2):
  range-diff: optionally include merge commits' diffs in the analysis
  range-diff: introduce the convenience option `--remerge-diff`
 Documentation/git-range-diff.txt | 17 ++++++++++++++++-
 builtin/range-diff.c             | 12 ++++++++++++
 range-diff.c                     | 17 ++++++++++++-----
 range-diff.h                     |  1 +
 t/t3206-range-diff.sh            | 16 ++++++++++++++++
 5 files changed, 57 insertions(+), 6 deletions(-)
base-commit: f94bfa151623d7e675db9465ae7ff0b33e4825f3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1734%2Fdscho%2Fsupport-diff-merges-option-in-range-diff-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1734/dscho/support-diff-merges-option-in-range-diff-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1734
Range-diff vs v2:
 1:  d01a395900b ! 1:  ef3c243da1b range-diff: optionally include merge commits' diffs in the analysis
     @@ Commit message
          merges, via the `--diff-merges=<format>` option.
      
          Let's add corresponding support for `git range-diff`, too. This makes it
     -    more convenient to spot differences between iterations of non-linear
     -    contributions, where so-called "evil merges" are sometimes necessary and
     -    need to be reviewed, too.
     +    more convenient to spot differences between commit ranges that contain
     +    merges.
      
     -    In my code reviews, I found the `--diff-merges=first-parent` option
     +    This is especially true in scenarios with non-trivial merges, i.e.
     +    merges introducing changes other than, or in addition to, what merge ORT
     +    would have produced. Merging a topic branch that changes a function
     +    signature into a branch that added a caller of that function, for
     +    example, would require the merge commit itself to adjust that caller to
     +    the modified signature.
     +
     +    In my code reviews, I found the `--diff-merges=remerge` option
          particularly useful.
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
     @@ Documentation/git-range-diff.txt: to revert to color all lines according to the
      +	corresponding `--diff-merges=<format>` option of linkgit:git-log[1],
      +	and include them in the comparison.
      ++
     -+Note: In the common case, the `first-parent` mode will be the most natural one
     -+to use, as it is consistent with the idea that a merge is kind of a "meta
     -+patch", comprising all the merged commits' patches into a single one.
     ++Note: In the common case, the `remerge` mode will be the most natural one
     ++to use, as it shows only the diff on top of what Git's merge machinery would
     ++have produced. In other words, if a merge commit is the result of a
     ++non-conflicting `git merge`, the `remerge` mode will represent it with an empty
     ++diff.
      +
       --[no-]notes[=<ref>]::
       	This flag is passed to the `git log` program
       	(see linkgit:git-log[1]) that generates the patches.
      
       ## builtin/range-diff.c ##
     -@@ builtin/range-diff.c: int cmd_range_diff(int argc,
     +@@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix)
       {
       	struct diff_options diffopt = { NULL };
       	struct strvec other_arg = STRVEC_INIT;
     @@ builtin/range-diff.c: int cmd_range_diff(int argc,
       	struct range_diff_options range_diff_opts = {
       		.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,
       		.diffopt = &diffopt,
     -@@ builtin/range-diff.c: int cmd_range_diff(int argc,
     +@@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix)
       		OPT_PASSTHRU_ARGV(0, "notes", &other_arg,
       				  N_("notes"), N_("passed to 'git log'"),
       				  PARSE_OPT_OPTARG),
     @@ builtin/range-diff.c: int cmd_range_diff(int argc,
       		OPT_BOOL(0, "left-only", &left_only,
       			 N_("only emit output related to the first range")),
       		OPT_BOOL(0, "right-only", &right_only,
     -@@ builtin/range-diff.c: int cmd_range_diff(int argc,
     +@@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix)
       	if (!simple_color)
       		diffopt.use_color = 1;
       
     @@ builtin/range-diff.c: int cmd_range_diff(int argc,
       	for (i = 0; i < argc; i++)
       		if (!strcmp(argv[i], "--")) {
       			dash_dash = i;
     -@@ builtin/range-diff.c: int cmd_range_diff(int argc,
     +@@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix)
       	res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
       
       	strvec_clear(&other_arg);
     @@ range-diff.c: static int read_patches(const char *range, struct string_list *lis
       		     /*
      @@ range-diff.c: static int read_patches(const char *range, struct string_list *list,
       		     "--pretty=medium",
     - 		     "--show-notes-by-default",
     + 		     "--notes",
       		     NULL);
      +	if (!include_merges)
      +		strvec_push(&cp.args, "--no-merges");
     @@ range-diff.c: static int read_patches(const char *range, struct string_list *lis
       				strbuf_reset(&buf);
       			}
       			CALLOC_ARRAY(util, 1);
     +-			if (get_oid(p, &util->oid)) {
      +			if (include_merges && (q = strstr(p, " (from ")))
      +				*q = '\0';
     - 			if (repo_get_oid(the_repository, p, &util->oid)) {
     ++			if (repo_get_oid(the_repository, p, &util->oid)) {
       				error(_("could not parse commit '%s'"), p);
       				FREE_AND_NULL(util);
     + 				string_list_clear(list, 1);
      @@ range-diff.c: int show_range_diff(const char *range1, const char *range2,
       
       	struct string_list branch1 = STRING_LIST_INIT_DUP;
 -:  ----------- > 2:  f99416acd5a range-diff: introduce the convenience option `--remerge-diff`
-- 
gitgitgadget
Previous: Junio C HamanoNext: Johannes Schindelin via GitGitGadget
Message 18 of 21 in “range-diff: optionally include merge commits' diffs in the analysis”
  1. range-diff: optionally include merge commits' diffs in the analysisJohannes Schindelin via GitGitGadget, Nov 7, 2024
  2. Junio C HamanoNov 8, 2024
  3. Johannes SixtNov 8, 2024
  4. Johannes SchindelinNov 8, 2024
  5. Elijah NewrenNov 9, 2024
  6. Junio C HamanoNov 11, 2024
  7. Junio C HamanoNov 8, 2024
  8. Elijah NewrenNov 8, 2024
  9. range-diff: optionally include merge commits' diffs in the analysisJohannes Schindelin via GitGitGadget, Nov 8, 2024
  10. Elijah NewrenNov 8, 2024
  11. Johannes SchindelinNov 11, 2024
  12. Philippe BlainNov 10, 2024
  13. Johannes SchindelinNov 11, 2024
  14. Junio C HamanoNov 26, 2024
  15. Junio C HamanoNov 11, 2024
  16. Elijah NewrenNov 11, 2024
  17. Junio C HamanoNov 12, 2024
  18. 0/2 Support diff merges option in range diffJohannes Schindelin via GitGitGadget, Dec 16, 2024
  19. 1/2 range-diff: optionally include merge commits' diffs in the analysisJohannes Schindelin via GitGitGadget, Dec 16, 2024
  20. 2/2 range-diff: introduce the convenience option `--remerge-diff`Johannes Schindelin via GitGitGadget, Dec 16, 2024
  21. Elijah NewrenNov 8, 2024

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.