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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 4, 2019, 21:50 UTC
Message-ID: <xmqqimvte8yr.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAHd499Dr5sjzFCFYvkwcS0WOo0W51_RyL7nLAg_MaGeFy5eQKQ@mail.gmail.com>

Robert Dailey <rcdailey.lists@gmail.com> writes:

> It might be fine within the realm of git itself, because git knows how
> to deal with them by peeling, as you say, but there are 3 reasons I
> dislike that this is allowed:

That sounds as if you have tools that forgets to peel when it should in mind. Isn't that what you should be looking into fixing? After all, tools that aim to work with Git should strive to come close to "within the realm of git itsef" in the modern world.

> 1. The intent by the user was to create a tag pointing to the commit
> that another tag points to.

You make it sound as if you are convinced that is the truth. I am not. If I want to tag the commit pointed at by tag v1.0, I'd say I want to tag "v1.0^0", because otherwise there won't be a way to say

    $ git tag -s -m "i am aware of this tag" initial v1.0

i.e. making a tag that points at a tag. So I take the lack of ^0 (or ^{commit}) peeling an explicit sign that shows the intent by the user (well, those who know the tool, anyway) is clearly to create a tag pointing to that tag. In other words, peeling at the tagging time is wrong, and rejecting tag creation is also wrong.

> 2. When users on my team do a `git show tag`, they see 2 descriptions
> and 2 tags. This creates a LOT of confusion.

So what?  Not everybody will forever stay to be a newbie ;-)

As I said, an opt-in tag.allowCommitOnly is fine. That would train their fingers to peel with ^{commit} when they want to tag a commit. An opt-in tag.autoPeelTags might also be fine, even though that would not help training their fingers (so they will have to be prepared for the same "confusion" on a fresh machine)

When the command line clearly tells us that the user wants to tag a tag, we should not get overly "smart" and refuse to create a tag of a tag, unless the user tells us otherwise with some means.

> 3. Even if Git internally handles peeling tags, external 3rd party
> tooling may not. As I mentioned in another thread, `git lfs migrate`
> was not programmed to peel tags. I reported the issue here:

That is good, and it is the right way. After all, external 3rd party tooling may tag a tag even after we castrate "git tag" to be incapable of doing so, so a bug like the one you are helping to fix in lfs needs to be fixed anyway. In other words, thats the only sensible way forward when you care about the entire Git ecosystem, not just the main/reference implementation we work on here.