santi...@nyu.edu writes:

> From: Santiago Torres <santi...@nyu.edu>
>
> This change is meant to prepare verify_tag for libification. Many
> existing modules/commands already do the refname to sha1 resolution, so
> should avoid resolving the refname twice. To avoid breaking
> builtin/verify-tag, we move the refname resolution outside of the
> verify_tag() call.
>
> Signed-off-by: Santiago Torres <santi...@nyu.edu>
> ---
>  builtin/verify-tag.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> index 1ca9a05..7a7c376 100644
> --- a/builtin/verify-tag.c
> +++ b/builtin/verify-tag.c
> @@ -42,25 +42,23 @@ static int run_gpg_verify(const char *buf, unsigned long 
> size, unsigned flags)
>       return ret;
>  }
>  
> -static int verify_tag(const char *name, unsigned flags)
> +static int verify_tag(const unsigned char *sha1, unsigned flags)
>  {
>       enum object_type type;
> -     unsigned char sha1[20];
>       char *buf;
> +     char *hex_sha1;
>       unsigned long size;
>       int ret;
>  
> -     if (get_sha1(name, sha1))
> -             return error("tag '%s' not found.", name);
> -
> +     hex_sha1 = sha1_to_hex(sha1);
>       type = sha1_object_info(sha1, NULL);
>       if (type != OBJ_TAG)
>               return error("%s: cannot verify a non-tag object of type %s.",
> -                             name, typename(type));
> +                             hex_sha1, typename(type));

So, if I said

    $ git verify-tag master

the code used to take "master" from argv[], fed it to verify_tag()
as parameter 'name', turned it to the object name of the commit,
noticed that it is not a tag, and complained that "master: cannot verify".

With this rewrite, the same invocation would cause "master" to be
turned into the object name, which is passed to verify_tag() and the
complaint is an overlong

    76bece327f490cb344b95ae8f869cbeb89a4d20b: cannot verify a non-tag object of 
type commit

That does not sound like a good change at all.

If you want to support a future caller of a libified version of
verify_tag() that has a raw object name but not the original name,
I'd suggest to make this function keep parameter 'name' while adding
the new parameter 'sha1'.  Then, the error reporting may become:

        return error("%s: cannot verify a non-tag object of type '%s'",
                     name ? name : sha1_to_hex(sha1), typename(type));

and the output would still be useful.  Further improvements may be

 - rename 'name' to 'report_name' to clarify that the parameter is
   only used for reporting, and that the tag object to verify is
   always identified by the new 'sha1' parameter.

 - use find_unique_abbrev() to shorten the fallback name displayed
   in the error message.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to