* [PATCH 0/5] fast-import: start controlling how tag signatures are handled
@ 2025-10-07 12:29 Christian Couder
2025-10-07 12:29 ` [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags Christian Couder
` (6 more replies)
0 siblings, 7 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-07 12:29 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Christian Couder
Introduction
------------
Tools like `git-filter-repo` should be able to control how tag
signatures are handled when regenerating repository content after it
has been filtered. For this purpose, they need a way for `git
fast-import` to control how tag signatures are handled.
A previous series [1] added a '--signed-commits=<mode>' option to `git
fast-import` to control how commit signatures are handled, so this is
adding a similar '--signed-tags=<mode>' for tag signatures.
For now this new option behaves in a very similar way as the option
with the same name that already exists in `git fast-export`.
Especially it supports exactly the same <mode>s and the same aliases
for these modes. For example "ignore" is a synonym for "verbatim".
This way, both `git fast-export` and `git fast-import` have both a
'--signed-tags=<mode>' and a '--signed-commits=<mode>' supporting the
same <mode>s.
In the future I want to implement new <mode>s like "strip-if-invalid",
"re-sign", "re-sign-if-invalid" in `git fast-import` for both tag and
commit signatures. These might be a bit more complex, so for now I
prefer to start with the simple modes.
[1] https://lore.kernel.org/git/20250917181427.3193500-1-christian.couder@gmail.com/
Note about the different patches
--------------------------------
Patch 1/5 (doc: git-tag: stop focussing on GPG signed tags) is a
documentation update for `git tag`. It could go in a separate series
or be dropped altogether, but while working on this I thought that it
would be a good thing to do, as the doc is quite outdated.
Patches 2/5, 3/5 and 4/5 are preparatory patches for the main one
which is patch 5/5 (fast-import: add '--signed-tags=<mode>' option).
I wanted '--signed-tags=<mode>' to work for all kinds of signature in
tags (OpenPGP, X.509 and SSH) but soon realized that the
'--signed-tags=<mode>' option of `git fast-export` worked only for
OpenPGP signatures, so I fixed that issue in patch 4/5 (fast-export:
handle all kinds of tag signatures).
While working on the tests in patch 4/5, I found a few things to
improve that could belong to other patches so that's how I came up
with patches 2/5 and 3/5.
CI tests:
---------
They have all passed except one on Windows where
"t8020-last-modified.sh" failed. I doubt it's related though. See:
https://github.com/chriscool/git/actions/runs/18311274807/job/52140205441
Christian Couder (5):
doc: git-tag: stop focussing on GPG signed tags
lib-gpg: allow tests with the GPGSM prereq first
t9350: properly count annotated tags
fast-export: handle all kinds of tag signatures
fast-import: add '--signed-tags=<mode>' option
Documentation/git-fast-import.adoc | 5 ++
Documentation/git-tag.adoc | 52 ++++++++++++-------
builtin/fast-export.c | 7 ++-
builtin/fast-import.c | 43 ++++++++++++++++
t/lib-gpg.sh | 2 +-
t/meson.build | 1 +
t/t9306-fast-import-signed-tags.sh | 80 ++++++++++++++++++++++++++++++
t/t9350-fast-export.sh | 60 ++++++++++++++++++++--
8 files changed, 224 insertions(+), 26 deletions(-)
create mode 100755 t/t9306-fast-import-signed-tags.sh
--
2.51.0.438.g6987fc0bae
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags
2025-10-07 12:29 [PATCH 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
@ 2025-10-07 12:29 ` Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-07 12:29 ` [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first Christian Couder
` (5 subsequent siblings)
6 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-07 12:29 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Christian Couder,
Christian Couder
It looks like the documentation of `git tag` is focussed a bit too
much on GPG signed tags.
This starts with the "NAME" section where the command is described
with:
"Create, list, delete or verify a tag object signed with GPG"
while for example `git branch` is described with simply:
"List, create, or delete branches"
This could give the false impression that `git tag` only works with
tag objects, not with lightweight tags, and that tag objects are
always GPG signed.
In the "DESCRIPTION" section, it looks like only "GnuPG signed tag
objects" can be created by the `-s` and `-u <key-id>` options, and it
seems `gpg.program` can only specify a "custom GnuPG binary".
This goes on in the "OPTIONS" section too, especially about the `-s`
and `-u <key-id>` options.
The "CONFIGURATION" also doesn't talk about how to configure the
command to work with X.509 and SSH signatures.
Let's rework all that to make sure users have a more accurate and
balanced view of what the command can do.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-tag.adoc | 52 +++++++++++++++++++++++++-------------
1 file changed, 35 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-tag.adoc b/Documentation/git-tag.adoc
index a4b1c0ec05..9117754ffb 100644
--- a/Documentation/git-tag.adoc
+++ b/Documentation/git-tag.adoc
@@ -3,7 +3,7 @@ git-tag(1)
NAME
----
-git-tag - Create, list, delete or verify a tag object signed with GPG
+git-tag - Create, list, delete or verify tags
SYNOPSIS
@@ -38,17 +38,18 @@ and `-a`, `-s`, and `-u <key-id>` are absent, `-a` is implied.
Otherwise, a tag reference that points directly at the given object
(i.e., a lightweight tag) is created.
-A GnuPG signed tag object will be created when `-s` or `-u
-<key-id>` is used. When `-u <key-id>` is not used, the
-committer identity for the current user is used to find the
-GnuPG key for signing. The configuration variable `gpg.program`
-is used to specify custom GnuPG binary.
+A cryptographically signed tag object will be created when `-s` or
+`-u <key-id>` is used. The signing backend (GPG, X.509, SSH, etc.) is
+controlled by the `gpg.format` configuration variable, defaulting to
+OpenPGP. When `-u <key-id>` is not used, the committer identity for
+the current user is used to find the key for signing. The
+configuration variable `gpg.program` is used to specify a custom
+signing binary.
Tag objects (created with `-a`, `-s`, or `-u`) are called "annotated"
tags; they contain a creation date, the tagger name and e-mail, a
-tagging message, and an optional GnuPG signature. Whereas a
-"lightweight" tag is simply a name for an object (usually a commit
-object).
+tagging message, and an optional signature. Whereas a "lightweight"
+tag is simply a name for an object (usually a commit object).
Annotated tags are meant for release while lightweight tags are meant
for private or temporary object labels. For this reason, some git
@@ -64,10 +65,12 @@ OPTIONS
-s::
--sign::
- Make a GPG-signed tag, using the default e-mail address's key.
- The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
- configuration variable if it exists, or disabled otherwise.
- See linkgit:git-config[1].
+ Make a signed tag, using the default signing key. The signing
+ backend used depends on the `gpg.format` configuration
+ variable. The default key is determined by the backend. For
+ GPG, it's based on the committer's email address, while for
+ SSH it may be a specific key file or agent identity. See
+ linkgit:git-config[1].
--no-sign::
Override `tag.gpgSign` configuration variable that is
@@ -75,7 +78,9 @@ OPTIONS
-u <key-id>::
--local-user=<key-id>::
- Make a GPG-signed tag, using the given key.
+ Make a signed tag using the given key. The format of the
+ <key-id> and the backend used depend on the `gpg.format`
+ configuration variable. See linkgit:git-config[1].
-f::
--force::
@@ -87,7 +92,7 @@ OPTIONS
-v::
--verify::
- Verify the GPG signature of the given tag names.
+ Verify the signature of the given tag names.
-n<num>::
<num> specifies how many lines from the annotation, if any,
@@ -236,12 +241,25 @@ it in the repository configuration as follows:
-------------------------------------
[user]
- signingKey = <gpg-key-id>
+ signingKey = <key-id>
-------------------------------------
+The signing backend is controlled by the `gpg.format` configuration
+variable, which defaults to `openpgp` for GPG signing. To sign tags
+using other technologies like X.509 or SSH, set this variable to
+`x509` or `ssh` respectively.
+
+You can also specify the path to the signing program for each
+format. The `gpg.program` variable (or its synonym
+`gpg.openpgp.program`) is used for the OpenPGP backend. For other
+backends, the configuration is `gpg.<format>.program`, for example
+`gpg.ssh.program` for SSH signing.
+
`pager.tag` is only respected when listing tags, i.e., when `-l` is
used or implied. The default is to use a pager.
-See linkgit:git-config[1].
+
+See linkgit:git-config[1] for more details and other configuration
+variables.
DISCUSSION
----------
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first
2025-10-07 12:29 [PATCH 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
2025-10-07 12:29 ` [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags Christian Couder
@ 2025-10-07 12:29 ` Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-07 12:29 ` [PATCH 3/5] t9350: properly count annotated tags Christian Couder
` (4 subsequent siblings)
6 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-07 12:29 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Christian Couder,
Christian Couder
When the 'GPG' prereq is lazily tested, `mkdir "$GNUPGHOME"` could
fail if the "$GNUPGHOME" directory already exists. This can happen if
the 'GPGSM' prereq has been lazily tested before as it uses
`mkdir -p "$GNUPGHOME"`.
To allow the GPGSM prereq to appear before the GPG prereq in some
test scripts, let's use `mkdir -p "$GNUPGHOME"` when the 'GPG' prereq
is lazily tested too.
This will be useful in a following commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/lib-gpg.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 937b876bd0..743985efab 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -38,7 +38,7 @@ test_lazy_prereq GPG '
# To export ownertrust:
# gpg --homedir /tmp/gpghome --export-ownertrust \
# > lib-gpg/ownertrust
- mkdir "$GNUPGHOME" &&
+ mkdir -p "$GNUPGHOME" &&
chmod 0700 "$GNUPGHOME" &&
(gpgconf --kill all || : ) &&
gpg --homedir "${GNUPGHOME}" --import \
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH 3/5] t9350: properly count annotated tags
2025-10-07 12:29 [PATCH 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
2025-10-07 12:29 ` [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags Christian Couder
2025-10-07 12:29 ` [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first Christian Couder
@ 2025-10-07 12:29 ` Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-07 12:29 ` [PATCH 4/5] fast-export: handle all kinds of tag signatures Christian Couder
` (3 subsequent siblings)
6 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-07 12:29 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Christian Couder,
Christian Couder
In t9350-fast-export.sh, these existing tests:
- 'fast-export | fast-import when main is tagged'
- 'cope with tagger-less tags'
are checking the number of annotated tags in the test repo by comparing
it with some hardcoded values.
This could be an issue if some new tests that have some prerequisites
add new annotated tags to the repo before these existing tests. When
the prerequisites would be satisfied, the number of annotated tags
would be different from when some prerequisites would not be satisfied.
As we are going to add new tests that add new annotated tags in a
following commit, let's properly count the number of annotated tag in
the repo by incrementing a counter each time a new annotated tag is
added, and then by comparing the number of annotated tags to the value
of the counter when checking the number of annotated tags.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t9350-fast-export.sh | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 8f85c69d62..21ff26939c 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -35,6 +35,7 @@ test_expect_success 'setup' '
git commit -m sitzt file2 &&
test_tick &&
git tag -a -m valentin muss &&
+ ANNOTATED_TAG_COUNT=1 &&
git merge -s ours main
'
@@ -229,7 +230,8 @@ EOF
test_expect_success 'set up faked signed tag' '
- git fast-import <signed-tag-import
+ git fast-import <signed-tag-import &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
'
@@ -491,8 +493,9 @@ test_expect_success 'fast-export -C -C | fast-import' '
test_expect_success 'fast-export | fast-import when main is tagged' '
git tag -m msg last &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1)) &&
git fast-export -C -C --signed-tags=strip --all > output &&
- test $(grep -c "^tag " output) = 3
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT
'
@@ -506,12 +509,13 @@ test_expect_success 'cope with tagger-less tags' '
TAG=$(git hash-object --literally -t tag -w tag-content) &&
git update-ref refs/tags/sonnenschein $TAG &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1)) &&
git fast-export -C -C --signed-tags=strip --all > output &&
- test $(grep -c "^tag " output) = 4 &&
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT &&
! grep "Unspecified Tagger" output &&
git fast-export -C -C --signed-tags=strip --all \
--fake-missing-tagger > output &&
- test $(grep -c "^tag " output) = 4 &&
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT &&
grep "Unspecified Tagger" output
'
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH 4/5] fast-export: handle all kinds of tag signatures
2025-10-07 12:29 [PATCH 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
` (2 preceding siblings ...)
2025-10-07 12:29 ` [PATCH 3/5] t9350: properly count annotated tags Christian Couder
@ 2025-10-07 12:29 ` Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-07 12:29 ` [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
` (2 subsequent siblings)
6 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-07 12:29 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Christian Couder,
Christian Couder
Currently the handle_tag() function in "builtin/fast-export.c" searches
only for "\n-----BEGIN PGP SIGNATURE-----\n" in the tag message to find
a tag signature.
This doesn't handle all kinds of OpenPGP signatures as some can start
with "-----BEGIN PGP MESSAGE-----" too, and this doesn't handle SSH and
X.509 signatures either as they use "-----BEGIN SSH SIGNATURE-----" and
"-----BEGIN SIGNED MESSAGE-----" respectively.
To handle all these kinds of tag signatures supported by Git, let's use
the parse_signed_buffer() function to properly find signatures in tag
messages.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/fast-export.c | 7 +++---
t/t9350-fast-export.sh | 48 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index dc2486f9a8..7adbc55f0d 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -931,9 +931,8 @@ static void handle_tag(const char *name, struct tag *tag)
/* handle signed tags */
if (message) {
- const char *signature = strstr(message,
- "\n-----BEGIN PGP SIGNATURE-----\n");
- if (signature)
+ size_t sig_offset = parse_signed_buffer(message, message_size);
+ if (sig_offset < message_size)
switch (signed_tag_mode) {
case SIGN_ABORT:
die("encountered signed tag %s; use "
@@ -950,7 +949,7 @@ static void handle_tag(const char *name, struct tag *tag)
oid_to_hex(&tag->object.oid));
/* fallthru */
case SIGN_STRIP:
- message_size = signature + 1 - message;
+ message_size = sig_offset;
break;
}
}
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 21ff26939c..5a46608f65 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -279,6 +279,54 @@ test_expect_success 'signed-tags=warn-strip' '
test -s err
'
+test_expect_success GPGSM 'setup X.509 signed tag' '
+
+ test_config gpg.format x509 &&
+ test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+
+ git tag -s -m "X.509 signed tag" x509-signed $(git rev-parse HEAD) &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
+
+'
+
+test_expect_success GPGSM 'signed-tags=verbatim with X.509' '
+
+ git fast-export --signed-tags=verbatim x509-signed > output &&
+ test_grep "SIGNED MESSAGE" output
+
+'
+
+test_expect_success GPGSM 'signed-tags=strip with X.509' '
+
+ git fast-export --signed-tags=strip x509-signed > output &&
+ test_grep ! "SIGNED MESSAGE" output
+
+'
+
+test_expect_success GPGSSH 'setup SSH signed tag' '
+
+ test_config gpg.format ssh &&
+ test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+
+ git tag -s -m "SSH signed tag" ssh-signed $(git rev-parse HEAD) &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
+
+'
+
+test_expect_success GPGSSH 'signed-tags=verbatim with SSH' '
+
+ git fast-export --signed-tags=verbatim ssh-signed > output &&
+ test_grep "SSH SIGNATURE" output
+
+'
+
+test_expect_success GPGSSH 'signed-tags=strip with SSH' '
+
+ git fast-export --signed-tags=strip ssh-signed > output &&
+ test_grep ! "SSH SIGNATURE" output
+
+'
+
test_expect_success GPG 'set up signed commit' '
# Generate a commit with both "gpgsig" and "encoding" set, so
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-07 12:29 [PATCH 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
` (3 preceding siblings ...)
2025-10-07 12:29 ` [PATCH 4/5] fast-export: handle all kinds of tag signatures Christian Couder
@ 2025-10-07 12:29 ` Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
6 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-07 12:29 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Christian Couder,
Christian Couder
Recently, eaaddf5791 (fast-import: add '--signed-commits=<mode>'
option, 2025-09-17) added support for controlling how signed commits
are handled by `git fast-import`, but there is no option yet to
decide about signed tags.
To remediate that, let's add a '--signed-tags=<mode>' option to
`git fast-import` too.
With this, both `git fast-export` and `git fast-import` have both
a '--signed-tags=<mode>' and a '--signed-commits=<mode>' supporting
the same <mode>s.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-import.adoc | 5 ++
builtin/fast-import.c | 43 ++++++++++++++++
t/meson.build | 1 +
t/t9306-fast-import-signed-tags.sh | 80 ++++++++++++++++++++++++++++++
4 files changed, 129 insertions(+)
create mode 100755 t/t9306-fast-import-signed-tags.sh
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 85ed7a7270..b74179a6c8 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -66,6 +66,11 @@ fast-import stream! This option is enabled automatically for
remote-helpers that use the `import` capability, as they are
already trusted to run their own code.
+--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
+ Specify how to handle signed tags. Behaves in the same way
+ as the same option in linkgit:git-fast-export[1], except that
+ default is 'verbatim' (instead of 'abort').
+
--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed commits. Behaves in the same way
as the same option in linkgit:git-fast-export[1], except that
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 2010e78475..668c926db5 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -188,6 +188,7 @@ static int global_argc;
static const char **global_argv;
static const char *global_prefix;
+static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
/* Memory pools */
@@ -2961,6 +2962,43 @@ static void parse_new_commit(const char *arg)
b->last_commit = object_count_by_type[OBJ_COMMIT];
}
+static void handle_tag_signature(struct strbuf *msg, const char *name)
+{
+ size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
+
+ /* If there is no signature, there is nothing to do. */
+ if (sig_offset >= msg->len)
+ return;
+
+ switch (signed_tag_mode) {
+
+ /* First, modes that don't change anything */
+ case SIGN_ABORT:
+ die("encountered signed tag; use "
+ "--signed-tags=<mode> to handle it");
+ case SIGN_WARN_VERBATIM:
+ warning(_("importing a tag signature verbatim for tag '%s'"), name);
+ /* fallthru */
+ case SIGN_VERBATIM:
+ /* Nothing to do, the signature will be put into the imported tag. */
+ break;
+
+ /* Second, modes that remove the signature */
+ case SIGN_WARN_STRIP:
+ warning(_("stripping a tag signature for tag '%s'"), name);
+ /* fallthru */
+ case SIGN_STRIP:
+ /* Truncate the buffer to remove the signature */
+ strbuf_setlen(msg, sig_offset);
+ break;
+
+ /* Third, BUG */
+ default:
+ BUG("invalid signed_tag_mode value %d from tag '%s'",
+ signed_tag_mode, name);
+ }
+}
+
static void parse_new_tag(const char *arg)
{
static struct strbuf msg = STRBUF_INIT;
@@ -3024,6 +3062,8 @@ static void parse_new_tag(const char *arg)
/* tag payload/message */
parse_data(&msg, 0, NULL);
+ handle_tag_signature(&msg, t->name);
+
/* build the tag object */
strbuf_reset(&new_data);
@@ -3544,6 +3584,9 @@ static int parse_one_option(const char *option)
} else if (skip_prefix(option, "signed-commits=", &option)) {
if (parse_sign_mode(option, &signed_commit_mode))
usagef(_("unknown --signed-commits mode '%s'"), option);
+ } else if (skip_prefix(option, "signed-tags=", &option)) {
+ if (parse_sign_mode(option, &signed_tag_mode))
+ usagef(_("unknown --signed-tags mode '%s'"), option);
} else if (!strcmp(option, "quiet")) {
show_stats = 0;
quiet = 1;
diff --git a/t/meson.build b/t/meson.build
index 11376b9e25..cb8c2b4b30 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1036,6 +1036,7 @@ integration_tests = [
't9303-fast-import-compression.sh',
't9304-fast-import-marks.sh',
't9305-fast-import-signatures.sh',
+ 't9306-fast-import-signed-tags.sh',
't9350-fast-export.sh',
't9351-fast-export-anonymize.sh',
't9400-git-cvsserver-server.sh',
diff --git a/t/t9306-fast-import-signed-tags.sh b/t/t9306-fast-import-signed-tags.sh
new file mode 100755
index 0000000000..363619e7d1
--- /dev/null
+++ b/t/t9306-fast-import-signed-tags.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+
+test_description='git fast-import --signed-tags=<mode>'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success 'set up unsigned initial commit and import repo' '
+ test_commit first &&
+ git init new
+'
+
+test_expect_success 'import no signed tag with --signed-tags=abort' '
+ git fast-export --signed-tags=verbatim >output &&
+ git -C new fast-import --quiet --signed-tags=abort <output
+'
+
+test_expect_success GPG 'set up OpenPGP signed tag' '
+ git tag -s -m "OpenPGP signed tag" openpgp-signed first &&
+ OPENPGP_SIGNED=$(git rev-parse --verify refs/tags/openpgp-signed) &&
+ git fast-export --signed-tags=verbatim openpgp-signed >output
+'
+
+test_expect_success GPG 'import OpenPGP signed tag with --signed-tags=abort' '
+ test_must_fail git -C new fast-import --quiet --signed-tags=abort <output
+'
+
+test_expect_success GPG 'import OpenPGP signed tag with --signed-tags=verbatim' '
+ git -C new fast-import --quiet --signed-tags=verbatim <output >log 2>&1 &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/openpgp-signed) &&
+ test $OPENPGP_SIGNED = $IMPORTED &&
+ test_must_be_empty log
+'
+
+test_expect_success GPGSM 'setup X.509 signed tag' '
+ test_config gpg.format x509 &&
+ test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+
+ git tag -s -m "X.509 signed tag" x509-signed first &&
+ X509_SIGNED=$(git rev-parse --verify refs/tags/x509-signed) &&
+ git fast-export --signed-tags=verbatim x509-signed >output
+'
+
+test_expect_success GPGSM 'import X.509 signed tag with --signed-tags=warn-strip' '
+ git -C new fast-import --quiet --signed-tags=warn-strip <output >log 2>&1 &&
+ test_grep "stripping a tag signature for tag '\''x509-signed'\''" log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/x509-signed) &&
+ test $X509_SIGNED != $IMPORTED &&
+ git -C new cat-file -p x509-signed >out &&
+ test_grep ! "SIGNED MESSAGE" out
+'
+
+test_expect_success GPGSSH 'setup SSH signed tag' '
+ test_config gpg.format ssh &&
+ test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+
+ git tag -s -m "SSH signed tag" ssh-signed first &&
+ SSH_SIGNED=$(git rev-parse --verify refs/tags/ssh-signed) &&
+ git fast-export --signed-tags=verbatim ssh-signed >output
+'
+
+test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=warn-verbatim' '
+ git -C new fast-import --quiet --signed-tags=warn-verbatim <output >log 2>&1 &&
+ test_grep "importing a tag signature verbatim for tag '\''ssh-signed'\''" log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/ssh-signed) &&
+ test $SSH_SIGNED = $IMPORTED
+'
+
+test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=strip' '
+ git -C new fast-import --quiet --signed-tags=strip <output >log 2>&1 &&
+ test_must_be_empty log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/ssh-signed) &&
+ test $SSH_SIGNED != $IMPORTED &&
+ git -C new cat-file -p ssh-signed >out &&
+ test_grep ! "SSH SIGNATURE" out
+'
+
+test_done
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags
2025-10-07 12:29 ` [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags Christian Couder
@ 2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 9:52 ` Christian Couder
0 siblings, 1 reply; 52+ messages in thread
From: Patrick Steinhardt @ 2025-10-08 7:14 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Tue, Oct 07, 2025 at 02:29:54PM +0200, Christian Couder wrote:
> diff --git a/Documentation/git-tag.adoc b/Documentation/git-tag.adoc
> index a4b1c0ec05..9117754ffb 100644
> --- a/Documentation/git-tag.adoc
> +++ b/Documentation/git-tag.adoc
> @@ -3,7 +3,7 @@ git-tag(1)
>
> NAME
> ----
> -git-tag - Create, list, delete or verify a tag object signed with GPG
> +git-tag - Create, list, delete or verify tags
This is an obvious improvement.
> @@ -38,17 +38,18 @@ and `-a`, `-s`, and `-u <key-id>` are absent, `-a` is implied.
> Otherwise, a tag reference that points directly at the given object
> (i.e., a lightweight tag) is created.
>
> -A GnuPG signed tag object will be created when `-s` or `-u
> -<key-id>` is used. When `-u <key-id>` is not used, the
> -committer identity for the current user is used to find the
> -GnuPG key for signing. The configuration variable `gpg.program`
> -is used to specify custom GnuPG binary.
> +A cryptographically signed tag object will be created when `-s` or
> +`-u <key-id>` is used. The signing backend (GPG, X.509, SSH, etc.) is
> +controlled by the `gpg.format` configuration variable, defaulting to
> +OpenPGP. When `-u <key-id>` is not used, the committer identity for
> +the current user is used to find the key for signing. The
> +configuration variable `gpg.program` is used to specify a custom
> +signing binary.
>
> Tag objects (created with `-a`, `-s`, or `-u`) are called "annotated"
> tags; they contain a creation date, the tagger name and e-mail, a
> -tagging message, and an optional GnuPG signature. Whereas a
> -"lightweight" tag is simply a name for an object (usually a commit
> -object).
> +tagging message, and an optional signature. Whereas a "lightweight"
Nit: let's rather say "cryptographic signature" here.
> +tag is simply a name for an object (usually a commit object).
>
> Annotated tags are meant for release while lightweight tags are meant
> for private or temporary object labels. For this reason, some git
> @@ -64,10 +65,12 @@ OPTIONS
>
> -s::
> --sign::
> - Make a GPG-signed tag, using the default e-mail address's key.
> - The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
> - configuration variable if it exists, or disabled otherwise.
> - See linkgit:git-config[1].
> + Make a signed tag, using the default signing key. The signing
Same here, let's say "cryptographically signed tag".
> @@ -75,7 +78,9 @@ OPTIONS
>
> -u <key-id>::
> --local-user=<key-id>::
> - Make a GPG-signed tag, using the given key.
> + Make a signed tag using the given key. The format of the
Same.
> + <key-id> and the backend used depend on the `gpg.format`
> + configuration variable. See linkgit:git-config[1].
>
> -f::
> --force::
> @@ -87,7 +92,7 @@ OPTIONS
>
> -v::
> --verify::
> - Verify the GPG signature of the given tag names.
> + Verify the signature of the given tag names.
Same.
> @@ -236,12 +241,25 @@ it in the repository configuration as follows:
>
> -------------------------------------
> [user]
> - signingKey = <gpg-key-id>
> + signingKey = <key-id>
> -------------------------------------
>
> +The signing backend is controlled by the `gpg.format` configuration
> +variable, which defaults to `openpgp` for GPG signing. To sign tags
> +using other technologies like X.509 or SSH, set this variable to
> +`x509` or `ssh` respectively.
> +
It might make sense to use a bulleted list here to list the different
available formats. On the other hand, we could just as well refer to
git-config(1) so that we don't have to repeat any of the information
here, but instead have it at a central place.
That might not be worth it though. In the end there aren't too many
different commands that write signed objects.
Overall this change makes a lot of sense to me, thanks!
Patrick
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first
2025-10-07 12:29 ` [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first Christian Couder
@ 2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 9:42 ` Christian Couder
0 siblings, 1 reply; 52+ messages in thread
From: Patrick Steinhardt @ 2025-10-08 7:14 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Tue, Oct 07, 2025 at 02:29:55PM +0200, Christian Couder wrote:
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index 937b876bd0..743985efab 100644
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -38,7 +38,7 @@ test_lazy_prereq GPG '
> # To export ownertrust:
> # gpg --homedir /tmp/gpghome --export-ownertrust \
> # > lib-gpg/ownertrust
> - mkdir "$GNUPGHOME" &&
> + mkdir -p "$GNUPGHOME" &&
> chmod 0700 "$GNUPGHOME" &&
> (gpgconf --kill all || : ) &&
> gpg --homedir "${GNUPGHOME}" --import \
Okay. I wonder why we even have to create the directory manually. We
don't do it in the GPGSM prereq either, as gpgsm seems to handle this
for us. Doesn't `gpg --homedir ... --import` create the home directory
in a similar way?
Patrick
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 3/5] t9350: properly count annotated tags
2025-10-07 12:29 ` [PATCH 3/5] t9350: properly count annotated tags Christian Couder
@ 2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 10:00 ` Christian Couder
0 siblings, 1 reply; 52+ messages in thread
From: Patrick Steinhardt @ 2025-10-08 7:14 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Tue, Oct 07, 2025 at 02:29:56PM +0200, Christian Couder wrote:
> In t9350-fast-export.sh, these existing tests:
>
> - 'fast-export | fast-import when main is tagged'
> - 'cope with tagger-less tags'
>
> are checking the number of annotated tags in the test repo by comparing
> it with some hardcoded values.
>
> This could be an issue if some new tests that have some prerequisites
> add new annotated tags to the repo before these existing tests. When
> the prerequisites would be satisfied, the number of annotated tags
> would be different from when some prerequisites would not be satisfied.
>
> As we are going to add new tests that add new annotated tags in a
> following commit, let's properly count the number of annotated tag in
> the repo by incrementing a counter each time a new annotated tag is
> added, and then by comparing the number of annotated tags to the value
> of the counter when checking the number of annotated tags.
Hm, okay. I think having tests interdepend on one another is bad test
design in the first place, but it's not a new problem you create. An
alternative solution could of course be to change the new test so that
it works in a standalone repository, or to add it towards the end of the
test suite.
Have you considered these alternatives?
Patrick
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 4/5] fast-export: handle all kinds of tag signatures
2025-10-07 12:29 ` [PATCH 4/5] fast-export: handle all kinds of tag signatures Christian Couder
@ 2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 10:02 ` Christian Couder
2025-10-09 12:33 ` Christian Couder
0 siblings, 2 replies; 52+ messages in thread
From: Patrick Steinhardt @ 2025-10-08 7:14 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Tue, Oct 07, 2025 at 02:29:57PM +0200, Christian Couder wrote:
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index dc2486f9a8..7adbc55f0d 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -931,9 +931,8 @@ static void handle_tag(const char *name, struct tag *tag)
>
> /* handle signed tags */
> if (message) {
> - const char *signature = strstr(message,
> - "\n-----BEGIN PGP SIGNATURE-----\n");
> - if (signature)
> + size_t sig_offset = parse_signed_buffer(message, message_size);
> + if (sig_offset < message_size)
Yup. The function either returns `message_size` in case there is no
signature, or it returns the offset at which the signature starts.
> switch (signed_tag_mode) {
> case SIGN_ABORT:
> die("encountered signed tag %s; use "
I was afraid at first that we're now open-coding all these different
signature formats. But this implementation makes me quite happy, as we
even remove the existing check instead of using a central function.
Nice.
> @@ -950,7 +949,7 @@ static void handle_tag(const char *name, struct tag *tag)
> oid_to_hex(&tag->object.oid));
> /* fallthru */
> case SIGN_STRIP:
> - message_size = signature + 1 - message;
> + message_size = sig_offset;
> break;
> }
> }
Makes sense.
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 21ff26939c..5a46608f65 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -279,6 +279,54 @@ test_expect_success 'signed-tags=warn-strip' '
> test -s err
> '
>
> +test_expect_success GPGSM 'setup X.509 signed tag' '
> +
> + test_config gpg.format x509 &&
> + test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> +
> + git tag -s -m "X.509 signed tag" x509-signed $(git rev-parse HEAD) &&
> + ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
> +
> +'
Nit for this test and all of the below ones: our modern style does not
have empty lines at the beginning and end of a test case.
Patrick
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-07 12:29 ` [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
@ 2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 10:50 ` Christian Couder
0 siblings, 1 reply; 52+ messages in thread
From: Patrick Steinhardt @ 2025-10-08 7:14 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Tue, Oct 07, 2025 at 02:29:58PM +0200, Christian Couder wrote:
> diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> index 85ed7a7270..b74179a6c8 100644
> --- a/Documentation/git-fast-import.adoc
> +++ b/Documentation/git-fast-import.adoc
> @@ -66,6 +66,11 @@ fast-import stream! This option is enabled automatically for
> remote-helpers that use the `import` capability, as they are
> already trusted to run their own code.
>
> +--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> + Specify how to handle signed tags. Behaves in the same way
> + as the same option in linkgit:git-fast-export[1], except that
> + default is 'verbatim' (instead of 'abort').
> +
Nit: I would've ordered this after "--signed-commits", mostly so that
these two are ordered alphabetically.
> --signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> Specify how to handle signed commits. Behaves in the same way
> as the same option in linkgit:git-fast-export[1], except that
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 2010e78475..668c926db5 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -2961,6 +2962,43 @@ static void parse_new_commit(const char *arg)
> b->last_commit = object_count_by_type[OBJ_COMMIT];
> }
>
> +static void handle_tag_signature(struct strbuf *msg, const char *name)
> +{
> + size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
> +
> + /* If there is no signature, there is nothing to do. */
> + if (sig_offset >= msg->len)
> + return;
> +
> + switch (signed_tag_mode) {
> +
> + /* First, modes that don't change anything */
> + case SIGN_ABORT:
> + die("encountered signed tag; use "
> + "--signed-tags=<mode> to handle it");
This message needs to be marked for translation.
> + case SIGN_WARN_VERBATIM:
> + warning(_("importing a tag signature verbatim for tag '%s'"), name);
> + /* fallthru */
This comment is misindented.
> + case SIGN_VERBATIM:
> + /* Nothing to do, the signature will be put into the imported tag. */
> + break;
> +
> + /* Second, modes that remove the signature */
> + case SIGN_WARN_STRIP:
> + warning(_("stripping a tag signature for tag '%s'"), name);
> + /* fallthru */
Same here, the comment is misindented.
> + case SIGN_STRIP:
> + /* Truncate the buffer to remove the signature */
> + strbuf_setlen(msg, sig_offset);
> + break;
I'm not familiar with the signature format, so it's probably a dumb
question: does the signature always extend until the end of the tag
message? Doesn't the tag message come after it?
Patrick
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first
2025-10-08 7:14 ` Patrick Steinhardt
@ 2025-10-08 9:42 ` Christian Couder
2025-10-09 1:29 ` Collin Funk
0 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-08 9:42 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Wed, Oct 8, 2025 at 11:21 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Oct 07, 2025 at 02:29:55PM +0200, Christian Couder wrote:
> > diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> > index 937b876bd0..743985efab 100644
> > --- a/t/lib-gpg.sh
> > +++ b/t/lib-gpg.sh
> > @@ -38,7 +38,7 @@ test_lazy_prereq GPG '
> > # To export ownertrust:
> > # gpg --homedir /tmp/gpghome --export-ownertrust \
> > # > lib-gpg/ownertrust
> > - mkdir "$GNUPGHOME" &&
> > + mkdir -p "$GNUPGHOME" &&
> > chmod 0700 "$GNUPGHOME" &&
> > (gpgconf --kill all || : ) &&
> > gpg --homedir "${GNUPGHOME}" --import \
>
> Okay. I wonder why we even have to create the directory manually. We
> don't do it in the GPGSM prereq either, as gpgsm seems to handle this
> for us.
Yeah, the GPGSSH prereq does `mkdir -p "$GNUPGHOME"`, but not the GPGSM prereq.
> Doesn't `gpg --homedir ... --import` create the home directory
> in a similar way?
I am not sure. It might depend on the gpg version. Or maybe gpgsm
does it but not gpg. I will check.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags
2025-10-08 7:14 ` Patrick Steinhardt
@ 2025-10-08 9:52 ` Christian Couder
2025-10-08 11:48 ` Patrick Steinhardt
0 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-08 9:52 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Wed, Oct 8, 2025 at 11:21 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Oct 07, 2025 at 02:29:54PM +0200, Christian Couder wrote:
> > diff --git a/Documentation/git-tag.adoc b/Documentation/git-tag.adoc
> > index a4b1c0ec05..9117754ffb 100644
> > --- a/Documentation/git-tag.adoc
> > +++ b/Documentation/git-tag.adoc
> > @@ -3,7 +3,7 @@ git-tag(1)
> >
> > NAME
> > ----
> > -git-tag - Create, list, delete or verify a tag object signed with GPG
> > +git-tag - Create, list, delete or verify tags
>
> This is an obvious improvement.
[...]
> > Tag objects (created with `-a`, `-s`, or `-u`) are called "annotated"
> > tags; they contain a creation date, the tagger name and e-mail, a
> > -tagging message, and an optional GnuPG signature. Whereas a
> > -"lightweight" tag is simply a name for an object (usually a commit
> > -object).
> > +tagging message, and an optional signature. Whereas a "lightweight"
>
> Nit: let's rather say "cryptographic signature" here.
OK, I will make this change in V2.
> > +tag is simply a name for an object (usually a commit object).
> >
> > Annotated tags are meant for release while lightweight tags are meant
> > for private or temporary object labels. For this reason, some git
> > @@ -64,10 +65,12 @@ OPTIONS
> >
> > -s::
> > --sign::
> > - Make a GPG-signed tag, using the default e-mail address's key.
> > - The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
> > - configuration variable if it exists, or disabled otherwise.
> > - See linkgit:git-config[1].
> > + Make a signed tag, using the default signing key. The signing
>
> Same here, let's say "cryptographically signed tag".
>
> > @@ -75,7 +78,9 @@ OPTIONS
> >
> > -u <key-id>::
> > --local-user=<key-id>::
> > - Make a GPG-signed tag, using the given key.
> > + Make a signed tag using the given key. The format of the
>
> Same.
>
> > + <key-id> and the backend used depend on the `gpg.format`
> > + configuration variable. See linkgit:git-config[1].
> >
> > -f::
> > --force::
> > @@ -87,7 +92,7 @@ OPTIONS
> >
> > -v::
> > --verify::
> > - Verify the GPG signature of the given tag names.
> > + Verify the signature of the given tag names.
>
> Same.
It's a bit cumbersome to have to say "cryptographic" or
"cryptographically" everywhere though. Maybe saying it a few times at
the beginning is enough?
> > @@ -236,12 +241,25 @@ it in the repository configuration as follows:
> >
> > -------------------------------------
> > [user]
> > - signingKey = <gpg-key-id>
> > + signingKey = <key-id>
> > -------------------------------------
> >
> > +The signing backend is controlled by the `gpg.format` configuration
> > +variable, which defaults to `openpgp` for GPG signing. To sign tags
> > +using other technologies like X.509 or SSH, set this variable to
> > +`x509` or `ssh` respectively.
> > +
>
> It might make sense to use a bulleted list here to list the different
> available formats.
What should we say about each format though?
> On the other hand, we could just as well refer to
> git-config(1) so that we don't have to repeat any of the information
> here, but instead have it at a central place.
>
> That might not be worth it though. In the end there aren't too many
> different commands that write signed objects.
I think this CONFIGURATION section should talk only briefly about the
most important config options and refer to the git-config(1) doc for
details and less important config options. So I am not sure what you
suggest exactly about this.
> Overall this change makes a lot of sense to me, thanks!
Thanks.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 3/5] t9350: properly count annotated tags
2025-10-08 7:14 ` Patrick Steinhardt
@ 2025-10-08 10:00 ` Christian Couder
0 siblings, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-08 10:00 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Wed, Oct 8, 2025 at 11:21 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Oct 07, 2025 at 02:29:56PM +0200, Christian Couder wrote:
> > In t9350-fast-export.sh, these existing tests:
> >
> > - 'fast-export | fast-import when main is tagged'
> > - 'cope with tagger-less tags'
> >
> > are checking the number of annotated tags in the test repo by comparing
> > it with some hardcoded values.
> >
> > This could be an issue if some new tests that have some prerequisites
> > add new annotated tags to the repo before these existing tests. When
> > the prerequisites would be satisfied, the number of annotated tags
> > would be different from when some prerequisites would not be satisfied.
> >
> > As we are going to add new tests that add new annotated tags in a
> > following commit, let's properly count the number of annotated tag in
> > the repo by incrementing a counter each time a new annotated tag is
> > added, and then by comparing the number of annotated tags to the value
> > of the counter when checking the number of annotated tags.
>
> Hm, okay. I think having tests interdepend on one another is bad test
> design in the first place, but it's not a new problem you create. An
> alternative solution could of course be to change the new test so that
> it works in a standalone repository, or to add it towards the end of the
> test suite.
>
> Have you considered these alternatives?
Yes, I have considered them, but I think those workarounds could make
the technical debt worse.
For example if I move those tests towards the end of the test script
or in another separate test script, then someone might wonder later
why they are not at the logical place where they should be. They would
then move them and realize that it creates problems with subsequent
tests. This would waste time.
So I think it's a good thing to make the interdependency clearly
visible instead. This is a bit ugly, but it shows the existing
technical debt instead of hiding it.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 4/5] fast-export: handle all kinds of tag signatures
2025-10-08 7:14 ` Patrick Steinhardt
@ 2025-10-08 10:02 ` Christian Couder
2025-10-09 12:33 ` Christian Couder
1 sibling, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-08 10:02 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Wed, Oct 8, 2025 at 11:21 AM Patrick Steinhardt <ps@pks.im> wrote:
> > diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> > index 21ff26939c..5a46608f65 100755
> > --- a/t/t9350-fast-export.sh
> > +++ b/t/t9350-fast-export.sh
> > @@ -279,6 +279,54 @@ test_expect_success 'signed-tags=warn-strip' '
> > test -s err
> > '
> >
> > +test_expect_success GPGSM 'setup X.509 signed tag' '
> > +
> > + test_config gpg.format x509 &&
> > + test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> > +
> > + git tag -s -m "X.509 signed tag" x509-signed $(git rev-parse HEAD) &&
> > + ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
> > +
> > +'
>
> Nit for this test and all of the below ones: our modern style does not
> have empty lines at the beginning and end of a test case.
OK, I will use the modern style (instead of the existing style in this
test script) in V2.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-08 7:14 ` Patrick Steinhardt
@ 2025-10-08 10:50 ` Christian Couder
2025-10-08 11:53 ` Patrick Steinhardt
0 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-08 10:50 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Wed, Oct 8, 2025 at 11:21 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Oct 07, 2025 at 02:29:58PM +0200, Christian Couder wrote:
> > diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> > index 85ed7a7270..b74179a6c8 100644
> > --- a/Documentation/git-fast-import.adoc
> > +++ b/Documentation/git-fast-import.adoc
> > @@ -66,6 +66,11 @@ fast-import stream! This option is enabled automatically for
> > remote-helpers that use the `import` capability, as they are
> > already trusted to run their own code.
> >
> > +--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> > + Specify how to handle signed tags. Behaves in the same way
> > + as the same option in linkgit:git-fast-export[1], except that
> > + default is 'verbatim' (instead of 'abort').
> > +
>
> Nit: I would've ordered this after "--signed-commits", mostly so that
> these two are ordered alphabetically.
In the fast-export doc --signed-tags is before --signed-commits. Also
in the previous patch series Junio mentioned that historically signed
tags came before signed commits. And the other options are not sorted
alphabetically.
> > --signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> > Specify how to handle signed commits. Behaves in the same way
> > as the same option in linkgit:git-fast-export[1], except that
> > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > index 2010e78475..668c926db5 100644
> > --- a/builtin/fast-import.c
> > +++ b/builtin/fast-import.c
> > @@ -2961,6 +2962,43 @@ static void parse_new_commit(const char *arg)
> > b->last_commit = object_count_by_type[OBJ_COMMIT];
> > }
> >
> > +static void handle_tag_signature(struct strbuf *msg, const char *name)
> > +{
> > + size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
> > +
> > + /* If there is no signature, there is nothing to do. */
> > + if (sig_offset >= msg->len)
> > + return;
> > +
> > + switch (signed_tag_mode) {
> > +
> > + /* First, modes that don't change anything */
> > + case SIGN_ABORT:
> > + die("encountered signed tag; use "
> > + "--signed-tags=<mode> to handle it");
>
> This message needs to be marked for translation.
Yeah, I will fix it in V2.
> > + case SIGN_WARN_VERBATIM:
> > + warning(_("importing a tag signature verbatim for tag '%s'"), name);
> > + /* fallthru */
>
> This comment is misindented.
Will fix it in V2. Same with other misindented comments.
> > + case SIGN_STRIP:
> > + /* Truncate the buffer to remove the signature */
> > + strbuf_setlen(msg, sig_offset);
> > + break;
>
> I'm not familiar with the signature format, so it's probably a dumb
> question: does the signature always extend until the end of the tag
> message? Doesn't the tag message come after it?
Users can add anything in a tag message, including signatures created
however they want and copy-pasted there, followed by whatever content
they want. I don't think we need to take care of those signatures,
except perhaps to warn in our docs that Git could mistake them with
the one Git creates.
When Git itself signs a tag, it appends the signature to the tag
message. See do_sign() in "builtin/tag.c" for more details. It looks
like 2 signatures can be created in "compat" mode, but the compat
signature is added into an object header, not appended to the tag
message.
So I think this is the right thing to do and relatively safe.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags
2025-10-08 9:52 ` Christian Couder
@ 2025-10-08 11:48 ` Patrick Steinhardt
0 siblings, 0 replies; 52+ messages in thread
From: Patrick Steinhardt @ 2025-10-08 11:48 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Wed, Oct 08, 2025 at 11:52:44AM +0200, Christian Couder wrote:
> On Wed, Oct 8, 2025 at 11:21 AM Patrick Steinhardt <ps@pks.im> wrote:
> > On Tue, Oct 07, 2025 at 02:29:54PM +0200, Christian Couder wrote:
> > > diff --git a/Documentation/git-tag.adoc b/Documentation/git-tag.adoc
> > > index a4b1c0ec05..9117754ffb 100644
> > > --- a/Documentation/git-tag.adoc
> > > +++ b/Documentation/git-tag.adoc
> > > @@ -236,12 +241,25 @@ it in the repository configuration as follows:
> > >
> > > -------------------------------------
> > > [user]
> > > - signingKey = <gpg-key-id>
> > > + signingKey = <key-id>
> > > -------------------------------------
> > >
> > > +The signing backend is controlled by the `gpg.format` configuration
> > > +variable, which defaults to `openpgp` for GPG signing. To sign tags
> > > +using other technologies like X.509 or SSH, set this variable to
> > > +`x509` or `ssh` respectively.
> > > +
> >
> > It might make sense to use a bulleted list here to list the different
> > available formats.
>
> What should we say about each format though?
>
> > On the other hand, we could just as well refer to
> > git-config(1) so that we don't have to repeat any of the information
> > here, but instead have it at a central place.
> >
> > That might not be worth it though. In the end there aren't too many
> > different commands that write signed objects.
>
> I think this CONFIGURATION section should talk only briefly about the
> most important config options and refer to the git-config(1) doc for
> details and less important config options. So I am not sure what you
> suggest exactly about this.
Yeah, I'm fine with referring to git-config(1). I mostly want to avoid
that we have N locations that we need to update every time something
changes here, as those are bound to become stale.
Maybe a solution would be to only point out the config keys without
going into much detail what the respective values are? In that case we
woulds imply refer to git-config(1) and call it a day.
Patrick
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-08 10:50 ` Christian Couder
@ 2025-10-08 11:53 ` Patrick Steinhardt
0 siblings, 0 replies; 52+ messages in thread
From: Patrick Steinhardt @ 2025-10-08 11:53 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Wed, Oct 08, 2025 at 12:50:53PM +0200, Christian Couder wrote:
> On Wed, Oct 8, 2025 at 11:21 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Tue, Oct 07, 2025 at 02:29:58PM +0200, Christian Couder wrote:
> > > diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> > > index 85ed7a7270..b74179a6c8 100644
> > > --- a/Documentation/git-fast-import.adoc
> > > +++ b/Documentation/git-fast-import.adoc
> > > @@ -66,6 +66,11 @@ fast-import stream! This option is enabled automatically for
> > > remote-helpers that use the `import` capability, as they are
> > > already trusted to run their own code.
> > >
> > > +--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> > > + Specify how to handle signed tags. Behaves in the same way
> > > + as the same option in linkgit:git-fast-export[1], except that
> > > + default is 'verbatim' (instead of 'abort').
> > > +
> >
> > Nit: I would've ordered this after "--signed-commits", mostly so that
> > these two are ordered alphabetically.
>
> In the fast-export doc --signed-tags is before --signed-commits. Also
> in the previous patch series Junio mentioned that historically signed
> tags came before signed commits. And the other options are not sorted
> alphabetically.
Okay, makes sense.
> > > + case SIGN_STRIP:
> > > + /* Truncate the buffer to remove the signature */
> > > + strbuf_setlen(msg, sig_offset);
> > > + break;
> >
> > I'm not familiar with the signature format, so it's probably a dumb
> > question: does the signature always extend until the end of the tag
> > message? Doesn't the tag message come after it?
>
> Users can add anything in a tag message, including signatures created
> however they want and copy-pasted there, followed by whatever content
> they want. I don't think we need to take care of those signatures,
> except perhaps to warn in our docs that Git could mistake them with
> the one Git creates.
>
> When Git itself signs a tag, it appends the signature to the tag
> message. See do_sign() in "builtin/tag.c" for more details. It looks
> like 2 signatures can be created in "compat" mode, but the compat
> signature is added into an object header, not appended to the tag
> message.
>
> So I think this is the right thing to do and relatively safe.
Okay, thanks for clarifying.
Patrick
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first
2025-10-08 9:42 ` Christian Couder
@ 2025-10-09 1:29 ` Collin Funk
2025-10-09 2:37 ` Todd Zullinger
2025-10-09 12:30 ` Christian Couder
0 siblings, 2 replies; 52+ messages in thread
From: Collin Funk @ 2025-10-09 1:29 UTC (permalink / raw)
To: Christian Couder
Cc: Patrick Steinhardt, git, Junio C Hamano, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Christian Couder
Christian Couder <christian.couder@gmail.com> writes:
>> Okay. I wonder why we even have to create the directory manually. We
>> don't do it in the GPGSM prereq either, as gpgsm seems to handle this
>> for us.
>
> Yeah, the GPGSSH prereq does `mkdir -p "$GNUPGHOME"`, but not the GPGSM prereq.
>
>> Doesn't `gpg --homedir ... --import` create the home directory
>> in a similar way?
>
> I am not sure. It might depend on the gpg version. Or maybe gpgsm
> does it but not gpg. I will check.
If $GNUPGHOME or --homedir is the default (usually ~/.gnupg) gets
created by 'gpg' and 'gpgsm':
$ ls ~/.gnupg
ls: cannot access '/root/.gnupg': No such file or directory
$ gpgsm
gpgsm: directory '/root/.gnupg' created
gpgsm: invalid command (there is no implicit command)
$ rm -rf ~/.gnupg && gpg
gpg: directory '/root/.gnupg' created
[...]
If it is not the default then it will not be created:
$ GNUPGHOME=$HOME/test gpgsm
gpgsm: keyblock resource '/root/test/pubring.kbx': No such file or directory
$ GNUPGHOME=$HOME/test gpg
gpg: keyblock resource '/root/test/pubring.kbx': No such file or directory
Collin
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first
2025-10-09 1:29 ` Collin Funk
@ 2025-10-09 2:37 ` Todd Zullinger
2025-10-09 12:29 ` Christian Couder
2025-10-09 18:18 ` Junio C Hamano
2025-10-09 12:30 ` Christian Couder
1 sibling, 2 replies; 52+ messages in thread
From: Todd Zullinger @ 2025-10-09 2:37 UTC (permalink / raw)
To: Collin Funk
Cc: Christian Couder, Patrick Steinhardt, git, Junio C Hamano,
Elijah Newren, Jeff King, brian m . carlson, Johannes Schindelin,
Christian Couder
Collin Funk wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>>> Okay. I wonder why we even have to create the directory manually. We
>>> don't do it in the GPGSM prereq either, as gpgsm seems to handle this
>>> for us.
>>
>> Yeah, the GPGSSH prereq does `mkdir -p "$GNUPGHOME"`, but not the GPGSM prereq.
>>
>>> Doesn't `gpg --homedir ... --import` create the home directory
>>> in a similar way?
>>
>> I am not sure. It might depend on the gpg version. Or maybe gpgsm
>> does it but not gpg. I will check.
>
> If $GNUPGHOME or --homedir is the default (usually ~/.gnupg) gets
> created by 'gpg' and 'gpgsm':
>
> $ ls ~/.gnupg
> ls: cannot access '/root/.gnupg': No such file or directory
> $ gpgsm
> gpgsm: directory '/root/.gnupg' created
> gpgsm: invalid command (there is no implicit command)
> $ rm -rf ~/.gnupg && gpg
> gpg: directory '/root/.gnupg' created
> [...]
>
> If it is not the default then it will not be created:
>
> $ GNUPGHOME=$HOME/test gpgsm
> gpgsm: keyblock resource '/root/test/pubring.kbx': No such file or directory
> $ GNUPGHOME=$HOME/test gpg
> gpg: keyblock resource '/root/test/pubring.kbx': No such file or directory
>
> Collin
>
I sent a series long ago to fix this issue¹, but it wasn't
picked up.
Fixing the issue exposes broken tests which use the gpg2
prereq. That breakage turns up in our CI and other build
environments, like Fedora's, but I was never able to
reliably trigger it locally and track down what was broken
about those test.
I believe I asked about it again a few months later and it
did not gain any attention.
I simply apply the patches locally and then disable those
tests -- tests which don't run reliably are not worth
running IMO. :)
¹ <20240703153738.916469-1-tmz@pobox.com>
--
Todd
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled
2025-10-07 12:29 [PATCH 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
` (4 preceding siblings ...)
2025-10-07 12:29 ` [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
@ 2025-10-09 12:24 ` Christian Couder
2025-10-09 12:24 ` [PATCH v2 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
` (5 more replies)
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
6 siblings, 6 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:24 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
Introduction
------------
Tools like `git-filter-repo` should be able to control how tag
signatures are handled when regenerating repository content after it
has been filtered. For this purpose, they need a way for `git
fast-import` to control how tag signatures are handled.
A previous series [1] added a '--signed-commits=<mode>' option to `git
fast-import` to control how commit signatures are handled, so this is
adding a similar '--signed-tags=<mode>' for tag signatures.
For now this new option behaves in a very similar way as the option
with the same name that already exists in `git fast-export`.
Especially it supports exactly the same <mode>s and the same aliases
for these modes. For example "ignore" is a synonym for "verbatim".
This way, both `git fast-export` and `git fast-import` have both a
'--signed-tags=<mode>' and a '--signed-commits=<mode>' supporting the
same <mode>s.
In the future I want to implement new <mode>s like "strip-if-invalid",
"re-sign", "re-sign-if-invalid" in `git fast-import` for both tag and
commit signatures. These might be a bit more complex, so for now I
prefer to start with the simple modes.
[1] https://lore.kernel.org/git/20250917181427.3193500-1-christian.couder@gmail.com/
Note about the different patches
--------------------------------
Patch 1/5 (doc: git-tag: stop focussing on GPG signed tags) is a
documentation update for `git tag`. It could go in a separate series
or be dropped altogether, but while working on this I thought that it
would be a good thing to do, as the doc is quite outdated.
Patches 2/5, 3/5 and 4/5 are preparatory patches for the main one
which is patch 5/5 (fast-import: add '--signed-tags=<mode>' option).
I wanted '--signed-tags=<mode>' to work for all kinds of signature in
tags (OpenPGP, X.509 and SSH) but soon realized that the
'--signed-tags=<mode>' option of `git fast-export` worked only for
OpenPGP signatures, so I fixed that issue in patch 4/5 (fast-export:
handle all kinds of tag signatures).
While working on the tests in patch 4/5, I found a few things to
improve that could belong to other patches so that's how I came up
with patches 2/5 and 3/5.
Changes since v1
----------------
Thanks to Patrick Steinhardt, Todd Zullinger and Collin Funk who
reviewed or commented on the v1.
- In patch 1/5, in the commit message:
- "focussing" and "focussed" have been replaced with "focusing" and
"focused" respectively as the former is UK Eglish while the latter
is US English,
- the missing "section" word has been added.
- In patch 1/5, in the git-tag doc:
- "cryptographic" or "cryptographically" has been added to number of
places,
- the changes to the CONFIGURATION section have been shortened by
not mentioning the supported signing backend (X.509 and SSH) other
than OpenPGP, and by referring to git-config(1) more.
- In patch 2/5, the approach taken is now the one previously used by
Todd Zullinger in:
https://lore.kernel.org/git/20240703153738.916469-2-tmz@pobox.com/
so this patch looks like a completely different patch in the range
diff.
- In patch 3/5, in the commit message:
- t9350-fast-export.sh has been quoted,
- some explanations about alternative solutions that have been
considered have been added.
- In patch 4/5, the added tests are now written in a modern style,
instead of the old style used elsewhere in the script.
- In patch 5/5, a die() message has been marked for translation and
some "/* fallthru */" comments have been properly indented.
CI tests
--------
They have all passed except again one on Windows where
"t8020-last-modified.sh" failed. See:
https://github.com/chriscool/git/actions/runs/18373100224
Range diff since v1
-------------------
1: 05d0b86de6 ! 1: eb65af631d doc: git-tag: stop focussing on GPG signed tags
@@ Metadata
Author: Christian Couder <chriscool@tuxfamily.org>
## Commit message ##
- doc: git-tag: stop focussing on GPG signed tags
+ doc: git-tag: stop focusing on GPG signed tags
- It looks like the documentation of `git tag` is focussed a bit too
+ It looks like the documentation of `git tag` is focused a bit too
much on GPG signed tags.
This starts with the "NAME" section where the command is described
@@ Commit message
This goes on in the "OPTIONS" section too, especially about the `-s`
and `-u <key-id>` options.
- The "CONFIGURATION" also doesn't talk about how to configure the
- command to work with X.509 and SSH signatures.
+ The "CONFIGURATION" section also doesn't talk about how to configure
+ the command to work with X.509 and SSH signatures.
Let's rework all that to make sure users have a more accurate and
balanced view of what the command can do.
+ Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
## Documentation/git-tag.adoc ##
@@ Documentation/git-tag.adoc: and `-a`, `-s`, and `-u <key-id>` are absent, `-a` i
Tag objects (created with `-a`, `-s`, or `-u`) are called "annotated"
tags; they contain a creation date, the tagger name and e-mail, a
-tagging message, and an optional GnuPG signature. Whereas a
--"lightweight" tag is simply a name for an object (usually a commit
--object).
-+tagging message, and an optional signature. Whereas a "lightweight"
-+tag is simply a name for an object (usually a commit object).
++tagging message, and an optional cryptographic signature. Whereas a
+ "lightweight" tag is simply a name for an object (usually a commit
+ object).
- Annotated tags are meant for release while lightweight tags are meant
- for private or temporary object labels. For this reason, some git
@@ Documentation/git-tag.adoc: OPTIONS
-s::
@@ Documentation/git-tag.adoc: OPTIONS
- The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
- configuration variable if it exists, or disabled otherwise.
- See linkgit:git-config[1].
-+ Make a signed tag, using the default signing key. The signing
-+ backend used depends on the `gpg.format` configuration
-+ variable. The default key is determined by the backend. For
-+ GPG, it's based on the committer's email address, while for
-+ SSH it may be a specific key file or agent identity. See
-+ linkgit:git-config[1].
++ Make a cryptographically signed tag, using the default signing
++ key. The signing backend used depends on the `gpg.format`
++ configuration variable. The default key is determined by the
++ backend. For GPG, it's based on the committer's email address,
++ while for SSH it may be a specific key file or agent
++ identity. See linkgit:git-config[1].
--no-sign::
Override `tag.gpgSign` configuration variable that is
@@ Documentation/git-tag.adoc: OPTIONS
-u <key-id>::
--local-user=<key-id>::
- Make a GPG-signed tag, using the given key.
-+ Make a signed tag using the given key. The format of the
-+ <key-id> and the backend used depend on the `gpg.format`
-+ configuration variable. See linkgit:git-config[1].
++ Make a cryptographically signed tag using the given key. The
++ format of the <key-id> and the backend used depend on the
++ `gpg.format` configuration variable. See
++ linkgit:git-config[1].
-f::
--force::
@@ Documentation/git-tag.adoc: it in the repository configuration as follows:
+ signingKey = <key-id>
-------------------------------------
-+The signing backend is controlled by the `gpg.format` configuration
-+variable, which defaults to `openpgp` for GPG signing. To sign tags
-+using other technologies like X.509 or SSH, set this variable to
-+`x509` or `ssh` respectively.
++The signing backend can be chosen via the `gpg.format` configuration
++variable, which defaults to `openpgp`. See linkgit:git-config[1]
++for a list of other supported formats.
+
-+You can also specify the path to the signing program for each
-+format. The `gpg.program` variable (or its synonym
-+`gpg.openpgp.program`) is used for the OpenPGP backend. For other
-+backends, the configuration is `gpg.<format>.program`, for example
-+`gpg.ssh.program` for SSH signing.
++The path to the program used for each signing backend can be specified
++with the `gpg.<format>.program` configuration variable. For the
++`openpgp` backend, `gpg.program` can be used as a synonym for
++`gpg.openpgp.program`. See linkgit:git-config[1] for details.
+
`pager.tag` is only respected when listing tags, i.e., when `-l` is
used or implied. The default is to use a pager.
2: 61a1116542 < -: ---------- lib-gpg: allow tests with the GPGSM prereq first
-: ---------- > 2: 640204ef26 lib-gpg: allow tests with GPGSM or GPGSSH prereq first
3: b2b703ae9d ! 3: 8f788bafe1 t9350: properly count annotated tags
@@ Metadata
## Commit message ##
t9350: properly count annotated tags
- In t9350-fast-export.sh, these existing tests:
+ In "t9350-fast-export.sh", these existing tests:
- 'fast-export | fast-import when main is tagged'
- 'cope with tagger-less tags'
@@ Commit message
added, and then by comparing the number of annotated tags to the value
of the counter when checking the number of annotated tags.
+ This is a bit ugly, but it makes it explicit that some tests are
+ interdependent. Alternative solutions, like moving the new tests to
+ the end of the script, were considered, but were rejected because they
+ would instead hide the technical debt and could confuse developers in
+ the future.
+
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
## t/t9350-fast-export.sh ##
4: b51e904f90 ! 4: d62a43905c fast-export: handle all kinds of tag signatures
@@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
'
+test_expect_success GPGSM 'setup X.509 signed tag' '
-+
+ test_config gpg.format x509 &&
+ test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+
+ git tag -s -m "X.509 signed tag" x509-signed $(git rev-parse HEAD) &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
-+
+'
+
+test_expect_success GPGSM 'signed-tags=verbatim with X.509' '
-+
+ git fast-export --signed-tags=verbatim x509-signed > output &&
+ test_grep "SIGNED MESSAGE" output
-+
+'
+
+test_expect_success GPGSM 'signed-tags=strip with X.509' '
-+
+ git fast-export --signed-tags=strip x509-signed > output &&
+ test_grep ! "SIGNED MESSAGE" output
-+
+'
+
+test_expect_success GPGSSH 'setup SSH signed tag' '
-+
+ test_config gpg.format ssh &&
+ test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+
+ git tag -s -m "SSH signed tag" ssh-signed $(git rev-parse HEAD) &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
-+
+'
+
+test_expect_success GPGSSH 'signed-tags=verbatim with SSH' '
-+
+ git fast-export --signed-tags=verbatim ssh-signed > output &&
+ test_grep "SSH SIGNATURE" output
-+
+'
+
+test_expect_success GPGSSH 'signed-tags=strip with SSH' '
-+
+ git fast-export --signed-tags=strip ssh-signed > output &&
+ test_grep ! "SSH SIGNATURE" output
-+
+'
+
test_expect_success GPG 'set up signed commit' '
5: 6987fc0bae ! 5: 9094f37b46 fast-import: add '--signed-tags=<mode>' option
@@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
+
+ /* First, modes that don't change anything */
+ case SIGN_ABORT:
-+ die("encountered signed tag; use "
-+ "--signed-tags=<mode> to handle it");
++ die(_("encountered signed tag; use "
++ "--signed-tags=<mode> to handle it"));
+ case SIGN_WARN_VERBATIM:
+ warning(_("importing a tag signature verbatim for tag '%s'"), name);
-+ /* fallthru */
++ /* fallthru */
+ case SIGN_VERBATIM:
+ /* Nothing to do, the signature will be put into the imported tag. */
+ break;
@@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
+ /* Second, modes that remove the signature */
+ case SIGN_WARN_STRIP:
+ warning(_("stripping a tag signature for tag '%s'"), name);
-+ /* fallthru */
++ /* fallthru */
+ case SIGN_STRIP:
+ /* Truncate the buffer to remove the signature */
+ strbuf_setlen(msg, sig_offset);
Christian Couder (5):
doc: git-tag: stop focusing on GPG signed tags
lib-gpg: allow tests with GPGSM or GPGSSH prereq first
t9350: properly count annotated tags
fast-export: handle all kinds of tag signatures
fast-import: add '--signed-tags=<mode>' option
Documentation/git-fast-import.adoc | 5 ++
Documentation/git-tag.adoc | 48 ++++++++++++------
builtin/fast-export.c | 7 ++-
builtin/fast-import.c | 43 ++++++++++++++++
t/lib-gpg.sh | 24 +++++++--
t/meson.build | 1 +
t/t9306-fast-import-signed-tags.sh | 80 ++++++++++++++++++++++++++++++
t/t9350-fast-export.sh | 48 ++++++++++++++++--
8 files changed, 229 insertions(+), 27 deletions(-)
create mode 100755 t/t9306-fast-import-signed-tags.sh
--
2.51.0.438.g6987fc0bae
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v2 1/5] doc: git-tag: stop focusing on GPG signed tags
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
@ 2025-10-09 12:24 ` Christian Couder
2025-10-10 1:19 ` Junio C Hamano
2025-10-09 12:24 ` [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first Christian Couder
` (4 subsequent siblings)
5 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:24 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
It looks like the documentation of `git tag` is focused a bit too
much on GPG signed tags.
This starts with the "NAME" section where the command is described
with:
"Create, list, delete or verify a tag object signed with GPG"
while for example `git branch` is described with simply:
"List, create, or delete branches"
This could give the false impression that `git tag` only works with
tag objects, not with lightweight tags, and that tag objects are
always GPG signed.
In the "DESCRIPTION" section, it looks like only "GnuPG signed tag
objects" can be created by the `-s` and `-u <key-id>` options, and it
seems `gpg.program` can only specify a "custom GnuPG binary".
This goes on in the "OPTIONS" section too, especially about the `-s`
and `-u <key-id>` options.
The "CONFIGURATION" section also doesn't talk about how to configure
the command to work with X.509 and SSH signatures.
Let's rework all that to make sure users have a more accurate and
balanced view of what the command can do.
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-tag.adoc | 48 ++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-tag.adoc b/Documentation/git-tag.adoc
index a4b1c0ec05..3519e5b9b2 100644
--- a/Documentation/git-tag.adoc
+++ b/Documentation/git-tag.adoc
@@ -3,7 +3,7 @@ git-tag(1)
NAME
----
-git-tag - Create, list, delete or verify a tag object signed with GPG
+git-tag - Create, list, delete or verify tags
SYNOPSIS
@@ -38,15 +38,17 @@ and `-a`, `-s`, and `-u <key-id>` are absent, `-a` is implied.
Otherwise, a tag reference that points directly at the given object
(i.e., a lightweight tag) is created.
-A GnuPG signed tag object will be created when `-s` or `-u
-<key-id>` is used. When `-u <key-id>` is not used, the
-committer identity for the current user is used to find the
-GnuPG key for signing. The configuration variable `gpg.program`
-is used to specify custom GnuPG binary.
+A cryptographically signed tag object will be created when `-s` or
+`-u <key-id>` is used. The signing backend (GPG, X.509, SSH, etc.) is
+controlled by the `gpg.format` configuration variable, defaulting to
+OpenPGP. When `-u <key-id>` is not used, the committer identity for
+the current user is used to find the key for signing. The
+configuration variable `gpg.program` is used to specify a custom
+signing binary.
Tag objects (created with `-a`, `-s`, or `-u`) are called "annotated"
tags; they contain a creation date, the tagger name and e-mail, a
-tagging message, and an optional GnuPG signature. Whereas a
+tagging message, and an optional cryptographic signature. Whereas a
"lightweight" tag is simply a name for an object (usually a commit
object).
@@ -64,10 +66,12 @@ OPTIONS
-s::
--sign::
- Make a GPG-signed tag, using the default e-mail address's key.
- The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
- configuration variable if it exists, or disabled otherwise.
- See linkgit:git-config[1].
+ Make a cryptographically signed tag, using the default signing
+ key. The signing backend used depends on the `gpg.format`
+ configuration variable. The default key is determined by the
+ backend. For GPG, it's based on the committer's email address,
+ while for SSH it may be a specific key file or agent
+ identity. See linkgit:git-config[1].
--no-sign::
Override `tag.gpgSign` configuration variable that is
@@ -75,7 +79,10 @@ OPTIONS
-u <key-id>::
--local-user=<key-id>::
- Make a GPG-signed tag, using the given key.
+ Make a cryptographically signed tag using the given key. The
+ format of the <key-id> and the backend used depend on the
+ `gpg.format` configuration variable. See
+ linkgit:git-config[1].
-f::
--force::
@@ -87,7 +94,7 @@ OPTIONS
-v::
--verify::
- Verify the GPG signature of the given tag names.
+ Verify the signature of the given tag names.
-n<num>::
<num> specifies how many lines from the annotation, if any,
@@ -236,12 +243,23 @@ it in the repository configuration as follows:
-------------------------------------
[user]
- signingKey = <gpg-key-id>
+ signingKey = <key-id>
-------------------------------------
+The signing backend can be chosen via the `gpg.format` configuration
+variable, which defaults to `openpgp`. See linkgit:git-config[1]
+for a list of other supported formats.
+
+The path to the program used for each signing backend can be specified
+with the `gpg.<format>.program` configuration variable. For the
+`openpgp` backend, `gpg.program` can be used as a synonym for
+`gpg.openpgp.program`. See linkgit:git-config[1] for details.
+
`pager.tag` is only respected when listing tags, i.e., when `-l` is
used or implied. The default is to use a pager.
-See linkgit:git-config[1].
+
+See linkgit:git-config[1] for more details and other configuration
+variables.
DISCUSSION
----------
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
2025-10-09 12:24 ` [PATCH v2 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
@ 2025-10-09 12:24 ` Christian Couder
2025-10-10 6:49 ` Patrick Steinhardt
2025-10-09 12:24 ` [PATCH v2 3/5] t9350: properly count annotated tags Christian Couder
` (3 subsequent siblings)
5 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:24 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
When the 'GPG' prereq is lazily tested, `mkdir "$GNUPGHOME"` could
fail if the "$GNUPGHOME" directory already exists. This can happen if
the 'GPGSM' or the 'GPGSSH' prereq has been lazily tested before as they
already create "$GNUPGHOME".
To allow the GPGSM or the GPGSSH prereq to appear before the GPG prereq
in some test scripts, let's refactor the creation and setup of the
"$GNUPGHOME"` directory in a new prepare_gnupghome() function that uses
`mkdir -p "$GNUPGHOME"`.
This will be useful in a following commit.
Unfortunately the new prepare_gnupghome() function cannot be used when
lazily testing the GPG2 prereq, because that would expose existing,
hidden bugs in "t1016-compatObjectFormat.sh", so let's just document
that with a NEEDSWORK comment.
Helped-by: Todd Zullinger <tmz@pobox.com>
Helped-by: Collin Funk <collin.funk1@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/lib-gpg.sh | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 937b876bd0..b99ae39a06 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -9,6 +9,16 @@
GNUPGHOME="$(pwd)/gpghome"
export GNUPGHOME
+# All the "test_lazy_prereq GPG*" below should use
+# `prepare_gnupghome()` either directly or through a call to
+# `test_have_prereq GPG*`. That's because `gpg` and `gpgsm`
+# only create the directory specified using "$GNUPGHOME" or
+# `--homedir` if it's the default (usually "~/.gnupg").
+prepare_gnupghome() {
+ mkdir -p "$GNUPGHOME" &&
+ chmod 0700 "$GNUPGHOME"
+}
+
test_lazy_prereq GPG '
gpg_version=$(gpg --version 2>&1)
test $? != 127 || exit 1
@@ -38,8 +48,7 @@ test_lazy_prereq GPG '
# To export ownertrust:
# gpg --homedir /tmp/gpghome --export-ownertrust \
# > lib-gpg/ownertrust
- mkdir "$GNUPGHOME" &&
- chmod 0700 "$GNUPGHOME" &&
+ prepare_gnupghome &&
(gpgconf --kill all || : ) &&
gpg --homedir "${GNUPGHOME}" --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
@@ -63,6 +72,14 @@ test_lazy_prereq GPG2 '
;;
*)
(gpgconf --kill all || : ) &&
+
+ # NEEDSWORK: prepare_gnupghome() should definitely be
+ # called here, but it looks like it exposes a
+ # pre-existing, hidden bug by allowing some tests in
+ # t1016-compatObjectFormat.sh to run instead of being
+ # skipped. See:
+ # https://lore.kernel.org/git/ZoV8b2RvYxLOotSJ@teonanacatl.net/
+
gpg --homedir "${GNUPGHOME}" --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" --import-ownertrust \
@@ -132,8 +149,7 @@ test_lazy_prereq GPGSSH '
test $? = 0 || exit 1;
# Setup some keys and an allowed signers file
- mkdir -p "${GNUPGHOME}" &&
- chmod 0700 "${GNUPGHOME}" &&
+ prepare_gnupghome &&
(setfacl -k "${GNUPGHOME}" 2>/dev/null || true) &&
ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_PRIMARY}" >/dev/null &&
ssh-keygen -t rsa -b 2048 -N "" -C "git rsa2048 key" -f "${GPGSSH_KEY_SECONDARY}" >/dev/null &&
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 3/5] t9350: properly count annotated tags
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
2025-10-09 12:24 ` [PATCH v2 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
2025-10-09 12:24 ` [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first Christian Couder
@ 2025-10-09 12:24 ` Christian Couder
2025-10-09 12:24 ` [PATCH v2 4/5] fast-export: handle all kinds of tag signatures Christian Couder
` (2 subsequent siblings)
5 siblings, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:24 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
In "t9350-fast-export.sh", these existing tests:
- 'fast-export | fast-import when main is tagged'
- 'cope with tagger-less tags'
are checking the number of annotated tags in the test repo by comparing
it with some hardcoded values.
This could be an issue if some new tests that have some prerequisites
add new annotated tags to the repo before these existing tests. When
the prerequisites would be satisfied, the number of annotated tags
would be different from when some prerequisites would not be satisfied.
As we are going to add new tests that add new annotated tags in a
following commit, let's properly count the number of annotated tag in
the repo by incrementing a counter each time a new annotated tag is
added, and then by comparing the number of annotated tags to the value
of the counter when checking the number of annotated tags.
This is a bit ugly, but it makes it explicit that some tests are
interdependent. Alternative solutions, like moving the new tests to
the end of the script, were considered, but were rejected because they
would instead hide the technical debt and could confuse developers in
the future.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t9350-fast-export.sh | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 8f85c69d62..21ff26939c 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -35,6 +35,7 @@ test_expect_success 'setup' '
git commit -m sitzt file2 &&
test_tick &&
git tag -a -m valentin muss &&
+ ANNOTATED_TAG_COUNT=1 &&
git merge -s ours main
'
@@ -229,7 +230,8 @@ EOF
test_expect_success 'set up faked signed tag' '
- git fast-import <signed-tag-import
+ git fast-import <signed-tag-import &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
'
@@ -491,8 +493,9 @@ test_expect_success 'fast-export -C -C | fast-import' '
test_expect_success 'fast-export | fast-import when main is tagged' '
git tag -m msg last &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1)) &&
git fast-export -C -C --signed-tags=strip --all > output &&
- test $(grep -c "^tag " output) = 3
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT
'
@@ -506,12 +509,13 @@ test_expect_success 'cope with tagger-less tags' '
TAG=$(git hash-object --literally -t tag -w tag-content) &&
git update-ref refs/tags/sonnenschein $TAG &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1)) &&
git fast-export -C -C --signed-tags=strip --all > output &&
- test $(grep -c "^tag " output) = 4 &&
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT &&
! grep "Unspecified Tagger" output &&
git fast-export -C -C --signed-tags=strip --all \
--fake-missing-tagger > output &&
- test $(grep -c "^tag " output) = 4 &&
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT &&
grep "Unspecified Tagger" output
'
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 4/5] fast-export: handle all kinds of tag signatures
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
` (2 preceding siblings ...)
2025-10-09 12:24 ` [PATCH v2 3/5] t9350: properly count annotated tags Christian Couder
@ 2025-10-09 12:24 ` Christian Couder
2025-10-09 12:24 ` [PATCH v2 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
2025-10-09 21:35 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Junio C Hamano
5 siblings, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:24 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
Currently the handle_tag() function in "builtin/fast-export.c" searches
only for "\n-----BEGIN PGP SIGNATURE-----\n" in the tag message to find
a tag signature.
This doesn't handle all kinds of OpenPGP signatures as some can start
with "-----BEGIN PGP MESSAGE-----" too, and this doesn't handle SSH and
X.509 signatures either as they use "-----BEGIN SSH SIGNATURE-----" and
"-----BEGIN SIGNED MESSAGE-----" respectively.
To handle all these kinds of tag signatures supported by Git, let's use
the parse_signed_buffer() function to properly find signatures in tag
messages.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/fast-export.c | 7 +++----
t/t9350-fast-export.sh | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index dc2486f9a8..7adbc55f0d 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -931,9 +931,8 @@ static void handle_tag(const char *name, struct tag *tag)
/* handle signed tags */
if (message) {
- const char *signature = strstr(message,
- "\n-----BEGIN PGP SIGNATURE-----\n");
- if (signature)
+ size_t sig_offset = parse_signed_buffer(message, message_size);
+ if (sig_offset < message_size)
switch (signed_tag_mode) {
case SIGN_ABORT:
die("encountered signed tag %s; use "
@@ -950,7 +949,7 @@ static void handle_tag(const char *name, struct tag *tag)
oid_to_hex(&tag->object.oid));
/* fallthru */
case SIGN_STRIP:
- message_size = signature + 1 - message;
+ message_size = sig_offset;
break;
}
}
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 21ff26939c..3d153a4805 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -279,6 +279,42 @@ test_expect_success 'signed-tags=warn-strip' '
test -s err
'
+test_expect_success GPGSM 'setup X.509 signed tag' '
+ test_config gpg.format x509 &&
+ test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+
+ git tag -s -m "X.509 signed tag" x509-signed $(git rev-parse HEAD) &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
+'
+
+test_expect_success GPGSM 'signed-tags=verbatim with X.509' '
+ git fast-export --signed-tags=verbatim x509-signed > output &&
+ test_grep "SIGNED MESSAGE" output
+'
+
+test_expect_success GPGSM 'signed-tags=strip with X.509' '
+ git fast-export --signed-tags=strip x509-signed > output &&
+ test_grep ! "SIGNED MESSAGE" output
+'
+
+test_expect_success GPGSSH 'setup SSH signed tag' '
+ test_config gpg.format ssh &&
+ test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+
+ git tag -s -m "SSH signed tag" ssh-signed $(git rev-parse HEAD) &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
+'
+
+test_expect_success GPGSSH 'signed-tags=verbatim with SSH' '
+ git fast-export --signed-tags=verbatim ssh-signed > output &&
+ test_grep "SSH SIGNATURE" output
+'
+
+test_expect_success GPGSSH 'signed-tags=strip with SSH' '
+ git fast-export --signed-tags=strip ssh-signed > output &&
+ test_grep ! "SSH SIGNATURE" output
+'
+
test_expect_success GPG 'set up signed commit' '
# Generate a commit with both "gpgsig" and "encoding" set, so
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
` (3 preceding siblings ...)
2025-10-09 12:24 ` [PATCH v2 4/5] fast-export: handle all kinds of tag signatures Christian Couder
@ 2025-10-09 12:24 ` Christian Couder
2025-10-09 21:35 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Junio C Hamano
5 siblings, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:24 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
Recently, eaaddf5791 (fast-import: add '--signed-commits=<mode>'
option, 2025-09-17) added support for controlling how signed commits
are handled by `git fast-import`, but there is no option yet to
decide about signed tags.
To remediate that, let's add a '--signed-tags=<mode>' option to
`git fast-import` too.
With this, both `git fast-export` and `git fast-import` have both
a '--signed-tags=<mode>' and a '--signed-commits=<mode>' supporting
the same <mode>s.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-import.adoc | 5 ++
builtin/fast-import.c | 43 ++++++++++++++++
t/meson.build | 1 +
t/t9306-fast-import-signed-tags.sh | 80 ++++++++++++++++++++++++++++++
4 files changed, 129 insertions(+)
create mode 100755 t/t9306-fast-import-signed-tags.sh
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 85ed7a7270..b74179a6c8 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -66,6 +66,11 @@ fast-import stream! This option is enabled automatically for
remote-helpers that use the `import` capability, as they are
already trusted to run their own code.
+--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
+ Specify how to handle signed tags. Behaves in the same way
+ as the same option in linkgit:git-fast-export[1], except that
+ default is 'verbatim' (instead of 'abort').
+
--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed commits. Behaves in the same way
as the same option in linkgit:git-fast-export[1], except that
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 2010e78475..60d6faa465 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -188,6 +188,7 @@ static int global_argc;
static const char **global_argv;
static const char *global_prefix;
+static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
/* Memory pools */
@@ -2961,6 +2962,43 @@ static void parse_new_commit(const char *arg)
b->last_commit = object_count_by_type[OBJ_COMMIT];
}
+static void handle_tag_signature(struct strbuf *msg, const char *name)
+{
+ size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
+
+ /* If there is no signature, there is nothing to do. */
+ if (sig_offset >= msg->len)
+ return;
+
+ switch (signed_tag_mode) {
+
+ /* First, modes that don't change anything */
+ case SIGN_ABORT:
+ die(_("encountered signed tag; use "
+ "--signed-tags=<mode> to handle it"));
+ case SIGN_WARN_VERBATIM:
+ warning(_("importing a tag signature verbatim for tag '%s'"), name);
+ /* fallthru */
+ case SIGN_VERBATIM:
+ /* Nothing to do, the signature will be put into the imported tag. */
+ break;
+
+ /* Second, modes that remove the signature */
+ case SIGN_WARN_STRIP:
+ warning(_("stripping a tag signature for tag '%s'"), name);
+ /* fallthru */
+ case SIGN_STRIP:
+ /* Truncate the buffer to remove the signature */
+ strbuf_setlen(msg, sig_offset);
+ break;
+
+ /* Third, BUG */
+ default:
+ BUG("invalid signed_tag_mode value %d from tag '%s'",
+ signed_tag_mode, name);
+ }
+}
+
static void parse_new_tag(const char *arg)
{
static struct strbuf msg = STRBUF_INIT;
@@ -3024,6 +3062,8 @@ static void parse_new_tag(const char *arg)
/* tag payload/message */
parse_data(&msg, 0, NULL);
+ handle_tag_signature(&msg, t->name);
+
/* build the tag object */
strbuf_reset(&new_data);
@@ -3544,6 +3584,9 @@ static int parse_one_option(const char *option)
} else if (skip_prefix(option, "signed-commits=", &option)) {
if (parse_sign_mode(option, &signed_commit_mode))
usagef(_("unknown --signed-commits mode '%s'"), option);
+ } else if (skip_prefix(option, "signed-tags=", &option)) {
+ if (parse_sign_mode(option, &signed_tag_mode))
+ usagef(_("unknown --signed-tags mode '%s'"), option);
} else if (!strcmp(option, "quiet")) {
show_stats = 0;
quiet = 1;
diff --git a/t/meson.build b/t/meson.build
index 11376b9e25..cb8c2b4b30 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1036,6 +1036,7 @@ integration_tests = [
't9303-fast-import-compression.sh',
't9304-fast-import-marks.sh',
't9305-fast-import-signatures.sh',
+ 't9306-fast-import-signed-tags.sh',
't9350-fast-export.sh',
't9351-fast-export-anonymize.sh',
't9400-git-cvsserver-server.sh',
diff --git a/t/t9306-fast-import-signed-tags.sh b/t/t9306-fast-import-signed-tags.sh
new file mode 100755
index 0000000000..363619e7d1
--- /dev/null
+++ b/t/t9306-fast-import-signed-tags.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+
+test_description='git fast-import --signed-tags=<mode>'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success 'set up unsigned initial commit and import repo' '
+ test_commit first &&
+ git init new
+'
+
+test_expect_success 'import no signed tag with --signed-tags=abort' '
+ git fast-export --signed-tags=verbatim >output &&
+ git -C new fast-import --quiet --signed-tags=abort <output
+'
+
+test_expect_success GPG 'set up OpenPGP signed tag' '
+ git tag -s -m "OpenPGP signed tag" openpgp-signed first &&
+ OPENPGP_SIGNED=$(git rev-parse --verify refs/tags/openpgp-signed) &&
+ git fast-export --signed-tags=verbatim openpgp-signed >output
+'
+
+test_expect_success GPG 'import OpenPGP signed tag with --signed-tags=abort' '
+ test_must_fail git -C new fast-import --quiet --signed-tags=abort <output
+'
+
+test_expect_success GPG 'import OpenPGP signed tag with --signed-tags=verbatim' '
+ git -C new fast-import --quiet --signed-tags=verbatim <output >log 2>&1 &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/openpgp-signed) &&
+ test $OPENPGP_SIGNED = $IMPORTED &&
+ test_must_be_empty log
+'
+
+test_expect_success GPGSM 'setup X.509 signed tag' '
+ test_config gpg.format x509 &&
+ test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+
+ git tag -s -m "X.509 signed tag" x509-signed first &&
+ X509_SIGNED=$(git rev-parse --verify refs/tags/x509-signed) &&
+ git fast-export --signed-tags=verbatim x509-signed >output
+'
+
+test_expect_success GPGSM 'import X.509 signed tag with --signed-tags=warn-strip' '
+ git -C new fast-import --quiet --signed-tags=warn-strip <output >log 2>&1 &&
+ test_grep "stripping a tag signature for tag '\''x509-signed'\''" log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/x509-signed) &&
+ test $X509_SIGNED != $IMPORTED &&
+ git -C new cat-file -p x509-signed >out &&
+ test_grep ! "SIGNED MESSAGE" out
+'
+
+test_expect_success GPGSSH 'setup SSH signed tag' '
+ test_config gpg.format ssh &&
+ test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+
+ git tag -s -m "SSH signed tag" ssh-signed first &&
+ SSH_SIGNED=$(git rev-parse --verify refs/tags/ssh-signed) &&
+ git fast-export --signed-tags=verbatim ssh-signed >output
+'
+
+test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=warn-verbatim' '
+ git -C new fast-import --quiet --signed-tags=warn-verbatim <output >log 2>&1 &&
+ test_grep "importing a tag signature verbatim for tag '\''ssh-signed'\''" log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/ssh-signed) &&
+ test $SSH_SIGNED = $IMPORTED
+'
+
+test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=strip' '
+ git -C new fast-import --quiet --signed-tags=strip <output >log 2>&1 &&
+ test_must_be_empty log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/ssh-signed) &&
+ test $SSH_SIGNED != $IMPORTED &&
+ git -C new cat-file -p ssh-signed >out &&
+ test_grep ! "SSH SIGNATURE" out
+'
+
+test_done
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first
2025-10-09 2:37 ` Todd Zullinger
@ 2025-10-09 12:29 ` Christian Couder
2025-10-09 18:18 ` Junio C Hamano
1 sibling, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:29 UTC (permalink / raw)
To: Todd Zullinger
Cc: Collin Funk, Patrick Steinhardt, git, Junio C Hamano,
Elijah Newren, Jeff King, brian m . carlson, Johannes Schindelin,
Christian Couder
On Thu, Oct 9, 2025 at 4:37 AM Todd Zullinger <tmz@pobox.com> wrote:
> I sent a series long ago to fix this issueš, but it wasn't
> picked up.
>
> Fixing the issue exposes broken tests which use the gpg2
> prereq. That breakage turns up in our CI and other build
> environments, like Fedora's, but I was never able to
> reliably trigger it locally and track down what was broken
> about those test.
>
> I believe I asked about it again a few months later and it
> did not gain any attention.
>
> I simply apply the patches locally and then disable those
> tests -- tests which don't run reliably are not worth
> running IMO. :)
>
> š <20240703153738.916469-1-tmz@pobox.com>
Thanks for mentioning it and sorry that your series didn't get more attention.
I have just sent a v2 of my series with a patch based on your first
patch. About the GPG2 prereq I added a NEEDSWORK comment but didn't
change the actual code to not trigger the test failures.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first
2025-10-09 1:29 ` Collin Funk
2025-10-09 2:37 ` Todd Zullinger
@ 2025-10-09 12:30 ` Christian Couder
1 sibling, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:30 UTC (permalink / raw)
To: Collin Funk
Cc: Patrick Steinhardt, git, Junio C Hamano, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Christian Couder
On Thu, Oct 9, 2025 at 3:29 AM Collin Funk <collin.funk1@gmail.com> wrote:
> > I am not sure. It might depend on the gpg version. Or maybe gpgsm
> > does it but not gpg. I will check.
>
> If $GNUPGHOME or --homedir is the default (usually ~/.gnupg) gets
> created by 'gpg' and 'gpgsm':
>
> $ ls ~/.gnupg
> ls: cannot access '/root/.gnupg': No such file or directory
> $ gpgsm
> gpgsm: directory '/root/.gnupg' created
> gpgsm: invalid command (there is no implicit command)
> $ rm -rf ~/.gnupg && gpg
> gpg: directory '/root/.gnupg' created
> [...]
>
> If it is not the default then it will not be created:
>
> $ GNUPGHOME=$HOME/test gpgsm
> gpgsm: keyblock resource '/root/test/pubring.kbx': No such file or directory
> $ GNUPGHOME=$HOME/test gpg
> gpg: keyblock resource '/root/test/pubring.kbx': No such file or directory
Thanks for this useful information. I have added a code comment to
explain this in the v2 series I just sent.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 4/5] fast-export: handle all kinds of tag signatures
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 10:02 ` Christian Couder
@ 2025-10-09 12:33 ` Christian Couder
1 sibling, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-09 12:33 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Christian Couder
On Wed, Oct 8, 2025 at 11:21 AM Patrick Steinhardt <ps@pks.im> wrote:
> > diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> > index 21ff26939c..5a46608f65 100755
> > --- a/t/t9350-fast-export.sh
> > +++ b/t/t9350-fast-export.sh
> > @@ -279,6 +279,54 @@ test_expect_success 'signed-tags=warn-strip' '
> > test -s err
> > '
> >
> > +test_expect_success GPGSM 'setup X.509 signed tag' '
> > +
> > + test_config gpg.format x509 &&
> > + test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> > +
> > + git tag -s -m "X.509 signed tag" x509-signed $(git rev-parse HEAD) &&
> > + ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
> > +
> > +'
>
> Nit for this test and all of the below ones: our modern style does not
> have empty lines at the beginning and end of a test case.
Thanks. I think I have addressed all your comments like this one in
the v2 I just sent.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first
2025-10-09 2:37 ` Todd Zullinger
2025-10-09 12:29 ` Christian Couder
@ 2025-10-09 18:18 ` Junio C Hamano
1 sibling, 0 replies; 52+ messages in thread
From: Junio C Hamano @ 2025-10-09 18:18 UTC (permalink / raw)
To: Todd Zullinger
Cc: Collin Funk, Christian Couder, Patrick Steinhardt, git,
Elijah Newren, Jeff King, brian m . carlson, Johannes Schindelin,
Christian Couder
Todd Zullinger <tmz@pobox.com> writes:
> I sent a series long ago to fix this issue¹, but it wasn't
> picked up.
>
> Fixing the issue exposes broken tests which use the gpg2
> prereq. That breakage turns up in our CI and other build
> environments, like Fedora's, but I was never able to
> reliably trigger it locally and track down what was broken
> about those test.
>
> I believe I asked about it again a few months later and it
> did not gain any attention.
>
> I simply apply the patches locally and then disable those
> tests -- tests which don't run reliably are not worth
> running IMO. :)
>
> ¹ <20240703153738.916469-1-tmz@pobox.com>
True, the archive shows that the two-patch series got no attention
from anybody, it seems. Perhaps nobody was looking at the list at
around the beginning of July last year?
Let me pick it up belatedly, but I'd appreciate an extra sets or two
of eyes while the issue is fresh in our minds.
Thanks.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
` (4 preceding siblings ...)
2025-10-09 12:24 ` [PATCH v2 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
@ 2025-10-09 21:35 ` Junio C Hamano
5 siblings, 0 replies; 52+ messages in thread
From: Junio C Hamano @ 2025-10-09 21:35 UTC (permalink / raw)
To: Christian Couder
Cc: git, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk
Christian Couder <christian.couder@gmail.com> writes:
> Changes since v1
> ----------------
>
> Thanks to Patrick Steinhardt, Todd Zullinger and Collin Funk who
> reviewed or commented on the v1.
>
> - In patch 1/5, in the commit message:
>
> - "focussing" and "focussed" have been replaced with "focusing" and
> "focused" respectively as the former is UK Eglish while the latter
> is US English,
>
> - the missing "section" word has been added.
>
> - In patch 1/5, in the git-tag doc:
>
> - "cryptographic" or "cryptographically" has been added to number of
> places,
>
> - the changes to the CONFIGURATION section have been shortened by
> not mentioning the supported signing backend (X.509 and SSH) other
> than OpenPGP, and by referring to git-config(1) more.
>
> - In patch 2/5, the approach taken is now the one previously used by
> Todd Zullinger in:
>
> https://lore.kernel.org/git/20240703153738.916469-2-tmz@pobox.com/
>
> so this patch looks like a completely different patch in the range
> diff.
>
> - In patch 3/5, in the commit message:
>
> - t9350-fast-export.sh has been quoted,
>
> - some explanations about alternative solutions that have been
> considered have been added.
>
> - In patch 4/5, the added tests are now written in a modern style,
> instead of the old style used elsewhere in the script.
>
> - In patch 5/5, a die() message has been marked for translation and
> some "/* fallthru */" comments have been properly indented.
Looking good. Thanks, will queue.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 1/5] doc: git-tag: stop focusing on GPG signed tags
2025-10-09 12:24 ` [PATCH v2 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
@ 2025-10-10 1:19 ` Junio C Hamano
2025-10-10 7:06 ` Christian Couder
0 siblings, 1 reply; 52+ messages in thread
From: Junio C Hamano @ 2025-10-10 1:19 UTC (permalink / raw)
To: Christian Couder
Cc: git, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
Christian Couder <christian.couder@gmail.com> writes:
> -s::
> --sign::
> - Make a GPG-signed tag, using the default e-mail address's key.
> ...
> + Make a cryptographically signed tag, using the default signing
> ...
> -u <key-id>::
> --local-user=<key-id>::
> - Make a GPG-signed tag, using the given key.
> + Make a cryptographically signed tag using the given key. The
Given the above ...
> -v::
> --verify::
> - Verify the GPG signature of the given tag names.
> + Verify the signature of the given tag names.
... it would be more consistent to say it with "cryptographic"
somewhere. Also what we verify are "tags", not their names.
So, something like
Verify the cryptographic signature of the given tags.
perhaps?
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first
2025-10-09 12:24 ` [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first Christian Couder
@ 2025-10-10 6:49 ` Patrick Steinhardt
2025-10-10 14:09 ` Todd Zullinger
0 siblings, 1 reply; 52+ messages in thread
From: Patrick Steinhardt @ 2025-10-10 6:49 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Elijah Newren, Jeff King, brian m . carlson,
Johannes Schindelin, Todd Zullinger, Collin Funk,
Christian Couder
On Thu, Oct 09, 2025 at 02:24:54PM +0200, Christian Couder wrote:
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index 937b876bd0..b99ae39a06 100644
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -9,6 +9,16 @@
> @@ -63,6 +72,14 @@ test_lazy_prereq GPG2 '
> ;;
> *)
> (gpgconf --kill all || : ) &&
> +
> + # NEEDSWORK: prepare_gnupghome() should definitely be
> + # called here, but it looks like it exposes a
> + # pre-existing, hidden bug by allowing some tests in
> + # t1016-compatObjectFormat.sh to run instead of being
> + # skipped. See:
> + # https://lore.kernel.org/git/ZoV8b2RvYxLOotSJ@teonanacatl.net/
> +
> gpg --homedir "${GNUPGHOME}" --import \
> "$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
> gpg --homedir "${GNUPGHOME}" --import-ownertrust \
Interesting. So I assume that these gpg commands here fail because the
GPG home doesn't exist, and thus we disable the prereq? Too bad, but I
agree that this doesn't necessarily have to be fixed by this patch
series.
The remaining patches look good to me and address my feedback, thanks!
Patrick
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 1/5] doc: git-tag: stop focusing on GPG signed tags
2025-10-10 1:19 ` Junio C Hamano
@ 2025-10-10 7:06 ` Christian Couder
0 siblings, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-10 7:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
On Fri, Oct 10, 2025 at 3:19 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > -s::
> > --sign::
> > - Make a GPG-signed tag, using the default e-mail address's key.
> > ...
> > + Make a cryptographically signed tag, using the default signing
> > ...
> > -u <key-id>::
> > --local-user=<key-id>::
> > - Make a GPG-signed tag, using the given key.
> > + Make a cryptographically signed tag using the given key. The
>
> Given the above ...
>
> > -v::
> > --verify::
> > - Verify the GPG signature of the given tag names.
> > + Verify the signature of the given tag names.
>
> ... it would be more consistent to say it with "cryptographic"
> somewhere. Also what we verify are "tags", not their names.
> So, something like
>
> Verify the cryptographic signature of the given tags.
>
> perhaps?
Yeah, it looks better. I have changed this in my current version, and
will send it in a v3 in a few days.
Thanks.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first
2025-10-10 6:49 ` Patrick Steinhardt
@ 2025-10-10 14:09 ` Todd Zullinger
2025-10-10 16:22 ` Junio C Hamano
0 siblings, 1 reply; 52+ messages in thread
From: Todd Zullinger @ 2025-10-10 14:09 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Christian Couder, git, Junio C Hamano, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Collin Funk,
Christian Couder
Patrick Steinhardt wrote:
> On Thu, Oct 09, 2025 at 02:24:54PM +0200, Christian Couder wrote:
>> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
>> index 937b876bd0..b99ae39a06 100644
>> --- a/t/lib-gpg.sh
>> +++ b/t/lib-gpg.sh
>> @@ -9,6 +9,16 @@
>> @@ -63,6 +72,14 @@ test_lazy_prereq GPG2 '
>> ;;
>> *)
>> (gpgconf --kill all || : ) &&
>> +
>> + # NEEDSWORK: prepare_gnupghome() should definitely be
>> + # called here, but it looks like it exposes a
>> + # pre-existing, hidden bug by allowing some tests in
>> + # t1016-compatObjectFormat.sh to run instead of being
>> + # skipped. See:
>> + # https://lore.kernel.org/git/ZoV8b2RvYxLOotSJ@teonanacatl.net/
>> +
>> gpg --homedir "${GNUPGHOME}" --import \
>> "$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
>> gpg --homedir "${GNUPGHOME}" --import-ownertrust \
>
> Interesting. So I assume that these gpg commands here fail because the
> GPG home doesn't exist, and thus we disable the prereq? Too bad, but I
> agree that this doesn't necessarily have to be fixed by this patch
> series.
I agree. But it is ugly that any tests we have which rely
on the GPG2 prereq simply never run. That should be fixed
and, if it were me, I'd do so by dropping the flaky tests in
t1016 initially. Someone who cares about those tests
running could debug it more and hopefully fix the problem.
As it stands, this breakage blocks tests in t1461-refs-list,
t6300-for-each-ref, and t7510-signed-commit. Anyone adding
a test with a GPG2 prereq should be aware that thoses tests
just won't be run.
The t1016-compatObjectFormat tests have been flaky since
they were added and no one really noticed. That's at least
partly a failure of our CI output, which hides these sort of
skipped tests that we just presume are running. I don't
have any good suggestions for fixing that, unfortunately.
--
Todd
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first
2025-10-10 14:09 ` Todd Zullinger
@ 2025-10-10 16:22 ` Junio C Hamano
2025-10-11 2:14 ` Todd Zullinger
0 siblings, 1 reply; 52+ messages in thread
From: Junio C Hamano @ 2025-10-10 16:22 UTC (permalink / raw)
To: Todd Zullinger
Cc: Patrick Steinhardt, Christian Couder, git, Elijah Newren,
Jeff King, brian m . carlson, Johannes Schindelin, Collin Funk,
Christian Couder
Todd Zullinger <tmz@pobox.com> writes:
> I agree. But it is ugly that any tests we have which rely
> on the GPG2 prereq simply never run. That should be fixed
> and, if it were me, I'd do so by dropping the flaky tests in
> t1016 initially. Someone who cares about those tests
> running could debug it more and hopefully fix the problem.
Let me queue your two patches as-is to leave the tip of 'seen'
broken for a few days to see if anybody bites ;-). After that, we
may do "s|test_expect_success|test_expect_failure|" on those tests
that you call "flaky". Are they flaky in the sense that they
sometimes pass sometimes fail depending on the timing, or just
simply buggy and always fail?
> The t1016-compatObjectFormat tests have been flaky since
> they were added and no one really noticed. That's at least
> partly a failure of our CI output, which hides these sort of
> skipped tests that we just presume are running. I don't
> have any good suggestions for fixing that, unfortunately.
Thanks.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first
2025-10-10 16:22 ` Junio C Hamano
@ 2025-10-11 2:14 ` Todd Zullinger
2025-10-12 0:15 ` Junio C Hamano
0 siblings, 1 reply; 52+ messages in thread
From: Todd Zullinger @ 2025-10-11 2:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Patrick Steinhardt, Christian Couder, git, Elijah Newren,
Jeff King, brian m . carlson, Johannes Schindelin, Collin Funk,
Christian Couder
Junio C Hamano wrote:
> Todd Zullinger <tmz@pobox.com> writes:
>
>> I agree. But it is ugly that any tests we have which rely
>> on the GPG2 prereq simply never run. That should be fixed
>> and, if it were me, I'd do so by dropping the flaky tests in
>> t1016 initially. Someone who cares about those tests
>> running could debug it more and hopefully fix the problem.
>
> Let me queue your two patches as-is to leave the tip of 'seen'
> broken for a few days to see if anybody bites ;-). After that, we
> may do "s|test_expect_success|test_expect_failure|" on those tests
> that you call "flaky". Are they flaky in the sense that they
> sometimes pass sometimes fail depending on the timing, or just
> simply buggy and always fail?
In my recollection, they fail all (or nearly all?) of the
time in our CI runs and when I was building git for Fedora
infrastructure, they failed consistently on the Fedora
builders as well.
They fail rarely (if ever) when I run them locally, even
with --stress options. That made it rather difficult to
work out the issue. I thought that it was a timing problem
for a while, but I wasn't able to find a way to demonstrate
that.
Thanks for the willingness to suffer some test breakage to
see if it can flush out a fix. :)
I suspect there are folks here who know the test suite and
code being tested well enough that it may be really obvious
to them. Whether there is an intersection of those folks
and spare "round tuits" is another matter.
--
Todd
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first
2025-10-11 2:14 ` Todd Zullinger
@ 2025-10-12 0:15 ` Junio C Hamano
0 siblings, 0 replies; 52+ messages in thread
From: Junio C Hamano @ 2025-10-12 0:15 UTC (permalink / raw)
To: Todd Zullinger
Cc: Patrick Steinhardt, Christian Couder, git, Elijah Newren,
Jeff King, brian m . carlson, Johannes Schindelin, Collin Funk,
Christian Couder
Todd Zullinger <tmz@pobox.com> writes:
> In my recollection, they fail all (or nearly all?) of the
> time in our CI runs and when I was building git for Fedora
> infrastructure, they failed consistently on the Fedora
> builders as well.
>
> They fail rarely (if ever) when I run them locally, even
> with --stress options. That made it rather difficult to
> work out the issue. I thought that it was a timing problem
> for a while, but I wasn't able to find a way to demonstrate
> that.
>
> Thanks for the willingness to suffer some test breakage to
> see if it can flush out a fix. :)
Or I can just revert these two patches if nothing happens ;-).
> I suspect there are folks here who know the test suite and
> code being tested well enough that it may be really obvious
> to them. Whether there is an intersection of those folks
> and spare "round tuits" is another matter.
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v3 0/5] fast-import: start controlling how tag signatures are handled
2025-10-07 12:29 [PATCH 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
` (5 preceding siblings ...)
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
@ 2025-10-13 8:48 ` Christian Couder
2025-10-13 8:48 ` [PATCH v3 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
` (5 more replies)
6 siblings, 6 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-13 8:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
Introduction
------------
Tools like `git-filter-repo` should be able to control how tag
signatures are handled when regenerating repository content after it
has been filtered. For this purpose, they need a way for `git
fast-import` to control how tag signatures are handled.
A previous series [1] added a '--signed-commits=<mode>' option to `git
fast-import` to control how commit signatures are handled, so this is
adding a similar '--signed-tags=<mode>' for tag signatures.
For now this new option behaves in a very similar way as the option
with the same name that already exists in `git fast-export`.
Especially it supports exactly the same <mode>s and the same aliases
for these modes. For example "ignore" is a synonym for "verbatim".
This way, both `git fast-export` and `git fast-import` have both a
'--signed-tags=<mode>' and a '--signed-commits=<mode>' supporting the
same <mode>s.
In the future I want to implement new <mode>s like "strip-if-invalid",
"re-sign", "re-sign-if-invalid" in `git fast-import` for both tag and
commit signatures. These might be a bit more complex, so for now I
prefer to start with the simple modes.
[1] https://lore.kernel.org/git/20250917181427.3193500-1-christian.couder@gmail.com/
Note about the different patches
--------------------------------
Patch 1/5 (doc: git-tag: stop focussing on GPG signed tags) is a
documentation update for `git tag`. It could go in a separate series
or be dropped altogether, but while working on this I thought that it
would be a good thing to do, as the doc is quite outdated.
Patches 2/5, 3/5 and 4/5 are preparatory patches for the main one
which is patch 5/5 (fast-import: add '--signed-tags=<mode>' option).
I wanted '--signed-tags=<mode>' to work for all kinds of signature in
tags (OpenPGP, X.509 and SSH) but soon realized that the
'--signed-tags=<mode>' option of `git fast-export` worked only for
OpenPGP signatures, so I fixed that issue in patch 4/5 (fast-export:
handle all kinds of tag signatures).
While working on the tests in patch 4/5, I found a few things to
improve that could belong to other patches so that's how I came up
with patches 2/5 and 3/5.
Changes since v2
----------------
Thanks to Patrick Steinhardt, Todd Zullinger and Collin Funk who
reviewed or commented on the v1 and v2.
There is a single change in the first patch (doc: git-tag: stop
focusing on GPG signed tags) where the description of the
`-v | --verify` option of `git tag` has been improved.
CI tests
--------
I haven't run CI tests because there is a single documentation change
since v2 that is very unlikely to make things break.
Range diff since v2
-------------------
1: eb65af631d ! 1: ac67d927ad doc: git-tag: stop focusing on GPG signed tags
@@ Documentation/git-tag.adoc: OPTIONS
-v::
--verify::
- Verify the GPG signature of the given tag names.
-+ Verify the signature of the given tag names.
++ Verify the cryptographic signature of the given tags.
-n<num>::
<num> specifies how many lines from the annotation, if any,
2: 640204ef26 = 2: f0208527ff lib-gpg: allow tests with GPGSM or GPGSSH prereq first
3: 8f788bafe1 = 3: e9e3d8c081 t9350: properly count annotated tags
4: d62a43905c = 4: 8d318a0046 fast-export: handle all kinds of tag signatures
5: 9094f37b46 = 5: 962ad96b4a fast-import: add '--signed-tags=<mode>' option
Christian Couder (5):
doc: git-tag: stop focusing on GPG signed tags
lib-gpg: allow tests with GPGSM or GPGSSH prereq first
t9350: properly count annotated tags
fast-export: handle all kinds of tag signatures
fast-import: add '--signed-tags=<mode>' option
Documentation/git-fast-import.adoc | 5 ++
Documentation/git-tag.adoc | 48 ++++++++++++------
builtin/fast-export.c | 7 ++-
builtin/fast-import.c | 43 ++++++++++++++++
t/lib-gpg.sh | 24 +++++++--
t/meson.build | 1 +
t/t9306-fast-import-signed-tags.sh | 80 ++++++++++++++++++++++++++++++
t/t9350-fast-export.sh | 48 ++++++++++++++++--
8 files changed, 229 insertions(+), 27 deletions(-)
create mode 100755 t/t9306-fast-import-signed-tags.sh
--
2.51.0.438.g6987fc0bae
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v3 1/5] doc: git-tag: stop focusing on GPG signed tags
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
@ 2025-10-13 8:48 ` Christian Couder
2025-10-24 2:03 ` Elijah Newren
2025-10-13 8:48 ` [PATCH v3 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first Christian Couder
` (4 subsequent siblings)
5 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-13 8:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
It looks like the documentation of `git tag` is focused a bit too
much on GPG signed tags.
This starts with the "NAME" section where the command is described
with:
"Create, list, delete or verify a tag object signed with GPG"
while for example `git branch` is described with simply:
"List, create, or delete branches"
This could give the false impression that `git tag` only works with
tag objects, not with lightweight tags, and that tag objects are
always GPG signed.
In the "DESCRIPTION" section, it looks like only "GnuPG signed tag
objects" can be created by the `-s` and `-u <key-id>` options, and it
seems `gpg.program` can only specify a "custom GnuPG binary".
This goes on in the "OPTIONS" section too, especially about the `-s`
and `-u <key-id>` options.
The "CONFIGURATION" section also doesn't talk about how to configure
the command to work with X.509 and SSH signatures.
Let's rework all that to make sure users have a more accurate and
balanced view of what the command can do.
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-tag.adoc | 48 ++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-tag.adoc b/Documentation/git-tag.adoc
index a4b1c0ec05..28d6fe4e1a 100644
--- a/Documentation/git-tag.adoc
+++ b/Documentation/git-tag.adoc
@@ -3,7 +3,7 @@ git-tag(1)
NAME
----
-git-tag - Create, list, delete or verify a tag object signed with GPG
+git-tag - Create, list, delete or verify tags
SYNOPSIS
@@ -38,15 +38,17 @@ and `-a`, `-s`, and `-u <key-id>` are absent, `-a` is implied.
Otherwise, a tag reference that points directly at the given object
(i.e., a lightweight tag) is created.
-A GnuPG signed tag object will be created when `-s` or `-u
-<key-id>` is used. When `-u <key-id>` is not used, the
-committer identity for the current user is used to find the
-GnuPG key for signing. The configuration variable `gpg.program`
-is used to specify custom GnuPG binary.
+A cryptographically signed tag object will be created when `-s` or
+`-u <key-id>` is used. The signing backend (GPG, X.509, SSH, etc.) is
+controlled by the `gpg.format` configuration variable, defaulting to
+OpenPGP. When `-u <key-id>` is not used, the committer identity for
+the current user is used to find the key for signing. The
+configuration variable `gpg.program` is used to specify a custom
+signing binary.
Tag objects (created with `-a`, `-s`, or `-u`) are called "annotated"
tags; they contain a creation date, the tagger name and e-mail, a
-tagging message, and an optional GnuPG signature. Whereas a
+tagging message, and an optional cryptographic signature. Whereas a
"lightweight" tag is simply a name for an object (usually a commit
object).
@@ -64,10 +66,12 @@ OPTIONS
-s::
--sign::
- Make a GPG-signed tag, using the default e-mail address's key.
- The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
- configuration variable if it exists, or disabled otherwise.
- See linkgit:git-config[1].
+ Make a cryptographically signed tag, using the default signing
+ key. The signing backend used depends on the `gpg.format`
+ configuration variable. The default key is determined by the
+ backend. For GPG, it's based on the committer's email address,
+ while for SSH it may be a specific key file or agent
+ identity. See linkgit:git-config[1].
--no-sign::
Override `tag.gpgSign` configuration variable that is
@@ -75,7 +79,10 @@ OPTIONS
-u <key-id>::
--local-user=<key-id>::
- Make a GPG-signed tag, using the given key.
+ Make a cryptographically signed tag using the given key. The
+ format of the <key-id> and the backend used depend on the
+ `gpg.format` configuration variable. See
+ linkgit:git-config[1].
-f::
--force::
@@ -87,7 +94,7 @@ OPTIONS
-v::
--verify::
- Verify the GPG signature of the given tag names.
+ Verify the cryptographic signature of the given tags.
-n<num>::
<num> specifies how many lines from the annotation, if any,
@@ -236,12 +243,23 @@ it in the repository configuration as follows:
-------------------------------------
[user]
- signingKey = <gpg-key-id>
+ signingKey = <key-id>
-------------------------------------
+The signing backend can be chosen via the `gpg.format` configuration
+variable, which defaults to `openpgp`. See linkgit:git-config[1]
+for a list of other supported formats.
+
+The path to the program used for each signing backend can be specified
+with the `gpg.<format>.program` configuration variable. For the
+`openpgp` backend, `gpg.program` can be used as a synonym for
+`gpg.openpgp.program`. See linkgit:git-config[1] for details.
+
`pager.tag` is only respected when listing tags, i.e., when `-l` is
used or implied. The default is to use a pager.
-See linkgit:git-config[1].
+
+See linkgit:git-config[1] for more details and other configuration
+variables.
DISCUSSION
----------
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v3 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
2025-10-13 8:48 ` [PATCH v3 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
@ 2025-10-13 8:48 ` Christian Couder
2025-10-13 8:48 ` [PATCH v3 3/5] t9350: properly count annotated tags Christian Couder
` (3 subsequent siblings)
5 siblings, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-13 8:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
When the 'GPG' prereq is lazily tested, `mkdir "$GNUPGHOME"` could
fail if the "$GNUPGHOME" directory already exists. This can happen if
the 'GPGSM' or the 'GPGSSH' prereq has been lazily tested before as they
already create "$GNUPGHOME".
To allow the GPGSM or the GPGSSH prereq to appear before the GPG prereq
in some test scripts, let's refactor the creation and setup of the
"$GNUPGHOME"` directory in a new prepare_gnupghome() function that uses
`mkdir -p "$GNUPGHOME"`.
This will be useful in a following commit.
Unfortunately the new prepare_gnupghome() function cannot be used when
lazily testing the GPG2 prereq, because that would expose existing,
hidden bugs in "t1016-compatObjectFormat.sh", so let's just document
that with a NEEDSWORK comment.
Helped-by: Todd Zullinger <tmz@pobox.com>
Helped-by: Collin Funk <collin.funk1@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/lib-gpg.sh | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 937b876bd0..b99ae39a06 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -9,6 +9,16 @@
GNUPGHOME="$(pwd)/gpghome"
export GNUPGHOME
+# All the "test_lazy_prereq GPG*" below should use
+# `prepare_gnupghome()` either directly or through a call to
+# `test_have_prereq GPG*`. That's because `gpg` and `gpgsm`
+# only create the directory specified using "$GNUPGHOME" or
+# `--homedir` if it's the default (usually "~/.gnupg").
+prepare_gnupghome() {
+ mkdir -p "$GNUPGHOME" &&
+ chmod 0700 "$GNUPGHOME"
+}
+
test_lazy_prereq GPG '
gpg_version=$(gpg --version 2>&1)
test $? != 127 || exit 1
@@ -38,8 +48,7 @@ test_lazy_prereq GPG '
# To export ownertrust:
# gpg --homedir /tmp/gpghome --export-ownertrust \
# > lib-gpg/ownertrust
- mkdir "$GNUPGHOME" &&
- chmod 0700 "$GNUPGHOME" &&
+ prepare_gnupghome &&
(gpgconf --kill all || : ) &&
gpg --homedir "${GNUPGHOME}" --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
@@ -63,6 +72,14 @@ test_lazy_prereq GPG2 '
;;
*)
(gpgconf --kill all || : ) &&
+
+ # NEEDSWORK: prepare_gnupghome() should definitely be
+ # called here, but it looks like it exposes a
+ # pre-existing, hidden bug by allowing some tests in
+ # t1016-compatObjectFormat.sh to run instead of being
+ # skipped. See:
+ # https://lore.kernel.org/git/ZoV8b2RvYxLOotSJ@teonanacatl.net/
+
gpg --homedir "${GNUPGHOME}" --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" --import-ownertrust \
@@ -132,8 +149,7 @@ test_lazy_prereq GPGSSH '
test $? = 0 || exit 1;
# Setup some keys and an allowed signers file
- mkdir -p "${GNUPGHOME}" &&
- chmod 0700 "${GNUPGHOME}" &&
+ prepare_gnupghome &&
(setfacl -k "${GNUPGHOME}" 2>/dev/null || true) &&
ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_PRIMARY}" >/dev/null &&
ssh-keygen -t rsa -b 2048 -N "" -C "git rsa2048 key" -f "${GPGSSH_KEY_SECONDARY}" >/dev/null &&
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v3 3/5] t9350: properly count annotated tags
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
2025-10-13 8:48 ` [PATCH v3 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
2025-10-13 8:48 ` [PATCH v3 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first Christian Couder
@ 2025-10-13 8:48 ` Christian Couder
2025-10-24 2:03 ` Elijah Newren
2025-10-13 8:48 ` [PATCH v3 4/5] fast-export: handle all kinds of tag signatures Christian Couder
` (2 subsequent siblings)
5 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-13 8:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
In "t9350-fast-export.sh", these existing tests:
- 'fast-export | fast-import when main is tagged'
- 'cope with tagger-less tags'
are checking the number of annotated tags in the test repo by comparing
it with some hardcoded values.
This could be an issue if some new tests that have some prerequisites
add new annotated tags to the repo before these existing tests. When
the prerequisites would be satisfied, the number of annotated tags
would be different from when some prerequisites would not be satisfied.
As we are going to add new tests that add new annotated tags in a
following commit, let's properly count the number of annotated tag in
the repo by incrementing a counter each time a new annotated tag is
added, and then by comparing the number of annotated tags to the value
of the counter when checking the number of annotated tags.
This is a bit ugly, but it makes it explicit that some tests are
interdependent. Alternative solutions, like moving the new tests to
the end of the script, were considered, but were rejected because they
would instead hide the technical debt and could confuse developers in
the future.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t9350-fast-export.sh | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 8f85c69d62..21ff26939c 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -35,6 +35,7 @@ test_expect_success 'setup' '
git commit -m sitzt file2 &&
test_tick &&
git tag -a -m valentin muss &&
+ ANNOTATED_TAG_COUNT=1 &&
git merge -s ours main
'
@@ -229,7 +230,8 @@ EOF
test_expect_success 'set up faked signed tag' '
- git fast-import <signed-tag-import
+ git fast-import <signed-tag-import &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
'
@@ -491,8 +493,9 @@ test_expect_success 'fast-export -C -C | fast-import' '
test_expect_success 'fast-export | fast-import when main is tagged' '
git tag -m msg last &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1)) &&
git fast-export -C -C --signed-tags=strip --all > output &&
- test $(grep -c "^tag " output) = 3
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT
'
@@ -506,12 +509,13 @@ test_expect_success 'cope with tagger-less tags' '
TAG=$(git hash-object --literally -t tag -w tag-content) &&
git update-ref refs/tags/sonnenschein $TAG &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1)) &&
git fast-export -C -C --signed-tags=strip --all > output &&
- test $(grep -c "^tag " output) = 4 &&
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT &&
! grep "Unspecified Tagger" output &&
git fast-export -C -C --signed-tags=strip --all \
--fake-missing-tagger > output &&
- test $(grep -c "^tag " output) = 4 &&
+ test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT &&
grep "Unspecified Tagger" output
'
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v3 4/5] fast-export: handle all kinds of tag signatures
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
` (2 preceding siblings ...)
2025-10-13 8:48 ` [PATCH v3 3/5] t9350: properly count annotated tags Christian Couder
@ 2025-10-13 8:48 ` Christian Couder
2025-10-24 2:03 ` Elijah Newren
2025-10-13 8:48 ` [PATCH v3 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
2025-10-13 9:09 ` [PATCH v3 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
5 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-13 8:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
Currently the handle_tag() function in "builtin/fast-export.c" searches
only for "\n-----BEGIN PGP SIGNATURE-----\n" in the tag message to find
a tag signature.
This doesn't handle all kinds of OpenPGP signatures as some can start
with "-----BEGIN PGP MESSAGE-----" too, and this doesn't handle SSH and
X.509 signatures either as they use "-----BEGIN SSH SIGNATURE-----" and
"-----BEGIN SIGNED MESSAGE-----" respectively.
To handle all these kinds of tag signatures supported by Git, let's use
the parse_signed_buffer() function to properly find signatures in tag
messages.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/fast-export.c | 7 +++----
t/t9350-fast-export.sh | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index dc2486f9a8..7adbc55f0d 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -931,9 +931,8 @@ static void handle_tag(const char *name, struct tag *tag)
/* handle signed tags */
if (message) {
- const char *signature = strstr(message,
- "\n-----BEGIN PGP SIGNATURE-----\n");
- if (signature)
+ size_t sig_offset = parse_signed_buffer(message, message_size);
+ if (sig_offset < message_size)
switch (signed_tag_mode) {
case SIGN_ABORT:
die("encountered signed tag %s; use "
@@ -950,7 +949,7 @@ static void handle_tag(const char *name, struct tag *tag)
oid_to_hex(&tag->object.oid));
/* fallthru */
case SIGN_STRIP:
- message_size = signature + 1 - message;
+ message_size = sig_offset;
break;
}
}
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 21ff26939c..3d153a4805 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -279,6 +279,42 @@ test_expect_success 'signed-tags=warn-strip' '
test -s err
'
+test_expect_success GPGSM 'setup X.509 signed tag' '
+ test_config gpg.format x509 &&
+ test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+
+ git tag -s -m "X.509 signed tag" x509-signed $(git rev-parse HEAD) &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
+'
+
+test_expect_success GPGSM 'signed-tags=verbatim with X.509' '
+ git fast-export --signed-tags=verbatim x509-signed > output &&
+ test_grep "SIGNED MESSAGE" output
+'
+
+test_expect_success GPGSM 'signed-tags=strip with X.509' '
+ git fast-export --signed-tags=strip x509-signed > output &&
+ test_grep ! "SIGNED MESSAGE" output
+'
+
+test_expect_success GPGSSH 'setup SSH signed tag' '
+ test_config gpg.format ssh &&
+ test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+
+ git tag -s -m "SSH signed tag" ssh-signed $(git rev-parse HEAD) &&
+ ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
+'
+
+test_expect_success GPGSSH 'signed-tags=verbatim with SSH' '
+ git fast-export --signed-tags=verbatim ssh-signed > output &&
+ test_grep "SSH SIGNATURE" output
+'
+
+test_expect_success GPGSSH 'signed-tags=strip with SSH' '
+ git fast-export --signed-tags=strip ssh-signed > output &&
+ test_grep ! "SSH SIGNATURE" output
+'
+
test_expect_success GPG 'set up signed commit' '
# Generate a commit with both "gpgsig" and "encoding" set, so
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v3 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
` (3 preceding siblings ...)
2025-10-13 8:48 ` [PATCH v3 4/5] fast-export: handle all kinds of tag signatures Christian Couder
@ 2025-10-13 8:48 ` Christian Couder
2025-10-24 2:03 ` Elijah Newren
2025-10-13 9:09 ` [PATCH v3 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
5 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-13 8:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder, Christian Couder
Recently, eaaddf5791 (fast-import: add '--signed-commits=<mode>'
option, 2025-09-17) added support for controlling how signed commits
are handled by `git fast-import`, but there is no option yet to
decide about signed tags.
To remediate that, let's add a '--signed-tags=<mode>' option to
`git fast-import` too.
With this, both `git fast-export` and `git fast-import` have both
a '--signed-tags=<mode>' and a '--signed-commits=<mode>' supporting
the same <mode>s.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-import.adoc | 5 ++
builtin/fast-import.c | 43 ++++++++++++++++
t/meson.build | 1 +
t/t9306-fast-import-signed-tags.sh | 80 ++++++++++++++++++++++++++++++
4 files changed, 129 insertions(+)
create mode 100755 t/t9306-fast-import-signed-tags.sh
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 85ed7a7270..b74179a6c8 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -66,6 +66,11 @@ fast-import stream! This option is enabled automatically for
remote-helpers that use the `import` capability, as they are
already trusted to run their own code.
+--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
+ Specify how to handle signed tags. Behaves in the same way
+ as the same option in linkgit:git-fast-export[1], except that
+ default is 'verbatim' (instead of 'abort').
+
--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed commits. Behaves in the same way
as the same option in linkgit:git-fast-export[1], except that
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 2010e78475..60d6faa465 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -188,6 +188,7 @@ static int global_argc;
static const char **global_argv;
static const char *global_prefix;
+static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
/* Memory pools */
@@ -2961,6 +2962,43 @@ static void parse_new_commit(const char *arg)
b->last_commit = object_count_by_type[OBJ_COMMIT];
}
+static void handle_tag_signature(struct strbuf *msg, const char *name)
+{
+ size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
+
+ /* If there is no signature, there is nothing to do. */
+ if (sig_offset >= msg->len)
+ return;
+
+ switch (signed_tag_mode) {
+
+ /* First, modes that don't change anything */
+ case SIGN_ABORT:
+ die(_("encountered signed tag; use "
+ "--signed-tags=<mode> to handle it"));
+ case SIGN_WARN_VERBATIM:
+ warning(_("importing a tag signature verbatim for tag '%s'"), name);
+ /* fallthru */
+ case SIGN_VERBATIM:
+ /* Nothing to do, the signature will be put into the imported tag. */
+ break;
+
+ /* Second, modes that remove the signature */
+ case SIGN_WARN_STRIP:
+ warning(_("stripping a tag signature for tag '%s'"), name);
+ /* fallthru */
+ case SIGN_STRIP:
+ /* Truncate the buffer to remove the signature */
+ strbuf_setlen(msg, sig_offset);
+ break;
+
+ /* Third, BUG */
+ default:
+ BUG("invalid signed_tag_mode value %d from tag '%s'",
+ signed_tag_mode, name);
+ }
+}
+
static void parse_new_tag(const char *arg)
{
static struct strbuf msg = STRBUF_INIT;
@@ -3024,6 +3062,8 @@ static void parse_new_tag(const char *arg)
/* tag payload/message */
parse_data(&msg, 0, NULL);
+ handle_tag_signature(&msg, t->name);
+
/* build the tag object */
strbuf_reset(&new_data);
@@ -3544,6 +3584,9 @@ static int parse_one_option(const char *option)
} else if (skip_prefix(option, "signed-commits=", &option)) {
if (parse_sign_mode(option, &signed_commit_mode))
usagef(_("unknown --signed-commits mode '%s'"), option);
+ } else if (skip_prefix(option, "signed-tags=", &option)) {
+ if (parse_sign_mode(option, &signed_tag_mode))
+ usagef(_("unknown --signed-tags mode '%s'"), option);
} else if (!strcmp(option, "quiet")) {
show_stats = 0;
quiet = 1;
diff --git a/t/meson.build b/t/meson.build
index 11376b9e25..cb8c2b4b30 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1036,6 +1036,7 @@ integration_tests = [
't9303-fast-import-compression.sh',
't9304-fast-import-marks.sh',
't9305-fast-import-signatures.sh',
+ 't9306-fast-import-signed-tags.sh',
't9350-fast-export.sh',
't9351-fast-export-anonymize.sh',
't9400-git-cvsserver-server.sh',
diff --git a/t/t9306-fast-import-signed-tags.sh b/t/t9306-fast-import-signed-tags.sh
new file mode 100755
index 0000000000..363619e7d1
--- /dev/null
+++ b/t/t9306-fast-import-signed-tags.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+
+test_description='git fast-import --signed-tags=<mode>'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success 'set up unsigned initial commit and import repo' '
+ test_commit first &&
+ git init new
+'
+
+test_expect_success 'import no signed tag with --signed-tags=abort' '
+ git fast-export --signed-tags=verbatim >output &&
+ git -C new fast-import --quiet --signed-tags=abort <output
+'
+
+test_expect_success GPG 'set up OpenPGP signed tag' '
+ git tag -s -m "OpenPGP signed tag" openpgp-signed first &&
+ OPENPGP_SIGNED=$(git rev-parse --verify refs/tags/openpgp-signed) &&
+ git fast-export --signed-tags=verbatim openpgp-signed >output
+'
+
+test_expect_success GPG 'import OpenPGP signed tag with --signed-tags=abort' '
+ test_must_fail git -C new fast-import --quiet --signed-tags=abort <output
+'
+
+test_expect_success GPG 'import OpenPGP signed tag with --signed-tags=verbatim' '
+ git -C new fast-import --quiet --signed-tags=verbatim <output >log 2>&1 &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/openpgp-signed) &&
+ test $OPENPGP_SIGNED = $IMPORTED &&
+ test_must_be_empty log
+'
+
+test_expect_success GPGSM 'setup X.509 signed tag' '
+ test_config gpg.format x509 &&
+ test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+
+ git tag -s -m "X.509 signed tag" x509-signed first &&
+ X509_SIGNED=$(git rev-parse --verify refs/tags/x509-signed) &&
+ git fast-export --signed-tags=verbatim x509-signed >output
+'
+
+test_expect_success GPGSM 'import X.509 signed tag with --signed-tags=warn-strip' '
+ git -C new fast-import --quiet --signed-tags=warn-strip <output >log 2>&1 &&
+ test_grep "stripping a tag signature for tag '\''x509-signed'\''" log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/x509-signed) &&
+ test $X509_SIGNED != $IMPORTED &&
+ git -C new cat-file -p x509-signed >out &&
+ test_grep ! "SIGNED MESSAGE" out
+'
+
+test_expect_success GPGSSH 'setup SSH signed tag' '
+ test_config gpg.format ssh &&
+ test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+
+ git tag -s -m "SSH signed tag" ssh-signed first &&
+ SSH_SIGNED=$(git rev-parse --verify refs/tags/ssh-signed) &&
+ git fast-export --signed-tags=verbatim ssh-signed >output
+'
+
+test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=warn-verbatim' '
+ git -C new fast-import --quiet --signed-tags=warn-verbatim <output >log 2>&1 &&
+ test_grep "importing a tag signature verbatim for tag '\''ssh-signed'\''" log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/ssh-signed) &&
+ test $SSH_SIGNED = $IMPORTED
+'
+
+test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=strip' '
+ git -C new fast-import --quiet --signed-tags=strip <output >log 2>&1 &&
+ test_must_be_empty log &&
+ IMPORTED=$(git -C new rev-parse --verify refs/tags/ssh-signed) &&
+ test $SSH_SIGNED != $IMPORTED &&
+ git -C new cat-file -p ssh-signed >out &&
+ test_grep ! "SSH SIGNATURE" out
+'
+
+test_done
--
2.51.0.438.g6987fc0bae
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v3 0/5] fast-import: start controlling how tag signatures are handled
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
` (4 preceding siblings ...)
2025-10-13 8:48 ` [PATCH v3 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
@ 2025-10-13 9:09 ` Christian Couder
2025-10-24 2:06 ` Elijah Newren
5 siblings, 1 reply; 52+ messages in thread
From: Christian Couder @ 2025-10-13 9:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Elijah Newren, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk
On Mon, Oct 13, 2025 at 10:49 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> Changes since v2
> ----------------
>
> Thanks to Patrick Steinhardt, Todd Zullinger and Collin Funk who
> reviewed or commented on the v1 and v2.
>
> There is a single change in the first patch (doc: git-tag: stop
> focusing on GPG signed tags) where the description of the
> `-v | --verify` option of `git tag` has been improved.
Sorry, this should have been sent in reply to the v2
(https://lore.kernel.org/git/20251009122457.1273701-1-christian.couder@gmail.com/)
instead of the v1.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v3 1/5] doc: git-tag: stop focusing on GPG signed tags
2025-10-13 8:48 ` [PATCH v3 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
@ 2025-10-24 2:03 ` Elijah Newren
0 siblings, 0 replies; 52+ messages in thread
From: Elijah Newren @ 2025-10-24 2:03 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
On Mon, Oct 13, 2025 at 4:49 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> It looks like the documentation of `git tag` is focused a bit too
> much on GPG signed tags.
>
> This starts with the "NAME" section where the command is described
> with:
>
> "Create, list, delete or verify a tag object signed with GPG"
>
> while for example `git branch` is described with simply:
>
> "List, create, or delete branches"
>
> This could give the false impression that `git tag` only works with
> tag objects, not with lightweight tags, and that tag objects are
> always GPG signed.
>
> In the "DESCRIPTION" section, it looks like only "GnuPG signed tag
> objects" can be created by the `-s` and `-u <key-id>` options, and it
> seems `gpg.program` can only specify a "custom GnuPG binary".
>
> This goes on in the "OPTIONS" section too, especially about the `-s`
> and `-u <key-id>` options.
>
> The "CONFIGURATION" section also doesn't talk about how to configure
> the command to work with X.509 and SSH signatures.
>
> Let's rework all that to make sure users have a more accurate and
> balanced view of what the command can do.
>
> Helped-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> Documentation/git-tag.adoc | 48 ++++++++++++++++++++++++++------------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/git-tag.adoc b/Documentation/git-tag.adoc
> index a4b1c0ec05..28d6fe4e1a 100644
> --- a/Documentation/git-tag.adoc
> +++ b/Documentation/git-tag.adoc
> @@ -3,7 +3,7 @@ git-tag(1)
>
> NAME
> ----
> -git-tag - Create, list, delete or verify a tag object signed with GPG
> +git-tag - Create, list, delete or verify tags
>
>
> SYNOPSIS
> @@ -38,15 +38,17 @@ and `-a`, `-s`, and `-u <key-id>` are absent, `-a` is implied.
> Otherwise, a tag reference that points directly at the given object
> (i.e., a lightweight tag) is created.
>
> -A GnuPG signed tag object will be created when `-s` or `-u
> -<key-id>` is used. When `-u <key-id>` is not used, the
> -committer identity for the current user is used to find the
> -GnuPG key for signing. The configuration variable `gpg.program`
> -is used to specify custom GnuPG binary.
> +A cryptographically signed tag object will be created when `-s` or
> +`-u <key-id>` is used. The signing backend (GPG, X.509, SSH, etc.) is
> +controlled by the `gpg.format` configuration variable, defaulting to
> +OpenPGP. When `-u <key-id>` is not used, the committer identity for
> +the current user is used to find the key for signing. The
> +configuration variable `gpg.program` is used to specify a custom
> +signing binary.
>
> Tag objects (created with `-a`, `-s`, or `-u`) are called "annotated"
> tags; they contain a creation date, the tagger name and e-mail, a
> -tagging message, and an optional GnuPG signature. Whereas a
> +tagging message, and an optional cryptographic signature. Whereas a
> "lightweight" tag is simply a name for an object (usually a commit
> object).
>
> @@ -64,10 +66,12 @@ OPTIONS
>
> -s::
> --sign::
> - Make a GPG-signed tag, using the default e-mail address's key.
> - The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
> - configuration variable if it exists, or disabled otherwise.
> - See linkgit:git-config[1].
> + Make a cryptographically signed tag, using the default signing
> + key. The signing backend used depends on the `gpg.format`
> + configuration variable. The default key is determined by the
> + backend. For GPG, it's based on the committer's email address,
> + while for SSH it may be a specific key file or agent
> + identity. See linkgit:git-config[1].
>
> --no-sign::
> Override `tag.gpgSign` configuration variable that is
> @@ -75,7 +79,10 @@ OPTIONS
>
> -u <key-id>::
> --local-user=<key-id>::
> - Make a GPG-signed tag, using the given key.
> + Make a cryptographically signed tag using the given key. The
> + format of the <key-id> and the backend used depend on the
> + `gpg.format` configuration variable. See
> + linkgit:git-config[1].
>
> -f::
> --force::
> @@ -87,7 +94,7 @@ OPTIONS
>
> -v::
> --verify::
> - Verify the GPG signature of the given tag names.
> + Verify the cryptographic signature of the given tags.
>
> -n<num>::
> <num> specifies how many lines from the annotation, if any,
> @@ -236,12 +243,23 @@ it in the repository configuration as follows:
>
> -------------------------------------
> [user]
> - signingKey = <gpg-key-id>
> + signingKey = <key-id>
> -------------------------------------
>
> +The signing backend can be chosen via the `gpg.format` configuration
> +variable, which defaults to `openpgp`. See linkgit:git-config[1]
> +for a list of other supported formats.
> +
> +The path to the program used for each signing backend can be specified
> +with the `gpg.<format>.program` configuration variable. For the
> +`openpgp` backend, `gpg.program` can be used as a synonym for
> +`gpg.openpgp.program`. See linkgit:git-config[1] for details.
> +
> `pager.tag` is only respected when listing tags, i.e., when `-l` is
> used or implied. The default is to use a pager.
> -See linkgit:git-config[1].
> +
> +See linkgit:git-config[1] for more details and other configuration
> +variables.
>
> DISCUSSION
> ----------
> --
> 2.51.0.438.g6987fc0bae
Looks like some nice clarifications & corrections.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v3 3/5] t9350: properly count annotated tags
2025-10-13 8:48 ` [PATCH v3 3/5] t9350: properly count annotated tags Christian Couder
@ 2025-10-24 2:03 ` Elijah Newren
0 siblings, 0 replies; 52+ messages in thread
From: Elijah Newren @ 2025-10-24 2:03 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
On Mon, Oct 13, 2025 at 4:49 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> In "t9350-fast-export.sh", these existing tests:
>
> - 'fast-export | fast-import when main is tagged'
> - 'cope with tagger-less tags'
>
> are checking the number of annotated tags in the test repo by comparing
> it with some hardcoded values.
>
> This could be an issue if some new tests that have some prerequisites
> add new annotated tags to the repo before these existing tests. When
> the prerequisites would be satisfied, the number of annotated tags
> would be different from when some prerequisites would not be satisfied.
>
> As we are going to add new tests that add new annotated tags in a
> following commit, let's properly count the number of annotated tag in
> the repo by incrementing a counter each time a new annotated tag is
> added, and then by comparing the number of annotated tags to the value
> of the counter when checking the number of annotated tags.
>
> This is a bit ugly, but it makes it explicit that some tests are
> interdependent. Alternative solutions, like moving the new tests to
> the end of the script, were considered, but were rejected because they
> would instead hide the technical debt and could confuse developers in
> the future.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> t/t9350-fast-export.sh | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 8f85c69d62..21ff26939c 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -35,6 +35,7 @@ test_expect_success 'setup' '
> git commit -m sitzt file2 &&
> test_tick &&
> git tag -a -m valentin muss &&
> + ANNOTATED_TAG_COUNT=1 &&
> git merge -s ours main
>
> '
> @@ -229,7 +230,8 @@ EOF
>
> test_expect_success 'set up faked signed tag' '
>
> - git fast-import <signed-tag-import
> + git fast-import <signed-tag-import &&
> + ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
>
> '
>
> @@ -491,8 +493,9 @@ test_expect_success 'fast-export -C -C | fast-import' '
> test_expect_success 'fast-export | fast-import when main is tagged' '
>
> git tag -m msg last &&
> + ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1)) &&
> git fast-export -C -C --signed-tags=strip --all > output &&
> - test $(grep -c "^tag " output) = 3
> + test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT
>
> '
>
> @@ -506,12 +509,13 @@ test_expect_success 'cope with tagger-less tags' '
>
> TAG=$(git hash-object --literally -t tag -w tag-content) &&
> git update-ref refs/tags/sonnenschein $TAG &&
> + ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1)) &&
> git fast-export -C -C --signed-tags=strip --all > output &&
> - test $(grep -c "^tag " output) = 4 &&
> + test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT &&
> ! grep "Unspecified Tagger" output &&
> git fast-export -C -C --signed-tags=strip --all \
> --fake-missing-tagger > output &&
> - test $(grep -c "^tag " output) = 4 &&
> + test $(grep -c "^tag " output) = $ANNOTATED_TAG_COUNT &&
> grep "Unspecified Tagger" output
>
> '
> --
> 2.51.0.438.g6987fc0bae
When tests are not read-only, I tend to prefer either giving each test
in a testfile its own repo, or doing hard resets with
test_when_finished. Either way allows tests to be more independent,
and allows new tests to be added or removed from the testsuite without
adverse affects on other tests. But, restructuring an existing
testfile is a significantly bigger change, so this seems like a
reasonable path you've proposed for your series.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v3 4/5] fast-export: handle all kinds of tag signatures
2025-10-13 8:48 ` [PATCH v3 4/5] fast-export: handle all kinds of tag signatures Christian Couder
@ 2025-10-24 2:03 ` Elijah Newren
0 siblings, 0 replies; 52+ messages in thread
From: Elijah Newren @ 2025-10-24 2:03 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
On Mon, Oct 13, 2025 at 4:49 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> Currently the handle_tag() function in "builtin/fast-export.c" searches
> only for "\n-----BEGIN PGP SIGNATURE-----\n" in the tag message to find
> a tag signature.
>
> This doesn't handle all kinds of OpenPGP signatures as some can start
> with "-----BEGIN PGP MESSAGE-----" too, and this doesn't handle SSH and
> X.509 signatures either as they use "-----BEGIN SSH SIGNATURE-----" and
> "-----BEGIN SIGNED MESSAGE-----" respectively.
>
> To handle all these kinds of tag signatures supported by Git, let's use
> the parse_signed_buffer() function to properly find signatures in tag
> messages.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> builtin/fast-export.c | 7 +++----
> t/t9350-fast-export.sh | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index dc2486f9a8..7adbc55f0d 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -931,9 +931,8 @@ static void handle_tag(const char *name, struct tag *tag)
>
> /* handle signed tags */
> if (message) {
> - const char *signature = strstr(message,
> - "\n-----BEGIN PGP SIGNATURE-----\n");
> - if (signature)
> + size_t sig_offset = parse_signed_buffer(message, message_size);
> + if (sig_offset < message_size)
Always nice to remove a special hard-coded (and incomplete) additional
parsing with a call to the official function we have to handle this.
> switch (signed_tag_mode) {
> case SIGN_ABORT:
> die("encountered signed tag %s; use "
> @@ -950,7 +949,7 @@ static void handle_tag(const char *name, struct tag *tag)
> oid_to_hex(&tag->object.oid));
> /* fallthru */
> case SIGN_STRIP:
> - message_size = signature + 1 - message;
> + message_size = sig_offset;
> break;
> }
> }
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 21ff26939c..3d153a4805 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -279,6 +279,42 @@ test_expect_success 'signed-tags=warn-strip' '
> test -s err
> '
>
> +test_expect_success GPGSM 'setup X.509 signed tag' '
> + test_config gpg.format x509 &&
> + test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> +
> + git tag -s -m "X.509 signed tag" x509-signed $(git rev-parse HEAD) &&
> + ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
> +'
> +
> +test_expect_success GPGSM 'signed-tags=verbatim with X.509' '
> + git fast-export --signed-tags=verbatim x509-signed > output &&
> + test_grep "SIGNED MESSAGE" output
> +'
> +
> +test_expect_success GPGSM 'signed-tags=strip with X.509' '
> + git fast-export --signed-tags=strip x509-signed > output &&
> + test_grep ! "SIGNED MESSAGE" output
> +'
> +
> +test_expect_success GPGSSH 'setup SSH signed tag' '
> + test_config gpg.format ssh &&
> + test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
> +
> + git tag -s -m "SSH signed tag" ssh-signed $(git rev-parse HEAD) &&
> + ANNOTATED_TAG_COUNT=$((ANNOTATED_TAG_COUNT + 1))
> +'
> +
> +test_expect_success GPGSSH 'signed-tags=verbatim with SSH' '
> + git fast-export --signed-tags=verbatim ssh-signed > output &&
> + test_grep "SSH SIGNATURE" output
> +'
> +
> +test_expect_success GPGSSH 'signed-tags=strip with SSH' '
> + git fast-export --signed-tags=strip ssh-signed > output &&
> + test_grep ! "SSH SIGNATURE" output
> +'
> +
> test_expect_success GPG 'set up signed commit' '
>
> # Generate a commit with both "gpgsig" and "encoding" set, so
> --
> 2.51.0.438.g6987fc0bae
Looks good.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v3 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-13 8:48 ` [PATCH v3 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
@ 2025-10-24 2:03 ` Elijah Newren
2025-10-24 9:27 ` Christian Couder
2025-10-24 15:03 ` Junio C Hamano
0 siblings, 2 replies; 52+ messages in thread
From: Elijah Newren @ 2025-10-24 2:03 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
On Mon, Oct 13, 2025 at 4:49 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> Recently, eaaddf5791 (fast-import: add '--signed-commits=<mode>'
> option, 2025-09-17) added support for controlling how signed commits
> are handled by `git fast-import`, but there is no option yet to
> decide about signed tags.
>
> To remediate that, let's add a '--signed-tags=<mode>' option to
> `git fast-import` too.
>
> With this, both `git fast-export` and `git fast-import` have both
> a '--signed-tags=<mode>' and a '--signed-commits=<mode>' supporting
> the same <mode>s.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> Documentation/git-fast-import.adoc | 5 ++
> builtin/fast-import.c | 43 ++++++++++++++++
> t/meson.build | 1 +
> t/t9306-fast-import-signed-tags.sh | 80 ++++++++++++++++++++++++++++++
> 4 files changed, 129 insertions(+)
> create mode 100755 t/t9306-fast-import-signed-tags.sh
>
> diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> index 85ed7a7270..b74179a6c8 100644
> --- a/Documentation/git-fast-import.adoc
> +++ b/Documentation/git-fast-import.adoc
> @@ -66,6 +66,11 @@ fast-import stream! This option is enabled automatically for
> remote-helpers that use the `import` capability, as they are
> already trusted to run their own code.
>
> +--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> + Specify how to handle signed tags. Behaves in the same way
> + as the same option in linkgit:git-fast-export[1], except that
> + default is 'verbatim' (instead of 'abort').
Sorry for not catching this earlier with the --signed-commits series
(was otherwise occupied), but this worries me. If we ship with this
as the default, people will come to depend upon it, and I think it's a
bad long term default. Long term, we'd want to check if the
signatures are valid and keep if so and do something else if not (e.g.
re-sign or abort or strip). Maybe verbatim is better than abort out
of the options you've implemented so far, but I think setting the
default now to verbatim means people start depending on it and we
cannot change it later. Could we change to 'abort', for both this and
--signed-commits, before the 2.52 release, and then re-discuss once
you have the other options implemented?
> +
> --signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> Specify how to handle signed commits. Behaves in the same way
> as the same option in linkgit:git-fast-export[1], except that
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 2010e78475..60d6faa465 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -188,6 +188,7 @@ static int global_argc;
> static const char **global_argv;
> static const char *global_prefix;
>
> +static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
> static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
Here's where you define the defaults, in case there is no
--signed-{tag,commit} flags. I think we should go with abort to allow
us to later change to a better default once one is implemented.
> /* Memory pools */
> @@ -2961,6 +2962,43 @@ static void parse_new_commit(const char *arg)
> b->last_commit = object_count_by_type[OBJ_COMMIT];
> }
>
> +static void handle_tag_signature(struct strbuf *msg, const char *name)
> +{
> + size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
> +
> + /* If there is no signature, there is nothing to do. */
> + if (sig_offset >= msg->len)
> + return;
> +
> + switch (signed_tag_mode) {
> +
> + /* First, modes that don't change anything */
> + case SIGN_ABORT:
> + die(_("encountered signed tag; use "
> + "--signed-tags=<mode> to handle it"));
> + case SIGN_WARN_VERBATIM:
> + warning(_("importing a tag signature verbatim for tag '%s'"), name);
> + /* fallthru */
> + case SIGN_VERBATIM:
> + /* Nothing to do, the signature will be put into the imported tag. */
> + break;
> +
> + /* Second, modes that remove the signature */
> + case SIGN_WARN_STRIP:
> + warning(_("stripping a tag signature for tag '%s'"), name);
> + /* fallthru */
> + case SIGN_STRIP:
> + /* Truncate the buffer to remove the signature */
> + strbuf_setlen(msg, sig_offset);
> + break;
> +
> + /* Third, BUG */
> + default:
> + BUG("invalid signed_tag_mode value %d from tag '%s'",
> + signed_tag_mode, name);
> + }
Makes sense, you either keep the signature as is, or strip it, with
potentially sending a warning (or error) beforehand.
> +}
> +
> static void parse_new_tag(const char *arg)
> {
> static struct strbuf msg = STRBUF_INIT;
> @@ -3024,6 +3062,8 @@ static void parse_new_tag(const char *arg)
> /* tag payload/message */
> parse_data(&msg, 0, NULL);
>
> + handle_tag_signature(&msg, t->name);
> +
> /* build the tag object */
> strbuf_reset(&new_data);
>
> @@ -3544,6 +3584,9 @@ static int parse_one_option(const char *option)
> } else if (skip_prefix(option, "signed-commits=", &option)) {
> if (parse_sign_mode(option, &signed_commit_mode))
> usagef(_("unknown --signed-commits mode '%s'"), option);
> + } else if (skip_prefix(option, "signed-tags=", &option)) {
> + if (parse_sign_mode(option, &signed_tag_mode))
> + usagef(_("unknown --signed-tags mode '%s'"), option);
Re-using the parse_sign_mode() function previously introduced for
--signed-commits...
> } else if (!strcmp(option, "quiet")) {
> show_stats = 0;
> quiet = 1;
> diff --git a/t/meson.build b/t/meson.build
> index 11376b9e25..cb8c2b4b30 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -1036,6 +1036,7 @@ integration_tests = [
> 't9303-fast-import-compression.sh',
> 't9304-fast-import-marks.sh',
> 't9305-fast-import-signatures.sh',
> + 't9306-fast-import-signed-tags.sh',
> 't9350-fast-export.sh',
> 't9351-fast-export-anonymize.sh',
> 't9400-git-cvsserver-server.sh',
> diff --git a/t/t9306-fast-import-signed-tags.sh b/t/t9306-fast-import-signed-tags.sh
> new file mode 100755
> index 0000000000..363619e7d1
> --- /dev/null
> +++ b/t/t9306-fast-import-signed-tags.sh
> @@ -0,0 +1,80 @@
> +#!/bin/sh
> +
> +test_description='git fast-import --signed-tags=<mode>'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-gpg.sh"
> +
> +test_expect_success 'set up unsigned initial commit and import repo' '
> + test_commit first &&
> + git init new
> +'
> +
> +test_expect_success 'import no signed tag with --signed-tags=abort' '
> + git fast-export --signed-tags=verbatim >output &&
> + git -C new fast-import --quiet --signed-tags=abort <output
> +'
> +
> +test_expect_success GPG 'set up OpenPGP signed tag' '
> + git tag -s -m "OpenPGP signed tag" openpgp-signed first &&
> + OPENPGP_SIGNED=$(git rev-parse --verify refs/tags/openpgp-signed) &&
> + git fast-export --signed-tags=verbatim openpgp-signed >output
> +'
> +
> +test_expect_success GPG 'import OpenPGP signed tag with --signed-tags=abort' '
> + test_must_fail git -C new fast-import --quiet --signed-tags=abort <output
> +'
> +
> +test_expect_success GPG 'import OpenPGP signed tag with --signed-tags=verbatim' '
> + git -C new fast-import --quiet --signed-tags=verbatim <output >log 2>&1 &&
> + IMPORTED=$(git -C new rev-parse --verify refs/tags/openpgp-signed) &&
> + test $OPENPGP_SIGNED = $IMPORTED &&
> + test_must_be_empty log
> +'
> +
> +test_expect_success GPGSM 'setup X.509 signed tag' '
> + test_config gpg.format x509 &&
> + test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> +
> + git tag -s -m "X.509 signed tag" x509-signed first &&
> + X509_SIGNED=$(git rev-parse --verify refs/tags/x509-signed) &&
> + git fast-export --signed-tags=verbatim x509-signed >output
> +'
> +
> +test_expect_success GPGSM 'import X.509 signed tag with --signed-tags=warn-strip' '
> + git -C new fast-import --quiet --signed-tags=warn-strip <output >log 2>&1 &&
> + test_grep "stripping a tag signature for tag '\''x509-signed'\''" log &&
> + IMPORTED=$(git -C new rev-parse --verify refs/tags/x509-signed) &&
> + test $X509_SIGNED != $IMPORTED &&
> + git -C new cat-file -p x509-signed >out &&
> + test_grep ! "SIGNED MESSAGE" out
> +'
> +
> +test_expect_success GPGSSH 'setup SSH signed tag' '
> + test_config gpg.format ssh &&
> + test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
> +
> + git tag -s -m "SSH signed tag" ssh-signed first &&
> + SSH_SIGNED=$(git rev-parse --verify refs/tags/ssh-signed) &&
> + git fast-export --signed-tags=verbatim ssh-signed >output
> +'
> +
> +test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=warn-verbatim' '
> + git -C new fast-import --quiet --signed-tags=warn-verbatim <output >log 2>&1 &&
> + test_grep "importing a tag signature verbatim for tag '\''ssh-signed'\''" log &&
> + IMPORTED=$(git -C new rev-parse --verify refs/tags/ssh-signed) &&
> + test $SSH_SIGNED = $IMPORTED
> +'
> +
> +test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=strip' '
> + git -C new fast-import --quiet --signed-tags=strip <output >log 2>&1 &&
> + test_must_be_empty log &&
> + IMPORTED=$(git -C new rev-parse --verify refs/tags/ssh-signed) &&
> + test $SSH_SIGNED != $IMPORTED &&
> + git -C new cat-file -p ssh-signed >out &&
> + test_grep ! "SSH SIGNATURE" out
> +'
> +
> +test_done
> --
> 2.51.0.438.g6987fc0bae
This all looks good to me, other than the default as noted above.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v3 0/5] fast-import: start controlling how tag signatures are handled
2025-10-13 9:09 ` [PATCH v3 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
@ 2025-10-24 2:06 ` Elijah Newren
0 siblings, 0 replies; 52+ messages in thread
From: Elijah Newren @ 2025-10-24 2:06 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk
On Mon, Oct 13, 2025 at 5:10 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Mon, Oct 13, 2025 at 10:49 AM Christian Couder
> <christian.couder@gmail.com> wrote:
>
> >
> > Changes since v2
> > ----------------
> >
> > Thanks to Patrick Steinhardt, Todd Zullinger and Collin Funk who
> > reviewed or commented on the v1 and v2.
> >
> > There is a single change in the first patch (doc: git-tag: stop
> > focusing on GPG signed tags) where the description of the
> > `-v | --verify` option of `git tag` has been improved.
>
> Sorry, this should have been sent in reply to the v2
> (https://lore.kernel.org/git/20251009122457.1273701-1-christian.couder@gmail.com/)
> instead of the v1.
Sorry for the delay in reviewing; the series looks really good, with
one minor exception: I'm worried about the default chosen in patch 5
(and also the default chosen for --signed-commits in the recently
merged eaaddf579124 (fast-import: add '--signed-commits=<mode>'
option, 2025-09-17)), as I commented on there.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v3 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-24 2:03 ` Elijah Newren
@ 2025-10-24 9:27 ` Christian Couder
2025-10-24 15:03 ` Junio C Hamano
1 sibling, 0 replies; 52+ messages in thread
From: Christian Couder @ 2025-10-24 9:27 UTC (permalink / raw)
To: Elijah Newren
Cc: git, Junio C Hamano, Patrick Steinhardt, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
On Fri, Oct 24, 2025 at 4:04 AM Elijah Newren <newren@gmail.com> wrote:
>
> On Mon, Oct 13, 2025 at 4:49 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> > +--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> > + Specify how to handle signed tags. Behaves in the same way
> > + as the same option in linkgit:git-fast-export[1], except that
> > + default is 'verbatim' (instead of 'abort').
>
> Sorry for not catching this earlier with the --signed-commits series
> (was otherwise occupied), but this worries me. If we ship with this
> as the default, people will come to depend upon it, and I think it's a
> bad long term default. Long term, we'd want to check if the
> signatures are valid and keep if so and do something else if not (e.g.
> re-sign or abort or strip). Maybe verbatim is better than abort out
> of the options you've implemented so far, but I think setting the
> default now to verbatim means people start depending on it and we
> cannot change it later. Could we change to 'abort', for both this and
> --signed-commits, before the 2.52 release, and then re-discuss once
> you have the other options implemented?
"verbatim" was already the default long before this patch series. Any
tag signature was copied as-is, as part of the tag message. So it's
possible that users have relied on this for a long time.
For the --signed-commits series, "verbatim" was also the default
before the series. Even if importing commit signatures has been
implemented more recently and even if this is marked as experimental,
it's the default in Git 2.51. So regular users could already rely on
it.
The --signed-commits series has been merged to 'master' and this
series has recently been merged to 'next'. They aren't part of a
release, but at this point I think we should send separate patches to
change the default if we want to do that.
As I plan to work soon on the new modes that would check signatures
and do something based on that, and as you say that it would likely be
better if such a new mode becomes the default, I am reluctant to
change the default mode right now, only to have to change it again
hopefully in a few weeks or months. If you want to do it, then feel
free to send patches changing the default though.
> This all looks good to me, other than the default as noted above.
Thanks for your review.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v3 5/5] fast-import: add '--signed-tags=<mode>' option
2025-10-24 2:03 ` Elijah Newren
2025-10-24 9:27 ` Christian Couder
@ 2025-10-24 15:03 ` Junio C Hamano
1 sibling, 0 replies; 52+ messages in thread
From: Junio C Hamano @ 2025-10-24 15:03 UTC (permalink / raw)
To: Elijah Newren
Cc: Christian Couder, git, Patrick Steinhardt, Jeff King,
brian m . carlson, Johannes Schindelin, Todd Zullinger,
Collin Funk, Christian Couder
Elijah Newren <newren@gmail.com> writes:
>> +--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
>> + Specify how to handle signed tags. Behaves in the same way
>> + as the same option in linkgit:git-fast-export[1], except that
>> + default is 'verbatim' (instead of 'abort').
>
> Sorry for not catching this earlier with the --signed-commits series
> (was otherwise occupied), but this worries me. If we ship with this
> as the default, people will come to depend upon it, and I think it's a
> bad long term default. Long term, we'd want to check if the
> signatures are valid and keep if so and do something else if not (e.g.
> re-sign or abort or strip). Maybe verbatim is better than abort out
> of the options you've implemented so far, but I think setting the
> default now to verbatim means people start depending on it and we
> cannot change it later. Could we change to 'abort', for both this and
> --signed-commits, before the 2.52 release, and then re-discuss once
> you have the other options implemented?
Isn't this series a response to the "we only copy verbatim and there
is no other choice", which we had from the beginning of fast import
& export? If we knew better, we may have made it abort when we did
the fast import & export, but we cannot go back and change it, and
we cannot change the default with this series without disrupting the
users, so the next best thing is to make it configurable, which is
the point of this series (and the other one), no?
^ permalink raw reply [flat|nested] 52+ messages in thread
end of thread, other threads:[~2025-10-24 15:03 UTC | newest]
Thread overview: 52+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-07 12:29 [PATCH 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
2025-10-07 12:29 ` [PATCH 1/5] doc: git-tag: stop focussing on GPG signed tags Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 9:52 ` Christian Couder
2025-10-08 11:48 ` Patrick Steinhardt
2025-10-07 12:29 ` [PATCH 2/5] lib-gpg: allow tests with the GPGSM prereq first Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 9:42 ` Christian Couder
2025-10-09 1:29 ` Collin Funk
2025-10-09 2:37 ` Todd Zullinger
2025-10-09 12:29 ` Christian Couder
2025-10-09 18:18 ` Junio C Hamano
2025-10-09 12:30 ` Christian Couder
2025-10-07 12:29 ` [PATCH 3/5] t9350: properly count annotated tags Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 10:00 ` Christian Couder
2025-10-07 12:29 ` [PATCH 4/5] fast-export: handle all kinds of tag signatures Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 10:02 ` Christian Couder
2025-10-09 12:33 ` Christian Couder
2025-10-07 12:29 ` [PATCH 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
2025-10-08 7:14 ` Patrick Steinhardt
2025-10-08 10:50 ` Christian Couder
2025-10-08 11:53 ` Patrick Steinhardt
2025-10-09 12:24 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
2025-10-09 12:24 ` [PATCH v2 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
2025-10-10 1:19 ` Junio C Hamano
2025-10-10 7:06 ` Christian Couder
2025-10-09 12:24 ` [PATCH v2 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first Christian Couder
2025-10-10 6:49 ` Patrick Steinhardt
2025-10-10 14:09 ` Todd Zullinger
2025-10-10 16:22 ` Junio C Hamano
2025-10-11 2:14 ` Todd Zullinger
2025-10-12 0:15 ` Junio C Hamano
2025-10-09 12:24 ` [PATCH v2 3/5] t9350: properly count annotated tags Christian Couder
2025-10-09 12:24 ` [PATCH v2 4/5] fast-export: handle all kinds of tag signatures Christian Couder
2025-10-09 12:24 ` [PATCH v2 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
2025-10-09 21:35 ` [PATCH v2 0/5] fast-import: start controlling how tag signatures are handled Junio C Hamano
2025-10-13 8:48 ` [PATCH v3 " Christian Couder
2025-10-13 8:48 ` [PATCH v3 1/5] doc: git-tag: stop focusing on GPG signed tags Christian Couder
2025-10-24 2:03 ` Elijah Newren
2025-10-13 8:48 ` [PATCH v3 2/5] lib-gpg: allow tests with GPGSM or GPGSSH prereq first Christian Couder
2025-10-13 8:48 ` [PATCH v3 3/5] t9350: properly count annotated tags Christian Couder
2025-10-24 2:03 ` Elijah Newren
2025-10-13 8:48 ` [PATCH v3 4/5] fast-export: handle all kinds of tag signatures Christian Couder
2025-10-24 2:03 ` Elijah Newren
2025-10-13 8:48 ` [PATCH v3 5/5] fast-import: add '--signed-tags=<mode>' option Christian Couder
2025-10-24 2:03 ` Elijah Newren
2025-10-24 9:27 ` Christian Couder
2025-10-24 15:03 ` Junio C Hamano
2025-10-13 9:09 ` [PATCH v3 0/5] fast-import: start controlling how tag signatures are handled Christian Couder
2025-10-24 2:06 ` Elijah Newren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).