| Message ID | 20250224142744.279643-1-christian.couder@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | fast-export, fast-import: add support for signed-commits | expand |
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.
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.
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
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(-) >
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.
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".
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(-)