rebase --continue: refuse to commit after failed command

If a commit cannot be picked because it would overwrite an untracked
file then "git rebase --continue" should refuse to commit any staged
changes as the commit was not picked. This is implemented by refusing to
commit if the message file is missing. The message file is chosen for
this check because it is only written when "git rebase" stops for the
user to resolve merge conflicts.

Existing commands that refuse to commit staged changes when continuing
such as a failed "exec" rely on checking for the absence of the author
script in run_git_commit(). This prevents the staged changes from being
committed but prints

    error: could not open '.git/rebase-merge/author-script' for
    reading

before the message about not being able to commit. This is confusing to
users and so checking for the message file instead improves the user
experience. The existing test for refusing to commit after a failed exec
is updated to check that we do not print the error message about a
missing author script anymore.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sequencer.c b/sequencer.c
index 575464358cdd80d3d88246ba5023b7f928014ce2..f9ef301d632980d3890aeabbba6e412dee765161 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -4963,6 +4963,11 @@ static int commit_staged_changes(struct repository *r,
 
        is_clean = !has_uncommitted_changes(r, 0);
 
+       if (!is_clean && !file_exists(rebase_path_message())) {
+               const char *gpg_opt = gpg_sign_opt_quoted(opts);
+
+               return error(_(staged_changes_advice), gpg_opt, gpg_opt);
+       }
        if (file_exists(rebase_path_amend())) {
                struct strbuf rev = STRBUF_INIT;
                struct object_id head, to_amend;

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 6d3788c588b1feed991be6f11f2bee7461f993bd..a8ad398956ab47c2de223094270fd8d6fb0887c7 100755 (executable)
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -604,7 +604,8 @@ test_expect_success 'clean error after failed "exec"' '
        echo "edited again" > file7 &&
        git add file7 &&
        test_must_fail git rebase --continue 2>error &&
-       test_i18ngrep "you have staged changes in your working tree" error
+       test_i18ngrep "you have staged changes in your working tree" error &&
+       test_i18ngrep ! "could not open.*for reading" error
 '
 
 test_expect_success 'rebase a detached HEAD' '
@@ -1290,6 +1291,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (pick)' '
        test_cmp_rev REBASE_HEAD I &&
        rm file6 &&
        test_path_is_missing .git/rebase-merge/patch &&
+       echo changed >file1 &&
+       git add file1 &&
+       test_must_fail git rebase --continue 2>err &&
+       grep "error: you have staged changes in your working tree" err &&
+       git reset --hard HEAD &&
        git rebase --continue &&
        test_cmp_rev HEAD I
 '
@@ -1310,6 +1316,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)'
        test_cmp_rev REBASE_HEAD I &&
        rm file6 &&
        test_path_is_missing .git/rebase-merge/patch &&
+       echo changed >file1 &&
+       git add file1 &&
+       test_must_fail git rebase --continue 2>err &&
+       grep "error: you have staged changes in your working tree" err &&
+       git reset --hard HEAD &&
        git rebase --continue &&
        test $(git cat-file commit HEAD | sed -ne \$p) = I &&
        git reset --hard original-branch2
@@ -1330,6 +1341,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' '
        test_cmp_rev REBASE_HEAD I &&
        rm file6 &&
        test_path_is_missing .git/rebase-merge/patch &&
+       echo changed >file1 &&
+       git add file1 &&
+       test_must_fail git rebase --continue 2>err &&
+       grep "error: you have staged changes in your working tree" err &&
+       git reset --hard HEAD &&
        git rebase --continue &&
        test $(git cat-file commit HEAD | sed -ne \$p) = I
 '

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 5514738abdb66264839b0a419de82471a9606731..bf75080281e334a48559b1ee022adb409ae21f86 100755 (executable)
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -169,6 +169,10 @@ test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' '
        grep "^merge -C .* G$" .git/rebase-merge/done &&
        grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&
        test_path_is_missing .git/rebase-merge/patch &&
+       echo changed >file1 &&
+       git add file1 &&
+       test_must_fail git rebase --continue 2>err &&
+       grep "error: you have staged changes in your working tree" err &&
 
        : fail because of merge conflict &&
        git reset --hard conflicting-G &&