[PATCH v3 0/2] fast-import: start controlling how commit signatures are handled

Tools like `git-filter-repo` should be able to control how commit signatures are handled when regenerating repository content after it has been filtered (see https://github.com/newren/git-filter-repo/issues/139). For this purpose, they need a way for `git fast-import` to control how commit signatures are handled.

This small patch series starts to implement such a way by adding a new `--signed-commits=<mode>` option to `git fast-import`.

For now this new option behaves in a very similar way as the option with the same name that already exists in `git fast-export`. Especially it supports exactly the same <mode>s and the same aliases for these modes. For example "ignore" is a synonym for "verbatim".

In the future I want to implement new <mode>s like "strip-if-invalid", "re-sign", "re-sign-if-invalid" that might be a bit more complex for this option. But for now I prefer to start with the simple modes to validate the general design of the new option.

In the future I also plan to add a similar `--signed-tags=<mode>` so that the import of tags can also be controlled. But I prefer to validate the general design of a single new option first.

In particular, I am interested in feedback about the following:

  - Should we keep "ignore" as a synonym for "verbatim" and "warn" as
    a synonym for "warn-verbatim"? My opinion is that they might be
    confusing, so we might want to remove them for `git fast-import`
    even if we keep them for `git fast-export`. The parsing code might
    be a bit more complex if we do that though, so for now I have kept
    the synonyms.

  - Are we still fine with most <mode>s having a "warn-*" variant
    (like the "warn-strip" variant of "strip" for example)? Or should
    we have a separate `--verbose` or maybe `--signed-commits-verbose`
    option dedicated to switching warnings on/off? I think it's good
    to decide about this before the number of <mode>s increases a lot
    with new <mode>s like "strip-if-invalid", "re-sign",
    "re-sign-if-invalid" and possibly others.

Changes since v2: =================

Thanks to Junio and Patrick for reviewing previous versions.

In patch 2/2:

  - The commit message has been improved to explain why new functions
    are added and how the 'strip' and 'warn-strip' modes are handled.

  - The code responsible for consuming a signature has been refactored
    into two new functions: import_one_signature() and
    discard_one_signature() functions. This makes it easier to follow
    the logic and add new modes in the future.

  - The SIGN_ABORT case is not handled separately from the other cases
    anymore.

  - Code comments have been added to the
    `switch (signed_commit_mode) { ... }` that handles the different
    cases, and a `default: ...` case that calls BUG() has been added
    to it.

CI tests: =========

They have all passed. See:

https://github.com/chriscool/git/actions/runs/17804251337

Range-diff vs V2: =================

1:  87149ae92d = 1:  87149ae92d gpg-interface: refactor 'enum sign_mode' parsing
2:  e34f015aea ! 2:  cf6ce66e1d fast-import: add '--signed-commits=<mode>' option
    @@ Commit message
         For now the supported <mode>s are the same as those supported by
         `git fast-export`.
     
    +    The code responsible for consuming a signature is refactored into
    +    the import_one_signature() and discard_one_signature() functions,
    +    which makes it easier to follow the logic and add new modes in the
    +    future.
    +
    +    In the 'strip' and 'warn-strip' modes, we deliberately use
    +    discard_one_signature() to discard the signature without parsing it.
    +    This ensures that even malformed signatures, which would cause the
    +    parser to fail, can be successfully stripped from a commit.
    +
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
      ## Documentation/git-fast-import.adoc ##
    @@ builtin/fast-import.c: static int global_argc;
      /* Memory pools */
      static struct mem_pool fi_mem_pool = {
      	.block_alloc = 2*1024*1024 - sizeof(struct mp_block),
    +@@ builtin/fast-import.c: static void parse_one_signature(struct signature_data *sig, const char *v)
    + 	parse_data(&sig->data, 0, NULL);
    + }
    + 
    ++static void discard_one_signature(void)
    ++{
    ++	struct strbuf data = STRBUF_INIT;
    ++
    ++	read_next_command();
    ++	parse_data(&data, 0, NULL);
    ++	strbuf_release(&data);
    ++}
    ++
    + static void add_gpgsig_to_commit(struct strbuf *commit_data,
    + 				 const char *header,
    + 				 struct signature_data *sig)
    +@@ builtin/fast-import.c: static void store_signature(struct signature_data *stored_sig,
    + 	}
    + }
    + 
    ++static void import_one_signature(struct signature_data *sig_sha1,
    ++				 struct signature_data *sig_sha256,
    ++				 const char *v)
    ++{
    ++	struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    ++
    ++	parse_one_signature(&sig, v);
    ++
    ++	if (!strcmp(sig.hash_algo, "sha1"))
    ++		store_signature(sig_sha1, &sig, "SHA-1");
    ++	else if (!strcmp(sig.hash_algo, "sha256"))
    ++		store_signature(sig_sha256, &sig, "SHA-256");
    ++	else
    ++		die(_("parse_one_signature() returned unknown hash algo"));
    ++}
    ++
    + static void parse_new_commit(const char *arg)
    + {
    + 	static struct strbuf msg = STRBUF_INIT;
     @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
      	if (!committer)
      		die("Expected committer but didn't get one");
      
     -	/* Process signatures (up to 2: one "sha1" and one "sha256") */
      	while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
    - 		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    - 
    +-		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    +-
     -		parse_one_signature(&sig, v);
    -+		if (signed_commit_mode == SIGN_ABORT)
    -+			die(_("encountered signed commit; use "
    -+			      "--signed-commits=<mode> to handle it"));
    ++		switch (signed_commit_mode) {
    ++
    ++		/* First, modes that don't need the signature to be parsed */
    ++		case SIGN_ABORT:
    ++			die("encountered signed commit; use "
    ++			    "--signed-commits=<mode> to handle it");
    ++		case SIGN_WARN_STRIP:
    ++			warning(_("stripping a commit signature"));
    ++			/* fallthru */
    ++		case SIGN_STRIP:
    ++			discard_one_signature();
    ++			break;
      
     -		if (!strcmp(sig.hash_algo, "sha1"))
     -			store_signature(&sig_sha1, &sig, "SHA-1");
    @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
     -			store_signature(&sig_sha256, &sig, "SHA-256");
     -		else
     -			BUG("parse_one_signature() returned unknown hash algo");
    -+		parse_one_signature(&sig, v);
    - 
    -+		switch (signed_commit_mode) {
    -+		case SIGN_ABORT:
    -+			BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
    -+			break;
    ++		/* Second, modes that parse the signature */
     +		case SIGN_WARN_VERBATIM:
     +			warning(_("importing a commit signature verbatim"));
     +			/* fallthru */
     +		case SIGN_VERBATIM:
    -+			if (!strcmp(sig.hash_algo, "sha1"))
    -+				store_signature(&sig_sha1, &sig, "SHA-1");
    -+			else if (!strcmp(sig.hash_algo, "sha256"))
    -+				store_signature(&sig_sha256, &sig, "SHA-256");
    -+			else
    -+				die(_("parse_one_signature() returned unknown hash algo"));
    -+			break;
    -+		case SIGN_WARN_STRIP:
    -+			warning(_("stripping a commit signature"));
    -+			/* fallthru */
    -+		case SIGN_STRIP:
    -+			/* Just discard signature data */
    -+			strbuf_release(&sig.data);
    -+			free(sig.hash_algo);
    ++			import_one_signature(&sig_sha1, &sig_sha256, v);
     +			break;
    + 
    ++		/* Third, BUG */
    ++		default:
    ++			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
     +		}
      		read_next_command();
      	}

Christian Couder (2):
  gpg-interface: refactor 'enum sign_mode' parsing
  fast-import: add '--signed-commits=<mode>' option

 Documentation/git-fast-import.adoc |   5 ++
 builtin/fast-export.c              |  19 ++----
 builtin/fast-import.c              |  63 ++++++++++++++---
 gpg-interface.c                    |  17 +++++
 gpg-interface.h                    |  15 ++++
 t/meson.build                      |   1 +
 t/t9305-fast-import-signatures.sh  | 106 +++++++++++++++++++++++++++++
 7 files changed, 202 insertions(+), 24 deletions(-)
 create mode 100755 t/t9305-fast-import-signatures.sh