On Thu, Apr 24, 2025 at 11:19 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Christian Couder <christian.couder@xxxxxxxxx> writes:
> > 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.
I am also not sure about the best way to approach this, so I tried a
new approach in v2.
Anyway thanks for your comments! I will address them if we decide that
the approach in this patch is still worth pursuing.
