Thread (27 messages) 27 messages, 3 authors, 2025-07-14

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help