Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

From: Christian Couder <christian.couder@gmail.com>
Date: Nov 18, 2025, 18:29 UTC
Message-ID: <CAP8UFD03YK47nONVRV_wqOEanC8Oth1iRzsFv=eFhbFs6Q5mPA@mail.gmail.com>
In-Reply-To: <CABPp-BHY4SLmWY=V5aHJ6igN0GWeg6V1MoWDwszPe2O38wqBhw@mail.gmail.com>

On Mon, Nov 17, 2025 at 8:52 PM Elijah Newren <newren@gmail.com> wrote:

Show 21 quoted lines

>
> On Sun, Nov 16, 2025 at 8:35 PM Christian Couder
> <christian.couder@gmail.com> wrote:
> > There are no code changes in this v2, only commit message,
> > documentation and test changes:
> >
> > * Rebased on current 'master'. This avoids the need to mark some
> >   strings for translation as a recent series doing that has been
> >   recently merged to 'master'.
> >
> > * In patch 3/3, improved the commit message to better justify the new
> >   feature using some sentences from Elijah.
> >
> > * In patch 3/3, removed tests with dual signatures. This avoids a
> >   conflict with a separate series from brian carlson that adds a
> >   "RUST" prereq that is then needed to run tests with dual signatures.
>
> I'm a bit surprised; from
> https://lore.kernel.org/git/xmqqms4rry7f.fsf@gitster.g/, I thought you
> were going to rearrange the tests to avoid the conflict, not delete
> them.  Are no tests of this new functionality needed?

There are still 5 new tests left in patch 3/3 that are testing the new 'strip-if-invalid' functionality after I removed the 2 tests that are related to dual signatures.

In "t/t9305-fast-import-signatures.sh", dual signatures are already tested to work with `git fast-import --signed-commits=<mode>` by the tests that brian's f6581e23 (repository: require Rust support for interoperability, 2025-10-27) modifies.

f6581e23 not only adds the RUST prereq to these tests, but it also introduces the RUST prereq itself in "t/test-lib.sh" with:

+test_lazy_prereq RUST '
+       test "$(build_option rust)" = enabled
+'

So it's much simpler to just remove the 2 new dual signature tests that will need the RUST prereq when f6581e23 is merged. We can still add back these 2 new tests after f6581e23 is merged if we think it's worth it.

To avoid the conflict I could introduce the RUST prereq itself in "t/test-lib.sh" with the same code that f6581e23 uses, but then how do I justify it? What happens if f6581e23 is not actually merged?

It seems to me that if we really want the 2 new dual signature tests in this series, we would have to wait until f6581e23 is merged or discarded.

Show 5 quoted lines

> > * In patch 3/3, improved documentation of the new option to say that
> >   validation behaves as the validation performed by `git
> >   verify-commit`.
>
> Looking over the range diff, the other changes look good.

Thanks for your review.