Re: [PATCH] fast-(import|export): improve on the signature algorithm name

> >
> > This is a follow up from cc/signed-fast-export-import that was merged
> > by 01d17c0530 (Merge branch 'cc/signed-fast-export-import', 2025-03-29)
> > and introduced the support for signed commits.
> >
> > The format that this series implemented was lacking a bit, so the goal
> > with this patch is to improve it and handle signed commits a bit more
> > consistently in the code base. It also shows in the tests and in our
> > documentation that SSH and X.509 signatures are supported.
>
> Thanks.
>
> It is a bit surprising and slightly sad that nobody bothered to
> report/complain about the brokenness until the original author
> follows up one month later X-<.  Nobody but the original author is
> using this feature?  I would have expected that use of signed
> commits were of high demand and many more people were actively
> interested in the topic.

> > @@ -2830,12 +2831,15 @@ static void parse_new_commit(const char *arg)
> >                       "encoding %s\n",
> >                       encoding);
> >       if (sig_alg) {
> > -             if (!strcmp(sig_alg, "sha1"))
> > -                     strbuf_addstr(&new_data, "gpgsig ");
> > -             else if (!strcmp(sig_alg, "sha256"))
> > +             if (!strcmp(sig_alg, "sha256"))
> >                       strbuf_addstr(&new_data, "gpgsig-sha256 ");
> > -             else
> > -                     die("Expected gpgsig algorithm sha1 or sha256, got %s", sig_alg);
> > +             else if (valid_signature_name(sig_alg))
> > +                     strbuf_addstr(&new_data, "gpgsig ");
> > +             else if (!strcmp(sig_alg, "unknown")) {
> > +                     warning("Unknown gpgsig algorithm name!");
> > +                     strbuf_addstr(&new_data, "gpgsig ");
> > +             } else
> > +                     die("Invalid gpgsig algorithm name, got '%s'", sig_alg);
>
> Hmph, we used to have special cases for sha1 and sha256 but now we
> can handle sha1 with a more generic "valid_signature_name()" logic?
> And yet we need to still special case sha256?  Not that I trust the
> old code all that much and take deviations from the patterns in the
> old code as a sign of something not right...
>
> The fast-export stream produced by the code with d9cb0e6f
> (fast-export, fast-import: add support for signed-commits,
> 2025-03-10) used to identify a signature algorithm "sha1", but this
> new version of fast-import lost the support for it, and will barf
> when seeing such an existing fast-export stream?  I am not sure what
> is going on around this code.
>
> I am not so worried about the other case, where the stream produced
> by fast-export contained in this version may or may not be readable
> by an older version of fast-import.