{"thread":{"id":"64536","subject":"[PATCH] replay: do not copy \"gpgsign-sha256\" header","startedAt":"2025-11-26T14:33:49Z","lastAt":"2025-12-01T09:18:49Z","messageCount":4,"participants":["Phillip Wood","Junio C Hamano","Elijah Newren","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"531299","messageId":"4f04af5790353b074cf122c450c1cd3f8d1cecf3.1764167611.git.phillip.wood@dunelm.org.uk","threadId":"64536","inReplyTo":null,"subject":"[PATCH] replay: do not copy \"gpgsign-sha256\" header","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-26T14:33:37Z","receivedAt":"2025-11-26T14:33:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen \"git replay\" replays a commit it copies the extended headers\nacross from the original commit. However, if the original commit\nwas signed, we do not want to copy the header associated with the\nsignature is it wont be valid for the new commit. The code already\nknows to avoid coping the \"gpgsig\" header but does not know to avoid\ncopying the \"gpgsig-sha256\" header. Add that header to the list of\nexclusions to match what \"git commit --amend\" does.\n\nSigned-off-by: Phillip Wood \n---\nWe should perhaps think about how we can centralize this list of\nexclusions as we now have three copies of it in builtin/commit.c,\nbuiltin/replay.c and sequencer.c.\n\nThis patch is based on maint to make it easier to backport.\nUnfortunately that means it conflicts with ps/history which moves the\ncode that's changed here to a new file. I'm happy to rebase on on top\nof that branch if we decide it is not worth backporting this.\n\nBase-Commit: 9a2fb147f2c61d0cab52c883e7e26f5b7948e3ed\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-do-not-copy-gpgsig-sha256-header%2Fv1\nView-Changes-At: https://github.com/phillipwood/git/compare/9a2fb147f...4f04af579\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-do-not-copy-gpgsig-sha256-header/v1\n\n builtin/replay.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/replay.c b/builtin/replay.c\nindex 6172c8aacc9..d12e4d54872 100644\n--- a/builtin/replay.c\n+++ b/builtin/replay.c\n@@ -67,7 +67,7 @@ static struct commit *create_commit(struct repository *repo,\n \tconst char *message = repo_logmsg_reencode(repo, based_on,\n \t\t\t\t\t\t NULL, out_enc);\n \tconst char *orig_message = NULL;\n-\tconst char *exclude_gpgsig[] = { \"gpgsig\", NULL };\n+\tconst char *exclude_gpgsig[] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n \n \tcommit_list_insert(parent, &parents);\n \textra = read_commit_extra_headers(based_on, exclude_gpgsig);\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"531316","messageId":"xmqqh5ugog2l.fsf@gitster.g","threadId":"64536","inReplyTo":"4f04af5790353b074cf122c450c1cd3f8d1cecf3.1764167611.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH] replay: do not copy \"gpgsign-sha256\" header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-26T17:32:18Z","receivedAt":"2025-11-26T17:32:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> When \"git replay\" replays a commit it copies the extended headers\n> across from the original commit. However, if the original commit\n> was signed, we do not want to copy the header associated with the\n> signature is it wont be valid for the new commit. The code already\n> knows to avoid coping the \"gpgsig\" header but does not know to avoid\n> copying the \"gpgsig-sha256\" header. Add that header to the list of\n> exclusions to match what \"git commit --amend\" does.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> We should perhaps think about how we can centralize this list of\n> exclusions as we now have three copies of it in builtin/commit.c,\n> builtin/replay.c and sequencer.c.\n>\n> This patch is based on maint to make it easier to backport.\n> Unfortunately that means it conflicts with ps/history which moves the\n> code that's changed here to a new file. I'm happy to rebase on on top\n> of that branch if we decide it is not worth backporting this.\n\nI'd rather give priority to fixes over new development.\n\nThanks.\n\n>\n> Base-Commit: 9a2fb147f2c61d0cab52c883e7e26f5b7948e3ed\n> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-do-not-copy-gpgsig-sha256-header%2Fv1\n> View-Changes-At: https://github.com/phillipwood/git/compare/9a2fb147f...4f04af579\n> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-do-not-copy-gpgsig-sha256-header/v1\n>\n> builtin/replay.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/replay.c b/builtin/replay.c\n> index 6172c8aacc9..d12e4d54872 100644\n> --- a/builtin/replay.c\n> +++ b/builtin/replay.c\n> @@ -67,7 +67,7 @@ static struct commit *create_commit(struct repository *repo,\n> \tconst char *message = repo_logmsg_reencode(repo, based_on,\n> \t\t\t\t\t\t NULL, out_enc);\n> \tconst char *orig_message = NULL;\n> -\tconst char *exclude_gpgsig[] = { \"gpgsig\", NULL };\n> +\tconst char *exclude_gpgsig[] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n> \n> \tcommit_list_insert(parent, &parents);\n> \textra = read_commit_extra_headers(based_on, exclude_gpgsig);\n"},{"id":"531319","messageId":"CABPp-BHLYBAkhJpmEkxWj+ujHkdb-h-8qAoWMFAe19FNSA+KfA@mail.gmail.com","threadId":"64536","inReplyTo":"4f04af5790353b074cf122c450c1cd3f8d1cecf3.1764167611.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH] replay: do not copy \"gpgsign-sha256\" header","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-11-26T17:36:03Z","receivedAt":"2025-11-26T17:36:14Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Nov 26, 2025 at 6:33 AM Phillip Wood wrote:\n>\n> From: Phillip Wood \n>\n> When \"git replay\" replays a commit it copies the extended headers\n> across from the original commit. However, if the original commit\n> was signed, we do not want to copy the header associated with the\n> signature is it wont be valid for the new commit. The code already\n> knows to avoid coping the \"gpgsig\" header but does not know to avoid\n> copying the \"gpgsig-sha256\" header. Add that header to the list of\n> exclusions to match what \"git commit --amend\" does.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> We should perhaps think about how we can centralize this list of\n> exclusions as we now have three copies of it in builtin/commit.c,\n> builtin/replay.c and sequencer.c.\n>\n> This patch is based on maint to make it easier to backport.\n> Unfortunately that means it conflicts with ps/history which moves the\n> code that's changed here to a new file. I'm happy to rebase on on top\n> of that branch if we decide it is not worth backporting this.\n>\n> Base-Commit: 9a2fb147f2c61d0cab52c883e7e26f5b7948e3ed\n> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-do-not-copy-gpgsig-sha256-header%2Fv1\n> View-Changes-At: https://github.com/phillipwood/git/compare/9a2fb147f...4f04af579\n> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-do-not-copy-gpgsig-sha256-header/v1\n>\n> builtin/replay.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/replay.c b/builtin/replay.c\n> index 6172c8aacc9..d12e4d54872 100644\n> --- a/builtin/replay.c\n> +++ b/builtin/replay.c\n> @@ -67,7 +67,7 @@ static struct commit *create_commit(struct repository *repo,\n> const char *message = repo_logmsg_reencode(repo, based_on,\n> NULL, out_enc);\n> const char *orig_message = NULL;\n> - const char *exclude_gpgsig[] = { \"gpgsig\", NULL };\n> + const char *exclude_gpgsig[] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n>\n> commit_list_insert(parent, &parents);\n> extra = read_commit_extra_headers(based_on, exclude_gpgsig);\n> --\n> 2.52.0.362.g884e03848a9\n\nGood catch, thanks.\n"},{"id":"531491","messageId":"aS1dcz6i5_phTUEG@pks.im","threadId":"64536","inReplyTo":"xmqqh5ugog2l.fsf@gitster.g","subject":"Re: [PATCH] replay: do not copy \"gpgsign-sha256\" header","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-01T09:18:43Z","receivedAt":"2025-12-01T09:18:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Nov 26, 2025 at 09:32:18AM -0800, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n> > From: Phillip Wood \n> >\n> > When \"git replay\" replays a commit it copies the extended headers\n> > across from the original commit. However, if the original commit\n> > was signed, we do not want to copy the header associated with the\n> > signature is it wont be valid for the new commit. The code already\n> > knows to avoid coping the \"gpgsig\" header but does not know to avoid\n> > copying the \"gpgsig-sha256\" header. Add that header to the list of\n> > exclusions to match what \"git commit --amend\" does.\n> >\n> > Signed-off-by: Phillip Wood \n> > ---\n> > We should perhaps think about how we can centralize this list of\n> > exclusions as we now have three copies of it in builtin/commit.c,\n> > builtin/replay.c and sequencer.c.\n\nYeah, that would make sense indeed. We've currently got three different\nversions of this array in \"builtin/replay.c\", \"builtin/commit.c\" and in\n\"sequencer.c\". Furthermore, we've got `gpg_sig_headers` declared as a\nvariable in `commit.c`, but that one is a bit different.\n\nAnyway, the patch itself is an obvious improvement and bug fixg, so\nimproving the maintainability is certainly something we can leave for\na future patch series. #leftoverbits\n\n> > This patch is based on maint to make it easier to backport.\n> > Unfortunately that means it conflicts with ps/history which moves the\n> > code that's changed here to a new file. I'm happy to rebase on on top\n> > of that branch if we decide it is not worth backporting this.\n> \n> I'd rather give priority to fixes over new development.\n\nI'll make sure to rebase git-history(1) on top of your patch in the next\nversion.\n\nThanks!\n\nPatrick\n"}]}