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

> > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > index 63880b595c..59e991a03c 100644
> > --- a/builtin/fast-import.c
> > +++ b/builtin/fast-import.c
> > @@ -29,6 +29,7 @@
> >  #include "commit-reach.h"
> >  #include "khash.h"
> >  #include "date.h"
> > +#include "gpg-interface.h"
> >
> >  #define PACK_ID_BITS 16
> >  #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
> > @@ -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.
>
> I am puzzled enough, so I'll stop here for now.