Re: [PATCH v2 2/2] fast-import: add '--signed-commits=<mode>' option

From: Christian Couder <christian.couder@gmail.com>
Date: Sep 15, 2025, 10:17 UTC
Message-ID: <CAP8UFD3-3zjBBHP-Y9nLO-qEyhkWhD_pxFZhk6rzrC31LanDMQ@mail.gmail.com>
In-Reply-To: <aMex06JNPVeHaw_d@pks.im>

On Mon, Sep 15, 2025 at 8:27 AM Patrick Steinhardt <ps@pks.im> wrote:

>
> On Fri, Sep 12, 2025 at 02:40:42PM +0200, Christian Couder wrote:

Show 25 quoted lines

> >
> > -     /* Process signatures (up to 2: one "sha1" and one "sha256") */
> >       while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
> >               struct signature_data sig = { NULL, NULL, STRBUF_INIT };
> >
> > -             parse_one_signature(&sig, v);
> > +             if (signed_commit_mode == SIGN_ABORT)
> > +                     die(_("encountered signed commit; use "
> > +                           "--signed-commits=<mode> to handle it"));
> >
> > -             if (!strcmp(sig.hash_algo, "sha1"))
> > -                     store_signature(&sig_sha1, &sig, "SHA-1");
> > -             else if (!strcmp(sig.hash_algo, "sha256"))
> > -                     store_signature(&sig_sha256, &sig, "SHA-256");
> > -             else
> > -                     BUG("parse_one_signature() returned unknown hash algo");
> > +             parse_one_signature(&sig, v);
> >
> > +             switch (signed_commit_mode) {
> > +             case SIGN_ABORT:
> > +                     BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
> > +                     break;
>
> Let's be defensive and convert this into a `default:` case so that any
> unhandled value will cause a BUG.

Ok, maybe something like BUG("invalid signed_commit_mode value %d", signed_commit_mode) then?

Note that if we later develop new modes like "re-sign" or "strip-if-invalid", and users tries one such mode with an old version of Git, that should already be handled by the following code in parse_one_option():

    } else if (skip_prefix(option, "signed-commits=", &option)) {
        if (parse_sign_mode(option, &signed_commit_mode))
            usagef(_("unknown --signed-commits mode '%s'"), option);
    } ...

Thanks, Christian.