Re: [PATCHv2] tag: add --points-at list option

From: Jeff King <peff@peff.net>
Date: Feb 8, 2012, 00:25 UTC
Message-ID: <20120208002554.GA6035@sigill.intra.peff.net>
In-Reply-To: <20120207220806.GD6264@tgrennan-laptop>

On Tue, Feb 07, 2012 at 02:08:06PM -0800, Tom Grennan wrote:

Show 8 quoted lines

> v1 and v2 wouldn't list lightweight tags of the points-at objects.
> Both versions behave like this:
>   $ git tag my-lw-v1.7.9 v1.7.9
>   $ git tag my-a-v1.7.9 v1.7.9
>   $ git tag my-s-v1.7.9 v1.7.9
>   $ git tag -l --points-at v1.7.9
>   my-a-v1.7.9
>   my-s-v1.7.9

I assume the 2nd and 3rd line should be:

  $ git tag -a my-a-v1.7.9 v1.7.9
  $ git tag -s my-s-v1.7.9 v1.7.9

Show 13 quoted lines

> static struct points_at *match_points_at(struct points_at *points_at,
> 					 const char *refname,
> 					 const unsigned char *sha1)
> {
> 	struct object *obj;
> 	struct points_at *pa;
> 	const unsigned char *tagged_sha1;
> 
> 	/* First look for lightweight tags - those with matching sha's
> 	 * but different names */
> 	for (pa = points_at; pa; pa = pa->next)
> 		if (!hashcmp(pa->sha1, sha1) && strcmp(pa->refname, refname))
> 			return pa;

OK, I see what you are trying to accomplish here. But I really don't like it. Two complaints:

  1. Why is the name of the tag relevant? That is, if you are interested
     in lightweight tags, and you have two tag refs, "refs/tags/a" and
     "refs/tags/b", both pointing to the same tag object, then in what
     situation is it useful to show "a" but not "b"?

     It seems to me you would either want lightweight tags or not. And I
     thought not, because the point of this was to reveal signatures or
     annotations about a tag. Your my-lw-v1.7.9 says neither. Why do we
     want to show it?

     Also, it's not symmetric. What if I say "git tag
     --points-at=my-lw-v1.7.9"? Then I would get your signed and
     annotated tags (even though they're _not_ saying anything about
     ny-lw-v1.7.9), and I would get v1.7.9 (even though it's not saying
     anything about it either; in fact, it's the opposite!).

  2. I thought --points-at was about providing an object name. But it's
     not. It's about providing a particular string. So with this code,
     "git tag --points-at=v1.7.9" and "git tag --points-at=$(git
     rev-parse v1.7.9)" are two different things. Which seems odd and
     un-git-like to me.

     Your documentation says "Only list annotated or signed tags of the
     given object", which implies to me that --points-at is an arbitrary
     object specifier, not a specific tagname.

It seems like your rationale is just avoiding a mention of v1.7.9 because, hey, it was obviously on the command line and the user isn't interested in it. But I don't think that's true. The user asked for every tag pointing to v1.7.9's object, and v1.7.9 is such a tag. It is no more or less true for v1.7.9 than it is for my-lw-v1.7.9.

-Peff