From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Kevin Bracey <kevin@bracey•fi>,
Jonathan Nieder <jrnieder@gmail•com>,
git@vger•kernel.org
Subject: Re: breakage in revision traversal with pathspec
Date: Fri, 20 Sep 2013 10:51:55 -0700 [thread overview]
Message-ID: <xmqqeh8jqt38.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20130920051107.GA17609@sigill.intra.peff.net> (Jeff King's message of "Fri, 20 Sep 2013 01:11:07 -0400")
Jeff King <peff@peff•net> 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
next prev parent reply other threads:[~2013-09-20 17:52 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-10 17:19 breakage in revision traversal with pathspec Junio C Hamano
2013-09-10 21:27 ` Kevin Bracey
2013-09-10 22:23 ` Junio C Hamano
2013-09-11 17:49 ` Kevin Bracey
2013-09-11 18:24 ` Jonathan Nieder
2013-09-11 19:21 ` Junio C Hamano
2013-09-11 19:39 ` Kevin Bracey
2013-09-11 21:15 ` Junio C Hamano
2013-09-19 21:35 ` Junio C Hamano
2013-09-20 3:35 ` Jeff King
2013-09-20 4:58 ` Junio C Hamano
2013-09-20 5:11 ` Jeff King
2013-09-20 17:51 ` Junio C Hamano [this message]
2013-09-25 9:12 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqeh8jqt38.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jrnieder@gmail$(echo .)com \
--cc=kevin@bracey$(echo .)fi \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox