Sorry for the delay; noticed this in what's cooking and wanted to
double check whether I missed anything, and decided to add a comment
here...
On Wed, Jul 9, 2025 at 7:13 AM Christian Couder
<christian.couder@xxxxxxxxx> wrote:
>
[...]
> Let's improve on these limitations by improving fast-export and
> fast-import so that:
>
> - all the signatures are exported,
> - at most one signature on the SHA-1 object and one on the SHA-256
> are imported,
> - if there is more than one signature on the SHA-1 object or on
> the SHA-256 object, fast-import emits a warning for each
> additional signature,
> - the output format is "gpgsig <git-hash-algo> <signature-format>",
> where <git-hash-algo> is the Git object format as before, and
> <signature-format> is the signature type ("openpgp", "x509",
> "ssh" or "unknown"),
> - the output is properly documented.
>
[...]
>
> +test_expect_success GPGSM 'setup X.509 signed commit' '
> +
> + git checkout -b x509-signing main &&
> + test_config gpg.format x509 &&
> + test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> + echo "X.509 content" >file &&
> + git add file &&
> + git commit -S -m "X.509 signed commit" &&
> + X509_COMMIT=$(git rev-parse HEAD) &&
> + git checkout main
> +
> +'
> +
> +test_expect_success GPGSM 'round-trip X.509 signed commit' '
> +
> + git fast-export --signed-commits=verbatim x509-signing >output &&
> + test_grep -E "^gpgsig $GIT_DEFAULT_HASH x509" output &&
> + (
> + cd new &&
> + git fast-import &&
> + git cat-file commit refs/heads/x509-signing >actual &&
> + grep "^gpgsig" actual &&
> + IMPORTED=$(git rev-parse refs/heads/x509-signing) &&
> + test $X509_COMMIT = $IMPORTED
> + ) <output
> +
[...]
> +
> +test_expect_success GPGSSH 'round-trip SSH signed commit' '
> +
> + git fast-export --signed-commits=verbatim ssh-signing >output &&
> + test_grep -E "^gpgsig $GIT_DEFAULT_HASH ssh" output &&
> + (
> + cd new &&
> + git fast-import &&
> + git cat-file commit refs/heads/ssh-signing >actual &&
> + grep "^gpgsig" actual &&
> + IMPORTED=$(git rev-parse refs/heads/ssh-signing) &&
> + test $SSH_COMMIT = $IMPORTED
> + ) <output
> +
> +'
> +
Overall, the patch looks great now. There's just one little nit-pick
left; I'm still not a fan of tests of the form
(
cd dir &&
git fast-import
...lots of other commands...
) <output
because I think the "<output" should really be moved to the "git
fast-import" line since it's only meant to be used there.
This series adds 2 such tests. You did point out in the discussion on
v5 that the testsuite already uses this idiom and you wanted to match
existing style. (Though there were only 2 tests previously that used
it, and you already modified both as part of this patch.)
However...we've been through enough rounds and this is really just a
nit-pick; I can submit a follow-on patch later to clean up the four
tests and see if others agree with me that this is an eyesore, or if
I'm just weird.
I think it's good to merge down.
