git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Max Gautier <max.gautier@redhat.com>, git@vger.kernel.org
Subject: Re: git rev-list fails to verify ssh-signed commits (but git log works)
Date: Thu, 09 Feb 2023 08:44:23 -0800	[thread overview]
Message-ID: <xmqqpmaiokk8.fsf@gitster.g> (raw)
In-Reply-To: <Y+TqEM21o+3TGx6D@coredump.intra.peff.net> (Jeff King's message of "Thu, 9 Feb 2023 07:41:52 -0500")

Jeff King <peff@peff.net> writes:

> I'd be more worried about correctness (command "foo" must not parse
> option "bar" because it is plumbing and must use the default). But
> looking over the options, I really don't see anything like that. The one
> I'd expect (or worry most about) is "we do/do not bother to enable
> signatures at all based on config" but I don't think that is the case.
> We default use_format to &gpg_format[0], so there is always a signature
> format defined, even if the config is skipped.

I arrived at the same conclusion after seeing what these
configuration affected.  If some plumbing codepaths should avoid
certain gpg-interface features, I didn't see a good reason for them
to be calling into gpg-interface.c functions (e.g. that would have
meant that "rev-list '--format=%G'" should have errored out with a
"%G is only available in log but not in rev-list which is a
plumbing" error message) and at that point reading configuration
is much lessor problem.

> If the gpg code used git_config_get_string(), etc, then they could just
> access each key on demand (efficiently, from an internal hash table),
> which reduces the risk of "oops, we forgot to initialize the config
> here". It does probably mean restructuring the code a little, though
> (since you'd often have an accessor function to get "foo.bar" rather
> than assuming "foo.bar" was parsed into an enum already, etc). That may
> not be worth the effort (and risk of regression) to convert.

Yup.  Care must be taken not to break "-u <keyid>" that is set via
calling set_signing_key() directly, which assumes that the default
key is already set from the config earlier, but the resulting code
may make it clearer that our rule is "command line first, and then
peek into configuration as a fallback value", if we did such a
conversion.

Thanks.

  reply	other threads:[~2023-02-09 16:44 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-08 15:56 git rev-list fails to verify ssh-signed commits (but git log works) Max Gautier
2023-02-08 16:43 ` Jeff King
2023-02-08 17:56   ` Junio C Hamano
2023-02-08 18:20     ` Junio C Hamano
2023-02-08 20:31       ` [PATCH] gpg-interface: lazily initialize and read the configuration Junio C Hamano
2023-02-09  0:17         ` Ævar Arnfjörð Bjarmason
2023-02-09  2:05           ` Junio C Hamano
2023-02-09  2:24             ` Ævar Arnfjörð Bjarmason
2023-02-09 12:49         ` Jeff King
2023-02-09 16:38           ` Junio C Hamano
2023-02-09 20:24             ` [PATCH v2] " Junio C Hamano
2023-02-26 22:40               ` Jeff King
2023-02-27 16:00                 ` Junio C Hamano
2023-03-08  8:34                 ` Ævar Arnfjörð Bjarmason
2023-03-09  3:28                   ` Jeff King
2023-03-09 17:03                     ` Junio C Hamano
2023-03-10  9:01                       ` Jeff King
2023-02-09 12:41     ` git rev-list fails to verify ssh-signed commits (but git log works) Jeff King
2023-02-09 16:44       ` Junio C Hamano [this message]
2023-02-08 17:00 ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=xmqqpmaiokk8.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=max.gautier@redhat.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.
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 NNTP newsgroup(s).