On Wed, Jul 9, 2025 at 1:08 AM Elijah Newren [off-list ref] wrote:

On Tue, Jul 8, 2025 at 2:18 AM Christian Couder
[off-list ref] wrote:

quoted

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 250d866652..89dec1108f 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc

@@ -445,7 +445,7 @@ one).
        original-oid?
        ('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
        'committer' (SP <name>)? SP LT <email> GT SP <when> LF
-       ('gpgsig' SP <alg> LF data)?
+       ('gpgsig' SP <algo> SP <format> LF data)?
        ('encoding' SP <encoding> LF)?
        data
        ('from' SP <commit-ish> LF)?

@@ -518,13 +518,37 @@ their syntax.
 ^^^^^^^^

 The optional `gpgsig` command is used to include a PGP/GPG signature
-that signs the commit data.
+or other cryptographic signature that signs the commit data.

-Here <alg> specifies which hashing algorithm is used for this
-signature, either `sha1` or `sha256`.
+....
+       'gpgsig' SP <git-hash-algo> SP <signature-format> LF
+       data

Should the `data` be moved to the line above, just to make it clear
it's associated with it?  (Similar to the first line you changed in
git-fast-import.adoc?)

Yeah, I have changed it in my current version.

quoted

+....
+
+The `gpgsig` command takes two arguments:
+
+* `<git-hash-algo>` specifies which Git object format this signature
+  applies to, either `sha1` or `sha256`. This allows to know which
+  representation of the commit was signed (the SHA-1 or the SHA-256
+  version) which helps with both signature verification and
+  interoperability between repos with different hash functions.

Should there also be a note added that fast-import is limited on what
signatures it can verify if extensions.compatObjectFormat is not set?

It cannot yet verify signatures, but yeah it's probably a good idea to
say that setting this config option might help with verifying
signatures when it will be able to do it. See below.

quoted

+* `<signature-format>` specifies the type of signature, such as
+  `openpgp`, `x509`, `ssh`, or `unknown`. This is a convenience for
+  tools that process the stream, so they don't have to parse the ASCII
+  armor to identify the signature type.
+
+A commit may have at most one signature for the SHA-1 object format
+(stored in the "gpgsig" header) and one for the SHA-256 object format
+(stored in the "gpgsig-sha256" header).
+
+See below for a detailed description of the `data` command which
+contains the raw signature data.
+
+Signatures are not yet checked in the current implementation though.

...or maybe that extra note could be added as a parenthetical comment
here for now?

Yeah, in my current version, there is:

"Signatures are not yet checked in the current implementation
though. (Already setting the `extensions.compatObjectFormat`
configuration option might help with verifying both SHA-1 and SHA-256
object format signatures when it will be implemented.)"

[...]

quoted

+test_expect_success GPG 'export and import of doubly signed commit' '
+       git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&
+       test_grep -E "^gpgsig sha1 openpgp" output &&
+       test_grep -E "^gpgsig sha256 openpgp" output &&
+
+       (
+               cd new &&
+               git fast-import &&
+               git cat-file commit refs/heads/dual-signed >actual &&
+               test_grep -E "^gpgsig " actual &&
+               test_grep -E "^gpgsig-sha256 " actual &&
+               IMPORTED=$(git rev-parse refs/heads/dual-signed) &&
+               if test "$GIT_DEFAULT_HASH" = "sha1"
+               then
+                       test $SHA1_B = $IMPORTED
+               else
+                       test $SHA256_B = $IMPORTED
+               fi
+       ) <output

This last bit seems a bit fragile; could the redirection of output to
the stdin of `git fast-import` be made specific to that one line
instead of to the whole range of commands?

I used the same style as many other tests in this file. For example
there are already:

test_expect_success GPG 'signed-commits=verbatim' '

    git fast-export --signed-commits=verbatim --reencode=no
commit-signing >output &&
    grep "^gpgsig sha" output &&
    grep "encoding ISO-8859-1" output &&
    (
        cd new &&
        git fast-import &&
        STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
        test $COMMIT_SIGNING = $STRIPPED
    ) <output

'

test_expect_success GPG 'signed-commits=warn-verbatim' '

    git fast-export --signed-commits=warn-verbatim --reencode=no
commit-signing >output 2>err &&
    grep "^gpgsig sha" output &&
    grep "encoding ISO-8859-1" output &&
    test -s err &&
    (
        cd new &&
        git fast-import &&
        STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
        test $COMMIT_SIGNING = $STRIPPED
    ) <output

'

etc.

So to keep a consistent style in the tests, I would likely have to add
a preparatory commit that changes the style of all these existing
tests. Not sure it's worth it at this point.

Otherwise, I very much appreciate the work to create a testcase with
both signature types on a single commit.

Thanks for the kind words and for your reviews!