[PATCH v3 18/27] rebase -i: remove no-op do_with_author git commit --amend

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The author script is a file in the state directory that contains
assignments of the environment variables

    GIT_AUTHOR_NAME
    GIT_AUTHOR_EMAIL
    GIT_AUTHOR_DATE

to be evaluated by the shell. It is used to store author information
and has two applications in `git-rebase--interactive.sh`. Firstly,
the authorship of squash commits is read from it while the squash
commit is being amended step by step. Secondly, after conflict
resolution `git rebase --continue` restores the author information of
the original commit by sourcing the author script. For the
assignments of the git environment variables to take effect,
git-rebase--interactive executes the respective git-commit commands
wrapped in `do_with_author`. That shell function executes the named
command in a subshell that exports the git environment variables.

Since

    git commit --amend

has been used instead of

    git reset --soft HEAD^
    git commit

to amend squash commits, wrapping git-commit in `do_with_author` has
become a no-op because, unless the `--reset-author` option is
specified, `git commit --amend` ignores the user environment and
reuses the authorship of the commit it amends. To make the code
clearer and until we decide to use other authorships for squash
commits than the one of the first commit, unwrap `git commit
--amend`.

Signed-off-by: Fabian Ruch <bafain@xxxxxxxxx>
---
 git-rebase--interactive.sh    | 17 +++++++----------
 t/t3404-rebase-interactive.sh | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f8be238..ab807e5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -622,9 +622,6 @@ do_next () {
 
 		mark_action_done
 		update_squash_messages $squash_style $sha1
-		author_script_content=$(get_author_ident_from_commit HEAD)
-		echo "$author_script_content" > "$author_script"
-		eval "$author_script_content"
 		if ! pick_one -n $sha1
 		then
 			git rev-parse --verify HEAD >"$amend"
@@ -634,7 +631,7 @@ do_next () {
 		squash|s|fixup|f)
 			# This is an intermediate commit; its message will only be
 			# used in case of trouble.  So use the long version:
-			do_with_author output git commit --allow-empty-message --allow-empty \
+			output git commit --allow-empty-message --allow-empty \
 				--amend --no-verify -F "$squash_msg" \
 				${gpg_sign_opt:+"$gpg_sign_opt"} ||
 				die_failed_squash $sha1 "$rest"
@@ -643,14 +640,14 @@ do_next () {
 			# This is the final command of this squash/fixup group
 			if test -f "$fixup_msg"
 			then
-				do_with_author output git commit --allow-empty-message --allow-empty \
+				output git commit --allow-empty-message --allow-empty \
 					--amend --no-verify -F "$fixup_msg" \
 					${gpg_sign_opt:+"$gpg_sign_opt"} ||
 					die_failed_squash $sha1 "$rest"
 			else
 				cp "$squash_msg" "$GIT_DIR"/SQUASH_MSG || exit
 				rm -f "$GIT_DIR"/MERGE_MSG
-				do_with_author output git commit --allow-empty --amend --no-pre-commit -F "$GIT_DIR"/SQUASH_MSG -e \
+				output git commit --allow-empty --amend --no-pre-commit -F "$GIT_DIR"/SQUASH_MSG -e \
 					${gpg_sign_opt:+"$gpg_sign_opt"} ||
 					die_failed_squash $sha1 "$rest"
 			fi
@@ -939,7 +936,7 @@ continue)
 	then
 		: Nothing to commit -- skip this
 	else
-		if ! test -f "$author_script"
+		if ! test -f "$author_script" && ! test -f "$amend"
 		then
 			gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
 			die "You have staged changes in your working tree. If these changes are meant to be
@@ -956,8 +953,6 @@ In both case, once you're done, continue with:
   git rebase --continue
 "
 		fi
-		. "$author_script" ||
-			die "Error trying to find the author identity to amend commit"
 		if test -f "$amend"
 		then
 			current_head=$(git rev-parse --verify HEAD)
@@ -965,10 +960,12 @@ In both case, once you're done, continue with:
 			die "\
 You have uncommitted changes in your working tree. Please, commit them
 first and then run 'git rebase --continue' again."
-			do_with_author git commit --amend --no-verify -F "$msg" -e \
+			git commit --amend --no-verify -F "$msg" -e \
 				${gpg_sign_opt:+"$gpg_sign_opt"} ||
 				die "Could not commit staged changes."
 		else
+			. "$author_script" ||
+				die "Error trying to find the author identity to amend commit"
 			do_with_author git commit --no-verify -F "$msg" -e \
 				${gpg_sign_opt:+"$gpg_sign_opt"} ||
 				die "Could not commit staged changes."
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8de7a39..c037a07 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1270,4 +1270,27 @@ test_expect_success 'fixup commits with empty commit log messages' '
 	env FAKE_LINES="1 fixup 2" git rebase -i master
 '
 
+test_expect_success 'squash commits have authorship of the first commit' '
+	git checkout -b squash-authorship master &&
+	git cat-file commit HEAD~3 | sed -n -e "/^$/q" -e "/^author /p" >expected.author &&
+	test_tick &&
+	set_fake_editor &&
+	FAKE_LINES="1 squash 2 squash 3 fixup 4" git rebase -i HEAD~4 &&
+	git cat-file commit HEAD | sed -n -e "/^$/q" -e "/^author /p" >actual.author &&
+	test_cmp expected.author actual.author
+'
+
+test_expect_success 'squash commits have authorship of the first commit after conflict' '
+	git checkout -b squash-authorship-conflict conflict-branch &&
+	git cat-file commit HEAD~3 | sed -n -e "/^$/q" -e "/^author /p" >expected.author &&
+	test_tick &&
+	set_fake_editor &&
+	test_must_fail env FAKE_LINES="1 squash 2 squash 4 fixup 3" git rebase -i HEAD~4 &&
+	git checkout --ours conflict &&
+	git add conflict &&
+	git rebase --continue &&
+	git cat-file commit HEAD | sed -n -e "/^$/q" -e "/^author /p" >actual.author &&
+	test_cmp expected.author actual.author
+'
+
 test_done
-- 
2.0.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]