nathan.panike@gmail.comIn my local git.git:
$ git rev-parse 73c6b3575bc638b7096ec913bd91193707e2265d^@ 57526fde5df201a99afa6d122c3266b3a1c5673a 942e6baa92846e5628752c65a22bc4957d8de4d0
$ git rev-parse --short 73c6b3575bc638b7096ec913bd91193707e2265d^@ 57526fd 942e6ba fatal: Needed a single revision
^^^ I don't believe this "fatal" message should be here
$ git version git version 1.7.8
Nathan Panike
Jeff KingRe: [BUG] in rev-parse
On Wed, Dec 14, 2011 at 12:49:27PM -0600, nathan.panike@gmail.com wrote:
> In my local git.git: > > $ git rev-parse 73c6b3575bc638b7096ec913bd91193707e2265d^@ > 57526fde5df201a99afa6d122c3266b3a1c5673a > 942e6baa92846e5628752c65a22bc4957d8de4d0 > > $ git rev-parse --short 73c6b3575bc638b7096ec913bd91193707e2265d^@ > 57526fd > 942e6ba > fatal: Needed a single revision > > ^^^ I don't believe this "fatal" message should be here
It looks like "--short" implies "--verify", and you cannot "--verify" multiple sha1s.
But the documentation for "--short" says only:
--short::
--short=number::
Instead of outputting the full SHA1 values of object names try to
abbreviate them to a shorter unique name. When no length is specified
7 is used. The minimum length is 4.which would imply to me that not only should your example work, but this should, too:
$ git rev-parse HEAD HEAD^ 803b1a83b0ec5f04dfb770e83e11211e0015630f 28c2058b6048d32abc0a23b827ab0b26a0332b9b
$ git rev-parse --short HEAD HEAD^ fatal: Needed a single revision
On the other hand, it has been like this since it was introduced in 2006, and I wonder if scripts rely on the --verify side effect.
As a work-around, you can get what you want with:
git rev-list --no-walk --abbrev-commit $sha1^@
-Peff
Junio C HamanoRe: [BUG] in rev-parse
Jeff King <peff@peff.net> writes:
> On the other hand, it has been like this since it was introduced in > 2006, and I wonder if scripts rely on the --verify side effect.
It would have been nicer if it did not to imply --verify at all; a long hexdigit that do not name an existing object at all will be shortened to its prefix that still do not collide with an abbreviated object name of an existing object, and even in such a case, the command should not error out only because it was fed a non-existing object (of course, if "--verify" is given at the same time, its "one input that names existing object only" rule should also kick in).
Jeff KingRe: [BUG] in rev-parse
On Wed, Dec 14, 2011 at 07:20:41PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes: > > > On the other hand, it has been like this since it was introduced in > > 2006, and I wonder if scripts rely on the --verify side effect. > > It would have been nicer if it did not to imply --verify at all; a long > hexdigit that do not name an existing object at all will be shortened to > its prefix that still do not collide with an abbreviated object name of an > existing object, and even in such a case, the command should not error out > only because it was fed a non-existing object (of course, if "--verify" is > given at the same time, its "one input that names existing object only" > rule should also kick in).
Dropping the implied verify is easy (see below). But handling non-existant sha1s is a much more complicated change, as the regular abbreviation machinery assumes that they exist. E.g., with the patch below:
$ good=73c6b3575bc638b7096ec913bd91193707e2265d
$ bad=${good#d}e
$ git rev-parse --short $good
73c6b35
$ git rev-parse --short $bad
[no output]Anyway, I'm not sure it's worth changing at this point. It's part of the plumbing API that has been that way forever, it's kind of a rare thing to ask for, and I've already shown a workaround using rev-list.
-Peff
---
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 98d1cbe..b365ca0 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -545,7 +545,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) if (!strcmp(arg, "--short") || !prefixcmp(arg, "--short=")) { filter &= ~(DO_FLAGS|DO_NOREV); - verify = 1; abbrev = DEFAULT_ABBREV; if (arg[7] == '=') abbrev = strtoul(arg + 8, NULL, 10);
Junio C HamanoRe: [BUG] in rev-parse
Jeff King <peff@peff.net> writes:
> Dropping the implied verify is easy (see below). But handling > non-existant sha1s is a much more complicated change, as the regular > abbreviation machinery assumes that they exist.
Hmm, I was very sure that I wrote a logic to do the "no such object" case at least once, so I looked and found one in find_unique_abbrev(). But that requires the caller to feed the full 160-bit sha1[] and perhaps it is not easily used in the context of rev-parse (I didn't look further).
> Anyway, I'm not sure it's worth changing at this point. It's part of the > plumbing API that has been that way forever,...
Ok.
Michael HaggertyRe: [BUG] in rev-parse
On 12/15/2011 08:05 AM, Jeff King wrote:
> On Wed, Dec 14, 2011 at 07:20:41PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> On the other hand, it has been like this since it was introduced in
>>> 2006, and I wonder if scripts rely on the --verify side effect.
>>
>> It would have been nicer if it did not to imply --verify at all; a long
>> hexdigit that do not name an existing object at all will be shortened to
>> its prefix that still do not collide with an abbreviated object name of an
>> existing object, and even in such a case, the command should not error out
>> only because it was fed a non-existing object (of course, if "--verify" is
>> given at the same time, its "one input that names existing object only"
>> rule should also kick in).
>
> Dropping the implied verify is easy (see below). But handling
> non-existant sha1s is a much more complicated change, as the regular
> abbreviation machinery assumes that they exist. E.g., with the patch
> below:
>
> $ good=73c6b3575bc638b7096ec913bd91193707e2265d
> $ bad=${good#d}e
> $ git rev-parse --short $good
> 73c6b35
> $ git rev-parse --short $bad
> [no output]
>
> Anyway, I'm not sure it's worth changing at this point. It's part of the
> plumbing API that has been that way forever, it's kind of a rare thing
> to ask for, and I've already shown a workaround using rev-list.I believe that the OP was more inconvenienced that "git rev-parse --short" chokes on multiple objects than by the fact that it insists that the objects exist. (And shortening the SHA1s of non-existent objects doesn't sound very useful anyway.) So I think that a useful compromise would be for "git rev-parse --short" to accept multiple args but continue to insist that each of the args is a valid object.
If that is considered too big a break with backwards compatibility, one could add a --no-verify option that turns off the verification behavior of --short. But IMHO this problem is not important enough to justify adding an extra option.
Michael
-- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/
Jeff KingRe: [BUG] in rev-parse
On Thu, Dec 15, 2011 at 11:53:50PM +0100, Michael Haggerty wrote:
> I believe that the OP was more inconvenienced that "git rev-parse > --short" chokes on multiple objects than by the fact that it insists > that the objects exist. (And shortening the SHA1s of non-existent > objects doesn't sound very useful anyway.) So I think that a useful > compromise would be for "git rev-parse --short" to accept multiple args > but continue to insist that each of the args is a valid object.
Part of the guarantee of "--verify" is that it returns a single object. I don't know how many callers rely on "--short" implying "--verify" implying a single object.
I agree in practice it would probably be an OK change, and it's very easy to do. I just don't think it's an important enough problem to worry about, given the available workaround. But if you want to write the patch, be my guest. :)
-Peff