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

From: Robert Dailey <rcdailey.lists@gmail.com>
Date: Apr 4, 2019, 13:47 UTC
Message-ID: <CAHd499Dr5sjzFCFYvkwcS0WOo0W51_RyL7nLAg_MaGeFy5eQKQ@mail.gmail.com>
In-Reply-To: <xmqqbm1mf74c.fsf@gitster-ct.c.googlers.com>

On Thu, Apr 4, 2019 at 4:32 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 19 quoted lines

>
> Robert Dailey <rcdailey.lists@gmail.com> writes:
>
> >> > more clear in the doc and/or in the proposed log message what
> >> > practical downside there are to the end users if we do not stop this
> >> > "mistake", that is.
> >>
> >> Having said all that, I can sort-of see that it may make sense to
> >> forbid tagging anything but a commit-ish, either by default, or a
> >> "git tag --forbid-no-committish" that can be turned on with a
> >> configuration.
> >
> > I don't really understand what part of the change is a negative for
> > you. Are you saying that `git tag` should peel the tags for you? Or
> > are you saying you don't like nested tagging to be opt-in and an error
> > otherwise?
>
> No, no and no.  I am saying "git tag -a -m 'message' tag anothertag"
> that creates a tag that points at another tag is perfectly fine.

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:

1. The intent by the user was to create a tag pointing to the commit
that another tag points to. So the peeling was expected to occur when
the tag was *created*, not when it is used. Again, the use case is
that I create a new annotated tag, then realize later I pointed it to
the wrong commit. So sometimes I correct it by pointing it at another
tag, but my intent was for both tags to point at the same commit, not
for one tag to point to a commit and the other to point to the tag.

2. When users on my team do a `git show tag`, they see 2 descriptions
and 2 tags. This creates a LOT of confusion. I wasted hours trying to
find out what this meant. It was very obscure and indirect. Most users
do not have your expert level of understanding of the internals of
Git. So I think there's a disconnect between how you like how the
machinery works internally with tags, and what users expect from the
porcelain interface. I think we should go with what's pragmatic (tags
that point to commits, not other tags) and design the interfaces for
the majority use case. Tags pointing to tags is a minority use case,
or an edge case. Given that, I think it should be opt-in.

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:

https://github.com/git-lfs/git-lfs/issues/3568

The author there admitted that migrating the repository *and* keeping the tags pointing to tags would be difficult. So I think the solution they're going to end up going with is that when you migrate a repository for LFS support, they will permanently peel your annotated tags. Which I personally think is the correct solution.

So in summary, I think it's better for tags to be peeled when they're created, or at least make the user aware of the fact that they're creating something that they might not be expecting. Just because Git handles peeled tags transparently doesn't mean that's what the user wants, or what the user expects. I've been using git for years and I never knew tags pointing to other tags could exist. It sounds like a technicality that most users shouldn't care about.

That's my feedback as a user, take it or leave it. I'm not really concerned with what's right or wrong from a git-internals perspective, what I want is a tool that makes sense for my day to day job, and I think this PR aligns with that.