Re: [PATCH v2.5 2/2] tag: prevent nested tags

From: Jeff King <peff@peff.net>
Date: Apr 4, 2019, 02:02 UTC
Message-ID: <20190404020226.GG4409@sigill.intra.peff.net>
In-Reply-To: <20190403213318.GA14137@dev-l>

On Wed, Apr 03, 2019 at 02:33:18PM -0700, Denton Liu wrote:

Show 14 quoted lines

> > I am not sure if this is so bad, actually.  Why do we need to treat
> > it as a mistake?  When a command that wants a commit is fed a tag
> > (either a tag that directly refers to a commit, or a tag that tags
> > another tag that refers to a commit), the command knows how to peel
> > so it's not like the user is forced to say "git log T^{commit}".
> 
> This patch came about because Robert Dailey expressed confusion after
> accidentally creating a tag-to-a-tag a while back by mistake when he
> actually meant to amend a tag.
> 
> In the discussion upthread, Peff noted that he has never seen a
> tag-to-a-tag in the wild before. I think the conclusion was that for
> the majority of users, doing this is an error. That is what this patch
> is guarding against.

I do still think it is likely to be a mistake. I think Junio's point, though is: who cares if the mistake was made? For the most part you can continue to use the tag as if the mistake had never been made, because Git peels through multiple layers as necessary.

The only thing that caused the discussion in the first place is that when "git show" displays each layer, there was a little head-scratching.

So if we were starting from scratch and designing the behavior, I think putting a safety valve around this mistake would probably be a no-brainer.

But given that we're changing long-standing behavior (that somebody _could_ be using intentionally), is it worth it to fix what is a mostly harmless outcome?

I'm on the fence personally. I think I do still lean slightly towards "yes, detect this in the porcelain", but I can see an argument both ways.

-Peff