Hi Christian, On Mon, Mar 10, 2025 at 8:58 AM Christian Couder <christian.couder@xxxxxxxxx> 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@xxxxxxxxxxx> > > ## 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@xxxxxxxxxxx> > Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx> > @@ 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@xxxxxxxxxxxxx> > > ## 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!