From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 08 Oct 2025 07:14:40 GMT
Subject: Re: [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option
Message-ID: <aOYPYEk5sT6b1kuS@pks.im>
In-Reply-To: <20251007122958.1089680-6-christian.couder@gmail.com>

On Tue, Oct 07, 2025 at 02:29:58PM +0200, Christian Couder wrote:
> diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> index 85ed7a7270..b74179a6c8 100644
> --- a/Documentation/git-fast-import.adoc
> +++ b/Documentation/git-fast-import.adoc
> @@ -66,6 +66,11 @@ fast-import stream! This option is enabled automatically for
>  remote-helpers that use the `import` capability, as they are
>  already trusted to run their own code.
>  
> +--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> +	Specify how to handle signed tags.  Behaves in the same way
> +	as the same option in linkgit:git-fast-export[1], except that
> +	default is 'verbatim' (instead of 'abort').
> +

Nit: I would've ordered this after "--signed-commits", mostly so that
these two are ordered alphabetically.

>  --signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
>  	Specify how to handle signed commits.  Behaves in the same way
>  	as the same option in linkgit:git-fast-export[1], except that
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 2010e78475..668c926db5 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -2961,6 +2962,43 @@ static void parse_new_commit(const char *arg)
>  	b->last_commit = object_count_by_type[OBJ_COMMIT];
>  }
>  
> +static void handle_tag_signature(struct strbuf *msg, const char *name)
> +{
> +	size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
> +
> +	/* If there is no signature, there is nothing to do. */
> +	if (sig_offset >= msg->len)
> +		return;
> +
> +	switch (signed_tag_mode) {
> +
> +	/* First, modes that don't change anything */
> +	case SIGN_ABORT:
> +		die("encountered signed tag; use "
> +		    "--signed-tags=<mode> to handle it");

This message needs to be marked for translation.

> +	case SIGN_WARN_VERBATIM:
> +		warning(_("importing a tag signature verbatim for tag '%s'"), name);
> +			/* fallthru */

This comment is misindented.

> +	case SIGN_VERBATIM:
> +		/* Nothing to do, the signature will be put into the imported tag. */
> +		break;
> +
> +	/* Second, modes that remove the signature */
> +	case SIGN_WARN_STRIP:
> +		warning(_("stripping a tag signature for tag '%s'"), name);
> +			/* fallthru */

Same here, the comment is misindented.

> +	case SIGN_STRIP:
> +		/* Truncate the buffer to remove the signature */
> +		strbuf_setlen(msg, sig_offset);
> +		break;

I'm not familiar with the signature format, so it's probably a dumb
question: does the signature always extend until the end of the tag
message? Doesn't the tag message come after it?

Patrick