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

From: Junio C Hamano <gitster@pobox.com>
To: Denton Liu <liu.denton@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
	"Robert Dailey" <rcdailey.lists@gmail.com>,
	"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: Wed, 03 Apr 2019 16:32:27 +0900	[thread overview]
Message-ID: <xmqqzhp7sfw4.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190402230345.GA5004@dev-l> (Denton Liu's message of "Tue, 2 Apr 2019 16:03:45 -0700")

Denton Liu <liu.denton@gmail.com> writes:

> This is the first use of the '%n$<fmt>' style of printf format in the
> *.[ch] files in our codebase, but it's supported by POSIX[1] and there
> are existing uses for it in po/*.po files, so hopefully it won't cause

The latter is a stronger indication that this should be OK than the
former ;-)  Thanks for digging and noting.

> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
> index 88620429ea..ec4f6ae658 100644
> --- a/Documentation/config/advice.txt
> +++ b/Documentation/config/advice.txt
> @@ -90,4 +90,6 @@ advice.*::
>  	waitingForEditor::
>  		Print a message to the terminal whenever Git is waiting for
>  		editor input from the user.
> +	nestedTag::
> +		Advice shown if a user attempts to recursively tag a tag object.
>  --

In addition to 'advice', we may have to add a configuration to help
projects that wants to tag tag objects regularly so that they do not
have to keep typing "--allow-nested-tag".  But that can wait until a
participant of such a project comes forward and makes a case for
their workflow.

> +chain of custody by signing someone else's signed tag. However, in
> +practice, this is typically a mistake so we prevent it from happening by
> +default unless specifically requested.

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}".

And if something that *MUST* take a commit refuses to (or more
likely, forges to) peel a tag down to a commit and yields an error,
I think that is what needs fixing, not the command that creates a
tag.

So, I am fairly negative on this change---unless it is made much
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.

> +Automatically erroring on nested tags was introduced in Git version
> +2.22.0.

And please do not write something like this.  A feature gets in a
release when it is ready, and we may not ship this in 2.22.

"git tag --help" the user is running may or may not have the
paragraph about "nested tag", depending on the existence of the
feature in the version of Git the user is running, so there is no
need to say something like that.

And no, I do not buy arguments like "random web servers serve
different versions of documentation without identifying which
version of Git they describe".