[PATCH] replay: do not copy "gpgsign-sha256" header

[PATCH] replay: do not copy "gpgsign-sha256" header

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

When "git replay" replays a commit it copies the extended headers
across from the original commit. However, if the original commit
was signed, we do not want to copy the header associated with the
signature is it wont be valid for the new commit. The code already
knows to avoid coping the "gpgsig" header but does not know to avoid
copying the "gpgsig-sha256" header.  Add that header to the list of
exclusions to match what "git commit --amend" does.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
We should perhaps think about how we can centralize this list of
exclusions as we now have three copies of it in builtin/commit.c,
builtin/replay.c and sequencer.c.

This patch is based on maint to make it easier to backport.
Unfortunately that means it conflicts with ps/history which moves the
code that's changed here to a new file. I'm happy to rebase on on top
of that branch if we decide it is not worth backporting this.

Base-Commit: 9a2fb147f2c61d0cab52c883e7e26f5b7948e3ed
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-do-not-copy-gpgsig-sha256-header%2Fv1
View-Changes-At: https://github.com/phillipwood/git/compare/9a2fb147f...4f04af579
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-do-not-copy-gpgsig-sha256-header/v1

 builtin/replay.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 6172c8aacc9..d12e4d54872 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -67,7 +67,7 @@ static struct commit *create_commit(struct repository *repo,
 	const char *message = repo_logmsg_reencode(repo, based_on,
 						   NULL, out_enc);
 	const char *orig_message = NULL;
-	const char *exclude_gpgsig[] = { "gpgsig", NULL };
+	const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
 
 	commit_list_insert(parent, &parents);
 	extra = read_commit_extra_headers(based_on, exclude_gpgsig);
-- 
2.52.0.362.g884e03848a9

Re: [PATCH] replay: do not copy "gpgsign-sha256" header

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> When "git replay" replays a commit it copies the extended headers
> across from the original commit. However, if the original commit
> was signed, we do not want to copy the header associated with the
> signature is it wont be valid for the new commit. The code already
> knows to avoid coping the "gpgsig" header but does not know to avoid
> copying the "gpgsig-sha256" header.  Add that header to the list of
> exclusions to match what "git commit --amend" does.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> We should perhaps think about how we can centralize this list of
> exclusions as we now have three copies of it in builtin/commit.c,
> builtin/replay.c and sequencer.c.
>
> This patch is based on maint to make it easier to backport.
> Unfortunately that means it conflicts with ps/history which moves the
> code that's changed here to a new file. I'm happy to rebase on on top
> of that branch if we decide it is not worth backporting this.

I'd rather give priority to fixes over new development.

Thanks.

>
> Base-Commit: 9a2fb147f2c61d0cab52c883e7e26f5b7948e3ed
> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-do-not-copy-gpgsig-sha256-header%2Fv1
> View-Changes-At: https://github.com/phillipwood/git/compare/9a2fb147f...4f04af579
> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-do-not-copy-gpgsig-sha256-header/v1
>
>  builtin/replay.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/replay.c b/builtin/replay.c
> index 6172c8aacc9..d12e4d54872 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -67,7 +67,7 @@ static struct commit *create_commit(struct repository *repo,
>  	const char *message = repo_logmsg_reencode(repo, based_on,
>  						   NULL, out_enc);
>  	const char *orig_message = NULL;
> -	const char *exclude_gpgsig[] = { "gpgsig", NULL };
> +	const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
>  
>  	commit_list_insert(parent, &parents);
>  	extra = read_commit_extra_headers(based_on, exclude_gpgsig);

Re: [PATCH] replay: do not copy "gpgsign-sha256" header

[Elijah Newren]

On Wed, Nov 26, 2025 at 6:33 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> When "git replay" replays a commit it copies the extended headers
> across from the original commit. However, if the original commit
> was signed, we do not want to copy the header associated with the
> signature is it wont be valid for the new commit. The code already
> knows to avoid coping the "gpgsig" header but does not know to avoid
> copying the "gpgsig-sha256" header.  Add that header to the list of
> exclusions to match what "git commit --amend" does.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> We should perhaps think about how we can centralize this list of
> exclusions as we now have three copies of it in builtin/commit.c,
> builtin/replay.c and sequencer.c.
>
> This patch is based on maint to make it easier to backport.
> Unfortunately that means it conflicts with ps/history which moves the
> code that's changed here to a new file. I'm happy to rebase on on top
> of that branch if we decide it is not worth backporting this.
>
> Base-Commit: 9a2fb147f2c61d0cab52c883e7e26f5b7948e3ed
> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-do-not-copy-gpgsig-sha256-header%2Fv1
> View-Changes-At: https://github.com/phillipwood/git/compare/9a2fb147f...4f04af579
> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-do-not-copy-gpgsig-sha256-header/v1
>
>  builtin/replay.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/replay.c b/builtin/replay.c
> index 6172c8aacc9..d12e4d54872 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -67,7 +67,7 @@ static struct commit *create_commit(struct repository *repo,
>         const char *message = repo_logmsg_reencode(repo, based_on,
>                                                    NULL, out_enc);
>         const char *orig_message = NULL;
> -       const char *exclude_gpgsig[] = { "gpgsig", NULL };
> +       const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
>
>         commit_list_insert(parent, &parents);
>         extra = read_commit_extra_headers(based_on, exclude_gpgsig);
> --
> 2.52.0.362.g884e03848a9

Good catch, thanks.

Re: [PATCH] replay: do not copy "gpgsign-sha256" header

[Patrick Steinhardt]

On Wed, Nov 26, 2025 at 09:32:18AM -0800, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> > From: Phillip Wood <phillip.wood@dunelm.org.uk>
> >
> > When "git replay" replays a commit it copies the extended headers
> > across from the original commit. However, if the original commit
> > was signed, we do not want to copy the header associated with the
> > signature is it wont be valid for the new commit. The code already
> > knows to avoid coping the "gpgsig" header but does not know to avoid
> > copying the "gpgsig-sha256" header.  Add that header to the list of
> > exclusions to match what "git commit --amend" does.
> >
> > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> > ---
> > We should perhaps think about how we can centralize this list of
> > exclusions as we now have three copies of it in builtin/commit.c,
> > builtin/replay.c and sequencer.c.

Yeah, that would make sense indeed. We've currently got three different
versions of this array in "builtin/replay.c", "builtin/commit.c" and in
"sequencer.c". Furthermore, we've got `gpg_sig_headers` declared as a
variable in `commit.c`, but that one is a bit different.

Anyway, the patch itself is an obvious improvement and bug fixg, so
improving the maintainability is certainly something we can leave for
a future patch series. #leftoverbits

> > This patch is based on maint to make it easier to backport.
> > Unfortunately that means it conflicts with ps/history which moves the
> > code that's changed here to a new file. I'm happy to rebase on on top
> > of that branch if we decide it is not worth backporting this.
> 
> I'd rather give priority to fixes over new development.

I'll make sure to rebase git-history(1) on top of your patch in the next
version.

Thanks!

Patrick