git-rebase--interactive handles empty log messages inconsistently
between enabled and disabled fast-forwards. By default, commits with
empty log messages are rebased successfully like in non-interactive
mode. In contrast, the `--no-ff` option aborts the replay of such
commits.

In line with not verifying rebased commits and behaving like
git-rebase for `pick` lines, use the `--allow-empty-message` option
to replay commits. Root commits are replayed by recreating them in
`do_pick` using git-commit and all other commits are replayed using
git-cherry-pick in `pick_one`. Apply the option, understood by both
git-commit and git-cherry-pick, at the respective sites.

In case of `reword` and `squash`, continue to abort the rebase if the
_resulting_ commit would have no commit message. The rationale behind
this default is that patches and their log messages should be
verified at least once. For unchanged commits this is assumed to have
happened according to the author's standards when she created the
commits for the first time. While the empty log message can always be
kept in place by editing and resuming the aborted rebase, a debatable
alternative could be to teach git-rebase--interactive the option
`--allow-empty-message` for disabling complaints about empty log
messages even in changed commits.

The `fixup` case is different again because it throws away the second
commit's log message and uses the first log message for the changed
commit. Do not abort the rebase if that message is empty either since
it is assumed to have been verified already.

The remaining to-do list command `edit` is handled just like `pick`
for this matter, because git-rebase--interactive replays the named
commit without changes before the rebase is interrupted and the user
can make her changes to the replayed commit.

Add tests. In particular, design the `squash`-specific test case such
that it involves interim commits and `fixup` steps. Interim commits
should not trigger failures themselves and `fixup` steps should not
let git-rebase--interactive forget that it is still dealing with a
`squash` result.

Signed-off-by: Fabian Ruch <baf...@gmail.com>
---
 git-rebase--interactive.sh    | 10 ++++++----
 t/t3404-rebase-interactive.sh | 38 ++++++++++++++++++++++++++++++++++++++
 t/t3412-rebase-root.sh        | 16 ++++++++++++++++
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b64dd28..3222bf6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -249,7 +249,7 @@ pick_one () {
 
        test -d "$rewritten" &&
                pick_one_preserving_merges "$@" && return
-       output eval git cherry-pick \
+       output eval git cherry-pick --allow-empty-message \
                        ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
                        "$strategy_args" $empty_args $ff "$@"
 }
@@ -363,7 +363,7 @@ pick_one_preserving_merges () {
                        echo "$sha1 $(git rev-parse HEAD^0)" >> 
"$rewritten_list"
                        ;;
                *)
-                       output eval git cherry-pick \
+                       output eval git cherry-pick --allow-empty-message \
                                ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
                                "$strategy_args" "$@" ||
                                die_with_patch $sha1 "Could not pick $sha1"
@@ -549,7 +549,8 @@ 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 --amend --no-verify -F 
"$squash_msg" \
+                       do_with_author output git commit --allow-empty-message \
+                               --amend --no-verify -F "$squash_msg" \
                                ${gpg_sign_opt:+"$gpg_sign_opt"} ||
                                die_failed_squash $sha1 "$rest"
                        ;;
@@ -557,7 +558,8 @@ do_next () {
                        # This is the final command of this squash/fixup group
                        if test -f "$fixup_msg"
                        then
-                               do_with_author git commit --amend --no-verify 
-F "$fixup_msg" \
+                               do_with_author git commit --allow-empty-message 
\
+                                       --amend --no-verify -F "$fixup_msg" \
                                        ${gpg_sign_opt:+"$gpg_sign_opt"} ||
                                        die_failed_squash $sha1 "$rest"
                        else
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed2..9c71835 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1039,4 +1039,42 @@ test_expect_success 'short SHA-1 collide' '
        )
 '
 
+test_expect_success 'setup commits with empty commit log messages' '
+       git checkout -b empty-log-messages master &&
+       test_commit no-msg-commit-1 &&
+       git commit --amend --allow-empty-message -F - </dev/null &&
+       test_commit no-msg-commit-2 &&
+       git commit --amend --allow-empty-message -F - </dev/null &&
+       test_commit no-msg-commit-3 &&
+       git commit --amend --allow-empty-message -F - </dev/null
+'
+
+test_expect_success 'rebase commits with empty commit log messages' '
+       git checkout -b rebase-empty-log-messages empty-log-messages &&
+       set_fake_editor &&
+       test_expect_code 0 env FAKE_LINES="1" git rebase -i master &&
+       test_expect_code 0 env FAKE_LINES="1" git rebase -i --no-ff master
+'
+
+test_expect_success 'reword commits with empty commit log messages' '
+       git checkout -b reword-empty-log-messages empty-log-messages &&
+       test_when_finished reset_rebase &&
+       set_fake_editor &&
+       test_must_fail env FAKE_LINES="reword 1" git rebase -i master
+'
+
+test_expect_success 'squash commits with empty commit log messages' '
+       git checkout -b squash-empty-log-messages empty-log-messages &&
+       set_fake_editor &&
+       test_must_fail env FAKE_LINES="1 squash 2 fixup 3" git rebase -i master 
&&
+       git commit --allow-empty-message --amend &&
+       git rebase --continue
+'
+
+test_expect_success 'fixup commits with empty commit log messages' '
+       git checkout -b fixup-empty-log-messages empty-log-messages &&
+       set_fake_editor &&
+       env FAKE_LINES="1 fixup 2" git rebase -i master
+'
+
 test_done
diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
index 0b52105..7add7a1 100755
--- a/t/t3412-rebase-root.sh
+++ b/t/t3412-rebase-root.sh
@@ -278,4 +278,20 @@ test_expect_success 'rebase -i -p --root with conflict 
(second part)' '
        test_cmp expect-conflict-p out
 '
 
+test_expect_success 'rebase --root root with empty log message' '
+       git checkout --orphan empty-log-messages-root master &&
+       test_commit no-msg-root-commit &&
+       git commit --amend --allow-empty-message -F - </dev/null &&
+       test_expect_code 0 git rebase --root &&
+       test_expect_code 0 git rebase --root --no-ff
+'
+
+test_expect_success 'rebase --root commits with empty log messages' '
+       git checkout -b empty-log-messages master &&
+       test_commit no-msg-commit &&
+       git commit --amend --allow-empty-message -F - </dev/null &&
+       test_expect_code 0 git rebase --root &&
+       test_expect_code 0 git rebase --root --no-ff
+'
+
 test_done
-- 
2.0.1

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

Reply via email to