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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 3, 2019, 07:32 UTC
Message-ID: <xmqqzhp7sfw4.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190402230345.GA5004@dev-l>

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.

Show 11 quoted lines

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