Jeff King <peff@xxxxxxxx> writes: > My original question was going to be: why bother peeling at all if we > are just going to push the outer objects, anyway? > > And after staring at it, I somehow convinced myself that the answer was > that you were pushing both. But that is not the case. Sorry for the > noise. But that is still a valid point, and the patch to avoid peeling for non symmetric diff does not look too bad, either. revision.c | 59 ++++++++++++++++++++++++++++++------------------ t/t6000-rev-list-misc.sh | 8 +++++++ 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/revision.c b/revision.c index 68545c8..7010aff 100644 --- a/revision.c +++ b/revision.c @@ -1157,41 +1157,56 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi } if (!get_sha1_committish(this, from_sha1) && !get_sha1_committish(next, sha1)) { - struct commit *a, *b; - struct commit_list *exclude; - - a = lookup_commit_reference(from_sha1); - b = lookup_commit_reference(sha1); - if (!a || !b) { - if (revs->ignore_missing) - return 0; - die(symmetric ? - "Invalid symmetric difference expression %s...%s" : - "Invalid revision range %s..%s", - arg, next); - } + struct object *a_obj, *b_obj; if (!cant_be_filename) { *dotdot = '.'; verify_non_filename(revs->prefix, arg); } - if (symmetric) { + a_obj = parse_object(from_sha1); + b_obj = parse_object(sha1); + if (!a_obj || !b_obj) { + missing: + if (revs->ignore_missing) + return 0; + die(symmetric + ? "Invalid symmetric difference expression %s" + : "Invalid revision range %s", arg); + } + + if (!symmetric) { + /* just A..B */ + a_flags = flags_exclude; + } else { + /* A...B -- find merge bases between the two */ + struct commit *a, *b; + struct commit_list *exclude; + + a = (a_obj->type == OBJ_COMMIT + ? (struct commit *)a_obj + : lookup_commit_reference(a_obj->sha1)); + b = (b_obj->type == OBJ_COMMIT + ? (struct commit *)b_obj + : lookup_commit_reference(b_obj->sha1)); + if (!a || !b) + goto missing; exclude = get_merge_bases(a, b, 1); add_pending_commit_list(revs, exclude, flags_exclude); free_commit_list(exclude); + a_flags = flags | SYMMETRIC_LEFT; - } else - a_flags = flags_exclude; - a->object.flags |= a_flags; - b->object.flags |= flags; - add_rev_cmdline(revs, &a->object, this, + } + + a_obj->flags |= a_flags; + b_obj->flags |= flags; + add_rev_cmdline(revs, a_obj, this, REV_CMD_LEFT, a_flags); - add_rev_cmdline(revs, &b->object, next, + add_rev_cmdline(revs, b_obj, next, REV_CMD_RIGHT, flags); - add_pending_object(revs, &a->object, this); - add_pending_object(revs, &b->object, next); + add_pending_object(revs, a_obj, this); + add_pending_object(revs, b_obj, next); return 0; } *dotdot = '.'; diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh index b10685a..15e3d64 100755 --- a/t/t6000-rev-list-misc.sh +++ b/t/t6000-rev-list-misc.sh @@ -48,4 +48,12 @@ test_expect_success 'rev-list --objects with pathspecs and copied files' ' ! grep one output ' +test_expect_success 'rev-list A..B and rev-list ^A B are the same' ' + git commit --allow-empty -m another && + git tag -a -m "annotated" v1.0 && + git rev-list --objects ^v1.0^ v1.0 >expect && + git rev-list --objects v1.0^..v1.0 >actual && + test_cmp expect actual +' + test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html