range-diff: optionally include merge commits' diffs in the analysis

The `git log` command already offers support for including diffs for
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 commit ranges that contain
merges.

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Johannes Schindelin
2024-12-16 14:11:21 +00:00
committed by Junio C Hamano
parent eb8374c652
commit f8043236c6
5 changed files with 50 additions and 5 deletions

View File

@ -10,7 +10,7 @@ SYNOPSIS
[verse] [verse]
'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>] 'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]
[--no-dual-color] [--creation-factor=<factor>] [--no-dual-color] [--creation-factor=<factor>]
[--left-only | --right-only] [--left-only | --right-only] [--diff-merges=<format>]
( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> ) ( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
[[--] <path>...] [[--] <path>...]
@ -81,6 +81,17 @@ to revert to color all lines according to the outer diff markers
Suppress commits that are missing from the second specified range Suppress commits that are missing from the second specified range
(or the "right range" when using the `<rev1>...<rev2>` format). (or the "right range" when using the `<rev1>...<rev2>` format).
--diff-merges=<format>::
Instead of ignoring merge commits, generate diffs for them using the
corresponding `--diff-merges=<format>` option of linkgit:git-log[1],
and include them in the comparison.
+
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>]:: --[no-]notes[=<ref>]::
This flag is passed to the `git log` program This flag is passed to the `git log` program
(see linkgit:git-log[1]) that generates the patches. (see linkgit:git-log[1]) that generates the patches.

View File

@ -21,6 +21,7 @@ int cmd_range_diff(int argc,
{ {
struct diff_options diffopt = { NULL }; struct diff_options diffopt = { NULL };
struct strvec other_arg = STRVEC_INIT; struct strvec other_arg = STRVEC_INIT;
struct strvec diff_merges_arg = STRVEC_INIT;
struct range_diff_options range_diff_opts = { struct range_diff_options range_diff_opts = {
.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT, .creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,
.diffopt = &diffopt, .diffopt = &diffopt,
@ -36,6 +37,8 @@ int cmd_range_diff(int argc,
OPT_PASSTHRU_ARGV(0, "notes", &other_arg, OPT_PASSTHRU_ARGV(0, "notes", &other_arg,
N_("notes"), N_("passed to 'git log'"), N_("notes"), N_("passed to 'git log'"),
PARSE_OPT_OPTARG), PARSE_OPT_OPTARG),
OPT_PASSTHRU_ARGV(0, "diff-merges", &diff_merges_arg,
N_("style"), N_("passed to 'git log'"), 0),
OPT_BOOL(0, "left-only", &left_only, OPT_BOOL(0, "left-only", &left_only,
N_("only emit output related to the first range")), N_("only emit output related to the first range")),
OPT_BOOL(0, "right-only", &right_only, OPT_BOOL(0, "right-only", &right_only,
@ -62,6 +65,12 @@ int cmd_range_diff(int argc,
if (!simple_color) if (!simple_color)
diffopt.use_color = 1; diffopt.use_color = 1;
/* If `--diff-merges` was specified, imply `--merges` */
if (diff_merges_arg.nr) {
range_diff_opts.include_merges = 1;
strvec_pushv(&other_arg, diff_merges_arg.v);
}
for (i = 0; i < argc; i++) for (i = 0; i < argc; i++)
if (!strcmp(argv[i], "--")) { if (!strcmp(argv[i], "--")) {
dash_dash = i; dash_dash = i;
@ -155,6 +164,7 @@ int cmd_range_diff(int argc,
res = show_range_diff(range1.buf, range2.buf, &range_diff_opts); res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
strvec_clear(&other_arg); strvec_clear(&other_arg);
strvec_clear(&diff_merges_arg);
strbuf_release(&range1); strbuf_release(&range1);
strbuf_release(&range2); strbuf_release(&range2);

View File

@ -38,7 +38,8 @@ struct patch_util {
* as struct object_id (will need to be free()d). * as struct object_id (will need to be free()d).
*/ */
static int read_patches(const char *range, struct string_list *list, static int read_patches(const char *range, struct string_list *list,
const struct strvec *other_arg) const struct strvec *other_arg,
unsigned int include_merges)
{ {
struct child_process cp = CHILD_PROCESS_INIT; struct child_process cp = CHILD_PROCESS_INIT;
struct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT; struct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;
@ -49,7 +50,7 @@ static int read_patches(const char *range, struct string_list *list,
size_t size; size_t size;
int ret = -1; int ret = -1;
strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges", strvec_pushl(&cp.args, "log", "--no-color", "-p",
"--reverse", "--date-order", "--decorate=no", "--reverse", "--date-order", "--decorate=no",
"--no-prefix", "--submodule=short", "--no-prefix", "--submodule=short",
/* /*
@ -64,6 +65,8 @@ static int read_patches(const char *range, struct string_list *list,
"--pretty=medium", "--pretty=medium",
"--show-notes-by-default", "--show-notes-by-default",
NULL); NULL);
if (!include_merges)
strvec_push(&cp.args, "--no-merges");
strvec_push(&cp.args, range); strvec_push(&cp.args, range);
if (other_arg) if (other_arg)
strvec_pushv(&cp.args, other_arg->v); strvec_pushv(&cp.args, other_arg->v);
@ -96,11 +99,14 @@ static int read_patches(const char *range, struct string_list *list,
} }
if (skip_prefix(line, "commit ", &p)) { if (skip_prefix(line, "commit ", &p)) {
char *q;
if (util) { if (util) {
string_list_append(list, buf.buf)->util = util; string_list_append(list, buf.buf)->util = util;
strbuf_reset(&buf); strbuf_reset(&buf);
} }
CALLOC_ARRAY(util, 1); CALLOC_ARRAY(util, 1);
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); error(_("could not parse commit '%s'"), p);
FREE_AND_NULL(util); FREE_AND_NULL(util);
@ -571,13 +577,14 @@ int show_range_diff(const char *range1, const char *range2,
struct string_list branch1 = STRING_LIST_INIT_DUP; struct string_list branch1 = STRING_LIST_INIT_DUP;
struct string_list branch2 = STRING_LIST_INIT_DUP; struct string_list branch2 = STRING_LIST_INIT_DUP;
unsigned int include_merges = range_diff_opts->include_merges;
if (range_diff_opts->left_only && range_diff_opts->right_only) if (range_diff_opts->left_only && range_diff_opts->right_only)
res = error(_("options '%s' and '%s' cannot be used together"), "--left-only", "--right-only"); res = error(_("options '%s' and '%s' cannot be used together"), "--left-only", "--right-only");
if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg)) if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg, include_merges))
res = error(_("could not parse log for '%s'"), range1); res = error(_("could not parse log for '%s'"), range1);
if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg)) if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg, include_merges))
res = error(_("could not parse log for '%s'"), range2); res = error(_("could not parse log for '%s'"), range2);
if (!res) { if (!res) {

View File

@ -16,6 +16,7 @@ struct range_diff_options {
int creation_factor; int creation_factor;
unsigned dual_color:1; unsigned dual_color:1;
unsigned left_only:1, right_only:1; unsigned left_only:1, right_only:1;
unsigned include_merges:1;
const struct diff_options *diffopt; /* may be NULL */ const struct diff_options *diffopt; /* may be NULL */
const struct strvec *other_arg; /* may be NULL */ const struct strvec *other_arg; /* may be NULL */
}; };

View File

@ -909,4 +909,20 @@ test_expect_success 'submodule changes are shown irrespective of diff.submodule'
test_cmp expect actual test_cmp expect actual
' '
test_expect_success '--diff-merges' '
renamed_oid=$(git rev-parse --short renamed-file) &&
tree=$(git merge-tree unmodified renamed-file) &&
clean=$(git commit-tree -m merge -p unmodified -p renamed-file $tree) &&
clean_oid=$(git rev-parse --short $clean) &&
conflict=$(git commit-tree -m merge -p unmodified -p renamed-file^ $tree) &&
conflict_oid=$(git rev-parse --short $conflict) &&
git range-diff --diff-merges=1 $clean...$conflict >actual &&
cat >expect <<-EOF &&
1: $renamed_oid < -: ------- s/12/B/
2: $clean_oid = 1: $conflict_oid merge
EOF
test_cmp expect actual
'
test_done test_done