git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Robert Dailey <rcdailey.lists@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Denton Liu" <liu.denton@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Elijah Newren" <newren@gmail.com>
Subject: Re: [PATCH v2.5 2/2] tag: prevent nested tags
Date: Thu, 4 Apr 2019 08:47:43 -0500	[thread overview]
Message-ID: <CAHd499Dr5sjzFCFYvkwcS0WOo0W51_RyL7nLAg_MaGeFy5eQKQ@mail.gmail.com> (raw)
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:
>
> 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.

  reply	other threads:[~2019-04-04 13:47 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-21 16:59 Strange annotated tag issue Robert Dailey
2019-03-21 19:04 ` Bryan Turner
2019-03-21 19:39   ` Jeff King
2019-03-21 19:29 ` Jeff King
2019-03-25 13:50   ` Robert Dailey
2019-03-25 14:49     ` Jeff King
2019-03-25 15:31       ` Robert Dailey
2019-03-25 18:43       ` Bryan Turner
2019-03-25 23:36         ` Jeff King
2019-03-25 19:19       ` Ævar Arnfjörð Bjarmason
2019-03-25 23:37         ` Jeff King
2019-03-26  7:53           ` [PATCH 0/3] tag: prevent recursive tags Denton Liu
2019-03-26  7:53             ` [PATCH 1/3] " Denton Liu
2019-03-26  8:51               ` Denton Liu
2019-03-26 10:10               ` Ævar Arnfjörð Bjarmason
2019-03-27  4:57               ` Elijah Newren
2019-03-27 10:27                 ` Ævar Arnfjörð Bjarmason
2019-03-28 19:02                   ` Robert Dailey
2019-03-26  7:53             ` [PATCH 2/3] t7004: ensure recursive tag behavior is working Denton Liu
2019-03-26 10:11               ` Ævar Arnfjörð Bjarmason
2019-03-26  7:53             ` [PATCH 3/3] git-tag.txt: document --allow-recursive-tag option Denton Liu
2019-03-26 10:16               ` Ævar Arnfjörð Bjarmason
2019-03-26 16:18             ` [PATCH 0/3] tag: prevent recursive tags Jeff King
2019-04-02  5:38             ` [PATCH v2 0/2] tag: prevent nested tags Denton Liu
2019-04-02  5:38               ` [PATCH v2 1/2] tag: fix formatting Denton Liu
2019-04-02  5:38               ` [PATCH v2 2/2] tag: prevent nested tags Denton Liu
2019-04-02 23:03                 ` [PATCH v2.5 " Denton Liu
2019-04-03  7:32                   ` Junio C Hamano
2019-04-03  8:49                     ` Junio C Hamano
2019-04-03 18:26                       ` Robert Dailey
2019-04-04  9:32                         ` Junio C Hamano
2019-04-04 13:47                           ` Robert Dailey [this message]
2019-04-04 21:50                             ` Junio C Hamano
2019-04-05  2:51                               ` Robert Dailey
2019-04-03 18:16                     ` Johannes Sixt
2019-04-03 21:33                     ` Denton Liu
2019-04-04  2:02                       ` Jeff King
2019-04-04  9:31                         ` Junio C Hamano
2019-04-04 12:27                           ` Jeff King
2019-04-04 21:54                             ` Junio C Hamano
2019-04-04 22:12                               ` Jeff King
2019-04-11 18:40                             ` Eckhard Maaß
2019-04-12  3:21                               ` Junio C Hamano
2019-04-05  0:36                           ` Elijah Newren
2019-04-05  5:29                             ` Junio C Hamano
2019-04-04 18:25               ` [PATCH v3 0/2] tag: advise on recursive tagging Denton Liu
2019-04-04 18:25               ` [PATCH v3 1/2] tag: fix formatting Denton Liu
2019-04-04 18:25               ` [PATCH v3 2/2] tag: advise on nested tags Denton Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHd499Dr5sjzFCFYvkwcS0WOo0W51_RyL7nLAg_MaGeFy5eQKQ@mail.gmail.com \
    --to=rcdailey.lists@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=liu.denton@gmail.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).