From 793c21182e34f109b4f438944b4272fb50862ad5 Mon Sep 17 00:00:00 2001 From: René Scharfe Date: Sat, 1 Oct 2022 12:25:36 +0200 Subject: revision: use strtol_i() for exclude_parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid silent overflow of the int exclude_parent by using the appropriate function, strtol_i(), to parse its value. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- revision.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/revision.c b/revision.c index 0c6e26cd9c..da9dfd405e 100644 --- a/revision.c +++ b/revision.c @@ -2112,9 +2112,8 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl int exclude_parent = 1; if (mark[2]) { - char *end; - exclude_parent = strtoul(mark + 2, &end, 10); - if (*end != '\0' || !exclude_parent) + if (strtol_i(mark + 2, 10, &exclude_parent) || + exclude_parent < 1) return -1; } -- cgit v1.3 From 9f91da752fa28e405e91dfd6bd7372f897bbae8d Mon Sep 17 00:00:00 2001 From: René Scharfe Date: Sat, 1 Oct 2022 12:26:34 +0200 Subject: revisions.txt: unspecify order of resolved parts of ^! MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gitrevisions(7) says that ^! resolves to and then all the parents of . revision.c::handle_revision_arg_1() actually adds all parents first, then . Change the documentation to leave the order unspecified, to avoid misleading readers. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/revisions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index e3e350126d..0d2e55d781 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -363,7 +363,7 @@ Revision Range Summary '{caret}!', e.g. 'HEAD{caret}!':: A suffix '{caret}' followed by an exclamation mark is the same - as giving commit '' and then all its parents prefixed with + as giving commit '' and all its parents prefixed with '{caret}' to exclude them (and their ancestors). '{caret}-', e.g. 'HEAD{caret}-, HEAD{caret}-2':: -- cgit v1.3 From a79c6b60817c74534815bf132f0b26aa8e325874 Mon Sep 17 00:00:00 2001 From: René Scharfe Date: Sat, 1 Oct 2022 12:28:07 +0200 Subject: diff: support ^! for merges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit revision.c::handle_revision_arg_1() resolves ^! by first adding the negated parents and then itself. builtin_diff_combined() expects the first tree to be the merge and the remaining ones to be the parents, though. This mismatch results in bogus diff output. Remember the first tree that doesn't belong to a parent and use it instead of blindly picking the first one. This makes "git diff ^!" consistent with "git show ^!". Reported-by: Tim Jaacks Suggested-by: Junio C Hamano Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/git-diff.txt | 8 ++++---- builtin/diff.c | 23 ++++++++++++++++++----- t/t4038-diff-combined.sh | 10 ++++++++++ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt index 6236c75c9b..44748fa802 100644 --- a/Documentation/git-diff.txt +++ b/Documentation/git-diff.txt @@ -79,10 +79,10 @@ If --merge-base is given, use the merge base of the two commits for the This form is to view the results of a merge commit. The first listed must be the merge itself; the remaining two or - more commits should be its parents. A convenient way to produce - the desired set of revisions is to use the `^@` suffix. - For instance, if `master` names a merge commit, `git diff master - master^@` gives the same combined diff as `git show master`. + more commits should be its parents. Convenient ways to produce + the desired set of revisions are to use the suffixes `^@` and + `^!`. If A is a merge commit, then `git diff A A^@`, + `git diff A^!` and `git show A` all give the same combined diff. 'git diff' [] .. [--] [...]:: diff --git a/builtin/diff.c b/builtin/diff.c index 54bb3de964..0e49919735 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -209,7 +209,7 @@ static int builtin_diff_tree(struct rev_info *revs, static int builtin_diff_combined(struct rev_info *revs, int argc, const char **argv, struct object_array_entry *ent, - int ents) + int ents, int first_non_parent) { struct oid_array parents = OID_ARRAY_INIT; int i; @@ -217,11 +217,18 @@ static int builtin_diff_combined(struct rev_info *revs, if (argc > 1) usage(builtin_diff_usage); + if (first_non_parent < 0) + die(_("no merge given, only parents.")); + if (first_non_parent >= ents) + BUG("first_non_parent out of range: %d", first_non_parent); + diff_merges_set_dense_combined_if_unset(revs); - for (i = 1; i < ents; i++) - oid_array_append(&parents, &ent[i].item->oid); - diff_tree_combined(&ent[0].item->oid, &parents, revs); + for (i = 0; i < ents; i++) { + if (i != first_non_parent) + oid_array_append(&parents, &ent[i].item->oid); + } + diff_tree_combined(&ent[first_non_parent].item->oid, &parents, revs); oid_array_clear(&parents); return 0; } @@ -385,6 +392,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) int i; struct rev_info rev; struct object_array ent = OBJECT_ARRAY_INIT; + int first_non_parent = -1; int blobs = 0, paths = 0; struct object_array_entry *blob[2]; int nongit = 0, no_index = 0; @@ -543,6 +551,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix) continue; obj->flags |= flags; add_object_array(obj, name, &ent); + if (first_non_parent < 0 && + (i >= rev.cmdline.nr || /* HEAD by hand. */ + rev.cmdline.rev[i].whence != REV_CMD_PARENTS_ONLY)) + first_non_parent = ent.nr - 1; } else if (obj->type == OBJ_BLOB) { if (2 <= blobs) die(_("more than two blobs given: '%s'"), name); @@ -590,7 +602,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix) &ent.objects[0], &ent.objects[1]); } else result = builtin_diff_combined(&rev, argc, argv, - ent.objects, ent.nr); + ent.objects, ent.nr, + first_non_parent); result = diff_result_code(&rev.diffopt, result); if (1 < rev.diffopt.skip_stat_unmatch) refresh_index_quietly(); diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh index 9a292bac70..2ce26e585c 100755 --- a/t/t4038-diff-combined.sh +++ b/t/t4038-diff-combined.sh @@ -80,11 +80,21 @@ test_expect_success 'check combined output (1)' ' verify_helper sidewithone ' +test_expect_success 'check combined output (1) with git diff ^!' ' + git diff sidewithone^! -- >sidewithone && + verify_helper sidewithone +' + test_expect_success 'check combined output (2)' ' git show sidesansone -- >sidesansone && verify_helper sidesansone ' +test_expect_success 'check combined output (2) with git diff ^!' ' + git diff sidesansone^! -- >sidesansone && + verify_helper sidesansone +' + test_expect_success 'diagnose truncated file' ' >file && git add file && -- cgit v1.3