Re: git rev-list fails to verify ssh-signed commits (but git log works)

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 8, 2023, 17:56 UTC
Message-ID: <xmqqmt5orqgv.fsf@gitster.g>
In-Reply-To: <Y+PRTYtFDoE73XEM@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

> +	/*
> +	 * yikes, this needs to come early in the function because it
> +	 * also handles user.signingkey, which would otherwise get
> +	 * shunted to git_ident_config() below
> +	 */
> +	if (git_gpg_config(var, value, cb) < 0)
> +		return -1;

Indeed.

Show 10 quoted lines

>  	if (starts_with(var, "user.") ||
>  	    starts_with(var, "author.") ||
>  	    starts_with(var, "committer."))
>
> but it would need a bit more work:
>
>   1. Somebody would need to dig into the reasons, if any, for not
>      calling git_gpg_config() everywhere. It might be fine, but there
>      may be a good reason which we're now violating. Digging in the
>      history and looking at the code might yield some hints.

Hmph, I didn't consider calling gpg_config unconditionally. It may do a bit more than what a typical config callback does (i.e. as opposed to just store the string values it gets, it tries table look-ups and stuff) but it is not too bad.

>   2. The individual calls to git_gpg_config() in other programs should
>      go away.

Naturally.

>   3. It's possible some refactoring may let us avoid the "yikes" comment
>      above (e.g., should user.signingkey just go into the normal ident
>      config handler?).

Hindsight is golden---if this were called gpg.signingkey we wouldn't be having this discussion X-<.