From: Justin Tobler <jltobler@gmail.com>
Date: Tue, 10 Mar 2026 18:09:47 GMT
Subject: Re: [PATCH v2 3/3] fast-import: add mode to re-sign invalid commit signatures
Message-ID: <abBdNO2Izp7vrOdM@denethor>
In-Reply-To: <CAP8UFD3p84U0FhjGXNqagtDi=Cd3+QBHqGb3_ceWy-tdeLc43g@mail.gmail.com>

On 26/03/10 10:27AM, Christian Couder wrote:
> On Fri, Mar 6, 2026 at 9:54 PM Justin Tobler <jltobler@gmail.com> wrote:
> 
> > @@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
> >                 case SIGN_STRIP_IF_INVALID:
> >                         die(_("'strip-if-invalid' is not a valid mode for "
> >                               "git fast-export with --signed-commits=<mode>"));
> > +               case SIGN_RESIGN_IF_INVALID:
> 
> Everywhere in this patch, I think "RE_SIGN" might be more consistent
> than "RESIGN" for this name.

That's fair, will change.

> > +                       die(_("'re-sign-if-invalid' is not a valid mode for "
> > +                             "git fast-export with --signed-commits=<mode>"));
> 
> [...]
> 
> > @@ -2856,15 +2858,52 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
> >                 const char *subject;
> >                 int subject_len = find_commit_subject(msg->buf, &subject);
> >
> > -               if (subject_len > 100)
> > -                       warning(_("stripping invalid signature for commit '%.100s...'\n"
> > -                                 "  allegedly by %s"), subject, signer);
> > -               else if (subject_len > 0)
> > -                       warning(_("stripping invalid signature for commit '%.*s'\n"
> > -                                 "  allegedly by %s"), subject_len, subject, signer);
> > -               else
> > -                       warning(_("stripping invalid signature for commit\n"
> > -                                 "  allegedly by %s"), signer);
> > +               if (mode == SIGN_STRIP_IF_INVALID) {
> > +                       if (subject_len > 100)
> > +                               warning(_("stripping invalid signature for commit '%.100s...'\n"
> > +                                         "  allegedly by %s"), subject, signer);
> > +                       else if (subject_len > 0)
> > +                               warning(_("stripping invalid signature for commit '%.*s'\n"
> > +                                         "  allegedly by %s"), subject_len, subject, signer);
> > +                       else
> > +                               warning(_("stripping invalid signature for commit\n"
> > +                                         "  allegedly by %s"), signer);
> > +               } else if (mode == SIGN_RESIGN_IF_INVALID) {
> > +                       struct strbuf signature = STRBUF_INIT;
> > +                       struct strbuf payload = STRBUF_INIT;
> > +
> > +                       if (subject_len > 100)
> > +                               warning(_("re-signing invalid signature for commit '%.100s...'\n"
> > +                                         "  allegedly by %s"), subject, signer);
> > +                       else if (subject_len > 0)
> > +                               warning(_("re-signing invalid signature for commit '%.*s'\n"
> > +                                         "  allegedly by %s"), subject_len, subject, signer);
> > +                       else
> > +                               warning(_("re-signing invalid signature for commit\n"
> > +                                         "  allegedly by %s"), signer);
> 
> Maybe a helper function could be used to avoid duplicating the warning logic.

Ya, I could extract this out to a helper that prints the appropriate
warning. Due to being translated, I'm not quite sure if there would be a
good way to make the message strings more generic though. Will update in
the next version.

-Justin