On Fri, Feb 26, 2016 at 07:27:44PM -0500, santiago@xxxxxxx wrote:
> From: Santiago Torres <santiago@xxxxxxx>
>
> The verify tag function converts the commit sha1 to hex and passes it as
> a command-line argument to builtin/verify-tag. Given that builtin/verify-tag
> already resolves the ref name sha1 equivalent, the sha1 to
> hex_sha1 conversion is unnecessary and the ref-name can be used instead.
Hrm. This is potentially racy, if git-tag is going to say something
about the ref, but git-verify-tag may have actually verified another tag
entirely.
AFAICT, though, git-tag doesn't say anything, and is just purely
forwarding work to verify-tag. So I can't see a real downside to passing
in the ref name, except that it is slightly less efficient (because
verify_tag has to re-resolve it). But...
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 1705c94..5de1161 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -105,8 +105,7 @@ static int verify_tag(const char *name, const char *ref,
> const unsigned char *sha1)
> {
> const char *argv_verify_tag[] = {"verify-tag",
> - "-v", "SHA1_HEX", NULL};
> - argv_verify_tag[2] = sha1_to_hex(sha1);
> + "-v", name, NULL};
You are passing in "name" here, not "ref". git-tag knows it is operating
specifically on tags, and completes a name like "foo" to
"refs/tags/foo". Whereas verify-tag is plumbing that can operate on any
ref, and will do the usual lookup for "foo", "refs/heads/foo",
"refs/tags/foo", etc.
So by passing the unqualified name, we may end up finding something
entirely different, generating "ambiguous name" errors, etc. So if we
_were_ to go this route, I think we'd need to use "ref" here, not
"name".
But I'm not really sure I see the upside.
A much more interesting change in this area, I think, would be to skip
verify-tag entirely. Once upon a time it had a lot of logic itself, but
these days it is a thin wrapper over run_gpg_verify(), and we could
improve the efficiency quite a bit by eliminates the sub-process
entirely.
-Peff
--
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