[PATCH] sequencer: honor signoff opt in run_git_commit

From: David Bimmler via GitGitGadget <gitgitgadget@gmail.com>
Date: Apr 4, 2024, 09:39 UTC
Message-ID: <pull.1707.git.1712223572933.gitgitgadget@gmail.com>

From: David Bimmler <david.bimmler@isovalent.com>

When rebasing interactively, --signoff would not take effect for commits which conflict. That is, commits applying cleanly would be signed off, but commits requiring intervention would miss the sign off trailer.

The reason is that run_git_commit did not check for the signoff replay opt, and hence even though the option was picked up and passed correctly, the actual committing dropped the ball.

The patch adds a test specifically for this case, as well as amending a squash test which codified the broken behaviour.

Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
---
    sequencer: honor signoff opt in run_git_commit

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1707%2Fbimmlerd%2Fsignoff-conflicting-commits-in-rebase-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1707/bimmlerd/signoff-conflicting-commits-in-rebase-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1707

 sequencer.c                     |  2 ++
 t/t3428-rebase-signoff.sh       | 33 +++++++++++++++++++++++++++++++++
 t/t3437/expected-squash-message |  2 ++
 3 files changed, 37 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index fa838f264f5..16595e26a17 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1121,6 +1121,8 @@ static int run_git_commit(const char *defmsg,
 		strvec_pushf(&cmd.args, "-S%s", opts->gpg_sign);
 	else
 		strvec_push(&cmd.args, "--no-gpg-sign");
+	if (opts->signoff)
+		strvec_push(&cmd.args, "--signoff");
 	if (defmsg)
 		strvec_pushl(&cmd.args, "-F", defmsg, NULL);
 	else if (!(flags & EDIT_MSG))
diff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh
index e1b1e947647..fcecdf41978 100755
--- a/t/t3428-rebase-signoff.sh
+++ b/t/t3428-rebase-signoff.sh
@@ -27,6 +27,13 @@ first
 Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
 EOF
 
+# Expected signed off message after resolving the conflict
+cat >expected-signed-after-conflict <<EOF
+update file on side
+
+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
+EOF
+
 # Expected commit message after rebase without --signoff (or with --no-signoff)
 cat >expected-unsigned <<EOF
 first
@@ -82,4 +89,30 @@ test_expect_success 'rebase -m --signoff fails' '
 	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
 	test_cmp expected-signed actual
 '
+
+test_expect_success 'rebase -i signs commits even if a conflict occurs' '
+	git branch -M main &&
+
+	git branch side &&
+	echo "b" >file &&
+	git add file &&
+	git commit -m"update file" &&
+	test_tick &&
+
+	git checkout side &&
+	echo "side" >file &&
+	git add file &&
+	git commit -m"update file on side" &&
+	test_tick &&
+
+	test_must_fail git rebase -i --signoff main &&
+
+	echo "merged" >file &&
+	git add file &&
+	git rebase --continue &&
+
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	test_cmp expected-signed-after-conflict actual
+'
+
 test_done
diff --git a/t/t3437/expected-squash-message b/t/t3437/expected-squash-message
index ab2434f90ed..d74af0bcf6b 100644
--- a/t/t3437/expected-squash-message
+++ b/t/t3437/expected-squash-message
@@ -48,4 +48,6 @@ edited 1
 edited 2
 
 edited 3
+
+Signed-off-by: Rebase Committer <rebase.committer@example.com>
 squashed

base-commit: 7774cfed6261ce2900c84e55906da708c711d601

-- 
gitgitgadget