git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits

From
Taylor Blau <ttaylorr@github.com>
Date
Apr 20, 2021, 01:45 UTC
Message-ID
<YH4yN2OAghWB9/97@nand.local>
In-Reply-To
<20210419225441.3139048-4-lukeshu@lukeshu.com>
On Mon, Apr 19, 2021 at 04:54:41PM -0600, Luke Shumaker wrote:
Show 54 quoted lines
> From: Luke Shumaker <lukeshu@datawire.io>
>
> fast-export has an existing --signed-tags= flag 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 signed-commits.
>
> On the fast-export side, try to be as much like signed-tags as possible,
> in both implementation and in user-interface; with the exception that
> the default should be `--signed-commits=strip` (compared to the default
> `--signed-tags=abort`), in order to continue defaulting to the
> historical behavior.  Only bother implementing "gpgsig", not
> "gpgsig-sha256"; the existing signed-tag support doesn't implement
> "gpgsig-sha256" either.
>
> On the fast-import side, I'm not entirely sure that I got the ordering
> correct between "gpgsig" and "encoding" when generating the commit
> object.
>
> Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
> ---
>  Documentation/git-fast-export.txt | 12 +++++
>  Documentation/git-fast-import.txt |  7 +++
>  builtin/fast-export.c             | 86 +++++++++++++++++++++++++------
>  builtin/fast-import.c             | 15 ++++++
>  t/t9350-fast-export.sh            | 70 +++++++++++++++++++++++++
>  5 files changed, 174 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
> index d4a2bfe037..6fdb678b54 100644
> --- a/Documentation/git-fast-export.txt
> +++ b/Documentation/git-fast-export.txt
> @@ -39,6 +39,18 @@ warning will be displayed, with 'verbatim', they will be silently
>  exported and with 'warn-verbatim', they will be exported, but you will
>  see a warning.
>
> +--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
> +	Specify how to handle signed commits.  Since any transformation
> +	after the export can change the commit (which can also happen
> +	when excluding revisions) the signatures will not match.
> ++
> +When asking to 'abort', this program will die when encountering a
> +signed commit.  With 'strip' (which is the default), the commits 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', they will be exported,
> +but you will see a warning.
> +

OK, this all seems normal to me. But it may be worth shortening it to say "behaves exactly as --signed-tags, but for commits", or something.

Show 12 quoted lines
>  --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..3d0c5dbf7d 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -437,6 +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
> +	('gpgsig' LF data)?
Is this missing a LF after data?
Show 18 quoted lines
> +static const char *find_signature(const char *begin, const char *end)
> +{
> +	const char *needle = "\ngpgsig ";
> +	char *bod, *eod, *eol;
> +
> +	bod = memmem(begin, end ? end - begin : strlen(begin),
> +		     needle, strlen(needle));
> +	if (!bod)
> +		return NULL;
> +	bod += strlen(needle);
> +	eod = strchrnul(bod, '\n');
> +	while (eod[0] == '\n' && eod[1] == ' ') {
> +		eod = strchrnul(eod+1, '\n');
> +	}
> +	*eod = '\0';
> +
> +	while ((eol = strstr(bod, "\n ")))
> +		memmove(eol+1, eol+2, strlen(eol+1));

Hmm. I'm not quite sure I follow these last two lines. Perhaps a comment would help? The rest of this patch looks reasonable to me.

Thanks, Taylor

Previous: Luke ShumakerNext: Luke Shumaker
Message 11 of 15 in “fast-export, fast-import: implement signed-commits”
  1. 0/3 fast-export, fast-import: implement signed-commitsLuke Shumaker, Apr 19, 2021
  2. 1/3 git-fast-import.txt: add missing LF in the BNFLuke Shumaker, Apr 19, 2021
  3. 2/3 fast-export: rename --signed-tags='warn' to 'warn-verbatim'Luke Shumaker, Apr 19, 2021
  4. Taylor BlauApr 20, 2021
  5. Luke ShumakerApr 20, 2021
  6. 3/3 fast-export, fast-import: implement signed-commitsLuke Shumaker, Apr 19, 2021
  7. brian m. carlsonApr 20, 2021
  8. Luke ShumakerApr 20, 2021
  9. brian m. carlsonApr 20, 2021
  10. Luke ShumakerApr 21, 2021
  11. Taylor BlauApr 20, 2021
  12. Luke ShumakerApr 20, 2021
  13. Luke ShumakerApr 20, 2021
  14. Elijah NewrenApr 21, 2021
  15. Luke ShumakerApr 21, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.