Re: [PATCH v4 5/5] fast-export, fast-import: add support for signed-commits

Luke Shumaker [off-list ref] writes:

From: Luke Shumaker <redacted>

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().

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 &&