Richard Hansen wrote:
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -677,6 +677,8 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
> sp++; /* beginning of type name, or closing brace for empty */
> if (!strncmp(commit_type, sp, 6) && sp[6] == '}')
> expected_type = OBJ_COMMIT;
> + else if (!strncmp(tag_type, sp, 3) && sp[3] == '}')
> + expected_type = OBJ_TAG;
Interesting.
> gitrevisions(7) implies that <rev>^{tag} should work, but before now
> it did not:
The wording (especially of <rev>^{}) special-cases tags as "objects to
dereference".
> $ git rev-parse --verify v1.8.3.1^{}^{object}
> 362de916c06521205276acb7f51c99f47db94727
> $ git rev-parse --verify v1.8.3.1^{}^{tag}
> error: v1.8.3.1^{}^{tag}: expected tag type, but the object dereferences to tree type
> fatal: Needed a single revision
And the points out the problem: while ^{object} means "expect
OBJ_ANY", ^{} means "expect OBJ_NONE". What does that even mean? See
sha1_name.c:704 where this is handled sneakily: it just calls
deref_tag(), bypassing peel_to_type() altogether. If anything, ^{} is
already a poor-man's version of your ^{tag}; the reason it's a
poor-man's version is precisely because it doesn't error out when
<rev> isn't a tag, while your ^{tag} does. I would argue that ^{} is
very poorly done and must be deprecated in favor of your ^{tag}. What
is the point of using ^{} if you can't even be sure that what you get
is a deref? peel_to_type() already does the right thing by not using
deref_tag(), and explicitly checking.
Your commit message needs some tweaking, but I'm happy with your patch
otherwise.
Thanks.
--
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