Re: [PATCH 2/2] replay: add the -S option

[Junio C Hamano <gitster@pobox.com>]

Patrick Monette <pmonette@google.com> writes:

> `git replay` currently doesn't support signing. In fact, there is a
> FIXME to address this.
>
> Add the -S option and its related options --gpg-sign and --no-gpg-sign.
>
> Signed-off-by: Patrick Monette <pmonette@google.com>
> ---
>  Documentation/git-replay.adoc |  10 +++-
>  builtin/replay.c              |  12 +++-
>  replay.c                      |  12 ++--
>  replay.h                      |   6 ++
>  t/meson.build                 |   1 +
>  t/t3651-replay-gpg-sign.sh    | 107 ++++++++++++++++++++++++++++++++++
>  6 files changed, 141 insertions(+), 7 deletions(-)
>  create mode 100755 t/t3651-replay-gpg-sign.sh

The main part of the patch, which is the change to replay.[ch], has
striking similarity to another topic from mid July [*].

  https://lore.kernel.org/git/20260717145142.39478-2-git@5ouma.me/

That topic has its latest reroll posted recently and it still looks
very similar.

  https://lore.kernel.org/git/20261002132718.3830-2-git@5ouma.me/

Instead of making duplicated effort, given that this community is
limited by reviewer bandwidth more than it is in need of new
patches, it would be very much appreciated if you can give a review
to the other topic to help another developer and move it forward.

There would be things your topic wanted to do that is different from
what they wanted to achieve.  Theirs is about "git history", and
this topic is about "git replay".  So after their topic stabilized,
you can salvage the remainder of your topic and rebase them on top
of their patch.

Thanks.


[Footnote]

 * It shows us that there are only certain ways to implement a
   thing, and it is hard to be "original" these days ;-)


> diff --git a/replay.c b/replay.c
> index ad87863565..9a84e297b1 100644
> --- a/replay.c
> +++ b/replay.c
> @@ -85,13 +85,13 @@ static struct commit *create_commit(struct repository *repo,
>  				    struct tree *tree,
>  				    struct commit *based_on,
>  				    struct commit *parent,
> -				    enum replay_mode mode)
> +				    enum replay_mode mode,
> +				    const char *sign_commit)
>  {
>  	struct object_id ret;
>  	struct object *obj = NULL;
>  	struct commit_list *parents = NULL;
>  	char *author = NULL;
> -	char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
>  	struct commit_extra_header *extra = NULL;
>  	struct strbuf msg = STRBUF_INIT;
>  	const char *out_enc = get_commit_output_encoding();
> @@ -288,7 +288,8 @@ static struct commit *pick_regular_commit(struct repository *repo,
>  					  struct merge_options *merge_opt,
>  					  struct merge_result *result,
>  					  enum replay_mode mode,
> -					  enum replay_empty_commit_action empty)
> +					  enum replay_empty_commit_action empty,
> +					  const char *sign_commit)
>  {
>  	struct tree *pickme_tree, *base_tree, *replayed_base_tree;
>  	struct commit *new_commit;
> @@ -363,7 +364,7 @@ static struct commit *pick_regular_commit(struct repository *repo,
>  	}
>  
>  	new_commit = create_commit(repo, result->tree, pickme, replayed_base,
> -				   mode);
> +				   mode, sign_commit);
>  	if (!new_commit)
>  		result->clean = -1;
>  	return new_commit;
> @@ -486,7 +487,8 @@ int replay_revisions(struct rev_info *revs,
>  
>  			last_commit = pick_regular_commit(revs->repo, commit, base,
>  							  &merge_opt, &result,
> -							  mode, opts->empty);
> +							  mode, opts->empty,
> +							  opts->sign_commit);
>  		}
>  
>  		if (!last_commit)
> diff --git a/replay.h b/replay.h
> index 2c71afbfde..7e93ab9565 100644
> --- a/replay.h
> +++ b/replay.h
> @@ -67,6 +67,12 @@ struct replay_revisions_options {
>  	 * Whether to linearize the commits (i.e. drop merge commits).
>  	 */
>  	int linearize;
> +
> +	/*
> +	 * If non-NULL, GPG-sign the new commits. An empty string signs with
> +	 * the default key (the committer identity); otherwise, the key ID.
> +	 */
> +	const char *sign_commit;
>  };
>  
>  /* This struct is used as an out-parameter by `replay_revisions()`. */