Skip to content

Support diff merges option in range diff

Johannes Schindelin via GitGitGadget git 3 versions – New

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v3 Latest2 patches 0 repliesNew
  2. v21 patch 8 repliesSuperseded
  3. v11 patch 8 repliesSuperseded

Changes in v3 from the cover letter

* 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

Patches in v3 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2range-diff: optionally include merge commits' diffs in the analysisDocumentation/git-range-diff.txt builtin/range-diff.c range-diff.c +2 more+51 -60 replies
2/2range-diff: introduce the convenience option `--remerge-diff`Documentation/git-range-diff.txt builtin/range-diff.c+6 -00 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v3 yet.

Read the v3 thread

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

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.

@@ 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

Signature · 1 line
gitgitgadget