* [PATCH v4 1/5] git-fast-import.txt: add missing LF in the BNF
2021-04-30 23:25 ` [PATCH v4 0/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
@ 2021-04-30 23:25 ` Luke Shumaker
2021-04-30 23:25 ` [PATCH v4 2/5] fast-export: rename --signed-tags='warn' to 'warn-verbatim' Luke Shumaker
` (4 subsequent siblings)
5 siblings, 0 replies; 60+ messages in thread
From: Luke Shumaker @ 2021-04-30 23:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Elijah Newren, Jeff King, Johannes Schindelin,
Nguyễn Thái Ngọc Duy, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker
From: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
Notes:
v2: no changes
v3: no changes
v4: no changes
Documentation/git-fast-import.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 39cfa05b28..458af0a2d6 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -437,7 +437,7 @@ change to the project.
original-oid?
('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
'committer' (SP <name>)? SP LT <email> GT SP <when> LF
- ('encoding' SP <encoding>)?
+ ('encoding' SP <encoding> LF)?
data
('from' SP <commit-ish> LF)?
('merge' SP <commit-ish> LF)*
--
2.31.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v4 2/5] fast-export: rename --signed-tags='warn' to 'warn-verbatim'
2021-04-30 23:25 ` [PATCH v4 0/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
2021-04-30 23:25 ` [PATCH v4 1/5] git-fast-import.txt: add missing LF in the BNF Luke Shumaker
@ 2021-04-30 23:25 ` Luke Shumaker
2021-04-30 23:25 ` [PATCH v4 3/5] git-fast-export.txt: clarify why 'verbatim' may not be a good idea Luke Shumaker
` (3 subsequent siblings)
5 siblings, 0 replies; 60+ messages in thread
From: Luke Shumaker @ 2021-04-30 23:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Elijah Newren, Jeff King, Johannes Schindelin,
Nguyễn Thái Ngọc Duy, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker
From: Luke Shumaker <lukeshu@datawire.io>
The --signed-tags= option takes one of five arguments specifying how to
handle signed tags during export. Among these arguments, 'strip' is to
'warn-strip' as 'verbatim' is to 'warn' (the unmentioned argument is
'abort', which stops the fast-export process entirely). That is,
signatures are either stripped or copied verbatim while exporting, with
or without a warning.
Match the pattern and rename 'warn' to 'warn-verbatim' to make it clear
that it instructs fast-export to copy signatures verbatim.
To maintain backwards compatibility, 'warn' is still recognized as
deprecated synonym of 'warn-verbatim'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
Notes:
v2:
- Reword commit message based on feedback from Taylor.
- Fix copy-pasto in the test, noticed by Taylor.
- Add a comment to the tests.
- Fix whitespace in the tests.
v3:
- Document that --signed-tags='warn' is a deprecated synonym for
--signed-tags='warn-verbatim', rather than leaving it
undocumented, based on feedback from Eric.
v4:
- Don't give the "deprecated synonym" mention in the docs its own
paragraph.
- Don't just rename the user-facing string, also rename the internal
enum item from WARN to WARN_VERBATIM.
Documentation/git-fast-export.txt | 6 +++---
builtin/fast-export.c | 8 ++++----
t/t9350-fast-export.sh | 18 ++++++++++++++++++
3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 1978dbdc6a..593be7e9a2 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -27,7 +27,7 @@ OPTIONS
Insert 'progress' statements every <n> objects, to be shown by
'git fast-import' during import.
---signed-tags=(verbatim|warn|warn-strip|strip|abort)::
+--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed tags. Since any transformation
after the export can change the tag names (which can also happen
when excluding revisions) the signatures will not match.
@@ -36,8 +36,8 @@ When asking to 'abort' (which is the default), this program will die
when encountering a signed tag. With 'strip', the tags will silently
be made unsigned, with 'warn-strip' they will be made unsigned but a
warning will be displayed, with 'verbatim', they will be silently
-exported and with 'warn', they will be exported, but you will see a
-warning.
+exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
+they will be exported, but you will see a warning.
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 85a76e0ef8..d1cb8a3183 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -31,7 +31,7 @@ static const char *fast_export_usage[] = {
};
static int progress;
-static enum { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+static enum { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
static enum { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
static enum { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
@@ -55,8 +55,8 @@ static int parse_opt_signed_tag_mode(const struct option *opt,
signed_tag_mode = SIGNED_TAG_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
signed_tag_mode = VERBATIM;
- else if (!strcmp(arg, "warn"))
- signed_tag_mode = WARN;
+ else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+ signed_tag_mode = WARN_VERBATIM;
else if (!strcmp(arg, "warn-strip"))
signed_tag_mode = WARN_STRIP;
else if (!strcmp(arg, "strip"))
@@ -834,7 +834,7 @@ static void handle_tag(const char *name, struct tag *tag)
die("encountered signed tag %s; use "
"--signed-tags=<mode> to handle it",
oid_to_hex(&tag->object.oid));
- case WARN:
+ case WARN_VERBATIM:
warning("exporting signed tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 409b48e244..892737439b 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -253,6 +253,24 @@ test_expect_success 'signed-tags=verbatim' '
'
+test_expect_success 'signed-tags=warn-verbatim' '
+
+ git fast-export --signed-tags=warn-verbatim sign-your-name >output 2>err &&
+ grep PGP output &&
+ test -s err
+
+'
+
+# 'warn' is an backward-compatibility alias for 'warn-verbatim'; test
+# that it keeps working.
+test_expect_success 'signed-tags=warn' '
+
+ git fast-export --signed-tags=warn sign-your-name >output 2>err &&
+ grep PGP output &&
+ test -s err
+
+'
+
test_expect_success 'signed-tags=strip' '
git fast-export --signed-tags=strip sign-your-name > output &&
--
2.31.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v4 3/5] git-fast-export.txt: clarify why 'verbatim' may not be a good idea
2021-04-30 23:25 ` [PATCH v4 0/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
2021-04-30 23:25 ` [PATCH v4 1/5] git-fast-import.txt: add missing LF in the BNF Luke Shumaker
2021-04-30 23:25 ` [PATCH v4 2/5] fast-export: rename --signed-tags='warn' to 'warn-verbatim' Luke Shumaker
@ 2021-04-30 23:25 ` Luke Shumaker
2021-04-30 23:25 ` [PATCH v4 4/5] fast-export: do not modify memory from get_commit_buffer Luke Shumaker
` (2 subsequent siblings)
5 siblings, 0 replies; 60+ messages in thread
From: Luke Shumaker @ 2021-04-30 23:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Elijah Newren, Jeff King, Johannes Schindelin,
Nguyễn Thái Ngọc Duy, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker
From: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
Notes:
v4: This commit is new in v4.
Documentation/git-fast-export.txt | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 593be7e9a2..a364812d9f 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -29,15 +29,19 @@ OPTIONS
--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed tags. Since any transformation
- after the export can change the tag names (which can also happen
- when excluding revisions) the signatures will not match.
+ after the export (or during the export, such as excluding
+ revisions) can change the hashes being signed, the signatures
+ may become invalid.
+
When asking to 'abort' (which is the default), this program will die
when encountering a signed tag. With 'strip', the tags will silently
be made unsigned, with 'warn-strip' they will be made unsigned but a
warning will be displayed, with 'verbatim', they will be silently
exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
-they will be exported, but you will see a warning.
+they will be exported, but you will see a warning. 'verbatim' and
+'warn-verbatim' should only be used if you know that no
+transformations affecting tags will be performed, or if you do not
+care that the resulting tag will have an invalid signature.
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
--
2.31.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v4 4/5] fast-export: do not modify memory from get_commit_buffer
2021-04-30 23:25 ` [PATCH v4 0/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
` (2 preceding siblings ...)
2021-04-30 23:25 ` [PATCH v4 3/5] git-fast-export.txt: clarify why 'verbatim' may not be a good idea Luke Shumaker
@ 2021-04-30 23:25 ` Luke Shumaker
2021-05-03 4:41 ` Junio C Hamano
2021-04-30 23:25 ` [PATCH v4 5/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
5 siblings, 1 reply; 60+ messages in thread
From: Luke Shumaker @ 2021-04-30 23:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Elijah Newren, Jeff King, Johannes Schindelin,
Nguyễn Thái Ngọc Duy, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker
From: Luke Shumaker <lukeshu@datawire.io>
fast-export's helper function find_encoding() takes a `const char *`, but
modifies that memory despite the `const`. Ultimately, this memory came
from get_commit_buffer(), and you're not supposed to modify the memory
that you get from get_commit_buffer().
So, get rid of find_encoding() in favor of commit.h:find_commit_header(),
which gives back a string length, rather than mutating the memory to
insert a '\0' terminator.
Because find_commit_header() detects the "\n\n" string that separates the
headers and the commit message, move the call to be above the
`message = strstr(..., "\n\n")` call. This helps readability, and allows
for the value of `encoding` to be used for a better value of "..." so that
the same memory doesn't need to be checked twice. Introduce a
`commit_buffer_cursor` variable to avoid writing an awkward
`encoding ? encoding + encoding_len : committer_end` expression.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
Notes:
v4: This commit is new in v4.
builtin/fast-export.c | 65 ++++++++++++++++++++++++-------------------
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d1cb8a3183..81f3fb1f05 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -499,21 +499,6 @@ static void show_filemodify(struct diff_queue_struct *q,
}
}
-static const char *find_encoding(const char *begin, const char *end)
-{
- const char *needle = "\nencoding ";
- char *bol, *eol;
-
- bol = memmem(begin, end ? end - begin : strlen(begin),
- needle, strlen(needle));
- if (!bol)
- return NULL;
- bol += strlen(needle);
- eol = strchrnul(bol, '\n');
- *eol = '\0';
- return bol;
-}
-
static char *anonymize_ref_component(void *data)
{
static int counter;
@@ -615,13 +600,26 @@ static void anonymize_ident_line(const char **beg, const char **end)
*end = out->buf + out->len;
}
+static char *reencode_message(const char *in_msg,
+ const char *in_encoding, size_t in_encoding_len)
+{
+ static struct strbuf in_encoding_buf = STRBUF_INIT;
+
+ strbuf_reset(&in_encoding_buf);
+ strbuf_add(&in_encoding_buf, in_encoding, in_encoding_len);
+
+ return reencode_string(in_msg, "UTF-8", in_encoding_buf.buf);
+}
+
static void handle_commit(struct commit *commit, struct rev_info *rev,
struct string_list *paths_of_changed_objects)
{
int saved_output_format = rev->diffopt.output_format;
- const char *commit_buffer;
+ const char *commit_buffer, *commit_buffer_cursor;
const char *author, *author_end, *committer, *committer_end;
- const char *encoding, *message;
+ const char *encoding;
+ size_t encoding_len;
+ const char *message;
char *reencoded = NULL;
struct commit_list *p;
const char *refname;
@@ -630,21 +628,31 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
rev->diffopt.output_format = DIFF_FORMAT_CALLBACK;
parse_commit_or_die(commit);
- commit_buffer = get_commit_buffer(commit, NULL);
- author = strstr(commit_buffer, "\nauthor ");
+ commit_buffer_cursor = commit_buffer = get_commit_buffer(commit, NULL);
+
+ author = strstr(commit_buffer_cursor, "\nauthor ");
if (!author)
die("could not find author in commit %s",
oid_to_hex(&commit->object.oid));
author++;
- author_end = strchrnul(author, '\n');
- committer = strstr(author_end, "\ncommitter ");
+ commit_buffer_cursor = author_end = strchrnul(author, '\n');
+
+ committer = strstr(commit_buffer_cursor, "\ncommitter ");
if (!committer)
die("could not find committer in commit %s",
oid_to_hex(&commit->object.oid));
committer++;
- committer_end = strchrnul(committer, '\n');
- message = strstr(committer_end, "\n\n");
- encoding = find_encoding(committer_end, message);
+ commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
+
+ /* find_commit_header() gets a `+ 1` because
+ * commit_buffer_cursor points at the trailing "\n" at the end
+ * of the previous line, but find_commit_header() wants a
+ * pointer to the beginning of the next line. */
+ encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
+ if (encoding)
+ commit_buffer_cursor = encoding + encoding_len;
+
+ message = strstr(commit_buffer_cursor, "\n\n");
if (message)
message += 2;
@@ -685,14 +693,15 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
} else if (encoding) {
switch(reencode_mode) {
case REENCODE_YES:
- reencoded = reencode_string(message, "UTF-8", encoding);
+ reencoded = reencode_message(message, encoding, encoding_len);
break;
case REENCODE_NO:
break;
case REENCODE_ABORT:
- die("Encountered commit-specific encoding %s in commit "
+ die("Encountered commit-specific encoding %.*s in commit "
"%s; use --reencode=[yes|no] to handle it",
- encoding, oid_to_hex(&commit->object.oid));
+ (int)encoding_len, encoding,
+ oid_to_hex(&commit->object.oid));
}
}
if (!commit->parents)
@@ -704,7 +713,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
(int)(author_end - author), author,
(int)(committer_end - committer), committer);
if (!reencoded && encoding)
- printf("encoding %s\n", encoding);
+ printf("encoding %.*s\n", (int)encoding_len, encoding);
printf("data %u\n%s",
(unsigned)(reencoded
? strlen(reencoded) : message
--
2.31.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v4 4/5] fast-export: do not modify memory from get_commit_buffer
2021-04-30 23:25 ` [PATCH v4 4/5] fast-export: do not modify memory from get_commit_buffer Luke Shumaker
@ 2021-05-03 4:41 ` Junio C Hamano
0 siblings, 0 replies; 60+ messages in thread
From: Junio C Hamano @ 2021-05-03 4:41 UTC (permalink / raw)
To: Luke Shumaker
Cc: git, Elijah Newren, Jeff King, Johannes Schindelin,
Nguyễn Thái Ngọc Duy, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker
Luke Shumaker <lukeshu@lukeshu.com> writes:
> +static char *reencode_message(const char *in_msg,
> + const char *in_encoding, size_t in_encoding_len)
> +{
> + static struct strbuf in_encoding_buf = STRBUF_INIT;
> +
> + strbuf_reset(&in_encoding_buf);
> + strbuf_add(&in_encoding_buf, in_encoding, in_encoding_len);
> +
> + return reencode_string(in_msg, "UTF-8", in_encoding_buf.buf);
> +}
There is only a single caller of this, so making it caller's
responsibility to do the strbuf thing would allow us to make this
thread-safe quite easily (and at that point we might not even have
this helper function).
> + committer = strstr(commit_buffer_cursor, "\ncommitter ");
> if (!committer)
> die("could not find committer in commit %s",
> oid_to_hex(&commit->object.oid));
> committer++;
> - committer_end = strchrnul(committer, '\n');
> - message = strstr(committer_end, "\n\n");
> - encoding = find_encoding(committer_end, message);
> + commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
> +
> + /* find_commit_header() gets a `+ 1` because
> + * commit_buffer_cursor points at the trailing "\n" at the end
> + * of the previous line, but find_commit_header() wants a
> + * pointer to the beginning of the next line. */
> + encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
/*
* Our multi-line comments have opening and closing
* slash-asterisk and asterisk-slash on their own
* lines.
*/
What if strchrnul() returned a pointer to the terminating NUL
instead of the LF at the end of the line? +1 will run past the end
of the buffer.
> + if (encoding)
> + commit_buffer_cursor = encoding + encoding_len;
> +
> + message = strstr(commit_buffer_cursor, "\n\n");
Good.
> @@ -685,14 +693,15 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
> } else if (encoding) {
> switch(reencode_mode) {
> case REENCODE_YES:
> - reencoded = reencode_string(message, "UTF-8", encoding);
> + reencoded = reencode_message(message, encoding, encoding_len);
> break;
Here is where we can do the temporary strbuf to hold encoding[0,
encoding_len] and directly call reencode_string().
Other than that, this step looks good to me.
Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v4 5/5] fast-export, fast-import: add support for signed-commits
2021-04-30 23:25 ` [PATCH v4 0/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
` (3 preceding siblings ...)
2021-04-30 23:25 ` [PATCH v4 4/5] fast-export: do not modify memory from get_commit_buffer Luke Shumaker
@ 2021-04-30 23:25 ` Luke Shumaker
2021-05-03 5:09 ` Junio C Hamano
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
5 siblings, 1 reply; 60+ messages in thread
From: Luke Shumaker @ 2021-04-30 23:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Elijah Newren, Jeff King, Johannes Schindelin,
Nguyễn Thái Ngọc Duy, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker
From: Luke Shumaker <lukeshu@datawire.io>
fast-export has a --signed-tags= option that controls how to handle tag
signatures. However, there is no equivalent for commit signatures; it
just silently strips the signature out of the commit (analogously to
--signed-tags=strip).
While signatures are generally problematic for fast-export/fast-import
(because hashes are likely to change), if they're going to support tag
signatures, there's no reason to not also support commit signatures.
So, implement a --signed-commits= option that mirrors the --signed-tags=
option.
On the fast-export side, try to be as much like signed-tags as possible,
in both implementation and in user-interface. This will changes the
default behavior to '--signed-commits=abort' from what is now
'--signed-commits=strip'. In order to provide an escape hatch for users
of third-party tools that call fast-export and do not yet know of the
--signed-commits= option, add an environment variable
'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' that changes the default to
'--signed-commits=warn-strip'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
Notes:
v2:
- Remove erroneous remark about ordering from the commit message.
- Adjust the stream syntax to include the hash algorithm, as
suggested by brian.
- Add support for sha256 (based on lots of useful information from
brian). It does not support multiply-signed commits.
- Shorten the documentation, based on feedback from Taylor.
- Add comments, based on feedback from Taylor.
- Change the default from `--signed-commits=strip` to
`--signed-commits=warn-strip`. This shouldn't break anyone, and
means that users get useful feedback by default.
v3: no changes
v4:
- Reword the commit message based on feedback from Junio.
- v1-v3 renamed enum items to SIGN_VERBATIM_WARN and SIGN_STRIP_WARN,
rename them to SIGN_WARN_VERBATIM and SIGN_WARN_STRIP instead.
- Rewrite find_signature() as find_commit_multiline_header(). Don't
have it butcher the memory that we pass to it; have it return its
own buffer.
- Change the default from `--signed-commits=warn-strip` to
`--signed-commits=abort`, to match `--signed-tags`.
- Add a FAST_EXPORT_SIGNED_COMMITS_NOABORT=1 env-var to change the
default to `--signed-commits=warn-strip`.
Documentation/git-fast-export.txt | 11 +++
Documentation/git-fast-import.txt | 18 +++++
builtin/fast-export.c | 120 +++++++++++++++++++++++++-----
builtin/fast-import.c | 23 ++++++
t/t9350-fast-export.sh | 86 +++++++++++++++++++++
5 files changed, 240 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index a364812d9f..7a946e2ede 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -43,6 +43,17 @@ they will be exported, but you will see a warning. 'verbatim' and
transformations affecting tags will be performed, or if you do not
care that the resulting tag will have an invalid signature.
+--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
+ Specify how to handle signed commits. Behaves exactly as
+ '--signed-tags', but for commits.
++
+Earlier versions this command that did not have '--signed-commits'
+behaved as if '--signed-commits=strip'. As an escape hatch for users
+of tools that call 'git fast-export' but do not yet support
+'--signed-commits', you may set the environment variable
+'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' in order to change the default
+from 'abort' to 'warn-strip'.
+
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
Since revisions and files to export can be limited by path,
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 458af0a2d6..4955c94305 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -431,12 +431,21 @@ and control the current import process. More detailed discussion
Create or update a branch with a new commit, recording one logical
change to the project.
+////
+Yes, it's intentional that the 'gpgsig' line doesn't have a trailing
+`LF`; the the definition of `data` has a byte-count prefix, so it
+doesn't need an `LF` to act as a terminator (and `data` also already
+includes an optional trailing `LF?` just in case you want to include
+one).
+////
+
....
'commit' SP <ref> LF
mark?
original-oid?
('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
'committer' (SP <name>)? SP LT <email> GT SP <when> LF
+ ('gpgsig' SP <alg> LF data)?
('encoding' SP <encoding> LF)?
data
('from' SP <commit-ish> LF)?
@@ -505,6 +514,15 @@ that was selected by the --date-format=<fmt> command-line option.
See ``Date Formats'' above for the set of supported formats, and
their syntax.
+`gpgsig`
+^^^^^^^^
+
+The optional `gpgsig` command is used to include a PGP/GPG signature
+that signs the commit data.
+
+Here <alg> specifies which hashing algorithm is used for this
+signature, either `sha1` or `sha256`.
+
`encoding`
^^^^^^^^^^
The optional `encoding` command indicates the encoding of the commit
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 81f3fb1f05..075630f185 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -30,8 +30,11 @@ static const char *fast_export_usage[] = {
NULL
};
+enum sign_mode { SIGN_ABORT, SIGN_VERBATIM, SIGN_STRIP, SIGN_WARN_VERBATIM, SIGN_WARN_STRIP };
+
static int progress;
-static enum { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+static enum sign_mode signed_tag_mode = SIGN_ABORT;
+static enum sign_mode signed_commit_mode = SIGN_ABORT;
static enum { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
static enum { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
@@ -48,21 +51,24 @@ static int anonymize;
static struct hashmap anonymized_seeds;
static struct revision_sources revision_sources;
-static int parse_opt_signed_tag_mode(const struct option *opt,
+static int parse_opt_sign_mode(const struct option *opt,
const char *arg, int unset)
{
- if (unset || !strcmp(arg, "abort"))
- signed_tag_mode = SIGNED_TAG_ABORT;
+ enum sign_mode *valptr = opt->value;
+ if (unset)
+ return 0;
+ else if (!strcmp(arg, "abort"))
+ *valptr = SIGN_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
- signed_tag_mode = VERBATIM;
+ *valptr = SIGN_VERBATIM;
else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
- signed_tag_mode = WARN_VERBATIM;
+ *valptr = SIGN_WARN_VERBATIM;
else if (!strcmp(arg, "warn-strip"))
- signed_tag_mode = WARN_STRIP;
+ *valptr = SIGN_WARN_STRIP;
else if (!strcmp(arg, "strip"))
- signed_tag_mode = STRIP;
+ *valptr = SIGN_STRIP;
else
- return error("Unknown signed-tags mode: %s", arg);
+ return error("Unknown %s mode: %s", opt->long_name, arg);
return 0;
}
@@ -600,6 +606,46 @@ static void anonymize_ident_line(const char **beg, const char **end)
*end = out->buf + out->len;
}
+/*
+ * find_commit_multiline_header is similar to find_commit_header,
+ * except that it handles multi-line headers, rathar than simply
+ * returning the first line of the header.
+ *
+ * The returned string has had the ' ' line continuation markers
+ * removed, and points to staticly allocated memory (not to memory
+ * within 'msg'), so it is only valid until the next call to
+ * find_commit_multiline_header.
+ *
+ * If the header is found, then *end is set to point at the '\n' in
+ * msg that immediately follows the header value.
+ */
+static const char *find_commit_multiline_header(const char *msg,
+ const char *key,
+ const char **end)
+{
+ static struct strbuf val = STRBUF_INIT;
+ const char *bol, *eol;
+ size_t len;
+
+ strbuf_reset(&val);
+
+ bol = find_commit_header(msg, key, &len);
+ if (!bol)
+ return NULL;
+ eol = bol + len;
+ strbuf_add(&val, bol, len);
+
+ while (eol[0] == '\n' && eol[1] == ' ') {
+ bol = eol + 2;
+ eol = strchrnul(bol, '\n');
+ strbuf_addch(&val, '\n');
+ strbuf_add(&val, bol, eol - bol);
+ }
+
+ *end = eol;
+ return val.buf;
+}
+
static char *reencode_message(const char *in_msg,
const char *in_encoding, size_t in_encoding_len)
{
@@ -619,6 +665,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
const char *author, *author_end, *committer, *committer_end;
const char *encoding;
size_t encoding_len;
+ const char *signature_alg = NULL, *signature;
const char *message;
char *reencoded = NULL;
struct commit_list *p;
@@ -644,14 +691,20 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
committer++;
commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
- /* find_commit_header() gets a `+ 1` because
- * commit_buffer_cursor points at the trailing "\n" at the end
- * of the previous line, but find_commit_header() wants a
+ /* find_commit_header() and find_commit_multiline_header() get
+ * a `+ 1` because commit_buffer_cursor points at the trailing
+ * "\n" at the end of the previous line, but they want a
* pointer to the beginning of the next line. */
+
encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
if (encoding)
commit_buffer_cursor = encoding + encoding_len;
+ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
+ signature_alg = "sha1";
+ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
+ signature_alg = "sha256";
+
message = strstr(commit_buffer_cursor, "\n\n");
if (message)
message += 2;
@@ -712,6 +765,29 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
printf("%.*s\n%.*s\n",
(int)(author_end - author), author,
(int)(committer_end - committer), committer);
+ if (signature)
+ switch(signed_commit_mode) {
+ case SIGN_ABORT:
+ die("encountered signed commit %s; use "
+ "--signed-commits=<mode> to handle it",
+ oid_to_hex(&commit->object.oid));
+ case SIGN_WARN_VERBATIM:
+ warning("exporting signed commit %s",
+ oid_to_hex(&commit->object.oid));
+ /* fallthru */
+ case SIGN_VERBATIM:
+ printf("gpgsig %s\ndata %u\n%s",
+ signature_alg,
+ (unsigned)strlen(signature),
+ signature);
+ break;
+ case SIGN_WARN_STRIP:
+ warning("stripping signature from commit %s",
+ oid_to_hex(&commit->object.oid));
+ /* fallthru */
+ case SIGN_STRIP:
+ break;
+ }
if (!reencoded && encoding)
printf("encoding %.*s\n", (int)encoding_len, encoding);
printf("data %u\n%s",
@@ -839,21 +915,21 @@ static void handle_tag(const char *name, struct tag *tag)
"\n-----BEGIN PGP SIGNATURE-----\n");
if (signature)
switch(signed_tag_mode) {
- case SIGNED_TAG_ABORT:
+ case SIGN_ABORT:
die("encountered signed tag %s; use "
"--signed-tags=<mode> to handle it",
oid_to_hex(&tag->object.oid));
- case WARN_VERBATIM:
+ case SIGN_WARN_VERBATIM:
warning("exporting signed tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
- case VERBATIM:
+ case SIGN_VERBATIM:
break;
- case WARN_STRIP:
+ case SIGN_WARN_STRIP:
warning("stripping signature from tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
- case STRIP:
+ case SIGN_STRIP:
message_size = signature + 1 - message;
break;
}
@@ -1192,6 +1268,7 @@ static int parse_opt_anonymize_map(const struct option *opt,
int cmd_fast_export(int argc, const char **argv, const char *prefix)
{
+ const char *env_signed_commits_noabort;
struct rev_info revs;
struct object_array commits = OBJECT_ARRAY_INIT;
struct commit *commit;
@@ -1206,7 +1283,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
N_("show progress after <n> objects")),
OPT_CALLBACK(0, "signed-tags", &signed_tag_mode, N_("mode"),
N_("select handling of signed tags"),
- parse_opt_signed_tag_mode),
+ parse_opt_sign_mode),
+ OPT_CALLBACK(0, "signed-commits", &signed_commit_mode, N_("mode"),
+ N_("select handling of signed commits"),
+ parse_opt_sign_mode),
OPT_CALLBACK(0, "tag-of-filtered-object", &tag_of_filtered_mode, N_("mode"),
N_("select handling of tags that tag filtered objects"),
parse_opt_tag_of_filtered_mode),
@@ -1247,6 +1327,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
if (argc == 1)
usage_with_options (fast_export_usage, options);
+ env_signed_commits_noabort = getenv("FAST_EXPORT_SIGNED_COMMITS_NOABORT");
+ if (env_signed_commits_noabort && *env_signed_commits_noabort)
+ signed_commit_mode = SIGN_WARN_STRIP;
+
/* we handle encodings */
git_config(git_default_config, NULL);
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 3afa81cf9a..ee7516dd38 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2669,10 +2669,13 @@ static struct hash_list *parse_merge(unsigned int *count)
static void parse_new_commit(const char *arg)
{
+ static struct strbuf sig = STRBUF_INIT;
static struct strbuf msg = STRBUF_INIT;
+ struct string_list siglines = STRING_LIST_INIT_NODUP;
struct branch *b;
char *author = NULL;
char *committer = NULL;
+ char *sig_alg = NULL;
char *encoding = NULL;
struct hash_list *merge_list = NULL;
unsigned int merge_count;
@@ -2696,6 +2699,13 @@ static void parse_new_commit(const char *arg)
}
if (!committer)
die("Expected committer but didn't get one");
+ if (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
+ sig_alg = xstrdup(v);
+ read_next_command();
+ parse_data(&sig, 0, NULL);
+ read_next_command();
+ } else
+ strbuf_setlen(&sig, 0);
if (skip_prefix(command_buf.buf, "encoding ", &v)) {
encoding = xstrdup(v);
read_next_command();
@@ -2769,10 +2779,23 @@ static void parse_new_commit(const char *arg)
strbuf_addf(&new_data,
"encoding %s\n",
encoding);
+ if (sig_alg) {
+ if (!strcmp(sig_alg, "sha1"))
+ strbuf_addstr(&new_data, "gpgsig ");
+ else if (!strcmp(sig_alg, "sha256"))
+ strbuf_addstr(&new_data, "gpgsig-sha256 ");
+ else
+ die("Expected gpgsig algorithm sha1 or sha256, got %s", sig_alg);
+ string_list_split_in_place(&siglines, sig.buf, '\n', -1);
+ strbuf_add_separated_string_list(&new_data, "\n ", &siglines);
+ strbuf_addch(&new_data, '\n');
+ }
strbuf_addch(&new_data, '\n');
strbuf_addbuf(&new_data, &msg);
+ string_list_clear(&siglines, 1);
free(author);
free(committer);
+ free(sig_alg);
free(encoding);
if (!store_object(OBJ_COMMIT, &new_data, NULL, &b->oid, next_mark))
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 892737439b..cd51c78418 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -8,6 +8,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
test_expect_success 'setup' '
@@ -284,9 +285,94 @@ test_expect_success 'signed-tags=warn-strip' '
test -s err
'
+test_expect_success GPG 'set up signed commit' '
+
+ # Generate a commit with both "gpgsig" and "encoding" set, so
+ # that we can test that fast-import gets the ordering correct
+ # between the two.
+ test_config i18n.commitEncoding ISO-8859-1 &&
+ git checkout -f -b commit-signing main &&
+ echo Sign your name > file-sign &&
+ git add file-sign &&
+ git commit -S -m "signed commit" &&
+ COMMIT_SIGNING=$(git rev-parse --verify commit-signing)
+
+'
+
+test_expect_success GPG 'signed-commits default' '
+
+ unset FAST_EXPORT_SIGNED_COMMITS_NOABORT &&
+ test_must_fail git fast-export --reencode=no commit-signing &&
+
+ FAST_EXPORT_SIGNED_COMMITS_NOABORT=1 git fast-export --reencode=no commit-signing >output 2>err &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ test -s err &&
+ sed "s/commit-signing/commit-strip-signing/" output |
+ (cd new &&
+ git fast-import &&
+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
+
+'
+
+test_expect_success GPG 'signed-commits=abort' '
+
+ test_must_fail git fast-export --signed-commits=abort commit-signing
+
+'
+
+test_expect_success GPG 'signed-commits=verbatim' '
+
+ git fast-export --signed-commits=verbatim --reencode=no commit-signing >output &&
+ grep "^gpgsig sha" output &&
+ grep "encoding ISO-8859-1" output &&
+ (cd new &&
+ git fast-import &&
+ test $COMMIT_SIGNING = $(git rev-parse --verify refs/heads/commit-signing)) <output
+
+'
+
+test_expect_success GPG 'signed-commits=warn-verbatim' '
+
+ git fast-export --signed-commits=warn-verbatim --reencode=no commit-signing >output 2>err &&
+ grep "^gpgsig sha" output &&
+ grep "encoding ISO-8859-1" output &&
+ test -s err &&
+ (cd new &&
+ git fast-import &&
+ test $COMMIT_SIGNING = $(git rev-parse --verify refs/heads/commit-signing)) <output
+
+'
+
+test_expect_success GPG 'signed-commits=strip' '
+
+ git fast-export --signed-commits=strip --reencode=no commit-signing >output &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ sed "s/commit-signing/commit-strip-signing/" output |
+ (cd new &&
+ git fast-import &&
+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
+
+'
+
+test_expect_success GPG 'signed-commits=warn-strip' '
+
+ git fast-export --signed-commits=warn-strip --reencode=no commit-signing >output 2>err &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ test -s err &&
+ sed "s/commit-signing/commit-strip-signing/" output |
+ (cd new &&
+ git fast-import &&
+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
+
+'
+
test_expect_success 'setup submodule' '
git checkout -f main &&
+ { git update-ref -d refs/heads/commit-signing || true; } &&
mkdir sub &&
(
cd sub &&
--
2.31.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v4 5/5] fast-export, fast-import: add support for signed-commits
2021-04-30 23:25 ` [PATCH v4 5/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
@ 2021-05-03 5:09 ` Junio C Hamano
0 siblings, 0 replies; 60+ messages in thread
From: Junio C Hamano @ 2021-05-03 5:09 UTC (permalink / raw)
To: Luke Shumaker
Cc: git, Elijah Newren, Jeff King, Johannes Schindelin,
Nguyễn Thái Ngọc Duy, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker
Luke Shumaker <lukeshu@lukeshu.com> writes:
> From: Luke Shumaker <lukeshu@datawire.io>
>
> fast-export has a --signed-tags= option that controls how to handle tag
> signatures. However, there is no equivalent for commit signatures; it
> just silently strips the signature out of the commit (analogously to
> --signed-tags=strip).
>
> While signatures are generally problematic for fast-export/fast-import
> (because hashes are likely to change), if they're going to support tag
> signatures, there's no reason to not also support commit signatures.
>
> So, implement a --signed-commits= option that mirrors the --signed-tags=
> option.
>
> On the fast-export side, try to be as much like signed-tags as possible,
> in both implementation and in user-interface. This will changes the
s/changes/change/;
> default behavior to '--signed-commits=abort' from what is now
> '--signed-commits=strip'. In order to provide an escape hatch for users
> of third-party tools that call fast-export and do not yet know of the
> --signed-commits= option, add an environment variable
> 'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' that changes the default to
> '--signed-commits=warn-strip'.
Nicely explained.
> +static const char *find_commit_multiline_header(const char *msg,
> + const char *key,
> + const char **end)
> +{
> + static struct strbuf val = STRBUF_INIT;
> + const char *bol, *eol;
> + size_t len;
> +
> + strbuf_reset(&val);
> +
> + bol = find_commit_header(msg, key, &len);
> + if (!bol)
> + return NULL;
> + eol = bol + len;
> + strbuf_add(&val, bol, len);
> +
> + while (eol[0] == '\n' && eol[1] == ' ') {
> + bol = eol + 2;
> + eol = strchrnul(bol, '\n');
> + strbuf_addch(&val, '\n');
> + strbuf_add(&val, bol, eol - bol);
> + }
> +
> + *end = eol;
> + return val.buf;
It is not exactly wrong per se, but using non-static (on stack)
strbuf would make it easier to follow. You can then lose the
strbuf_reset() upfront, and then this will call strbuf_detach().
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 892737439b..cd51c78418 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -8,6 +8,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> . ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-gpg.sh"
>
> test_expect_success 'setup' '
>
> @@ -284,9 +285,94 @@ test_expect_success 'signed-tags=warn-strip' '
> test -s err
> '
>
> +test_expect_success GPG 'set up signed commit' '
> +
> + # Generate a commit with both "gpgsig" and "encoding" set, so
> + # that we can test that fast-import gets the ordering correct
> + # between the two.
> + test_config i18n.commitEncoding ISO-8859-1 &&
> + git checkout -f -b commit-signing main &&
> + echo Sign your name > file-sign &&
Style. >file-sign (lose SP between the redirection operator and its
operand).
> + git add file-sign &&
> + git commit -S -m "signed commit" &&
> + COMMIT_SIGNING=$(git rev-parse --verify commit-signing)
> +
> +'
> +
> +test_expect_success GPG 'signed-commits default' '
> +
> + unset FAST_EXPORT_SIGNED_COMMITS_NOABORT &&
sane_unset would be safer here.
> + test_must_fail git fast-export --reencode=no commit-signing &&
> +
> + FAST_EXPORT_SIGNED_COMMITS_NOABORT=1 git fast-export --reencode=no commit-signing >output 2>err &&
> + ! grep ^gpgsig output &&
> + grep "^encoding ISO-8859-1" output &&
> + test -s err &&
> + sed "s/commit-signing/commit-strip-signing/" output |
> + (cd new &&
> + git fast-import &&
> + test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
Let's not force readers to match nested parentheses visually
(applies to multiple places in this patch):
sed "s/commit-signing/commit-strip-signing/" output | (
cd new &&
git fast-import &&
STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
test $COMMIT_SIGNING != $STRIPPED
)
> test_expect_success 'setup submodule' '
>
> git checkout -f main &&
> + { git update-ref -d refs/heads/commit-signing || true; } &&
test_might_fail git update-ref -d refs/heads/commit-signing &&
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2021-04-30 23:25 ` [PATCH v4 0/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
` (4 preceding siblings ...)
2021-04-30 23:25 ` [PATCH v4 5/5] fast-export, fast-import: add support for signed-commits Luke Shumaker
@ 2025-02-24 14:27 ` Christian Couder
2025-02-24 14:27 ` [PATCH v5 1/6] git-fast-import.adoc: add missing LF in the BNF Christian Couder
` (8 more replies)
5 siblings, 9 replies; 60+ messages in thread
From: Christian Couder @ 2025-02-24 14:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Christian Couder
Luke Shumaker sent the first 4 versions of this series in April 2021,
but it looks like he stopped before it got merged. Let's finish
polishing it.
Goal
~~~~
fast-export has an existing --signed-tags= option that controls how to
handle tag signatures. However, there is no equivalent for commit
signatures; it just silently strips the signature out of the commit
(analogously to --signed-tags=strip).
So implement a --signed-commits= flag in fast-export, and implement
the receiving side of it in fast-import.
Overview of the changes since v4
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This revision addresses all the feedback from v4 and has a few small
improvements that I came accross when rebasing it on top of current
master and working on it.
This doesn't address the following (that weren't addressed either by
earlier versions of this series) though:
- Elijah's suggestion to implement a flag on fast-import to validate
signatures. This could be a useful feature, but Luke considered
it was beyond the scope of this work, and I agree with him.
- The added tests still use `test -s err`, as that's still what's
used by the other existing tests. I would be fine with adding a
preparatory patch to address this, if people think it would be
worth it. On the other hand, it could be part of a small separate
series to modernize the whole "t/t9350-fast-export.sh".
Details of the changes since v4
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- Rebased on top of b838bf1938 (Merge branch 'master' of
https://github.com/j6t/gitk, 2025-02-20) to fix a few conflicts
and avoid issues with file name changes, as v4 was based on
v2.31.0 which is very old.
- Added patch 2/6 (fast-export: fix missing whitespace after switch)
as a preparatory patch to fix a style issue related to 'switch'
statements in "builtin/fast-export.c".
- In patch 3/6 (fast-export: rename --signed-tags='warn' to
'warn-verbatim'), fixed an "a" vs "an" typo in
"t9350-fast-export.sh".
- In patch 4/6 (git-fast-export.txt: clarify why 'verbatim' may not
be a good idea), fixed a "transformation" vs "transformations"
typo in "git-fast-export.adoc".
- In patch 5/6 (fast-export: do not modify memory from
get_commit_buffer) there are a few small changes in
"builtin/fast-export.c" that were suggested by Junio:
* removed reencode_message() and instead put the encoding code in
the `case REENCODE_YES:` in handle_commit(),
* fixed multi-line comment style,
* fixed potential past end of buffer read by surrounding some code
with `if (*commit_buffer_cursor == '\n') { ... }`.
- In patch 6/6 (fast-export, fast-import: add support for
signed-commits) there are a number of small changes:
* some typo fixes:
- "changes" vs "change" in the commit message,
- "the the" vs "the" in "git-fast-import.adoc",
- "staticly" vs "statically" in a comment in "builtin/fast-export.c",
* some code changes, all suggested by Junio except the last one,
in "builtin/fast-export.c":
- made a 'strbuf' non-static in anonymize_ident_line(),
- fixed potential past end of buffer read by surrounding some code
with `if (*commit_buffer_cursor == '\n') { ... }`,
- added a `free((char *)signature)` call to avoid a memory leak
found by the CI tests,
* some test improvements suggested by Junio in
"t/t9350-fast-export.sh":
- removed whitespace between ">" and "file-sign",
- replaced `unset` with `sane_unset`,
- better indented lines where a `( ... )` subshell is used,
- replaced `{ ... || true }` with `test_might_fail`.
CI tests
~~~~~~~~
All the CI tests passed, see:
https://github.com/chriscool/git/actions/runs/13496792476
Range diff compared to version 4
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1: 53d8dd60ce ! 1: f97247e17d git-fast-import.txt: add missing LF in the BNF
@@ Metadata
Author: Luke Shumaker <lukeshu@datawire.io>
## Commit message ##
- git-fast-import.txt: add missing LF in the BNF
+ git-fast-import.adoc: add missing LF in the BNF
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
+ Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
- ## Documentation/git-fast-import.txt ##
-@@ Documentation/git-fast-import.txt: change to the project.
+ ## Documentation/git-fast-import.adoc ##
+@@ Documentation/git-fast-import.adoc: change to the project.
original-oid?
('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
'committer' (SP <name>)? SP LT <email> GT SP <when> LF
-: ---------- > 2: b71588563d fast-export: fix missing whitespace after switch
2: 454a58a398 ! 3: 947bc267e6 fast-export: rename --signed-tags='warn' to 'warn-verbatim'
@@ Commit message
deprecated synonym of 'warn-verbatim'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
+ Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
- ## Documentation/git-fast-export.txt ##
-@@ Documentation/git-fast-export.txt: OPTIONS
+ ## Documentation/git-fast-export.adoc ##
+@@ Documentation/git-fast-export.adoc: OPTIONS
Insert 'progress' statements every <n> objects, to be shown by
'git fast-import' during import.
@@ Documentation/git-fast-export.txt: OPTIONS
Specify how to handle signed tags. Since any transformation
after the export can change the tag names (which can also happen
when excluding revisions) the signatures will not match.
-@@ Documentation/git-fast-export.txt: When asking to 'abort' (which is the default), this program will die
+@@ Documentation/git-fast-export.adoc: When asking to 'abort' (which is the default), this program will die
when encountering a signed tag. With 'strip', the tags will silently
be made unsigned, with 'warn-strip' they will be made unsigned but a
warning will be displayed, with 'verbatim', they will be silently
@@ builtin/fast-export.c: static const char *fast_export_usage[] = {
};
static int progress;
--static enum { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
-+static enum { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
- static enum { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
- static enum { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
+-static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
++static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+ static enum tag_of_filtered_mode { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
+ static enum reencode_mode { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
@@ builtin/fast-export.c: static int parse_opt_signed_tag_mode(const struct option *opt,
- signed_tag_mode = SIGNED_TAG_ABORT;
+ *val = SIGNED_TAG_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
- signed_tag_mode = VERBATIM;
+ *val = VERBATIM;
- else if (!strcmp(arg, "warn"))
-- signed_tag_mode = WARN;
+- *val = WARN;
+ else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
-+ signed_tag_mode = WARN_VERBATIM;
++ *val = WARN_VERBATIM;
else if (!strcmp(arg, "warn-strip"))
- signed_tag_mode = WARN_STRIP;
+ *val = WARN_STRIP;
else if (!strcmp(arg, "strip"))
@@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
die("encountered signed tag %s; use "
@@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=verbatim' '
+
+'
+
-+# 'warn' is an backward-compatibility alias for 'warn-verbatim'; test
++# 'warn' is a backward-compatibility alias for 'warn-verbatim'; test
+# that it keeps working.
+test_expect_success 'signed-tags=warn' '
+
3: ee0d84c34a ! 4: 45087db345 git-fast-export.txt: clarify why 'verbatim' may not be a good idea
@@ Commit message
git-fast-export.txt: clarify why 'verbatim' may not be a good idea
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
+ Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
- ## Documentation/git-fast-export.txt ##
-@@ Documentation/git-fast-export.txt: OPTIONS
+ ## Documentation/git-fast-export.adoc ##
+@@ Documentation/git-fast-export.adoc: OPTIONS
--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed tags. Since any transformation
@@ Documentation/git-fast-export.txt: OPTIONS
-they will be exported, but you will see a warning.
+they will be exported, but you will see a warning. 'verbatim' and
+'warn-verbatim' should only be used if you know that no
-+transformations affecting tags will be performed, or if you do not
++transformation affecting tags will be performed, or if you do not
+care that the resulting tag will have an invalid signature.
--tag-of-filtered-object=(abort|drop|rewrite)::
4: 36463ee3a8 ! 5: 20f085a790 fast-export: do not modify memory from get_commit_buffer
@@ Commit message
`encoding ? encoding + encoding_len : committer_end` expression.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
+ Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
## builtin/fast-export.c ##
@@ builtin/fast-export.c: static void show_filemodify(struct diff_queue_struct *q,
@@ builtin/fast-export.c: static void show_filemodify(struct diff_queue_struct *q,
- return bol;
-}
-
- static char *anonymize_ref_component(void *data)
+ static char *anonymize_ref_component(void)
{
static int counter;
-@@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const char **end)
- *end = out->buf + out->len;
- }
-
-+static char *reencode_message(const char *in_msg,
-+ const char *in_encoding, size_t in_encoding_len)
-+{
-+ static struct strbuf in_encoding_buf = STRBUF_INIT;
-+
-+ strbuf_reset(&in_encoding_buf);
-+ strbuf_add(&in_encoding_buf, in_encoding, in_encoding_len);
-+
-+ return reencode_string(in_msg, "UTF-8", in_encoding_buf.buf);
-+}
-+
- static void handle_commit(struct commit *commit, struct rev_info *rev,
+@@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,
struct string_list *paths_of_changed_objects)
{
int saved_output_format = rev->diffopt.output_format;
@@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const
+ const char *commit_buffer, *commit_buffer_cursor;
const char *author, *author_end, *committer, *committer_end;
- const char *encoding, *message;
-+ const char *encoding;
++ const char *encoding = NULL;
+ size_t encoding_len;
+ const char *message;
char *reencoded = NULL;
@@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
rev->diffopt.output_format = DIFF_FORMAT_CALLBACK;
parse_commit_or_die(commit);
-- commit_buffer = get_commit_buffer(commit, NULL);
+- commit_buffer = repo_get_commit_buffer(the_repository, commit, NULL);
- author = strstr(commit_buffer, "\nauthor ");
-+ commit_buffer_cursor = commit_buffer = get_commit_buffer(commit, NULL);
++ commit_buffer_cursor = commit_buffer = repo_get_commit_buffer(the_repository, commit, NULL);
+
+ author = strstr(commit_buffer_cursor, "\nauthor ");
if (!author)
@@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
- encoding = find_encoding(committer_end, message);
+ commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
+
-+ /* find_commit_header() gets a `+ 1` because
++ /*
++ * find_commit_header() gets a `+ 1` because
+ * commit_buffer_cursor points at the trailing "\n" at the end
+ * of the previous line, but find_commit_header() wants a
-+ * pointer to the beginning of the next line. */
-+ encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
-+ if (encoding)
-+ commit_buffer_cursor = encoding + encoding_len;
++ * pointer to the beginning of the next line.
++ */
++ if (*commit_buffer_cursor == '\n') {
++ encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
++ if (encoding)
++ commit_buffer_cursor = encoding + encoding_len;
++ }
+
+ message = strstr(commit_buffer_cursor, "\n\n");
if (message)
message += 2;
@@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,
+ if (anonymize) {
+ reencoded = anonymize_commit_message();
} else if (encoding) {
- switch(reencode_mode) {
++ char *buf;
+ switch (reencode_mode) {
case REENCODE_YES:
- reencoded = reencode_string(message, "UTF-8", encoding);
-+ reencoded = reencode_message(message, encoding, encoding_len);
++ buf = xstrfmt("%.*s", (int)encoding_len, encoding);
++ reencoded = reencode_string(message, "UTF-8", buf);
++ free(buf);
break;
case REENCODE_NO:
break;
5: 8ff33e2e88 ! 6: 48e0d4203c fast-export, fast-import: add support for signed-commits
@@ Commit message
option.
On the fast-export side, try to be as much like signed-tags as possible,
- in both implementation and in user-interface. This will changes the
+ in both implementation and in user-interface. This will change the
default behavior to '--signed-commits=abort' from what is now
'--signed-commits=strip'. In order to provide an escape hatch for users
of third-party tools that call fast-export and do not yet know of the
@@ Commit message
'--signed-commits=warn-strip'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
+ Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
- ## Documentation/git-fast-export.txt ##
-@@ Documentation/git-fast-export.txt: they will be exported, but you will see a warning. 'verbatim' and
- transformations affecting tags will be performed, or if you do not
+ ## Documentation/git-fast-export.adoc ##
+@@ Documentation/git-fast-export.adoc: they will be exported, but you will see a warning. 'verbatim' and
+ transformation affecting tags will be performed, or if you do not
care that the resulting tag will have an invalid signature.
+--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
@@ Documentation/git-fast-export.txt: they will be exported, but you will see a war
Specify how to handle tags whose tagged object is filtered out.
Since revisions and files to export can be limited by path,
- ## Documentation/git-fast-import.txt ##
-@@ Documentation/git-fast-import.txt: and control the current import process. More detailed discussion
+ ## Documentation/git-fast-import.adoc ##
+@@ Documentation/git-fast-import.adoc: and control the current import process. More detailed discussion
Create or update a branch with a new commit, recording one logical
change to the project.
+////
+Yes, it's intentional that the 'gpgsig' line doesn't have a trailing
-+`LF`; the the definition of `data` has a byte-count prefix, so it
++`LF`; the definition of `data` has a byte-count prefix, so it
+doesn't need an `LF` to act as a terminator (and `data` also already
+includes an optional trailing `LF?` just in case you want to include
+one).
@@ Documentation/git-fast-import.txt: and control the current import process. More
('encoding' SP <encoding> LF)?
data
('from' SP <commit-ish> LF)?
-@@ Documentation/git-fast-import.txt: that was selected by the --date-format=<fmt> command-line option.
+@@ Documentation/git-fast-import.adoc: that was selected by the --date-format=<fmt> command-line option.
See ``Date Formats'' above for the set of supported formats, and
their syntax.
@@ builtin/fast-export.c: static const char *fast_export_usage[] = {
+enum sign_mode { SIGN_ABORT, SIGN_VERBATIM, SIGN_STRIP, SIGN_WARN_VERBATIM, SIGN_WARN_STRIP };
+
static int progress;
--static enum { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+-static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+static enum sign_mode signed_tag_mode = SIGN_ABORT;
+static enum sign_mode signed_commit_mode = SIGN_ABORT;
- static enum { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
- static enum { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
+ static enum tag_of_filtered_mode { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
+ static enum reencode_mode { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
@@ builtin/fast-export.c: static int anonymize;
static struct hashmap anonymized_seeds;
@@ builtin/fast-export.c: static int anonymize;
+static int parse_opt_sign_mode(const struct option *opt,
const char *arg, int unset)
{
+- enum signed_tag_mode *val = opt->value;
+-
- if (unset || !strcmp(arg, "abort"))
-- signed_tag_mode = SIGNED_TAG_ABORT;
-+ enum sign_mode *valptr = opt->value;
+- *val = SIGNED_TAG_ABORT;
++ enum sign_mode *val = opt->value;
+ if (unset)
+ return 0;
+ else if (!strcmp(arg, "abort"))
-+ *valptr = SIGN_ABORT;
++ *val = SIGN_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
-- signed_tag_mode = VERBATIM;
-+ *valptr = SIGN_VERBATIM;
+- *val = VERBATIM;
++ *val = SIGN_VERBATIM;
else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
-- signed_tag_mode = WARN_VERBATIM;
-+ *valptr = SIGN_WARN_VERBATIM;
+- *val = WARN_VERBATIM;
++ *val = SIGN_WARN_VERBATIM;
else if (!strcmp(arg, "warn-strip"))
-- signed_tag_mode = WARN_STRIP;
-+ *valptr = SIGN_WARN_STRIP;
+- *val = WARN_STRIP;
++ *val = SIGN_WARN_STRIP;
else if (!strcmp(arg, "strip"))
-- signed_tag_mode = STRIP;
-+ *valptr = SIGN_STRIP;
+- *val = STRIP;
++ *val = SIGN_STRIP;
else
- return error("Unknown signed-tags mode: %s", arg);
+ return error("Unknown %s mode: %s", opt->long_name, arg);
@@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const
+ * returning the first line of the header.
+ *
+ * The returned string has had the ' ' line continuation markers
-+ * removed, and points to staticly allocated memory (not to memory
++ * removed, and points to statically allocated memory (not to memory
+ * within 'msg'), so it is only valid until the next call to
+ * find_commit_multiline_header.
+ *
@@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const
+ const char *key,
+ const char **end)
+{
-+ static struct strbuf val = STRBUF_INIT;
++ struct strbuf val = STRBUF_INIT;
+ const char *bol, *eol;
+ size_t len;
+
-+ strbuf_reset(&val);
-+
+ bol = find_commit_header(msg, key, &len);
+ if (!bol)
+ return NULL;
@@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const
+ }
+
+ *end = eol;
-+ return val.buf;
++ return strbuf_detach(&val, NULL);
+}
+
- static char *reencode_message(const char *in_msg,
- const char *in_encoding, size_t in_encoding_len)
+ static void handle_commit(struct commit *commit, struct rev_info *rev,
+ struct string_list *paths_of_changed_objects)
{
@@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,
const char *author, *author_end, *committer, *committer_end;
- const char *encoding;
+ const char *encoding = NULL;
size_t encoding_len;
-+ const char *signature_alg = NULL, *signature;
++ const char *signature_alg = NULL, *signature = NULL;
const char *message;
char *reencoded = NULL;
struct commit_list *p;
@@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,
- committer++;
commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
-- /* find_commit_header() gets a `+ 1` because
+ /*
+- * find_commit_header() gets a `+ 1` because
- * commit_buffer_cursor points at the trailing "\n" at the end
- * of the previous line, but find_commit_header() wants a
-+ /* find_commit_header() and find_commit_multiline_header() get
++ * find_commit_header() and find_commit_multiline_header() get
+ * a `+ 1` because commit_buffer_cursor points at the trailing
+ * "\n" at the end of the previous line, but they want a
- * pointer to the beginning of the next line. */
+ * pointer to the beginning of the next line.
+ */
+
- encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
- if (encoding)
- commit_buffer_cursor = encoding + encoding_len;
+ if (*commit_buffer_cursor == '\n') {
+ encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
+ if (encoding)
+ commit_buffer_cursor = encoding + encoding_len;
+ }
-+ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
-+ signature_alg = "sha1";
-+ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
-+ signature_alg = "sha256";
++ if (*commit_buffer_cursor == '\n') {
++ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
++ signature_alg = "sha1";
++ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
++ signature_alg = "sha256";
++ }
+
message = strstr(commit_buffer_cursor, "\n\n");
if (message)
@@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
printf("%.*s\n%.*s\n",
(int)(author_end - author), author,
(int)(committer_end - committer), committer);
-+ if (signature)
-+ switch(signed_commit_mode) {
++ if (signature) {
++ switch (signed_commit_mode) {
+ case SIGN_ABORT:
+ die("encountered signed commit %s; use "
+ "--signed-commits=<mode> to handle it",
@@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
+ case SIGN_STRIP:
+ break;
+ }
++ free((char *)signature);
++ }
if (!reencoded && encoding)
printf("encoding %.*s\n", (int)encoding_len, encoding);
printf("data %u\n%s",
@@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
"\n-----BEGIN PGP SIGNATURE-----\n");
if (signature)
- switch(signed_tag_mode) {
+ switch (signed_tag_mode) {
- case SIGNED_TAG_ABORT:
+ case SIGN_ABORT:
die("encountered signed tag %s; use "
@@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
message_size = signature + 1 - message;
break;
}
-@@ builtin/fast-export.c: static int parse_opt_anonymize_map(const struct option *opt,
-
- int cmd_fast_export(int argc, const char **argv, const char *prefix)
+@@ builtin/fast-export.c: int cmd_fast_export(int argc,
+ const char *prefix,
+ struct repository *repo UNUSED)
{
+ const char *env_signed_commits_noabort;
struct rev_info revs;
- struct object_array commits = OBJECT_ARRAY_INIT;
struct commit *commit;
-@@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const char *prefix)
+ char *export_filename = NULL,
+@@ builtin/fast-export.c: int cmd_fast_export(int argc,
N_("show progress after <n> objects")),
OPT_CALLBACK(0, "signed-tags", &signed_tag_mode, N_("mode"),
N_("select handling of signed tags"),
@@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const ch
OPT_CALLBACK(0, "tag-of-filtered-object", &tag_of_filtered_mode, N_("mode"),
N_("select handling of tags that tag filtered objects"),
parse_opt_tag_of_filtered_mode),
-@@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const char *prefix)
+@@ builtin/fast-export.c: int cmd_fast_export(int argc,
if (argc == 1)
usage_with_options (fast_export_usage, options);
@@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
+ strbuf_addstr(&new_data, "gpgsig-sha256 ");
+ else
+ die("Expected gpgsig algorithm sha1 or sha256, got %s", sig_alg);
-+ string_list_split_in_place(&siglines, sig.buf, '\n', -1);
++ string_list_split_in_place(&siglines, sig.buf, "\n", -1);
+ strbuf_add_separated_string_list(&new_data, "\n ", &siglines);
+ strbuf_addch(&new_data, '\n');
+ }
@@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
+ # between the two.
+ test_config i18n.commitEncoding ISO-8859-1 &&
+ git checkout -f -b commit-signing main &&
-+ echo Sign your name > file-sign &&
++ echo Sign your name >file-sign &&
+ git add file-sign &&
+ git commit -S -m "signed commit" &&
+ COMMIT_SIGNING=$(git rev-parse --verify commit-signing)
@@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
+
+test_expect_success GPG 'signed-commits default' '
+
-+ unset FAST_EXPORT_SIGNED_COMMITS_NOABORT &&
++ sane_unset FAST_EXPORT_SIGNED_COMMITS_NOABORT &&
+ test_must_fail git fast-export --reencode=no commit-signing &&
+
+ FAST_EXPORT_SIGNED_COMMITS_NOABORT=1 git fast-export --reencode=no commit-signing >output 2>err &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ test -s err &&
-+ sed "s/commit-signing/commit-strip-signing/" output |
-+ (cd new &&
-+ git fast-import &&
-+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
++ sed "s/commit-signing/commit-strip-signing/" output | (
++ cd new &&
++ git fast-import &&
++ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
++ test $COMMIT_SIGNING != $STRIPPED
++ )
+
+'
+
@@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
+ git fast-export --signed-commits=verbatim --reencode=no commit-signing >output &&
+ grep "^gpgsig sha" output &&
+ grep "encoding ISO-8859-1" output &&
-+ (cd new &&
-+ git fast-import &&
-+ test $COMMIT_SIGNING = $(git rev-parse --verify refs/heads/commit-signing)) <output
++ (
++ cd new &&
++ git fast-import &&
++ STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
++ test $COMMIT_SIGNING = $STRIPPED
++ ) <output
+
+'
+
@@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
+ grep "^gpgsig sha" output &&
+ grep "encoding ISO-8859-1" output &&
+ test -s err &&
-+ (cd new &&
-+ git fast-import &&
-+ test $COMMIT_SIGNING = $(git rev-parse --verify refs/heads/commit-signing)) <output
++ (
++ cd new &&
++ git fast-import &&
++ STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
++ test $COMMIT_SIGNING = $STRIPPED
++ ) <output
+
+'
+
@@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
+ git fast-export --signed-commits=strip --reencode=no commit-signing >output &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
-+ sed "s/commit-signing/commit-strip-signing/" output |
-+ (cd new &&
-+ git fast-import &&
-+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
++ sed "s/commit-signing/commit-strip-signing/" output | (
++ cd new &&
++ git fast-import &&
++ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
++ test $COMMIT_SIGNING != $STRIPPED
++ )
+
+'
+
@@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ test -s err &&
-+ sed "s/commit-signing/commit-strip-signing/" output |
-+ (cd new &&
-+ git fast-import &&
-+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
++ sed "s/commit-signing/commit-strip-signing/" output | (
++ cd new &&
++ git fast-import &&
++ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
++ test $COMMIT_SIGNING != $STRIPPED
++ )
+
+'
+
test_expect_success 'setup submodule' '
+ test_config_global protocol.file.allow always &&
git checkout -f main &&
-+ { git update-ref -d refs/heads/commit-signing || true; } &&
++ test_might_fail git update-ref -d refs/heads/commit-signing &&
mkdir sub &&
(
cd sub &&
Christian Couder (1):
fast-export: fix missing whitespace after switch
Luke Shumaker (5):
git-fast-import.adoc: add missing LF in the BNF
fast-export: rename --signed-tags='warn' to 'warn-verbatim'
git-fast-export.txt: clarify why 'verbatim' may not be a good idea
fast-export: do not modify memory from get_commit_buffer
fast-export, fast-import: add support for signed-commits
Documentation/git-fast-export.adoc | 25 +++-
Documentation/git-fast-import.adoc | 20 ++-
builtin/fast-export.c | 189 +++++++++++++++++++++--------
builtin/fast-import.c | 23 ++++
t/t9350-fast-export.sh | 116 ++++++++++++++++++
5 files changed, 317 insertions(+), 56 deletions(-)
--
2.48.1.401.g48e0d4203c
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v5 1/6] git-fast-import.adoc: add missing LF in the BNF
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
@ 2025-02-24 14:27 ` Christian Couder
2025-02-24 14:27 ` [PATCH v5 2/6] fast-export: fix missing whitespace after switch Christian Couder
` (7 subsequent siblings)
8 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-02-24 14:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-import.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 58a2eaa51a..8e0de618c0 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -437,7 +437,7 @@ change to the project.
original-oid?
('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
'committer' (SP <name>)? SP LT <email> GT SP <when> LF
- ('encoding' SP <encoding>)?
+ ('encoding' SP <encoding> LF)?
data
('from' SP <commit-ish> LF)?
('merge' SP <commit-ish> LF)*
--
2.48.1.401.g48e0d4203c
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v5 2/6] fast-export: fix missing whitespace after switch
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
2025-02-24 14:27 ` [PATCH v5 1/6] git-fast-import.adoc: add missing LF in the BNF Christian Couder
@ 2025-02-24 14:27 ` Christian Couder
2025-02-24 14:27 ` [PATCH v5 3/6] fast-export: rename --signed-tags='warn' to 'warn-verbatim' Christian Couder
` (6 subsequent siblings)
8 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-02-24 14:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Christian Couder, Christian Couder
"Documentation/CodingGuidelines" says that there should be whitespaces
around operators like 'if', 'switch', 'for', etc.
Let's fix this in "builtin/fast-export.c".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/fast-export.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index a5c82eef1d..2bf787191a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -694,7 +694,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
if (anonymize) {
reencoded = anonymize_commit_message();
} else if (encoding) {
- switch(reencode_mode) {
+ switch (reencode_mode) {
case REENCODE_YES:
reencoded = reencode_string(message, "UTF-8", encoding);
break;
@@ -828,7 +828,7 @@ static void handle_tag(const char *name, struct tag *tag)
const char *signature = strstr(message,
"\n-----BEGIN PGP SIGNATURE-----\n");
if (signature)
- switch(signed_tag_mode) {
+ switch (signed_tag_mode) {
case SIGNED_TAG_ABORT:
die("encountered signed tag %s; use "
"--signed-tags=<mode> to handle it",
@@ -853,7 +853,7 @@ static void handle_tag(const char *name, struct tag *tag)
tagged = tag->tagged;
tagged_mark = get_object_mark(tagged);
if (!tagged_mark) {
- switch(tag_of_filtered_mode) {
+ switch (tag_of_filtered_mode) {
case TAG_FILTERING_ABORT:
die("tag %s tags unexported object; use "
"--tag-of-filtered-object=<mode> to handle it",
@@ -965,7 +965,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
continue;
}
- switch(commit->object.type) {
+ switch (commit->object.type) {
case OBJ_COMMIT:
break;
case OBJ_BLOB:
--
2.48.1.401.g48e0d4203c
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v5 3/6] fast-export: rename --signed-tags='warn' to 'warn-verbatim'
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
2025-02-24 14:27 ` [PATCH v5 1/6] git-fast-import.adoc: add missing LF in the BNF Christian Couder
2025-02-24 14:27 ` [PATCH v5 2/6] fast-export: fix missing whitespace after switch Christian Couder
@ 2025-02-24 14:27 ` Christian Couder
2025-02-24 14:27 ` [PATCH v5 4/6] git-fast-export.txt: clarify why 'verbatim' may not be a good idea Christian Couder
` (5 subsequent siblings)
8 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-02-24 14:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
The --signed-tags= option takes one of five arguments specifying how to
handle signed tags during export. Among these arguments, 'strip' is to
'warn-strip' as 'verbatim' is to 'warn' (the unmentioned argument is
'abort', which stops the fast-export process entirely). That is,
signatures are either stripped or copied verbatim while exporting, with
or without a warning.
Match the pattern and rename 'warn' to 'warn-verbatim' to make it clear
that it instructs fast-export to copy signatures verbatim.
To maintain backwards compatibility, 'warn' is still recognized as
deprecated synonym of 'warn-verbatim'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-export.adoc | 6 +++---
builtin/fast-export.c | 8 ++++----
t/t9350-fast-export.sh | 18 ++++++++++++++++++
3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
index 752e4b9b01..ab9a315fa9 100644
--- a/Documentation/git-fast-export.adoc
+++ b/Documentation/git-fast-export.adoc
@@ -27,7 +27,7 @@ OPTIONS
Insert 'progress' statements every <n> objects, to be shown by
'git fast-import' during import.
---signed-tags=(verbatim|warn|warn-strip|strip|abort)::
+--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed tags. Since any transformation
after the export can change the tag names (which can also happen
when excluding revisions) the signatures will not match.
@@ -36,8 +36,8 @@ When asking to 'abort' (which is the default), this program will die
when encountering a signed tag. With 'strip', the tags will silently
be made unsigned, with 'warn-strip' they will be made unsigned but a
warning will be displayed, with 'verbatim', they will be silently
-exported and with 'warn', they will be exported, but you will see a
-warning.
+exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
+they will be exported, but you will see a warning.
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 2bf787191a..2de2adc30e 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -36,7 +36,7 @@ static const char *fast_export_usage[] = {
};
static int progress;
-static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
static enum tag_of_filtered_mode { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
static enum reencode_mode { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
@@ -62,8 +62,8 @@ static int parse_opt_signed_tag_mode(const struct option *opt,
*val = SIGNED_TAG_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
*val = VERBATIM;
- else if (!strcmp(arg, "warn"))
- *val = WARN;
+ else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+ *val = WARN_VERBATIM;
else if (!strcmp(arg, "warn-strip"))
*val = WARN_STRIP;
else if (!strcmp(arg, "strip"))
@@ -833,7 +833,7 @@ static void handle_tag(const char *name, struct tag *tag)
die("encountered signed tag %s; use "
"--signed-tags=<mode> to handle it",
oid_to_hex(&tag->object.oid));
- case WARN:
+ case WARN_VERBATIM:
warning("exporting signed tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 40427883ec..cc110727fb 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -253,6 +253,24 @@ test_expect_success 'signed-tags=verbatim' '
'
+test_expect_success 'signed-tags=warn-verbatim' '
+
+ git fast-export --signed-tags=warn-verbatim sign-your-name >output 2>err &&
+ grep PGP output &&
+ test -s err
+
+'
+
+# 'warn' is a backward-compatibility alias for 'warn-verbatim'; test
+# that it keeps working.
+test_expect_success 'signed-tags=warn' '
+
+ git fast-export --signed-tags=warn sign-your-name >output 2>err &&
+ grep PGP output &&
+ test -s err
+
+'
+
test_expect_success 'signed-tags=strip' '
git fast-export --signed-tags=strip sign-your-name > output &&
--
2.48.1.401.g48e0d4203c
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v5 4/6] git-fast-export.txt: clarify why 'verbatim' may not be a good idea
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
` (2 preceding siblings ...)
2025-02-24 14:27 ` [PATCH v5 3/6] fast-export: rename --signed-tags='warn' to 'warn-verbatim' Christian Couder
@ 2025-02-24 14:27 ` Christian Couder
2025-02-24 19:26 ` Elijah Newren
2025-02-24 14:27 ` [PATCH v5 5/6] fast-export: do not modify memory from get_commit_buffer Christian Couder
` (4 subsequent siblings)
8 siblings, 1 reply; 60+ messages in thread
From: Christian Couder @ 2025-02-24 14:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-export.adoc | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
index ab9a315fa9..1b19f17b78 100644
--- a/Documentation/git-fast-export.adoc
+++ b/Documentation/git-fast-export.adoc
@@ -29,15 +29,19 @@ OPTIONS
--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed tags. Since any transformation
- after the export can change the tag names (which can also happen
- when excluding revisions) the signatures will not match.
+ after the export (or during the export, such as excluding
+ revisions) can change the hashes being signed, the signatures
+ may become invalid.
+
When asking to 'abort' (which is the default), this program will die
when encountering a signed tag. With 'strip', the tags will silently
be made unsigned, with 'warn-strip' they will be made unsigned but a
warning will be displayed, with 'verbatim', they will be silently
exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
-they will be exported, but you will see a warning.
+they will be exported, but you will see a warning. 'verbatim' and
+'warn-verbatim' should only be used if you know that no
+transformation affecting tags will be performed, or if you do not
+care that the resulting tag will have an invalid signature.
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
--
2.48.1.401.g48e0d4203c
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v5 4/6] git-fast-export.txt: clarify why 'verbatim' may not be a good idea
2025-02-24 14:27 ` [PATCH v5 4/6] git-fast-export.txt: clarify why 'verbatim' may not be a good idea Christian Couder
@ 2025-02-24 19:26 ` Elijah Newren
2025-03-10 15:58 ` Christian Couder
0 siblings, 1 reply; 60+ messages in thread
From: Elijah Newren @ 2025-02-24 19:26 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Jeff King,
Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Luke Shumaker, Christian Couder
On Mon, Feb 24, 2025 at 6:28 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> From: Luke Shumaker <lukeshu@datawire.io>
>
> Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> Documentation/git-fast-export.adoc | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
> index ab9a315fa9..1b19f17b78 100644
> --- a/Documentation/git-fast-export.adoc
> +++ b/Documentation/git-fast-export.adoc
> @@ -29,15 +29,19 @@ OPTIONS
>
> --signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> Specify how to handle signed tags. Since any transformation
> - after the export can change the tag names (which can also happen
> - when excluding revisions) the signatures will not match.
> + after the export (or during the export, such as excluding
> + revisions) can change the hashes being signed, the signatures
> + may become invalid.
> +
> When asking to 'abort' (which is the default), this program will die
> when encountering a signed tag. With 'strip', the tags will silently
> be made unsigned, with 'warn-strip' they will be made unsigned but a
> warning will be displayed, with 'verbatim', they will be silently
> exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
> -they will be exported, but you will see a warning.
> +they will be exported, but you will see a warning. 'verbatim' and
> +'warn-verbatim' should only be used if you know that no
> +transformation affecting tags will be performed, or if you do not
perhaps it'd be worth clarifying this slightly to
"...transformation affecting tags or any commit in their history will
be performed..."
Although, I'm not sure if that's strong enough either. Even if users
don't transform the fast-export output, the fast-export output will
have already possibly undergone transformations and fast-import might
send it through more. For example, if someone had a permission
recorded as 644 or 100640 it'd be canonicalized to 100644. If they
had a duplicate tree entry or an improperly sorted tree in their
history, that would be corrected by fast-export + fast-import. If
they had extended headers other than a commit signature, those would
be dropped. So, maybe it needs to be something more like
"..transformation affecting tags or any commit in their history will
be performed by you or by fast-export or fast-import, or if you do
not....
> +care that the resulting tag will have an invalid signature.
>
> --tag-of-filtered-object=(abort|drop|rewrite)::
> Specify how to handle tags whose tagged object is filtered out.
> --
> 2.48.1.401.g48e0d4203c
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 4/6] git-fast-export.txt: clarify why 'verbatim' may not be a good idea
2025-02-24 19:26 ` Elijah Newren
@ 2025-03-10 15:58 ` Christian Couder
0 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:58 UTC (permalink / raw)
To: Elijah Newren
Cc: git, Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Jeff King,
Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Luke Shumaker, Christian Couder
On Mon, Feb 24, 2025 at 8:26 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Mon, Feb 24, 2025 at 6:28 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> >
> > From: Luke Shumaker <lukeshu@datawire.io>
> >
> > Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> > ---
> > Documentation/git-fast-export.adoc | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
> > index ab9a315fa9..1b19f17b78 100644
> > --- a/Documentation/git-fast-export.adoc
> > +++ b/Documentation/git-fast-export.adoc
> > @@ -29,15 +29,19 @@ OPTIONS
> >
> > --signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> > Specify how to handle signed tags. Since any transformation
> > - after the export can change the tag names (which can also happen
> > - when excluding revisions) the signatures will not match.
> > + after the export (or during the export, such as excluding
> > + revisions) can change the hashes being signed, the signatures
> > + may become invalid.
> > +
> > When asking to 'abort' (which is the default), this program will die
> > when encountering a signed tag. With 'strip', the tags will silently
> > be made unsigned, with 'warn-strip' they will be made unsigned but a
> > warning will be displayed, with 'verbatim', they will be silently
> > exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
> > -they will be exported, but you will see a warning.
> > +they will be exported, but you will see a warning. 'verbatim' and
> > +'warn-verbatim' should only be used if you know that no
> > +transformation affecting tags will be performed, or if you do not
>
> perhaps it'd be worth clarifying this slightly to
>
> "...transformation affecting tags or any commit in their history will
> be performed..."
>
> Although, I'm not sure if that's strong enough either. Even if users
> don't transform the fast-export output, the fast-export output will
> have already possibly undergone transformations and fast-import might
> send it through more. For example, if someone had a permission
> recorded as 644 or 100640 it'd be canonicalized to 100644. If they
> had a duplicate tree entry or an improperly sorted tree in their
> history, that would be corrected by fast-export + fast-import. If
> they had extended headers other than a commit signature, those would
> be dropped. So, maybe it needs to be something more like
>
> "..transformation affecting tags or any commit in their history will
> be performed by you or by fast-export or fast-import, or if you do
> not....
I agree it's better like this, so this is used in the next version.
> > +care that the resulting tag will have an invalid signature.
Thanks!
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v5 5/6] fast-export: do not modify memory from get_commit_buffer
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
` (3 preceding siblings ...)
2025-02-24 14:27 ` [PATCH v5 4/6] git-fast-export.txt: clarify why 'verbatim' may not be a good idea Christian Couder
@ 2025-02-24 14:27 ` Christian Couder
2025-02-24 14:27 ` [PATCH v5 6/6] fast-export, fast-import: add support for signed-commits Christian Couder
` (3 subsequent siblings)
8 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-02-24 14:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
fast-export's helper function find_encoding() takes a `const char *`, but
modifies that memory despite the `const`. Ultimately, this memory came
from get_commit_buffer(), and you're not supposed to modify the memory
that you get from get_commit_buffer().
So, get rid of find_encoding() in favor of commit.h:find_commit_header(),
which gives back a string length, rather than mutating the memory to
insert a '\0' terminator.
Because find_commit_header() detects the "\n\n" string that separates the
headers and the commit message, move the call to be above the
`message = strstr(..., "\n\n")` call. This helps readability, and allows
for the value of `encoding` to be used for a better value of "..." so that
the same memory doesn't need to be checked twice. Introduce a
`commit_buffer_cursor` variable to avoid writing an awkward
`encoding ? encoding + encoding_len : committer_end` expression.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/fast-export.c | 61 +++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 2de2adc30e..39d43c2a29 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -510,21 +510,6 @@ static void show_filemodify(struct diff_queue_struct *q,
}
}
-static const char *find_encoding(const char *begin, const char *end)
-{
- const char *needle = "\nencoding ";
- char *bol, *eol;
-
- bol = memmem(begin, end ? end - begin : strlen(begin),
- needle, strlen(needle));
- if (!bol)
- return NULL;
- bol += strlen(needle);
- eol = strchrnul(bol, '\n');
- *eol = '\0';
- return bol;
-}
-
static char *anonymize_ref_component(void)
{
static int counter;
@@ -630,9 +615,11 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
struct string_list *paths_of_changed_objects)
{
int saved_output_format = rev->diffopt.output_format;
- const char *commit_buffer;
+ const char *commit_buffer, *commit_buffer_cursor;
const char *author, *author_end, *committer, *committer_end;
- const char *encoding, *message;
+ const char *encoding = NULL;
+ size_t encoding_len;
+ const char *message;
char *reencoded = NULL;
struct commit_list *p;
const char *refname;
@@ -641,21 +628,35 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
rev->diffopt.output_format = DIFF_FORMAT_CALLBACK;
parse_commit_or_die(commit);
- commit_buffer = repo_get_commit_buffer(the_repository, commit, NULL);
- author = strstr(commit_buffer, "\nauthor ");
+ commit_buffer_cursor = commit_buffer = repo_get_commit_buffer(the_repository, commit, NULL);
+
+ author = strstr(commit_buffer_cursor, "\nauthor ");
if (!author)
die("could not find author in commit %s",
oid_to_hex(&commit->object.oid));
author++;
- author_end = strchrnul(author, '\n');
- committer = strstr(author_end, "\ncommitter ");
+ commit_buffer_cursor = author_end = strchrnul(author, '\n');
+
+ committer = strstr(commit_buffer_cursor, "\ncommitter ");
if (!committer)
die("could not find committer in commit %s",
oid_to_hex(&commit->object.oid));
committer++;
- committer_end = strchrnul(committer, '\n');
- message = strstr(committer_end, "\n\n");
- encoding = find_encoding(committer_end, message);
+ commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
+
+ /*
+ * find_commit_header() gets a `+ 1` because
+ * commit_buffer_cursor points at the trailing "\n" at the end
+ * of the previous line, but find_commit_header() wants a
+ * pointer to the beginning of the next line.
+ */
+ if (*commit_buffer_cursor == '\n') {
+ encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
+ if (encoding)
+ commit_buffer_cursor = encoding + encoding_len;
+ }
+
+ message = strstr(commit_buffer_cursor, "\n\n");
if (message)
message += 2;
@@ -694,16 +695,20 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
if (anonymize) {
reencoded = anonymize_commit_message();
} else if (encoding) {
+ char *buf;
switch (reencode_mode) {
case REENCODE_YES:
- reencoded = reencode_string(message, "UTF-8", encoding);
+ buf = xstrfmt("%.*s", (int)encoding_len, encoding);
+ reencoded = reencode_string(message, "UTF-8", buf);
+ free(buf);
break;
case REENCODE_NO:
break;
case REENCODE_ABORT:
- die("Encountered commit-specific encoding %s in commit "
+ die("Encountered commit-specific encoding %.*s in commit "
"%s; use --reencode=[yes|no] to handle it",
- encoding, oid_to_hex(&commit->object.oid));
+ (int)encoding_len, encoding,
+ oid_to_hex(&commit->object.oid));
}
}
if (!commit->parents)
@@ -715,7 +720,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
(int)(author_end - author), author,
(int)(committer_end - committer), committer);
if (!reencoded && encoding)
- printf("encoding %s\n", encoding);
+ printf("encoding %.*s\n", (int)encoding_len, encoding);
printf("data %u\n%s",
(unsigned)(reencoded
? strlen(reencoded) : message
--
2.48.1.401.g48e0d4203c
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v5 6/6] fast-export, fast-import: add support for signed-commits
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
` (4 preceding siblings ...)
2025-02-24 14:27 ` [PATCH v5 5/6] fast-export: do not modify memory from get_commit_buffer Christian Couder
@ 2025-02-24 14:27 ` Christian Couder
2025-02-25 7:35 ` Elijah Newren
2025-02-24 17:01 ` [PATCH v5 0/6] " Junio C Hamano
` (2 subsequent siblings)
8 siblings, 1 reply; 60+ messages in thread
From: Christian Couder @ 2025-02-24 14:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
fast-export has a --signed-tags= option that controls how to handle tag
signatures. However, there is no equivalent for commit signatures; it
just silently strips the signature out of the commit (analogously to
--signed-tags=strip).
While signatures are generally problematic for fast-export/fast-import
(because hashes are likely to change), if they're going to support tag
signatures, there's no reason to not also support commit signatures.
So, implement a --signed-commits= option that mirrors the --signed-tags=
option.
On the fast-export side, try to be as much like signed-tags as possible,
in both implementation and in user-interface. This will change the
default behavior to '--signed-commits=abort' from what is now
'--signed-commits=strip'. In order to provide an escape hatch for users
of third-party tools that call fast-export and do not yet know of the
--signed-commits= option, add an environment variable
'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' that changes the default to
'--signed-commits=warn-strip'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-export.adoc | 11 +++
Documentation/git-fast-import.adoc | 18 +++++
builtin/fast-export.c | 124 ++++++++++++++++++++++++-----
builtin/fast-import.c | 23 ++++++
t/t9350-fast-export.sh | 98 +++++++++++++++++++++++
5 files changed, 254 insertions(+), 20 deletions(-)
diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
index 1b19f17b78..8750dd150b 100644
--- a/Documentation/git-fast-export.adoc
+++ b/Documentation/git-fast-export.adoc
@@ -43,6 +43,17 @@ they will be exported, but you will see a warning. 'verbatim' and
transformation affecting tags will be performed, or if you do not
care that the resulting tag will have an invalid signature.
+--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
+ Specify how to handle signed commits. Behaves exactly as
+ '--signed-tags', but for commits.
++
+Earlier versions this command that did not have '--signed-commits'
+behaved as if '--signed-commits=strip'. As an escape hatch for users
+of tools that call 'git fast-export' but do not yet support
+'--signed-commits', you may set the environment variable
+'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' in order to change the default
+from 'abort' to 'warn-strip'.
+
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
Since revisions and files to export can be limited by path,
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 8e0de618c0..7b107f5e8e 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -431,12 +431,21 @@ and control the current import process. More detailed discussion
Create or update a branch with a new commit, recording one logical
change to the project.
+////
+Yes, it's intentional that the 'gpgsig' line doesn't have a trailing
+`LF`; the definition of `data` has a byte-count prefix, so it
+doesn't need an `LF` to act as a terminator (and `data` also already
+includes an optional trailing `LF?` just in case you want to include
+one).
+////
+
....
'commit' SP <ref> LF
mark?
original-oid?
('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
'committer' (SP <name>)? SP LT <email> GT SP <when> LF
+ ('gpgsig' SP <alg> LF data)?
('encoding' SP <encoding> LF)?
data
('from' SP <commit-ish> LF)?
@@ -505,6 +514,15 @@ that was selected by the --date-format=<fmt> command-line option.
See ``Date Formats'' above for the set of supported formats, and
their syntax.
+`gpgsig`
+^^^^^^^^
+
+The optional `gpgsig` command is used to include a PGP/GPG signature
+that signs the commit data.
+
+Here <alg> specifies which hashing algorithm is used for this
+signature, either `sha1` or `sha256`.
+
`encoding`
^^^^^^^^^^
The optional `encoding` command indicates the encoding of the commit
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 39d43c2a29..e34adb9ae8 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -35,8 +35,11 @@ static const char *fast_export_usage[] = {
NULL
};
+enum sign_mode { SIGN_ABORT, SIGN_VERBATIM, SIGN_STRIP, SIGN_WARN_VERBATIM, SIGN_WARN_STRIP };
+
static int progress;
-static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+static enum sign_mode signed_tag_mode = SIGN_ABORT;
+static enum sign_mode signed_commit_mode = SIGN_ABORT;
static enum tag_of_filtered_mode { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
static enum reencode_mode { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
@@ -53,23 +56,24 @@ static int anonymize;
static struct hashmap anonymized_seeds;
static struct revision_sources revision_sources;
-static int parse_opt_signed_tag_mode(const struct option *opt,
+static int parse_opt_sign_mode(const struct option *opt,
const char *arg, int unset)
{
- enum signed_tag_mode *val = opt->value;
-
- if (unset || !strcmp(arg, "abort"))
- *val = SIGNED_TAG_ABORT;
+ enum sign_mode *val = opt->value;
+ if (unset)
+ return 0;
+ else if (!strcmp(arg, "abort"))
+ *val = SIGN_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
- *val = VERBATIM;
+ *val = SIGN_VERBATIM;
else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
- *val = WARN_VERBATIM;
+ *val = SIGN_WARN_VERBATIM;
else if (!strcmp(arg, "warn-strip"))
- *val = WARN_STRIP;
+ *val = SIGN_WARN_STRIP;
else if (!strcmp(arg, "strip"))
- *val = STRIP;
+ *val = SIGN_STRIP;
else
- return error("Unknown signed-tags mode: %s", arg);
+ return error("Unknown %s mode: %s", opt->long_name, arg);
return 0;
}
@@ -611,6 +615,44 @@ static void anonymize_ident_line(const char **beg, const char **end)
*end = out->buf + out->len;
}
+/*
+ * find_commit_multiline_header is similar to find_commit_header,
+ * except that it handles multi-line headers, rathar than simply
+ * returning the first line of the header.
+ *
+ * The returned string has had the ' ' line continuation markers
+ * removed, and points to statically allocated memory (not to memory
+ * within 'msg'), so it is only valid until the next call to
+ * find_commit_multiline_header.
+ *
+ * If the header is found, then *end is set to point at the '\n' in
+ * msg that immediately follows the header value.
+ */
+static const char *find_commit_multiline_header(const char *msg,
+ const char *key,
+ const char **end)
+{
+ struct strbuf val = STRBUF_INIT;
+ const char *bol, *eol;
+ size_t len;
+
+ bol = find_commit_header(msg, key, &len);
+ if (!bol)
+ return NULL;
+ eol = bol + len;
+ strbuf_add(&val, bol, len);
+
+ while (eol[0] == '\n' && eol[1] == ' ') {
+ bol = eol + 2;
+ eol = strchrnul(bol, '\n');
+ strbuf_addch(&val, '\n');
+ strbuf_add(&val, bol, eol - bol);
+ }
+
+ *end = eol;
+ return strbuf_detach(&val, NULL);
+}
+
static void handle_commit(struct commit *commit, struct rev_info *rev,
struct string_list *paths_of_changed_objects)
{
@@ -619,6 +661,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
const char *author, *author_end, *committer, *committer_end;
const char *encoding = NULL;
size_t encoding_len;
+ const char *signature_alg = NULL, *signature = NULL;
const char *message;
char *reencoded = NULL;
struct commit_list *p;
@@ -645,17 +688,25 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
/*
- * find_commit_header() gets a `+ 1` because
- * commit_buffer_cursor points at the trailing "\n" at the end
- * of the previous line, but find_commit_header() wants a
+ * find_commit_header() and find_commit_multiline_header() get
+ * a `+ 1` because commit_buffer_cursor points at the trailing
+ * "\n" at the end of the previous line, but they want a
* pointer to the beginning of the next line.
*/
+
if (*commit_buffer_cursor == '\n') {
encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
if (encoding)
commit_buffer_cursor = encoding + encoding_len;
}
+ if (*commit_buffer_cursor == '\n') {
+ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
+ signature_alg = "sha1";
+ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
+ signature_alg = "sha256";
+ }
+
message = strstr(commit_buffer_cursor, "\n\n");
if (message)
message += 2;
@@ -719,6 +770,31 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
printf("%.*s\n%.*s\n",
(int)(author_end - author), author,
(int)(committer_end - committer), committer);
+ if (signature) {
+ switch (signed_commit_mode) {
+ case SIGN_ABORT:
+ die("encountered signed commit %s; use "
+ "--signed-commits=<mode> to handle it",
+ oid_to_hex(&commit->object.oid));
+ case SIGN_WARN_VERBATIM:
+ warning("exporting signed commit %s",
+ oid_to_hex(&commit->object.oid));
+ /* fallthru */
+ case SIGN_VERBATIM:
+ printf("gpgsig %s\ndata %u\n%s",
+ signature_alg,
+ (unsigned)strlen(signature),
+ signature);
+ break;
+ case SIGN_WARN_STRIP:
+ warning("stripping signature from commit %s",
+ oid_to_hex(&commit->object.oid));
+ /* fallthru */
+ case SIGN_STRIP:
+ break;
+ }
+ free((char *)signature);
+ }
if (!reencoded && encoding)
printf("encoding %.*s\n", (int)encoding_len, encoding);
printf("data %u\n%s",
@@ -834,21 +910,21 @@ static void handle_tag(const char *name, struct tag *tag)
"\n-----BEGIN PGP SIGNATURE-----\n");
if (signature)
switch (signed_tag_mode) {
- case SIGNED_TAG_ABORT:
+ case SIGN_ABORT:
die("encountered signed tag %s; use "
"--signed-tags=<mode> to handle it",
oid_to_hex(&tag->object.oid));
- case WARN_VERBATIM:
+ case SIGN_WARN_VERBATIM:
warning("exporting signed tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
- case VERBATIM:
+ case SIGN_VERBATIM:
break;
- case WARN_STRIP:
+ case SIGN_WARN_STRIP:
warning("stripping signature from tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
- case STRIP:
+ case SIGN_STRIP:
message_size = signature + 1 - message;
break;
}
@@ -1194,6 +1270,7 @@ int cmd_fast_export(int argc,
const char *prefix,
struct repository *repo UNUSED)
{
+ const char *env_signed_commits_noabort;
struct rev_info revs;
struct commit *commit;
char *export_filename = NULL,
@@ -1207,7 +1284,10 @@ int cmd_fast_export(int argc,
N_("show progress after <n> objects")),
OPT_CALLBACK(0, "signed-tags", &signed_tag_mode, N_("mode"),
N_("select handling of signed tags"),
- parse_opt_signed_tag_mode),
+ parse_opt_sign_mode),
+ OPT_CALLBACK(0, "signed-commits", &signed_commit_mode, N_("mode"),
+ N_("select handling of signed commits"),
+ parse_opt_sign_mode),
OPT_CALLBACK(0, "tag-of-filtered-object", &tag_of_filtered_mode, N_("mode"),
N_("select handling of tags that tag filtered objects"),
parse_opt_tag_of_filtered_mode),
@@ -1248,6 +1328,10 @@ int cmd_fast_export(int argc,
if (argc == 1)
usage_with_options (fast_export_usage, options);
+ env_signed_commits_noabort = getenv("FAST_EXPORT_SIGNED_COMMITS_NOABORT");
+ if (env_signed_commits_noabort && *env_signed_commits_noabort)
+ signed_commit_mode = SIGN_WARN_STRIP;
+
/* we handle encodings */
git_config(git_default_config, NULL);
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index d6a368a566..a5b33eb91e 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2719,10 +2719,13 @@ static struct hash_list *parse_merge(unsigned int *count)
static void parse_new_commit(const char *arg)
{
+ static struct strbuf sig = STRBUF_INIT;
static struct strbuf msg = STRBUF_INIT;
+ struct string_list siglines = STRING_LIST_INIT_NODUP;
struct branch *b;
char *author = NULL;
char *committer = NULL;
+ char *sig_alg = NULL;
char *encoding = NULL;
struct hash_list *merge_list = NULL;
unsigned int merge_count;
@@ -2746,6 +2749,13 @@ static void parse_new_commit(const char *arg)
}
if (!committer)
die("Expected committer but didn't get one");
+ if (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
+ sig_alg = xstrdup(v);
+ read_next_command();
+ parse_data(&sig, 0, NULL);
+ read_next_command();
+ } else
+ strbuf_setlen(&sig, 0);
if (skip_prefix(command_buf.buf, "encoding ", &v)) {
encoding = xstrdup(v);
read_next_command();
@@ -2819,10 +2829,23 @@ static void parse_new_commit(const char *arg)
strbuf_addf(&new_data,
"encoding %s\n",
encoding);
+ if (sig_alg) {
+ if (!strcmp(sig_alg, "sha1"))
+ strbuf_addstr(&new_data, "gpgsig ");
+ else if (!strcmp(sig_alg, "sha256"))
+ strbuf_addstr(&new_data, "gpgsig-sha256 ");
+ else
+ die("Expected gpgsig algorithm sha1 or sha256, got %s", sig_alg);
+ string_list_split_in_place(&siglines, sig.buf, "\n", -1);
+ strbuf_add_separated_string_list(&new_data, "\n ", &siglines);
+ strbuf_addch(&new_data, '\n');
+ }
strbuf_addch(&new_data, '\n');
strbuf_addbuf(&new_data, &msg);
+ string_list_clear(&siglines, 1);
free(author);
free(committer);
+ free(sig_alg);
free(encoding);
if (!store_object(OBJ_COMMIT, &new_data, NULL, &b->oid, next_mark))
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index cc110727fb..304bac5b1d 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -8,6 +8,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
test_expect_success 'setup' '
@@ -284,10 +285,107 @@ test_expect_success 'signed-tags=warn-strip' '
test -s err
'
+test_expect_success GPG 'set up signed commit' '
+
+ # Generate a commit with both "gpgsig" and "encoding" set, so
+ # that we can test that fast-import gets the ordering correct
+ # between the two.
+ test_config i18n.commitEncoding ISO-8859-1 &&
+ git checkout -f -b commit-signing main &&
+ echo Sign your name >file-sign &&
+ git add file-sign &&
+ git commit -S -m "signed commit" &&
+ COMMIT_SIGNING=$(git rev-parse --verify commit-signing)
+
+'
+
+test_expect_success GPG 'signed-commits default' '
+
+ sane_unset FAST_EXPORT_SIGNED_COMMITS_NOABORT &&
+ test_must_fail git fast-export --reencode=no commit-signing &&
+
+ FAST_EXPORT_SIGNED_COMMITS_NOABORT=1 git fast-export --reencode=no commit-signing >output 2>err &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ test -s err &&
+ sed "s/commit-signing/commit-strip-signing/" output | (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
+ test $COMMIT_SIGNING != $STRIPPED
+ )
+
+'
+
+test_expect_success GPG 'signed-commits=abort' '
+
+ test_must_fail git fast-export --signed-commits=abort commit-signing
+
+'
+
+test_expect_success GPG 'signed-commits=verbatim' '
+
+ git fast-export --signed-commits=verbatim --reencode=no commit-signing >output &&
+ grep "^gpgsig sha" output &&
+ grep "encoding ISO-8859-1" output &&
+ (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
+ test $COMMIT_SIGNING = $STRIPPED
+ ) <output
+
+'
+
+test_expect_success GPG 'signed-commits=warn-verbatim' '
+
+ git fast-export --signed-commits=warn-verbatim --reencode=no commit-signing >output 2>err &&
+ grep "^gpgsig sha" output &&
+ grep "encoding ISO-8859-1" output &&
+ test -s err &&
+ (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
+ test $COMMIT_SIGNING = $STRIPPED
+ ) <output
+
+'
+
+test_expect_success GPG 'signed-commits=strip' '
+
+ git fast-export --signed-commits=strip --reencode=no commit-signing >output &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ sed "s/commit-signing/commit-strip-signing/" output | (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
+ test $COMMIT_SIGNING != $STRIPPED
+ )
+
+'
+
+test_expect_success GPG 'signed-commits=warn-strip' '
+
+ git fast-export --signed-commits=warn-strip --reencode=no commit-signing >output 2>err &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ test -s err &&
+ sed "s/commit-signing/commit-strip-signing/" output | (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
+ test $COMMIT_SIGNING != $STRIPPED
+ )
+
+'
+
test_expect_success 'setup submodule' '
test_config_global protocol.file.allow always &&
git checkout -f main &&
+ test_might_fail git update-ref -d refs/heads/commit-signing &&
mkdir sub &&
(
cd sub &&
--
2.48.1.401.g48e0d4203c
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v5 6/6] fast-export, fast-import: add support for signed-commits
2025-02-24 14:27 ` [PATCH v5 6/6] fast-export, fast-import: add support for signed-commits Christian Couder
@ 2025-02-25 7:35 ` Elijah Newren
2025-02-25 16:25 ` Junio C Hamano
2025-03-10 15:58 ` Christian Couder
0 siblings, 2 replies; 60+ messages in thread
From: Elijah Newren @ 2025-02-25 7:35 UTC (permalink / raw)
To: Christian Couder
Cc: Git Mailing List, Junio C Hamano, Patrick Steinhardt,
Luke Shumaker, Jeff King, Johannes Schindelin, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker, Christian Couder
On Mon, Feb 24, 2025 at 6:28 AM Christian Couder
<christian.couder@gmail.com> wrote:
[...snip...]
> diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
> index 1b19f17b78..8750dd150b 100644
> --- a/Documentation/git-fast-export.adoc
> +++ b/Documentation/git-fast-export.adoc
> @@ -43,6 +43,17 @@ they will be exported, but you will see a warning. 'verbatim' and
> transformation affecting tags will be performed, or if you do not
> care that the resulting tag will have an invalid signature.
>
> +--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> + Specify how to handle signed commits. Behaves exactly as
> + '--signed-tags', but for commits.
Should this also explicitly call out that the default is abort? Yes,
I know that...
> ++
> +Earlier versions this command that did not have '--signed-commits'
> +behaved as if '--signed-commits=strip'. As an escape hatch for users
> +of tools that call 'git fast-export' but do not yet support
> +'--signed-commits', you may set the environment variable
> +'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' in order to change the default
> +from 'abort' to 'warn-strip'.
...this paragraph implies abort is the default, but I imagine we
eventually drop this paragraph, but
it'd still be useful to have the default called out.
[...snip...]
> @@ -611,6 +615,44 @@ static void anonymize_ident_line(const char **beg, const char **end)
> *end = out->buf + out->len;
> }
>
> +/*
> + * find_commit_multiline_header is similar to find_commit_header,
> + * except that it handles multi-line headers, rathar than simply
s/rathar/rather/
[...snip...]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 6/6] fast-export, fast-import: add support for signed-commits
2025-02-25 7:35 ` Elijah Newren
@ 2025-02-25 16:25 ` Junio C Hamano
2025-03-10 15:58 ` Christian Couder
1 sibling, 0 replies; 60+ messages in thread
From: Junio C Hamano @ 2025-02-25 16:25 UTC (permalink / raw)
To: Elijah Newren
Cc: Christian Couder, Git Mailing List, Patrick Steinhardt,
Luke Shumaker, Jeff King, Johannes Schindelin, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker, Christian Couder
Elijah Newren <newren@gmail.com> writes:
> On Mon, Feb 24, 2025 at 6:28 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> [...snip...]
>> diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
>> index 1b19f17b78..8750dd150b 100644
>> --- a/Documentation/git-fast-export.adoc
>> +++ b/Documentation/git-fast-export.adoc
>> @@ -43,6 +43,17 @@ they will be exported, but you will see a warning. 'verbatim' and
>> transformation affecting tags will be performed, or if you do not
>> care that the resulting tag will have an invalid signature.
>>
>> +--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
>> + Specify how to handle signed commits. Behaves exactly as
>> + '--signed-tags', but for commits.
>
> Should this also explicitly call out that the default is abort? Yes,
> I know that...
Thanks. We all tend to assume that readers know more than they
actually are reasonably expected to know.
I would have of course expected that any sensible designer would
pick 'abort' as the default, but I didn't know what we actually
chose without looking at the code ;-)
It would make sense to spell it out.
Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 6/6] fast-export, fast-import: add support for signed-commits
2025-02-25 7:35 ` Elijah Newren
2025-02-25 16:25 ` Junio C Hamano
@ 2025-03-10 15:58 ` Christian Couder
1 sibling, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:58 UTC (permalink / raw)
To: Elijah Newren
Cc: Git Mailing List, Junio C Hamano, Patrick Steinhardt,
Luke Shumaker, Jeff King, Johannes Schindelin, Taylor Blau,
brian m . carlson, Eric Sunshine, Luke Shumaker, Christian Couder
On Tue, Feb 25, 2025 at 8:36 AM Elijah Newren <newren@gmail.com> wrote:
>
> On Mon, Feb 24, 2025 at 6:28 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> [...snip...]
> > diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
> > index 1b19f17b78..8750dd150b 100644
> > --- a/Documentation/git-fast-export.adoc
> > +++ b/Documentation/git-fast-export.adoc
> > @@ -43,6 +43,17 @@ they will be exported, but you will see a warning. 'verbatim' and
> > transformation affecting tags will be performed, or if you do not
> > care that the resulting tag will have an invalid signature.
> >
> > +--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> > + Specify how to handle signed commits. Behaves exactly as
> > + '--signed-tags', but for commits.
>
> Should this also explicitly call out that the default is abort?
Yeah, that might help, so "Default is 'abort'." has been added in the
next version.
> Yes,
> I know that...
>
> > ++
> > +Earlier versions this command that did not have '--signed-commits'
> > +behaved as if '--signed-commits=strip'. As an escape hatch for users
> > +of tools that call 'git fast-export' but do not yet support
> > +'--signed-commits', you may set the environment variable
> > +'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' in order to change the default
> > +from 'abort' to 'warn-strip'.
>
> ...this paragraph implies abort is the default, but I imagine we
> eventually drop this paragraph, but
> it'd still be useful to have the default called out.
We could still rely on the fact that the doc above says "Behaves
exactly as '--signed-tags', but for commits." and the default for
'--signed-tags' is 'abort', but I agree that it can still help to
spell it out.
> [...snip...]
>
> > @@ -611,6 +615,44 @@ static void anonymize_ident_line(const char **beg, const char **end)
> > *end = out->buf + out->len;
> > }
> >
> > +/*
> > + * find_commit_multiline_header is similar to find_commit_header,
> > + * except that it handles multi-line headers, rathar than simply
>
> s/rathar/rather/
Fixed in the next version. Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
` (5 preceding siblings ...)
2025-02-24 14:27 ` [PATCH v5 6/6] fast-export, fast-import: add support for signed-commits Christian Couder
@ 2025-02-24 17:01 ` Junio C Hamano
2025-02-25 7:35 ` Elijah Newren
2025-02-25 14:53 ` Phillip Wood
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
8 siblings, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2025-02-24 17:01 UTC (permalink / raw)
To: Christian Couder
Cc: git, Patrick Steinhardt, Luke Shumaker, Elijah Newren, Jeff King,
Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine
Christian Couder <christian.couder@gmail.com> writes:
> Luke Shumaker sent the first 4 versions of this series in April 2021,
> but it looks like he stopped before it got merged. Let's finish
> polishing it.
Nice to see an old topic resurrected.
> fast-export has an existing --signed-tags= option that controls how to
> handle tag signatures. However, there is no equivalent for commit
> signatures; it just silently strips the signature out of the commit
> (analogously to --signed-tags=strip).
>
> So implement a --signed-commits= flag in fast-export, and implement
> the receiving side of it in fast-import.
Nice.
I haven't thought about this topic obviously for a looooong time,
but I wonder we may want to have an option, which is independent
from these --signed-tags/--signed-commits options addressed here,
that allows the person who performed the import to attest to the
result by adding their own signature on tags and commits, whether
these tags and commits were originally signed or not.
Obviously totally independent, orthogonal, and outside of the scope
of this topic.
Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2025-02-24 17:01 ` [PATCH v5 0/6] " Junio C Hamano
@ 2025-02-25 7:35 ` Elijah Newren
2025-02-25 7:51 ` Patrick Steinhardt
0 siblings, 1 reply; 60+ messages in thread
From: Elijah Newren @ 2025-02-25 7:35 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, git, Patrick Steinhardt, Luke Shumaker,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine
On Mon, Feb 24, 2025 at 9:01 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > Luke Shumaker sent the first 4 versions of this series in April 2021,
> > but it looks like he stopped before it got merged. Let's finish
> > polishing it.
>
> Nice to see an old topic resurrected.
>
> > fast-export has an existing --signed-tags= option that controls how to
> > handle tag signatures. However, there is no equivalent for commit
> > signatures; it just silently strips the signature out of the commit
> > (analogously to --signed-tags=strip).
> >
> > So implement a --signed-commits= flag in fast-export, and implement
> > the receiving side of it in fast-import.
>
> Nice.
>
> I haven't thought about this topic obviously for a looooong time,
> but I wonder we may want to have an option, which is independent
> from these --signed-tags/--signed-commits options addressed here,
> that allows the person who performed the import to attest to the
> result by adding their own signature on tags and commits, whether
> these tags and commits were originally signed or not.
For what it's worth, this has been requested multiple times of
git-filter-repo, so there is some desire for this feature.
> Obviously totally independent, orthogonal, and outside of the scope
> of this topic.
Agreed.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2025-02-25 7:35 ` Elijah Newren
@ 2025-02-25 7:51 ` Patrick Steinhardt
2025-02-25 16:48 ` Elijah Newren
0 siblings, 1 reply; 60+ messages in thread
From: Patrick Steinhardt @ 2025-02-25 7:51 UTC (permalink / raw)
To: Elijah Newren
Cc: Junio C Hamano, Christian Couder, git, Luke Shumaker, Jeff King,
Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine
On Mon, Feb 24, 2025 at 11:35:00PM -0800, Elijah Newren wrote:
> On Mon, Feb 24, 2025 at 9:01 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Christian Couder <christian.couder@gmail.com> writes:
> >
> > > Luke Shumaker sent the first 4 versions of this series in April 2021,
> > > but it looks like he stopped before it got merged. Let's finish
> > > polishing it.
> >
> > Nice to see an old topic resurrected.
> >
> > > fast-export has an existing --signed-tags= option that controls how to
> > > handle tag signatures. However, there is no equivalent for commit
> > > signatures; it just silently strips the signature out of the commit
> > > (analogously to --signed-tags=strip).
> > >
> > > So implement a --signed-commits= flag in fast-export, and implement
> > > the receiving side of it in fast-import.
> >
> > Nice.
> >
> > I haven't thought about this topic obviously for a looooong time,
> > but I wonder we may want to have an option, which is independent
> > from these --signed-tags/--signed-commits options addressed here,
> > that allows the person who performed the import to attest to the
> > result by adding their own signature on tags and commits, whether
> > these tags and commits were originally signed or not.
>
> For what it's worth, this has been requested multiple times of
> git-filter-repo, so there is some desire for this feature.
This is also exactly the usecase we have been reviving this effort for
:) We recently hit such a case where a customer was basically unable to
use git-filter-repo(1) due to commit signatures, so we wanted to help
out and get this patch series landed so that the issue can ultimately be
addressed in git-filter-repo(1).
Patrick
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2025-02-25 7:51 ` Patrick Steinhardt
@ 2025-02-25 16:48 ` Elijah Newren
2025-02-25 16:56 ` Junio C Hamano
0 siblings, 1 reply; 60+ messages in thread
From: Elijah Newren @ 2025-02-25 16:48 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Junio C Hamano, Christian Couder, git, Luke Shumaker, Jeff King,
Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine
On Mon, Feb 24, 2025 at 11:51 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Mon, Feb 24, 2025 at 11:35:00PM -0800, Elijah Newren wrote:
> > On Mon, Feb 24, 2025 at 9:01 AM Junio C Hamano <gitster@pobox.com> wrote:
> > >
> > > Christian Couder <christian.couder@gmail.com> writes:
> > >
> > > > Luke Shumaker sent the first 4 versions of this series in April 2021,
> > > > but it looks like he stopped before it got merged. Let's finish
> > > > polishing it.
> > >
> > > Nice to see an old topic resurrected.
> > >
> > > > fast-export has an existing --signed-tags= option that controls how to
> > > > handle tag signatures. However, there is no equivalent for commit
> > > > signatures; it just silently strips the signature out of the commit
> > > > (analogously to --signed-tags=strip).
> > > >
> > > > So implement a --signed-commits= flag in fast-export, and implement
> > > > the receiving side of it in fast-import.
> > >
> > > Nice.
> > >
> > > I haven't thought about this topic obviously for a looooong time,
> > > but I wonder we may want to have an option, which is independent
> > > from these --signed-tags/--signed-commits options addressed here,
> > > that allows the person who performed the import to attest to the
> > > result by adding their own signature on tags and commits, whether
> > > these tags and commits were originally signed or not.
> >
> > For what it's worth, this has been requested multiple times of
> > git-filter-repo, so there is some desire for this feature.
>
> This is also exactly the usecase we have been reviving this effort for
> :) We recently hit such a case where a customer was basically unable to
> use git-filter-repo(1) due to commit signatures, so we wanted to help
> out and get this patch series landed so that the issue can ultimately be
> addressed in git-filter-repo(1).
I'm confused; this patch series doesn't implement the option Junio and
I were talking about. It only allows existing signatures to be
carried as-is, as opposed to resigning all the commits with the
current user's signature.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2025-02-25 16:48 ` Elijah Newren
@ 2025-02-25 16:56 ` Junio C Hamano
2025-03-10 15:59 ` Christian Couder
0 siblings, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2025-02-25 16:56 UTC (permalink / raw)
To: Elijah Newren
Cc: Patrick Steinhardt, Christian Couder, git, Luke Shumaker,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine
Elijah Newren <newren@gmail.com> writes:
>> This is also exactly the usecase we have been reviving this effort for
>> :) We recently hit such a case where a customer was basically unable to
>> use git-filter-repo(1) due to commit signatures, so we wanted to help
>> out and get this patch series landed so that the issue can ultimately be
>> addressed in git-filter-repo(1).
>
> I'm confused; this patch series doesn't implement the option Junio and
> I were talking about. It only allows existing signatures to be
> carried as-is, as opposed to resigning all the commits with the
> current user's signature.
I read the "can ultimately be" as "this series lays the groundwork
by upstreaming what the earlier effort started and stops there. a
future follow-up work will build on this to add more".
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2025-02-25 16:56 ` Junio C Hamano
@ 2025-03-10 15:59 ` Christian Couder
0 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Elijah Newren, Patrick Steinhardt, git, Luke Shumaker, Jeff King,
Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine
On Tue, Feb 25, 2025 at 5:56 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> >> This is also exactly the usecase we have been reviving this effort for
> >> :) We recently hit such a case where a customer was basically unable to
> >> use git-filter-repo(1) due to commit signatures, so we wanted to help
> >> out and get this patch series landed so that the issue can ultimately be
> >> addressed in git-filter-repo(1).
> >
> > I'm confused; this patch series doesn't implement the option Junio and
> > I were talking about. It only allows existing signatures to be
> > carried as-is, as opposed to resigning all the commits with the
> > current user's signature.
>
> I read the "can ultimately be" as "this series lays the groundwork
> by upstreaming what the earlier effort started and stops there. a
> future follow-up work will build on this to add more".
Yeah, this is our goal. I have added the following section to the
cover letter to clarify this:
Big picture goal
~~~~~~~~~~~~~~~~
Independent from these --signed-tags/--signed-commits options
addressed in this series, we want to have an option, that allows the
person who performed the import to attest to the result by adding
their own signature on tags and commits, whether these tags and
commits were originally signed or not.
This series lays the groundwork for that future option by upstreaming
the earlier effort started by Luke Shumaker and stops there. Future
follow-up work will build on it towards the big picture goal.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
` (6 preceding siblings ...)
2025-02-24 17:01 ` [PATCH v5 0/6] " Junio C Hamano
@ 2025-02-25 14:53 ` Phillip Wood
2025-03-10 15:59 ` Christian Couder
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
8 siblings, 1 reply; 60+ messages in thread
From: Phillip Wood @ 2025-02-25 14:53 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine
Hi Christian
I've only glanced over this series, but I did notice a memory leak
On 24/02/2025 14:27, Christian Couder wrote:
>
> + * The returned string has had the ' ' line continuation markers
> -+ * removed, and points to staticly allocated memory (not to memory
> ++ * removed, and points to statically allocated memory (not to memory
This corrects the spelling but the changes below remove the static
buffer so the user is now responsible for freeing the returned string.
That means this comment is wrong and I don't see any corresponding
changes to the callers to free the memory.
> + * within 'msg'), so it is only valid until the next call to
> + * find_commit_multiline_header.
> + *
> @@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const
> + const char *key,
> + const char **end)
> +{
> -+ static struct strbuf val = STRBUF_INIT;
> ++ struct strbuf val = STRBUF_INIT;
> + const char *bol, *eol;
> + size_t len;
> +
> -+ strbuf_reset(&val);
> -+
> + bol = find_commit_header(msg, key, &len);
> + if (!bol)
> + return NULL;
> @@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const
> + }
> +
> + *end = eol;
> -+ return val.buf;
> ++ return strbuf_detach(&val, NULL);
> +}
Best Wishes
Phillip
> - static char *reencode_message(const char *in_msg,
> - const char *in_encoding, size_t in_encoding_len)
> + static void handle_commit(struct commit *commit, struct rev_info *rev,
> + struct string_list *paths_of_changed_objects)
> {
> @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,
> const char *author, *author_end, *committer, *committer_end;
> - const char *encoding;
> + const char *encoding = NULL;
> size_t encoding_len;
> -+ const char *signature_alg = NULL, *signature;
> ++ const char *signature_alg = NULL, *signature = NULL;
> const char *message;
> char *reencoded = NULL;
> struct commit_list *p;
> @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,
> - committer++;
> commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
>
> -- /* find_commit_header() gets a `+ 1` because
> + /*
> +- * find_commit_header() gets a `+ 1` because
> - * commit_buffer_cursor points at the trailing "\n" at the end
> - * of the previous line, but find_commit_header() wants a
> -+ /* find_commit_header() and find_commit_multiline_header() get
> ++ * find_commit_header() and find_commit_multiline_header() get
> + * a `+ 1` because commit_buffer_cursor points at the trailing
> + * "\n" at the end of the previous line, but they want a
> - * pointer to the beginning of the next line. */
> + * pointer to the beginning of the next line.
> + */
> +
> - encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
> - if (encoding)
> - commit_buffer_cursor = encoding + encoding_len;
> + if (*commit_buffer_cursor == '\n') {
> + encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
> + if (encoding)
> + commit_buffer_cursor = encoding + encoding_len;
> + }
>
> -+ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
> -+ signature_alg = "sha1";
> -+ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
> -+ signature_alg = "sha256";
> ++ if (*commit_buffer_cursor == '\n') {
> ++ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
> ++ signature_alg = "sha1";
> ++ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
> ++ signature_alg = "sha256";
> ++ }
> +
> message = strstr(commit_buffer_cursor, "\n\n");
> if (message)
> @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
> printf("%.*s\n%.*s\n",
> (int)(author_end - author), author,
> (int)(committer_end - committer), committer);
> -+ if (signature)
> -+ switch(signed_commit_mode) {
> ++ if (signature) {
> ++ switch (signed_commit_mode) {
> + case SIGN_ABORT:
> + die("encountered signed commit %s; use "
> + "--signed-commits=<mode> to handle it",
> @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
> + case SIGN_STRIP:
> + break;
> + }
> ++ free((char *)signature);
> ++ }
> if (!reencoded && encoding)
> printf("encoding %.*s\n", (int)encoding_len, encoding);
> printf("data %u\n%s",
> @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
> "\n-----BEGIN PGP SIGNATURE-----\n");
> if (signature)
> - switch(signed_tag_mode) {
> + switch (signed_tag_mode) {
> - case SIGNED_TAG_ABORT:
> + case SIGN_ABORT:
> die("encountered signed tag %s; use "
> @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
> message_size = signature + 1 - message;
> break;
> }
> -@@ builtin/fast-export.c: static int parse_opt_anonymize_map(const struct option *opt,
> -
> - int cmd_fast_export(int argc, const char **argv, const char *prefix)
> +@@ builtin/fast-export.c: int cmd_fast_export(int argc,
> + const char *prefix,
> + struct repository *repo UNUSED)
> {
> + const char *env_signed_commits_noabort;
> struct rev_info revs;
> - struct object_array commits = OBJECT_ARRAY_INIT;
> struct commit *commit;
> -@@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const char *prefix)
> + char *export_filename = NULL,
> +@@ builtin/fast-export.c: int cmd_fast_export(int argc,
> N_("show progress after <n> objects")),
> OPT_CALLBACK(0, "signed-tags", &signed_tag_mode, N_("mode"),
> N_("select handling of signed tags"),
> @@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const ch
> OPT_CALLBACK(0, "tag-of-filtered-object", &tag_of_filtered_mode, N_("mode"),
> N_("select handling of tags that tag filtered objects"),
> parse_opt_tag_of_filtered_mode),
> -@@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const char *prefix)
> +@@ builtin/fast-export.c: int cmd_fast_export(int argc,
> if (argc == 1)
> usage_with_options (fast_export_usage, options);
>
> @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
> + strbuf_addstr(&new_data, "gpgsig-sha256 ");
> + else
> + die("Expected gpgsig algorithm sha1 or sha256, got %s", sig_alg);
> -+ string_list_split_in_place(&siglines, sig.buf, '\n', -1);
> ++ string_list_split_in_place(&siglines, sig.buf, "\n", -1);
> + strbuf_add_separated_string_list(&new_data, "\n ", &siglines);
> + strbuf_addch(&new_data, '\n');
> + }
> @@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
> + # between the two.
> + test_config i18n.commitEncoding ISO-8859-1 &&
> + git checkout -f -b commit-signing main &&
> -+ echo Sign your name > file-sign &&
> ++ echo Sign your name >file-sign &&
> + git add file-sign &&
> + git commit -S -m "signed commit" &&
> + COMMIT_SIGNING=$(git rev-parse --verify commit-signing)
> @@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
> +
> +test_expect_success GPG 'signed-commits default' '
> +
> -+ unset FAST_EXPORT_SIGNED_COMMITS_NOABORT &&
> ++ sane_unset FAST_EXPORT_SIGNED_COMMITS_NOABORT &&
> + test_must_fail git fast-export --reencode=no commit-signing &&
> +
> + FAST_EXPORT_SIGNED_COMMITS_NOABORT=1 git fast-export --reencode=no commit-signing >output 2>err &&
> + ! grep ^gpgsig output &&
> + grep "^encoding ISO-8859-1" output &&
> + test -s err &&
> -+ sed "s/commit-signing/commit-strip-signing/" output |
> -+ (cd new &&
> -+ git fast-import &&
> -+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
> ++ sed "s/commit-signing/commit-strip-signing/" output | (
> ++ cd new &&
> ++ git fast-import &&
> ++ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
> ++ test $COMMIT_SIGNING != $STRIPPED
> ++ )
> +
> +'
> +
> @@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
> + git fast-export --signed-commits=verbatim --reencode=no commit-signing >output &&
> + grep "^gpgsig sha" output &&
> + grep "encoding ISO-8859-1" output &&
> -+ (cd new &&
> -+ git fast-import &&
> -+ test $COMMIT_SIGNING = $(git rev-parse --verify refs/heads/commit-signing)) <output
> ++ (
> ++ cd new &&
> ++ git fast-import &&
> ++ STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
> ++ test $COMMIT_SIGNING = $STRIPPED
> ++ ) <output
> +
> +'
> +
> @@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
> + grep "^gpgsig sha" output &&
> + grep "encoding ISO-8859-1" output &&
> + test -s err &&
> -+ (cd new &&
> -+ git fast-import &&
> -+ test $COMMIT_SIGNING = $(git rev-parse --verify refs/heads/commit-signing)) <output
> ++ (
> ++ cd new &&
> ++ git fast-import &&
> ++ STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
> ++ test $COMMIT_SIGNING = $STRIPPED
> ++ ) <output
> +
> +'
> +
> @@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
> + git fast-export --signed-commits=strip --reencode=no commit-signing >output &&
> + ! grep ^gpgsig output &&
> + grep "^encoding ISO-8859-1" output &&
> -+ sed "s/commit-signing/commit-strip-signing/" output |
> -+ (cd new &&
> -+ git fast-import &&
> -+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
> ++ sed "s/commit-signing/commit-strip-signing/" output | (
> ++ cd new &&
> ++ git fast-import &&
> ++ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
> ++ test $COMMIT_SIGNING != $STRIPPED
> ++ )
> +
> +'
> +
> @@ t/t9350-fast-export.sh: test_expect_success 'signed-tags=warn-strip' '
> + ! grep ^gpgsig output &&
> + grep "^encoding ISO-8859-1" output &&
> + test -s err &&
> -+ sed "s/commit-signing/commit-strip-signing/" output |
> -+ (cd new &&
> -+ git fast-import &&
> -+ test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))
> ++ sed "s/commit-signing/commit-strip-signing/" output | (
> ++ cd new &&
> ++ git fast-import &&
> ++ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
> ++ test $COMMIT_SIGNING != $STRIPPED
> ++ )
> +
> +'
> +
> test_expect_success 'setup submodule' '
>
> + test_config_global protocol.file.allow always &&
> git checkout -f main &&
> -+ { git update-ref -d refs/heads/commit-signing || true; } &&
> ++ test_might_fail git update-ref -d refs/heads/commit-signing &&
> mkdir sub &&
> (
> cd sub &&
>
>
> Christian Couder (1):
> fast-export: fix missing whitespace after switch
>
> Luke Shumaker (5):
> git-fast-import.adoc: add missing LF in the BNF
> fast-export: rename --signed-tags='warn' to 'warn-verbatim'
> git-fast-export.txt: clarify why 'verbatim' may not be a good idea
> fast-export: do not modify memory from get_commit_buffer
> fast-export, fast-import: add support for signed-commits
>
> Documentation/git-fast-export.adoc | 25 +++-
> Documentation/git-fast-import.adoc | 20 ++-
> builtin/fast-export.c | 189 +++++++++++++++++++++--------
> builtin/fast-import.c | 23 ++++
> t/t9350-fast-export.sh | 116 ++++++++++++++++++
> 5 files changed, 317 insertions(+), 56 deletions(-)
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/6] fast-export, fast-import: add support for signed-commits
2025-02-25 14:53 ` Phillip Wood
@ 2025-03-10 15:59 ` Christian Couder
0 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:59 UTC (permalink / raw)
To: phillip.wood
Cc: git, Junio C Hamano, Patrick Steinhardt, Luke Shumaker,
Elijah Newren, Jeff King, Johannes Schindelin, Taylor Blau,
brian m . carlson, Eric Sunshine
Hi Phillip,
On Tue, Feb 25, 2025 at 3:53 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Christian
>
> I've only glanced over this series,
Thanks for taking a look at it!
> but I did notice a memory leak
>
> On 24/02/2025 14:27, Christian Couder wrote:
> >
> > + * The returned string has had the ' ' line continuation markers
> > -+ * removed, and points to staticly allocated memory (not to memory
> > ++ * removed, and points to statically allocated memory (not to memory
>
> This corrects the spelling but the changes below remove the static
> buffer so the user is now responsible for freeing the returned string.
> That means this comment is wrong
Yeah, this part of the comment is wrong. I have changed it in the next
version to the following:
* The returned string has had the ' ' line continuation markers
* removed, and points to allocated memory that must be free()d (not
* to memory within 'msg').
> and I don't see any corresponding
> changes to the callers to free the memory.
It is called by the following lines:
> > -+ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
> > -+ signature_alg = "sha1";
> > -+ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
> > -+ signature_alg = "sha256";
> > ++ if (*commit_buffer_cursor == '\n') {
> > ++ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
> > ++ signature_alg = "sha1";
> > ++ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
> > ++ signature_alg = "sha256";
> > ++ }
so the 'signature' variable points to the allocated memory, and then
it's used like this:
> > @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
> > printf("%.*s\n%.*s\n",
> > (int)(author_end - author), author,
> > (int)(committer_end - committer), committer);
> > -+ if (signature)
> > -+ switch(signed_commit_mode) {
> > ++ if (signature) {
> > ++ switch (signed_commit_mode) {
> > + case SIGN_ABORT:
> > + die("encountered signed commit %s; use "
> > + "--signed-commits=<mode> to handle it",
> > @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
> > + case SIGN_STRIP:
> > + break;
> > + }
> > ++ free((char *)signature);
And eventually the memory is freed by the added call to free() above.
> > ++ }
But yeah, the description of the changes since the previous version in
the cover letter might have done a better job of explaining this.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v6 0/6] fast-export, fast-import: add support for signed-commits
2025-02-24 14:27 ` [PATCH v5 0/6] " Christian Couder
` (7 preceding siblings ...)
2025-02-25 14:53 ` Phillip Wood
@ 2025-03-10 15:57 ` Christian Couder
2025-03-10 15:57 ` [PATCH v6 1/6] git-fast-import.adoc: add missing LF in the BNF Christian Couder
` (6 more replies)
8 siblings, 7 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:57 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Phillip Wood, Christian Couder
Luke Shumaker sent the first 4 versions of this series in April 2021,
but it looks like he stopped before it got merged. Let's finish
polishing it.
Goal of this series
~~~~~~~~~~~~~~~~~~~
fast-export has an existing --signed-tags= option that controls how to
handle tag signatures. However, there is no equivalent for commit
signatures; it just silently strips the signature out of the commit
(analogously to --signed-tags=strip).
So implement a --signed-commits= flag in fast-export, and implement
the receiving side of it in fast-import.
Big picture goal
~~~~~~~~~~~~~~~~
Independent from these --signed-tags/--signed-commits options
addressed in this series, we want to have an option, that allows the
person who performed the import to attest to the result by adding
their own signature on tags and commits, whether these tags and
commits were originally signed or not.
This series lays the groundwork for that future option by upstreaming
the earlier effort started by Luke Shumaker and stops there. Future
follow-up work will build on it towards the big picture goal.
Overview of the changes since v5
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
There is no real code change since v5, only a commit message, the
documentation and some code comments are improved.
Details of the changes since v5
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- Rebased on top of current 'master' branch at a36e024e98 (Merge
branch 'js/win-2.49-build-fixes', 2025-03-06). This is to get a
base as close as possible to v2.49.0 final.
- In patch 4/6 the commit message subject started with
"git-fast-export.txt:" instead of "git-fast-export.adoc" which has
been fixed.
- In patch 4/6 the documentation for `--signed-tags` in
"Documentation/git-fast-export.adoc" is improved to better explain
when it makes sense to use 'verbatim' and 'warn-verbatim', thanks
to Elijah.
- In patch 6/6 the documentation for `--signed-commits` in
"Documentation/git-fast-export.adoc" now spells out that its
default is 'abort', thanks to Elijah.
- In patch 6/6 a code comment in front of
find_commit_multiline_header() in "builtin/fast-export.c" has been
improved:
- a "rathar" vs "rather" typo has been fixed, thanks to Elijah,
- what should be done to the memory returned by the function has
been corrected, thanks to Phillip Wood.
CI tests
~~~~~~~~
All the CI tests passed, except perhaps the osx-gcc one which isn't
finished yet, see:
https://github.com/chriscool/git/actions/runs/13767984505
Range diff compared to version 5
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1: f97247e17d = 1: 395dc9b1d9 git-fast-import.adoc: add missing LF in the BNF
2: b71588563d = 2: 6265fd51aa fast-export: fix missing whitespace after switch
3: 947bc267e6 = 3: 9e290bab22 fast-export: rename --signed-tags='warn' to 'warn-verbatim'
4: 45087db345 ! 4: 923885134f git-fast-export.txt: clarify why 'verbatim' may not be a good idea
@@ Metadata
Author: Luke Shumaker <lukeshu@datawire.io>
## Commit message ##
- git-fast-export.txt: clarify why 'verbatim' may not be a good idea
+ git-fast-export.adoc: clarify why 'verbatim' may not be a good idea
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@@ Documentation/git-fast-export.adoc: OPTIONS
exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
-they will be exported, but you will see a warning.
+they will be exported, but you will see a warning. 'verbatim' and
-+'warn-verbatim' should only be used if you know that no
-+transformation affecting tags will be performed, or if you do not
-+care that the resulting tag will have an invalid signature.
++'warn-verbatim' should only be used if you know that no transformation
++affecting tags or any commit in their history will be performed by you
++or by fast-export or fast-import, or if you do not care that the
++resulting tag will have an invalid signature.
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
5: 20f085a790 = 5: 49f73ee6ef fast-export: do not modify memory from get_commit_buffer
6: 48e0d4203c ! 6: 542c692e67 fast-export, fast-import: add support for signed-commits
@@ Commit message
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
## Documentation/git-fast-export.adoc ##
-@@ Documentation/git-fast-export.adoc: they will be exported, but you will see a warning. 'verbatim' and
- transformation affecting tags will be performed, or if you do not
- care that the resulting tag will have an invalid signature.
+@@ Documentation/git-fast-export.adoc: affecting tags or any commit in their history will be performed by you
+ or by fast-export or fast-import, or if you do not care that the
+ resulting tag will have an invalid signature.
+--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
+ Specify how to handle signed commits. Behaves exactly as
-+ '--signed-tags', but for commits.
++ '--signed-tags', but for commits. Default is 'abort'.
++
+Earlier versions this command that did not have '--signed-commits'
+behaved as if '--signed-commits=strip'. As an escape hatch for users
@@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const
+/*
+ * find_commit_multiline_header is similar to find_commit_header,
-+ * except that it handles multi-line headers, rathar than simply
++ * except that it handles multi-line headers, rather than simply
+ * returning the first line of the header.
+ *
+ * The returned string has had the ' ' line continuation markers
-+ * removed, and points to statically allocated memory (not to memory
-+ * within 'msg'), so it is only valid until the next call to
-+ * find_commit_multiline_header.
++ * removed, and points to allocated memory that must be free()d (not
++ * to memory within 'msg').
+ *
+ * If the header is found, then *end is set to point at the '\n' in
+ * msg that immediately follows the header value.
Christian Couder (1):
fast-export: fix missing whitespace after switch
Luke Shumaker (5):
git-fast-import.adoc: add missing LF in the BNF
fast-export: rename --signed-tags='warn' to 'warn-verbatim'
git-fast-export.adoc: clarify why 'verbatim' may not be a good idea
fast-export: do not modify memory from get_commit_buffer
fast-export, fast-import: add support for signed-commits
Documentation/git-fast-export.adoc | 26 +++-
Documentation/git-fast-import.adoc | 20 ++-
builtin/fast-export.c | 188 +++++++++++++++++++++--------
builtin/fast-import.c | 23 ++++
t/t9350-fast-export.sh | 116 ++++++++++++++++++
5 files changed, 317 insertions(+), 56 deletions(-)
--
2.49.0.rc1.89.g148d1db992
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v6 1/6] git-fast-import.adoc: add missing LF in the BNF
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
@ 2025-03-10 15:57 ` Christian Couder
2025-03-10 15:57 ` [PATCH v6 2/6] fast-export: fix missing whitespace after switch Christian Couder
` (5 subsequent siblings)
6 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:57 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Phillip Wood, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-import.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 58a2eaa51a..8e0de618c0 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -437,7 +437,7 @@ change to the project.
original-oid?
('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
'committer' (SP <name>)? SP LT <email> GT SP <when> LF
- ('encoding' SP <encoding>)?
+ ('encoding' SP <encoding> LF)?
data
('from' SP <commit-ish> LF)?
('merge' SP <commit-ish> LF)*
--
2.49.0.rc1.89.g148d1db992
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v6 2/6] fast-export: fix missing whitespace after switch
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
2025-03-10 15:57 ` [PATCH v6 1/6] git-fast-import.adoc: add missing LF in the BNF Christian Couder
@ 2025-03-10 15:57 ` Christian Couder
2025-03-10 15:57 ` [PATCH v6 3/6] fast-export: rename --signed-tags='warn' to 'warn-verbatim' Christian Couder
` (4 subsequent siblings)
6 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:57 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Phillip Wood, Christian Couder, Christian Couder
"Documentation/CodingGuidelines" says that there should be whitespaces
around operators like 'if', 'switch', 'for', etc.
Let's fix this in "builtin/fast-export.c".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/fast-export.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index a5c82eef1d..2bf787191a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -694,7 +694,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
if (anonymize) {
reencoded = anonymize_commit_message();
} else if (encoding) {
- switch(reencode_mode) {
+ switch (reencode_mode) {
case REENCODE_YES:
reencoded = reencode_string(message, "UTF-8", encoding);
break;
@@ -828,7 +828,7 @@ static void handle_tag(const char *name, struct tag *tag)
const char *signature = strstr(message,
"\n-----BEGIN PGP SIGNATURE-----\n");
if (signature)
- switch(signed_tag_mode) {
+ switch (signed_tag_mode) {
case SIGNED_TAG_ABORT:
die("encountered signed tag %s; use "
"--signed-tags=<mode> to handle it",
@@ -853,7 +853,7 @@ static void handle_tag(const char *name, struct tag *tag)
tagged = tag->tagged;
tagged_mark = get_object_mark(tagged);
if (!tagged_mark) {
- switch(tag_of_filtered_mode) {
+ switch (tag_of_filtered_mode) {
case TAG_FILTERING_ABORT:
die("tag %s tags unexported object; use "
"--tag-of-filtered-object=<mode> to handle it",
@@ -965,7 +965,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
continue;
}
- switch(commit->object.type) {
+ switch (commit->object.type) {
case OBJ_COMMIT:
break;
case OBJ_BLOB:
--
2.49.0.rc1.89.g148d1db992
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v6 3/6] fast-export: rename --signed-tags='warn' to 'warn-verbatim'
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
2025-03-10 15:57 ` [PATCH v6 1/6] git-fast-import.adoc: add missing LF in the BNF Christian Couder
2025-03-10 15:57 ` [PATCH v6 2/6] fast-export: fix missing whitespace after switch Christian Couder
@ 2025-03-10 15:57 ` Christian Couder
2025-03-10 15:57 ` [PATCH v6 4/6] git-fast-export.adoc: clarify why 'verbatim' may not be a good idea Christian Couder
` (3 subsequent siblings)
6 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:57 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Phillip Wood, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
The --signed-tags= option takes one of five arguments specifying how to
handle signed tags during export. Among these arguments, 'strip' is to
'warn-strip' as 'verbatim' is to 'warn' (the unmentioned argument is
'abort', which stops the fast-export process entirely). That is,
signatures are either stripped or copied verbatim while exporting, with
or without a warning.
Match the pattern and rename 'warn' to 'warn-verbatim' to make it clear
that it instructs fast-export to copy signatures verbatim.
To maintain backwards compatibility, 'warn' is still recognized as
deprecated synonym of 'warn-verbatim'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-export.adoc | 6 +++---
builtin/fast-export.c | 8 ++++----
t/t9350-fast-export.sh | 18 ++++++++++++++++++
3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
index 752e4b9b01..ab9a315fa9 100644
--- a/Documentation/git-fast-export.adoc
+++ b/Documentation/git-fast-export.adoc
@@ -27,7 +27,7 @@ OPTIONS
Insert 'progress' statements every <n> objects, to be shown by
'git fast-import' during import.
---signed-tags=(verbatim|warn|warn-strip|strip|abort)::
+--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed tags. Since any transformation
after the export can change the tag names (which can also happen
when excluding revisions) the signatures will not match.
@@ -36,8 +36,8 @@ When asking to 'abort' (which is the default), this program will die
when encountering a signed tag. With 'strip', the tags will silently
be made unsigned, with 'warn-strip' they will be made unsigned but a
warning will be displayed, with 'verbatim', they will be silently
-exported and with 'warn', they will be exported, but you will see a
-warning.
+exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
+they will be exported, but you will see a warning.
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 2bf787191a..2de2adc30e 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -36,7 +36,7 @@ static const char *fast_export_usage[] = {
};
static int progress;
-static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
static enum tag_of_filtered_mode { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
static enum reencode_mode { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
@@ -62,8 +62,8 @@ static int parse_opt_signed_tag_mode(const struct option *opt,
*val = SIGNED_TAG_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
*val = VERBATIM;
- else if (!strcmp(arg, "warn"))
- *val = WARN;
+ else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+ *val = WARN_VERBATIM;
else if (!strcmp(arg, "warn-strip"))
*val = WARN_STRIP;
else if (!strcmp(arg, "strip"))
@@ -833,7 +833,7 @@ static void handle_tag(const char *name, struct tag *tag)
die("encountered signed tag %s; use "
"--signed-tags=<mode> to handle it",
oid_to_hex(&tag->object.oid));
- case WARN:
+ case WARN_VERBATIM:
warning("exporting signed tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 40427883ec..cc110727fb 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -253,6 +253,24 @@ test_expect_success 'signed-tags=verbatim' '
'
+test_expect_success 'signed-tags=warn-verbatim' '
+
+ git fast-export --signed-tags=warn-verbatim sign-your-name >output 2>err &&
+ grep PGP output &&
+ test -s err
+
+'
+
+# 'warn' is a backward-compatibility alias for 'warn-verbatim'; test
+# that it keeps working.
+test_expect_success 'signed-tags=warn' '
+
+ git fast-export --signed-tags=warn sign-your-name >output 2>err &&
+ grep PGP output &&
+ test -s err
+
+'
+
test_expect_success 'signed-tags=strip' '
git fast-export --signed-tags=strip sign-your-name > output &&
--
2.49.0.rc1.89.g148d1db992
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v6 4/6] git-fast-export.adoc: clarify why 'verbatim' may not be a good idea
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
` (2 preceding siblings ...)
2025-03-10 15:57 ` [PATCH v6 3/6] fast-export: rename --signed-tags='warn' to 'warn-verbatim' Christian Couder
@ 2025-03-10 15:57 ` Christian Couder
2025-03-10 15:57 ` [PATCH v6 5/6] fast-export: do not modify memory from get_commit_buffer Christian Couder
` (2 subsequent siblings)
6 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:57 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Phillip Wood, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-export.adoc | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
index ab9a315fa9..2bb52261a0 100644
--- a/Documentation/git-fast-export.adoc
+++ b/Documentation/git-fast-export.adoc
@@ -29,15 +29,20 @@ OPTIONS
--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
Specify how to handle signed tags. Since any transformation
- after the export can change the tag names (which can also happen
- when excluding revisions) the signatures will not match.
+ after the export (or during the export, such as excluding
+ revisions) can change the hashes being signed, the signatures
+ may become invalid.
+
When asking to 'abort' (which is the default), this program will die
when encountering a signed tag. With 'strip', the tags will silently
be made unsigned, with 'warn-strip' they will be made unsigned but a
warning will be displayed, with 'verbatim', they will be silently
exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
-they will be exported, but you will see a warning.
+they will be exported, but you will see a warning. 'verbatim' and
+'warn-verbatim' should only be used if you know that no transformation
+affecting tags or any commit in their history will be performed by you
+or by fast-export or fast-import, or if you do not care that the
+resulting tag will have an invalid signature.
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
--
2.49.0.rc1.89.g148d1db992
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v6 5/6] fast-export: do not modify memory from get_commit_buffer
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
` (3 preceding siblings ...)
2025-03-10 15:57 ` [PATCH v6 4/6] git-fast-export.adoc: clarify why 'verbatim' may not be a good idea Christian Couder
@ 2025-03-10 15:57 ` Christian Couder
2025-03-10 15:57 ` [PATCH v6 6/6] fast-export, fast-import: add support for signed-commits Christian Couder
2025-03-10 22:36 ` [PATCH v6 0/6] " Elijah Newren
6 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:57 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Phillip Wood, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
fast-export's helper function find_encoding() takes a `const char *`, but
modifies that memory despite the `const`. Ultimately, this memory came
from get_commit_buffer(), and you're not supposed to modify the memory
that you get from get_commit_buffer().
So, get rid of find_encoding() in favor of commit.h:find_commit_header(),
which gives back a string length, rather than mutating the memory to
insert a '\0' terminator.
Because find_commit_header() detects the "\n\n" string that separates the
headers and the commit message, move the call to be above the
`message = strstr(..., "\n\n")` call. This helps readability, and allows
for the value of `encoding` to be used for a better value of "..." so that
the same memory doesn't need to be checked twice. Introduce a
`commit_buffer_cursor` variable to avoid writing an awkward
`encoding ? encoding + encoding_len : committer_end` expression.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/fast-export.c | 61 +++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 2de2adc30e..39d43c2a29 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -510,21 +510,6 @@ static void show_filemodify(struct diff_queue_struct *q,
}
}
-static const char *find_encoding(const char *begin, const char *end)
-{
- const char *needle = "\nencoding ";
- char *bol, *eol;
-
- bol = memmem(begin, end ? end - begin : strlen(begin),
- needle, strlen(needle));
- if (!bol)
- return NULL;
- bol += strlen(needle);
- eol = strchrnul(bol, '\n');
- *eol = '\0';
- return bol;
-}
-
static char *anonymize_ref_component(void)
{
static int counter;
@@ -630,9 +615,11 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
struct string_list *paths_of_changed_objects)
{
int saved_output_format = rev->diffopt.output_format;
- const char *commit_buffer;
+ const char *commit_buffer, *commit_buffer_cursor;
const char *author, *author_end, *committer, *committer_end;
- const char *encoding, *message;
+ const char *encoding = NULL;
+ size_t encoding_len;
+ const char *message;
char *reencoded = NULL;
struct commit_list *p;
const char *refname;
@@ -641,21 +628,35 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
rev->diffopt.output_format = DIFF_FORMAT_CALLBACK;
parse_commit_or_die(commit);
- commit_buffer = repo_get_commit_buffer(the_repository, commit, NULL);
- author = strstr(commit_buffer, "\nauthor ");
+ commit_buffer_cursor = commit_buffer = repo_get_commit_buffer(the_repository, commit, NULL);
+
+ author = strstr(commit_buffer_cursor, "\nauthor ");
if (!author)
die("could not find author in commit %s",
oid_to_hex(&commit->object.oid));
author++;
- author_end = strchrnul(author, '\n');
- committer = strstr(author_end, "\ncommitter ");
+ commit_buffer_cursor = author_end = strchrnul(author, '\n');
+
+ committer = strstr(commit_buffer_cursor, "\ncommitter ");
if (!committer)
die("could not find committer in commit %s",
oid_to_hex(&commit->object.oid));
committer++;
- committer_end = strchrnul(committer, '\n');
- message = strstr(committer_end, "\n\n");
- encoding = find_encoding(committer_end, message);
+ commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
+
+ /*
+ * find_commit_header() gets a `+ 1` because
+ * commit_buffer_cursor points at the trailing "\n" at the end
+ * of the previous line, but find_commit_header() wants a
+ * pointer to the beginning of the next line.
+ */
+ if (*commit_buffer_cursor == '\n') {
+ encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
+ if (encoding)
+ commit_buffer_cursor = encoding + encoding_len;
+ }
+
+ message = strstr(commit_buffer_cursor, "\n\n");
if (message)
message += 2;
@@ -694,16 +695,20 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
if (anonymize) {
reencoded = anonymize_commit_message();
} else if (encoding) {
+ char *buf;
switch (reencode_mode) {
case REENCODE_YES:
- reencoded = reencode_string(message, "UTF-8", encoding);
+ buf = xstrfmt("%.*s", (int)encoding_len, encoding);
+ reencoded = reencode_string(message, "UTF-8", buf);
+ free(buf);
break;
case REENCODE_NO:
break;
case REENCODE_ABORT:
- die("Encountered commit-specific encoding %s in commit "
+ die("Encountered commit-specific encoding %.*s in commit "
"%s; use --reencode=[yes|no] to handle it",
- encoding, oid_to_hex(&commit->object.oid));
+ (int)encoding_len, encoding,
+ oid_to_hex(&commit->object.oid));
}
}
if (!commit->parents)
@@ -715,7 +720,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
(int)(author_end - author), author,
(int)(committer_end - committer), committer);
if (!reencoded && encoding)
- printf("encoding %s\n", encoding);
+ printf("encoding %.*s\n", (int)encoding_len, encoding);
printf("data %u\n%s",
(unsigned)(reencoded
? strlen(reencoded) : message
--
2.49.0.rc1.89.g148d1db992
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v6 6/6] fast-export, fast-import: add support for signed-commits
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
` (4 preceding siblings ...)
2025-03-10 15:57 ` [PATCH v6 5/6] fast-export: do not modify memory from get_commit_buffer Christian Couder
@ 2025-03-10 15:57 ` Christian Couder
2025-03-10 22:36 ` [PATCH v6 0/6] " Elijah Newren
6 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2025-03-10 15:57 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Elijah Newren,
Jeff King, Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Phillip Wood, Luke Shumaker, Christian Couder
From: Luke Shumaker <lukeshu@datawire.io>
fast-export has a --signed-tags= option that controls how to handle tag
signatures. However, there is no equivalent for commit signatures; it
just silently strips the signature out of the commit (analogously to
--signed-tags=strip).
While signatures are generally problematic for fast-export/fast-import
(because hashes are likely to change), if they're going to support tag
signatures, there's no reason to not also support commit signatures.
So, implement a --signed-commits= option that mirrors the --signed-tags=
option.
On the fast-export side, try to be as much like signed-tags as possible,
in both implementation and in user-interface. This will change the
default behavior to '--signed-commits=abort' from what is now
'--signed-commits=strip'. In order to provide an escape hatch for users
of third-party tools that call fast-export and do not yet know of the
--signed-commits= option, add an environment variable
'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' that changes the default to
'--signed-commits=warn-strip'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-fast-export.adoc | 11 +++
Documentation/git-fast-import.adoc | 18 +++++
builtin/fast-export.c | 123 ++++++++++++++++++++++++-----
builtin/fast-import.c | 23 ++++++
t/t9350-fast-export.sh | 98 +++++++++++++++++++++++
5 files changed, 253 insertions(+), 20 deletions(-)
diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
index 2bb52261a0..413a527496 100644
--- a/Documentation/git-fast-export.adoc
+++ b/Documentation/git-fast-export.adoc
@@ -44,6 +44,17 @@ affecting tags or any commit in their history will be performed by you
or by fast-export or fast-import, or if you do not care that the
resulting tag will have an invalid signature.
+--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
+ Specify how to handle signed commits. Behaves exactly as
+ '--signed-tags', but for commits. Default is 'abort'.
++
+Earlier versions this command that did not have '--signed-commits'
+behaved as if '--signed-commits=strip'. As an escape hatch for users
+of tools that call 'git fast-export' but do not yet support
+'--signed-commits', you may set the environment variable
+'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' in order to change the default
+from 'abort' to 'warn-strip'.
+
--tag-of-filtered-object=(abort|drop|rewrite)::
Specify how to handle tags whose tagged object is filtered out.
Since revisions and files to export can be limited by path,
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 8e0de618c0..7b107f5e8e 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -431,12 +431,21 @@ and control the current import process. More detailed discussion
Create or update a branch with a new commit, recording one logical
change to the project.
+////
+Yes, it's intentional that the 'gpgsig' line doesn't have a trailing
+`LF`; the definition of `data` has a byte-count prefix, so it
+doesn't need an `LF` to act as a terminator (and `data` also already
+includes an optional trailing `LF?` just in case you want to include
+one).
+////
+
....
'commit' SP <ref> LF
mark?
original-oid?
('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
'committer' (SP <name>)? SP LT <email> GT SP <when> LF
+ ('gpgsig' SP <alg> LF data)?
('encoding' SP <encoding> LF)?
data
('from' SP <commit-ish> LF)?
@@ -505,6 +514,15 @@ that was selected by the --date-format=<fmt> command-line option.
See ``Date Formats'' above for the set of supported formats, and
their syntax.
+`gpgsig`
+^^^^^^^^
+
+The optional `gpgsig` command is used to include a PGP/GPG signature
+that signs the commit data.
+
+Here <alg> specifies which hashing algorithm is used for this
+signature, either `sha1` or `sha256`.
+
`encoding`
^^^^^^^^^^
The optional `encoding` command indicates the encoding of the commit
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 39d43c2a29..126980f724 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -35,8 +35,11 @@ static const char *fast_export_usage[] = {
NULL
};
+enum sign_mode { SIGN_ABORT, SIGN_VERBATIM, SIGN_STRIP, SIGN_WARN_VERBATIM, SIGN_WARN_STRIP };
+
static int progress;
-static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN_VERBATIM, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+static enum sign_mode signed_tag_mode = SIGN_ABORT;
+static enum sign_mode signed_commit_mode = SIGN_ABORT;
static enum tag_of_filtered_mode { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
static enum reencode_mode { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
@@ -53,23 +56,24 @@ static int anonymize;
static struct hashmap anonymized_seeds;
static struct revision_sources revision_sources;
-static int parse_opt_signed_tag_mode(const struct option *opt,
+static int parse_opt_sign_mode(const struct option *opt,
const char *arg, int unset)
{
- enum signed_tag_mode *val = opt->value;
-
- if (unset || !strcmp(arg, "abort"))
- *val = SIGNED_TAG_ABORT;
+ enum sign_mode *val = opt->value;
+ if (unset)
+ return 0;
+ else if (!strcmp(arg, "abort"))
+ *val = SIGN_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
- *val = VERBATIM;
+ *val = SIGN_VERBATIM;
else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
- *val = WARN_VERBATIM;
+ *val = SIGN_WARN_VERBATIM;
else if (!strcmp(arg, "warn-strip"))
- *val = WARN_STRIP;
+ *val = SIGN_WARN_STRIP;
else if (!strcmp(arg, "strip"))
- *val = STRIP;
+ *val = SIGN_STRIP;
else
- return error("Unknown signed-tags mode: %s", arg);
+ return error("Unknown %s mode: %s", opt->long_name, arg);
return 0;
}
@@ -611,6 +615,43 @@ static void anonymize_ident_line(const char **beg, const char **end)
*end = out->buf + out->len;
}
+/*
+ * find_commit_multiline_header is similar to find_commit_header,
+ * except that it handles multi-line headers, rather than simply
+ * returning the first line of the header.
+ *
+ * The returned string has had the ' ' line continuation markers
+ * removed, and points to allocated memory that must be free()d (not
+ * to memory within 'msg').
+ *
+ * If the header is found, then *end is set to point at the '\n' in
+ * msg that immediately follows the header value.
+ */
+static const char *find_commit_multiline_header(const char *msg,
+ const char *key,
+ const char **end)
+{
+ struct strbuf val = STRBUF_INIT;
+ const char *bol, *eol;
+ size_t len;
+
+ bol = find_commit_header(msg, key, &len);
+ if (!bol)
+ return NULL;
+ eol = bol + len;
+ strbuf_add(&val, bol, len);
+
+ while (eol[0] == '\n' && eol[1] == ' ') {
+ bol = eol + 2;
+ eol = strchrnul(bol, '\n');
+ strbuf_addch(&val, '\n');
+ strbuf_add(&val, bol, eol - bol);
+ }
+
+ *end = eol;
+ return strbuf_detach(&val, NULL);
+}
+
static void handle_commit(struct commit *commit, struct rev_info *rev,
struct string_list *paths_of_changed_objects)
{
@@ -619,6 +660,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
const char *author, *author_end, *committer, *committer_end;
const char *encoding = NULL;
size_t encoding_len;
+ const char *signature_alg = NULL, *signature = NULL;
const char *message;
char *reencoded = NULL;
struct commit_list *p;
@@ -645,17 +687,25 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
commit_buffer_cursor = committer_end = strchrnul(committer, '\n');
/*
- * find_commit_header() gets a `+ 1` because
- * commit_buffer_cursor points at the trailing "\n" at the end
- * of the previous line, but find_commit_header() wants a
+ * find_commit_header() and find_commit_multiline_header() get
+ * a `+ 1` because commit_buffer_cursor points at the trailing
+ * "\n" at the end of the previous line, but they want a
* pointer to the beginning of the next line.
*/
+
if (*commit_buffer_cursor == '\n') {
encoding = find_commit_header(commit_buffer_cursor + 1, "encoding", &encoding_len);
if (encoding)
commit_buffer_cursor = encoding + encoding_len;
}
+ if (*commit_buffer_cursor == '\n') {
+ if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
+ signature_alg = "sha1";
+ else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
+ signature_alg = "sha256";
+ }
+
message = strstr(commit_buffer_cursor, "\n\n");
if (message)
message += 2;
@@ -719,6 +769,31 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
printf("%.*s\n%.*s\n",
(int)(author_end - author), author,
(int)(committer_end - committer), committer);
+ if (signature) {
+ switch (signed_commit_mode) {
+ case SIGN_ABORT:
+ die("encountered signed commit %s; use "
+ "--signed-commits=<mode> to handle it",
+ oid_to_hex(&commit->object.oid));
+ case SIGN_WARN_VERBATIM:
+ warning("exporting signed commit %s",
+ oid_to_hex(&commit->object.oid));
+ /* fallthru */
+ case SIGN_VERBATIM:
+ printf("gpgsig %s\ndata %u\n%s",
+ signature_alg,
+ (unsigned)strlen(signature),
+ signature);
+ break;
+ case SIGN_WARN_STRIP:
+ warning("stripping signature from commit %s",
+ oid_to_hex(&commit->object.oid));
+ /* fallthru */
+ case SIGN_STRIP:
+ break;
+ }
+ free((char *)signature);
+ }
if (!reencoded && encoding)
printf("encoding %.*s\n", (int)encoding_len, encoding);
printf("data %u\n%s",
@@ -834,21 +909,21 @@ static void handle_tag(const char *name, struct tag *tag)
"\n-----BEGIN PGP SIGNATURE-----\n");
if (signature)
switch (signed_tag_mode) {
- case SIGNED_TAG_ABORT:
+ case SIGN_ABORT:
die("encountered signed tag %s; use "
"--signed-tags=<mode> to handle it",
oid_to_hex(&tag->object.oid));
- case WARN_VERBATIM:
+ case SIGN_WARN_VERBATIM:
warning("exporting signed tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
- case VERBATIM:
+ case SIGN_VERBATIM:
break;
- case WARN_STRIP:
+ case SIGN_WARN_STRIP:
warning("stripping signature from tag %s",
oid_to_hex(&tag->object.oid));
/* fallthru */
- case STRIP:
+ case SIGN_STRIP:
message_size = signature + 1 - message;
break;
}
@@ -1194,6 +1269,7 @@ int cmd_fast_export(int argc,
const char *prefix,
struct repository *repo UNUSED)
{
+ const char *env_signed_commits_noabort;
struct rev_info revs;
struct commit *commit;
char *export_filename = NULL,
@@ -1207,7 +1283,10 @@ int cmd_fast_export(int argc,
N_("show progress after <n> objects")),
OPT_CALLBACK(0, "signed-tags", &signed_tag_mode, N_("mode"),
N_("select handling of signed tags"),
- parse_opt_signed_tag_mode),
+ parse_opt_sign_mode),
+ OPT_CALLBACK(0, "signed-commits", &signed_commit_mode, N_("mode"),
+ N_("select handling of signed commits"),
+ parse_opt_sign_mode),
OPT_CALLBACK(0, "tag-of-filtered-object", &tag_of_filtered_mode, N_("mode"),
N_("select handling of tags that tag filtered objects"),
parse_opt_tag_of_filtered_mode),
@@ -1248,6 +1327,10 @@ int cmd_fast_export(int argc,
if (argc == 1)
usage_with_options (fast_export_usage, options);
+ env_signed_commits_noabort = getenv("FAST_EXPORT_SIGNED_COMMITS_NOABORT");
+ if (env_signed_commits_noabort && *env_signed_commits_noabort)
+ signed_commit_mode = SIGN_WARN_STRIP;
+
/* we handle encodings */
git_config(git_default_config, NULL);
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 397a6f46ad..e432e8d5a1 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2719,10 +2719,13 @@ static struct hash_list *parse_merge(unsigned int *count)
static void parse_new_commit(const char *arg)
{
+ static struct strbuf sig = STRBUF_INIT;
static struct strbuf msg = STRBUF_INIT;
+ struct string_list siglines = STRING_LIST_INIT_NODUP;
struct branch *b;
char *author = NULL;
char *committer = NULL;
+ char *sig_alg = NULL;
char *encoding = NULL;
struct hash_list *merge_list = NULL;
unsigned int merge_count;
@@ -2746,6 +2749,13 @@ static void parse_new_commit(const char *arg)
}
if (!committer)
die("Expected committer but didn't get one");
+ if (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
+ sig_alg = xstrdup(v);
+ read_next_command();
+ parse_data(&sig, 0, NULL);
+ read_next_command();
+ } else
+ strbuf_setlen(&sig, 0);
if (skip_prefix(command_buf.buf, "encoding ", &v)) {
encoding = xstrdup(v);
read_next_command();
@@ -2819,10 +2829,23 @@ static void parse_new_commit(const char *arg)
strbuf_addf(&new_data,
"encoding %s\n",
encoding);
+ if (sig_alg) {
+ if (!strcmp(sig_alg, "sha1"))
+ strbuf_addstr(&new_data, "gpgsig ");
+ else if (!strcmp(sig_alg, "sha256"))
+ strbuf_addstr(&new_data, "gpgsig-sha256 ");
+ else
+ die("Expected gpgsig algorithm sha1 or sha256, got %s", sig_alg);
+ string_list_split_in_place(&siglines, sig.buf, "\n", -1);
+ strbuf_add_separated_string_list(&new_data, "\n ", &siglines);
+ strbuf_addch(&new_data, '\n');
+ }
strbuf_addch(&new_data, '\n');
strbuf_addbuf(&new_data, &msg);
+ string_list_clear(&siglines, 1);
free(author);
free(committer);
+ free(sig_alg);
free(encoding);
if (!store_object(OBJ_COMMIT, &new_data, NULL, &b->oid, next_mark))
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index cc110727fb..304bac5b1d 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -8,6 +8,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
test_expect_success 'setup' '
@@ -284,10 +285,107 @@ test_expect_success 'signed-tags=warn-strip' '
test -s err
'
+test_expect_success GPG 'set up signed commit' '
+
+ # Generate a commit with both "gpgsig" and "encoding" set, so
+ # that we can test that fast-import gets the ordering correct
+ # between the two.
+ test_config i18n.commitEncoding ISO-8859-1 &&
+ git checkout -f -b commit-signing main &&
+ echo Sign your name >file-sign &&
+ git add file-sign &&
+ git commit -S -m "signed commit" &&
+ COMMIT_SIGNING=$(git rev-parse --verify commit-signing)
+
+'
+
+test_expect_success GPG 'signed-commits default' '
+
+ sane_unset FAST_EXPORT_SIGNED_COMMITS_NOABORT &&
+ test_must_fail git fast-export --reencode=no commit-signing &&
+
+ FAST_EXPORT_SIGNED_COMMITS_NOABORT=1 git fast-export --reencode=no commit-signing >output 2>err &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ test -s err &&
+ sed "s/commit-signing/commit-strip-signing/" output | (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
+ test $COMMIT_SIGNING != $STRIPPED
+ )
+
+'
+
+test_expect_success GPG 'signed-commits=abort' '
+
+ test_must_fail git fast-export --signed-commits=abort commit-signing
+
+'
+
+test_expect_success GPG 'signed-commits=verbatim' '
+
+ git fast-export --signed-commits=verbatim --reencode=no commit-signing >output &&
+ grep "^gpgsig sha" output &&
+ grep "encoding ISO-8859-1" output &&
+ (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
+ test $COMMIT_SIGNING = $STRIPPED
+ ) <output
+
+'
+
+test_expect_success GPG 'signed-commits=warn-verbatim' '
+
+ git fast-export --signed-commits=warn-verbatim --reencode=no commit-signing >output 2>err &&
+ grep "^gpgsig sha" output &&
+ grep "encoding ISO-8859-1" output &&
+ test -s err &&
+ (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-signing) &&
+ test $COMMIT_SIGNING = $STRIPPED
+ ) <output
+
+'
+
+test_expect_success GPG 'signed-commits=strip' '
+
+ git fast-export --signed-commits=strip --reencode=no commit-signing >output &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ sed "s/commit-signing/commit-strip-signing/" output | (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
+ test $COMMIT_SIGNING != $STRIPPED
+ )
+
+'
+
+test_expect_success GPG 'signed-commits=warn-strip' '
+
+ git fast-export --signed-commits=warn-strip --reencode=no commit-signing >output 2>err &&
+ ! grep ^gpgsig output &&
+ grep "^encoding ISO-8859-1" output &&
+ test -s err &&
+ sed "s/commit-signing/commit-strip-signing/" output | (
+ cd new &&
+ git fast-import &&
+ STRIPPED=$(git rev-parse --verify refs/heads/commit-strip-signing) &&
+ test $COMMIT_SIGNING != $STRIPPED
+ )
+
+'
+
test_expect_success 'setup submodule' '
test_config_global protocol.file.allow always &&
git checkout -f main &&
+ test_might_fail git update-ref -d refs/heads/commit-signing &&
mkdir sub &&
(
cd sub &&
--
2.49.0.rc1.89.g148d1db992
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v6 0/6] fast-export, fast-import: add support for signed-commits
2025-03-10 15:57 ` [PATCH v6 " Christian Couder
` (5 preceding siblings ...)
2025-03-10 15:57 ` [PATCH v6 6/6] fast-export, fast-import: add support for signed-commits Christian Couder
@ 2025-03-10 22:36 ` Elijah Newren
6 siblings, 0 replies; 60+ messages in thread
From: Elijah Newren @ 2025-03-10 22:36 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Luke Shumaker, Jeff King,
Johannes Schindelin, Taylor Blau, brian m . carlson,
Eric Sunshine, Phillip Wood
Hi Christian,
On Mon, Mar 10, 2025 at 8:58 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> Luke Shumaker sent the first 4 versions of this series in April 2021,
> but it looks like he stopped before it got merged. Let's finish
> polishing it.
>
> Goal of this series
> ~~~~~~~~~~~~~~~~~~~
>
> fast-export has an existing --signed-tags= option that controls how to
> handle tag signatures. However, there is no equivalent for commit
> signatures; it just silently strips the signature out of the commit
> (analogously to --signed-tags=strip).
>
> So implement a --signed-commits= flag in fast-export, and implement
> the receiving side of it in fast-import.
>
> Big picture goal
> ~~~~~~~~~~~~~~~~
>
> Independent from these --signed-tags/--signed-commits options
> addressed in this series, we want to have an option, that allows the
> person who performed the import to attest to the result by adding
> their own signature on tags and commits, whether these tags and
> commits were originally signed or not.
>
> This series lays the groundwork for that future option by upstreaming
> the earlier effort started by Luke Shumaker and stops there. Future
> follow-up work will build on it towards the big picture goal.
>
> Overview of the changes since v5
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> There is no real code change since v5, only a commit message, the
> documentation and some code comments are improved.
>
> Details of the changes since v5
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> - Rebased on top of current 'master' branch at a36e024e98 (Merge
> branch 'js/win-2.49-build-fixes', 2025-03-06). This is to get a
> base as close as possible to v2.49.0 final.
>
> - In patch 4/6 the commit message subject started with
> "git-fast-export.txt:" instead of "git-fast-export.adoc" which has
> been fixed.
>
> - In patch 4/6 the documentation for `--signed-tags` in
> "Documentation/git-fast-export.adoc" is improved to better explain
> when it makes sense to use 'verbatim' and 'warn-verbatim', thanks
> to Elijah.
>
> - In patch 6/6 the documentation for `--signed-commits` in
> "Documentation/git-fast-export.adoc" now spells out that its
> default is 'abort', thanks to Elijah.
>
> - In patch 6/6 a code comment in front of
> find_commit_multiline_header() in "builtin/fast-export.c" has been
> improved:
>
> - a "rathar" vs "rather" typo has been fixed, thanks to Elijah,
>
> - what should be done to the memory returned by the function has
> been corrected, thanks to Phillip Wood.
>
> CI tests
> ~~~~~~~~
>
> All the CI tests passed, except perhaps the osx-gcc one which isn't
> finished yet, see:
>
> https://github.com/chriscool/git/actions/runs/13767984505
>
> Range diff compared to version 5
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> 1: f97247e17d = 1: 395dc9b1d9 git-fast-import.adoc: add missing LF in the BNF
> 2: b71588563d = 2: 6265fd51aa fast-export: fix missing whitespace after switch
> 3: 947bc267e6 = 3: 9e290bab22 fast-export: rename --signed-tags='warn' to 'warn-verbatim'
> 4: 45087db345 ! 4: 923885134f git-fast-export.txt: clarify why 'verbatim' may not be a good idea
> @@ Metadata
> Author: Luke Shumaker <lukeshu@datawire.io>
>
> ## Commit message ##
> - git-fast-export.txt: clarify why 'verbatim' may not be a good idea
> + git-fast-export.adoc: clarify why 'verbatim' may not be a good idea
>
> Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> @@ Documentation/git-fast-export.adoc: OPTIONS
> exported and with 'warn-verbatim' (or 'warn', a deprecated synonym),
> -they will be exported, but you will see a warning.
> +they will be exported, but you will see a warning. 'verbatim' and
> -+'warn-verbatim' should only be used if you know that no
> -+transformation affecting tags will be performed, or if you do not
> -+care that the resulting tag will have an invalid signature.
> ++'warn-verbatim' should only be used if you know that no transformation
> ++affecting tags or any commit in their history will be performed by you
> ++or by fast-export or fast-import, or if you do not care that the
> ++resulting tag will have an invalid signature.
>
> --tag-of-filtered-object=(abort|drop|rewrite)::
> Specify how to handle tags whose tagged object is filtered out.
> 5: 20f085a790 = 5: 49f73ee6ef fast-export: do not modify memory from get_commit_buffer
> 6: 48e0d4203c ! 6: 542c692e67 fast-export, fast-import: add support for signed-commits
> @@ Commit message
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>
> ## Documentation/git-fast-export.adoc ##
> -@@ Documentation/git-fast-export.adoc: they will be exported, but you will see a warning. 'verbatim' and
> - transformation affecting tags will be performed, or if you do not
> - care that the resulting tag will have an invalid signature.
> +@@ Documentation/git-fast-export.adoc: affecting tags or any commit in their history will be performed by you
> + or by fast-export or fast-import, or if you do not care that the
> + resulting tag will have an invalid signature.
>
> +--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> + Specify how to handle signed commits. Behaves exactly as
> -+ '--signed-tags', but for commits.
> ++ '--signed-tags', but for commits. Default is 'abort'.
> ++
> +Earlier versions this command that did not have '--signed-commits'
> +behaved as if '--signed-commits=strip'. As an escape hatch for users
> @@ builtin/fast-export.c: static void anonymize_ident_line(const char **beg, const
>
> +/*
> + * find_commit_multiline_header is similar to find_commit_header,
> -+ * except that it handles multi-line headers, rathar than simply
> ++ * except that it handles multi-line headers, rather than simply
> + * returning the first line of the header.
> + *
> + * The returned string has had the ' ' line continuation markers
> -+ * removed, and points to statically allocated memory (not to memory
> -+ * within 'msg'), so it is only valid until the next call to
> -+ * find_commit_multiline_header.
> ++ * removed, and points to allocated memory that must be free()d (not
> ++ * to memory within 'msg').
> + *
> + * If the header is found, then *end is set to point at the '\n' in
> + * msg that immediately follows the header value.
I didn't look closely at Phillip's comments or your changes related to
those, but the other changes in the range-diff address my comments
from v5, so this version looks good to me.
Thanks!
^ permalink raw reply [flat|nested] 60+ messages in thread