Re: [PATCH v6] fast-(import|export): improve on commit signature output format
From: Elijah Newren <hidden>
Date: 2025-07-14 21:07:27
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 [off-list ref] 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.