cherry-pick: don't forget -s on failure

In case 'git cherry-pick -s <commit>' failed, the user had to use 'git
commit -s' (i.e. state the -s option again), which is easy to forget
about.  Instead, write the signed-off-by line early, so plain 'git
commit' will have the same result.

Also update 'git commit -s', so that in case there is already a relevant
Signed-off-by line before the Conflicts: line, it won't add one more at
the end of the message. If there is no such line, then add it before the
the Conflicts: line.

Signed-off-by: Miklos Vajna <vmiklos@suse.cz>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

3 files changed, 51 insertions, 8 deletions

diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 0c81b3c427..c82f7210c4 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -30,6 +30,7 @@ test_expect_success setup '
 	test_commit initial foo a &&
 	test_commit base foo b &&
 	test_commit picked foo c &&
+	test_commit --signoff picked-signed foo d &&
 	git config advice.detachedhead false
 
 '
@@ -340,4 +341,35 @@ test_expect_success 'revert conflict, diff3 -m style' '
 	test_cmp expected actual
 '
 
+test_expect_success 'failed cherry-pick does not forget -s' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick -s picked &&
+	test_i18ngrep -e "Signed-off-by" .git/MERGE_MSG
+'
+
+test_expect_success 'commit after failed cherry-pick does not add duplicated -s' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick -s picked-signed &&
+	git commit -a -s &&
+	test $(git show -s |grep -c "Signed-off-by") = 1
+'
+
+test_expect_success 'commit after failed cherry-pick adds -s at the right place' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick picked &&
+	git commit -a -s &&
+	pwd &&
+	cat <<EOF > expected &&
+picked
+
+Signed-off-by: C O Mitter <committer@example.com>
+
+Conflicts:
+	foo
+EOF
+
+	git show -s --pretty=format:%B > actual &&
+	test_cmp expected actual
+'
+
 test_done
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index f4e6450d6a..b5fb527b2e 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -410,7 +410,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '
 	grep "cherry picked from.*$picked" msg
 '
 
-test_expect_success '--signoff is not automatically propagated to resolved conflict' '
+test_expect_failure '--signoff is automatically propagated to resolved conflict' '
 	pristine_detach initial &&
 	test_expect_code 1 git cherry-pick --signoff base..anotherpick &&
 	echo "c" >foo &&
@@ -428,7 +428,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl
 	grep "Signed-off-by:" anotherpick_msg
 '
 
-test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '
+test_expect_failure '--signoff dropped for implicit commit of resolution, multi-pick case' '
 	pristine_detach initial &&
 	test_must_fail git cherry-pick -s picked anotherpick &&
 	echo c >foo &&
@@ -441,7 +441,7 @@ test_expect_success '--signoff dropped for implicit commit of resolution, multi-
 	! grep Signed-off-by: msg
 '
 
-test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '
+test_expect_failure 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '
 	pristine_detach initial &&
 	test_must_fail git cherry-pick -s picked &&
 	echo c >foo &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 9bc57d27e9..8889ba5104 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -144,11 +144,22 @@ test_pause () {
 
 test_commit () {
 	notick= &&
-	if test "z$1" = "z--notick"
-	then
-		notick=yes
+	signoff= &&
+	while test $# != 0
+	do
+		case "$1" in
+		--notick)
+			notick=yes
+			;;
+		--signoff)
+			signoff="$1"
+			;;
+		*)
+			break
+			;;
+		esac
 		shift
-	fi &&
+	done &&
 	file=${2:-"$1.t"} &&
 	echo "${3-$1}" > "$file" &&
 	git add "$file" &&
@@ -156,7 +167,7 @@ test_commit () {
 	then
 		test_tick
 	fi &&
-	git commit -m "$1" &&
+	git commit $signoff -m "$1" &&
 	git tag "$1"
 }