Merge branch 'pw/rebase-i-after-failure' into maint-2.42

Various fixes to the behaviour of "rebase -i" when the command got
interrupted by conflicting changes.
cf. <6b927687-cf6e-d73e-78fb-bd4f46736928@gmx.de>

* pw/rebase-i-after-failure:
  rebase -i: fix adding failed command to the todo list
  rebase --continue: refuse to commit after failed command
  rebase: fix rewritten list for failed pick
  sequencer: factor out part of pick_commits()
  sequencer: use rebase_path_message()
  rebase -i: remove patch file after conflict resolution
  rebase -i: move unlink() calls

diff --git a/sequencer.c b/sequencer.c
index bc6b7b6a7680b20a7d80f9c1be9a9df9d2f4d0f5..81e1ad5832b242054a9796cff9ac16c5fd4b6499 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -147,6 +147,11 @@ static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
  * the commit object name of the corresponding patch.
  */
 static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
+/*
+ * When we stop for the user to resolve conflicts this file contains
+ * the patch of the commit that is being picked.
+ */
+static GIT_PATH_FUNC(rebase_path_patch, "rebase-merge/patch")
 /*
  * For the post-rewrite hook, we make a list of rewritten commits and
  * their new sha1s.  The rewritten-pending list keeps the sha1s of
@@ -3401,7 +3406,8 @@ give_advice:
        return -1;
 }
 
-static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
+static int save_todo(struct todo_list *todo_list, struct replay_opts *opts,
+                    int reschedule)
 {
        struct lock_file todo_lock = LOCK_INIT;
        const char *todo_path = get_todo_path(opts);
@@ -3411,7 +3417,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
         * rebase -i writes "git-rebase-todo" without the currently executing
         * command, appending it to "done" instead.
         */
-       if (is_rebase_i(opts))
+       if (is_rebase_i(opts) && !reschedule)
                next++;
 
        fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
@@ -3424,7 +3430,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
        if (commit_lock_file(&todo_lock) < 0)
                return error(_("failed to finalize '%s'"), todo_path);
 
-       if (is_rebase_i(opts) && next > 0) {
+       if (is_rebase_i(opts) && !reschedule && next > 0) {
                const char *done = rebase_path_done();
                int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666);
                int ret = 0;
@@ -3505,18 +3511,19 @@ static int make_patch(struct repository *r,
                      struct commit *commit,
                      struct replay_opts *opts)
 {
-       struct strbuf buf = STRBUF_INIT;
        struct rev_info log_tree_opt;
        const char *subject;
        char hex[GIT_MAX_HEXSZ + 1];
        int res = 0;
 
+       if (!is_rebase_i(opts))
+               BUG("make_patch should only be called when rebasing");
+
        oid_to_hex_r(hex, &commit->object.oid);
        if (write_message(hex, strlen(hex), rebase_path_stopped_sha(), 1) < 0)
                return -1;
        res |= write_rebase_head(&commit->object.oid);
 
-       strbuf_addf(&buf, "%s/patch", get_dir(opts));
        memset(&log_tree_opt, 0, sizeof(log_tree_opt));
        repo_init_revisions(r, &log_tree_opt, NULL);
        log_tree_opt.abbrev = 0;
@@ -3524,28 +3531,26 @@ static int make_patch(struct repository *r,
        log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH;
        log_tree_opt.disable_stdin = 1;
        log_tree_opt.no_commit_id = 1;
-       log_tree_opt.diffopt.file = fopen(buf.buf, "w");
+       log_tree_opt.diffopt.file = fopen(rebase_path_patch(), "w");
        log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
        if (!log_tree_opt.diffopt.file)
-               res |= error_errno(_("could not open '%s'"), buf.buf);
+               res |= error_errno(_("could not open '%s'"),
+                                  rebase_path_patch());
        else {
                res |= log_tree_commit(&log_tree_opt, commit);
                fclose(log_tree_opt.diffopt.file);
        }
-       strbuf_reset(&buf);
 
-       strbuf_addf(&buf, "%s/message", get_dir(opts));
-       if (!file_exists(buf.buf)) {
+       if (!file_exists(rebase_path_message())) {
                const char *encoding = get_commit_output_encoding();
                const char *commit_buffer = repo_logmsg_reencode(r,
                                                                 commit, NULL,
                                                                 encoding);
                find_commit_subject(commit_buffer, &subject);
-               res |= write_message(subject, strlen(subject), buf.buf, 1);
+               res |= write_message(subject, strlen(subject), rebase_path_message(), 1);
                repo_unuse_commit_buffer(r, commit,
                                         commit_buffer);
        }
-       strbuf_release(&buf);
        release_revisions(&log_tree_opt);
 
        return res;
@@ -4163,6 +4168,7 @@ static int do_merge(struct repository *r,
        if (ret < 0) {
                error(_("could not even attempt to merge '%.*s'"),
                      merge_arg_len, arg);
+               unlink(git_path_merge_msg(r));
                goto leave_merge;
        }
        /*
@@ -4650,6 +4656,68 @@ N_("Could not execute the todo command\n"
 "    git rebase --edit-todo\n"
 "    git rebase --continue\n");
 
+static int pick_one_commit(struct repository *r,
+                          struct todo_list *todo_list,
+                          struct replay_opts *opts,
+                          int *check_todo, int* reschedule)
+{
+       int res;
+       struct todo_item *item = todo_list->items + todo_list->current;
+       const char *arg = todo_item_get_arg(todo_list, item);
+       if (is_rebase_i(opts))
+               opts->reflog_message = reflog_message(
+                       opts, command_to_string(item->command), NULL);
+
+       res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
+                            check_todo);
+       if (is_rebase_i(opts) && res < 0) {
+               /* Reschedule */
+               *reschedule = 1;
+               return -1;
+       }
+       if (item->command == TODO_EDIT) {
+               struct commit *commit = item->commit;
+               if (!res) {
+                       if (!opts->verbose)
+                               term_clear_line();
+                       fprintf(stderr, _("Stopped at %s...  %.*s\n"),
+                               short_commit_name(commit), item->arg_len, arg);
+               }
+               return error_with_patch(r, commit,
+                                       arg, item->arg_len, opts, res, !res);
+       }
+       if (is_rebase_i(opts) && !res)
+               record_in_rewritten(&item->commit->object.oid,
+                                   peek_command(todo_list, 1));
+       if (res && is_fixup(item->command)) {
+               if (res == 1)
+                       intend_to_amend();
+               return error_failed_squash(r, item->commit, opts,
+                                          item->arg_len, arg);
+       } else if (res && is_rebase_i(opts) && item->commit) {
+               int to_amend = 0;
+               struct object_id oid;
+
+               /*
+                * If we are rewording and have either
+                * fast-forwarded already, or are about to
+                * create a new root commit, we want to amend,
+                * otherwise we do not.
+                */
+               if (item->command == TODO_REWORD &&
+                   !repo_get_oid(r, "HEAD", &oid) &&
+                   (oideq(&item->commit->object.oid, &oid) ||
+                    (opts->have_squash_onto &&
+                     oideq(&opts->squash_onto, &oid))))
+                       to_amend = 1;
+
+               return res | error_with_patch(r, item->commit,
+                                             arg, item->arg_len, opts,
+                                             res, to_amend);
+       }
+       return res;
+}
+
 static int pick_commits(struct repository *r,
                        struct todo_list *todo_list,
                        struct replay_opts *opts)
@@ -4665,12 +4733,17 @@ static int pick_commits(struct repository *r,
        if (read_and_refresh_cache(r, opts))
                return -1;
 
+       unlink(rebase_path_message());
+       unlink(rebase_path_stopped_sha());
+       unlink(rebase_path_amend());
+       unlink(rebase_path_patch());
+
        while (todo_list->current < todo_list->nr) {
                struct todo_item *item = todo_list->items + todo_list->current;
                const char *arg = todo_item_get_arg(todo_list, item);
                int check_todo = 0;
 
-               if (save_todo(todo_list, opts))
+               if (save_todo(todo_list, opts, reschedule))
                        return -1;
                if (is_rebase_i(opts)) {
                        if (item->command != TODO_COMMENT) {
@@ -4688,10 +4761,7 @@ static int pick_commits(struct repository *r,
                                                todo_list->total_nr,
                                                opts->verbose ? "\n" : "\r");
                        }
-                       unlink(rebase_path_message());
                        unlink(rebase_path_author_script());
-                       unlink(rebase_path_stopped_sha());
-                       unlink(rebase_path_amend());
                        unlink(git_path_merge_head(r));
                        unlink(git_path_auto_merge(r));
                        delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
@@ -4703,66 +4773,10 @@ static int pick_commits(struct repository *r,
                        }
                }
                if (item->command <= TODO_SQUASH) {
-                       if (is_rebase_i(opts))
-                               opts->reflog_message = reflog_message(opts,
-                                     command_to_string(item->command), NULL);
-
-                       res = do_pick_commit(r, item, opts,
-                                            is_final_fixup(todo_list),
-                                            &check_todo);
-                       if (is_rebase_i(opts) && res < 0) {
-                               /* Reschedule */
-                               advise(_(rescheduled_advice),
-                                      get_item_line_length(todo_list,
-                                                           todo_list->current),
-                                      get_item_line(todo_list,
-                                                    todo_list->current));
-                               todo_list->current--;
-                               if (save_todo(todo_list, opts))
-                                       return -1;
-                       }
-                       if (item->command == TODO_EDIT) {
-                               struct commit *commit = item->commit;
-                               if (!res) {
-                                       if (!opts->verbose)
-                                               term_clear_line();
-                                       fprintf(stderr,
-                                               _("Stopped at %s...  %.*s\n"),
-                                               short_commit_name(commit),
-                                               item->arg_len, arg);
-                               }
-                               return error_with_patch(r, commit,
-                                       arg, item->arg_len, opts, res, !res);
-                       }
-                       if (is_rebase_i(opts) && !res)
-                               record_in_rewritten(&item->commit->object.oid,
-                                       peek_command(todo_list, 1));
-                       if (res && is_fixup(item->command)) {
-                               if (res == 1)
-                                       intend_to_amend();
-                               return error_failed_squash(r, item->commit, opts,
-                                       item->arg_len, arg);
-                       } else if (res && is_rebase_i(opts) && item->commit) {
-                               int to_amend = 0;
-                               struct object_id oid;
-
-                               /*
-                                * If we are rewording and have either
-                                * fast-forwarded already, or are about to
-                                * create a new root commit, we want to amend,
-                                * otherwise we do not.
-                                */
-                               if (item->command == TODO_REWORD &&
-                                   !repo_get_oid(r, "HEAD", &oid) &&
-                                   (oideq(&item->commit->object.oid, &oid) ||
-                                    (opts->have_squash_onto &&
-                                     oideq(&opts->squash_onto, &oid))))
-                                       to_amend = 1;
-
-                               return res | error_with_patch(r, item->commit,
-                                               arg, item->arg_len, opts,
-                                               res, to_amend);
-                       }
+                       res = pick_one_commit(r, todo_list, opts, &check_todo,
+                                             &reschedule);
+                       if (!res && item->command == TODO_EDIT)
+                               return 0;
                } else if (item->command == TODO_EXEC) {
                        char *end_of_arg = (char *)(arg + item->arg_len);
                        int saved = *end_of_arg;
@@ -4810,22 +4824,19 @@ static int pick_commits(struct repository *r,
                               get_item_line_length(todo_list,
                                                    todo_list->current),
                               get_item_line(todo_list, todo_list->current));
-                       todo_list->current--;
-                       if (save_todo(todo_list, opts))
+                       if (save_todo(todo_list, opts, reschedule))
                                return -1;
                        if (item->commit)
-                               return error_with_patch(r,
-                                                       item->commit,
-                                                       arg, item->arg_len,
-                                                       opts, res, 0);
+                               write_rebase_head(&item->commit->object.oid);
                } else if (is_rebase_i(opts) && check_todo && !res &&
                           reread_todo_if_changed(r, todo_list, opts)) {
                        return -1;
                }
 
-               todo_list->current++;
                if (res)
                        return res;
+
+               todo_list->current++;
        }
 
        if (is_rebase_i(opts)) {
@@ -4978,6 +4989,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 96a56aafbed67ca7e47ac173c00afdbe17b000ad..939fe8dfbc7fe2245765c296155ea8340c06ec9f 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' '
@@ -1276,20 +1277,34 @@ test_expect_success 'todo count' '
 '
 
 test_expect_success 'rebase -i commits that overwrite untracked files (pick)' '
-       git checkout --force branch2 &&
+       git checkout --force A &&
        git clean -f &&
+       cat >todo <<-EOF &&
+       exec >file2
+       pick $(git rev-parse B) B
+       pick $(git rev-parse C) C
+       pick $(git rev-parse D) D
+       exec cat .git/rebase-merge/done >actual
+       EOF
        (
-               set_fake_editor &&
-               FAKE_LINES="edit 1 2" git rebase -i A
-       ) &&
-       test_cmp_rev HEAD F &&
-       test_path_is_missing file6 &&
-       >file6 &&
-       test_must_fail git rebase --continue &&
-       test_cmp_rev HEAD F &&
-       rm file6 &&
+               set_replace_editor todo &&
+               test_must_fail git rebase -i A
+       ) &&
+       test_cmp_rev HEAD B &&
+       test_cmp_rev REBASE_HEAD C &&
+       head -n3 todo >expect &&
+       test_cmp expect .git/rebase-merge/done &&
+       rm file2 &&
+       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
+       test_cmp_rev HEAD D &&
+       tail -n3 todo >>expect &&
+       test_cmp expect actual
 '
 
 test_expect_success 'rebase -i commits that overwrite untracked files (squash)' '
@@ -1305,7 +1320,14 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)'
        >file6 &&
        test_must_fail git rebase --continue &&
        test_cmp_rev HEAD F &&
+       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
@@ -1323,7 +1345,14 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' '
        >file6 &&
        test_must_fail git rebase --continue &&
        test $(git cat-file commit HEAD | sed -ne \$p) = F &&
+       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/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index fb7b68990cc33a6cbd08ba7031d405713db87446..c4e2fcac67e5886d47a392097dd14e2bd0141a73 100755 (executable)
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -268,6 +268,24 @@ test_expect_success 'the todo command "break" works' '
        test_path_is_file execed
 '
 
+test_expect_success 'patch file is removed before break command' '
+       test_when_finished "git rebase --abort" &&
+       cat >todo <<-\EOF &&
+       pick commit-new-file-F2-on-topic-branch
+       break
+       EOF
+
+       (
+               set_replace_editor todo &&
+               test_must_fail git rebase -i --onto commit-new-file-F2 HEAD
+       ) &&
+       test_path_is_file .git/rebase-merge/patch &&
+       echo 22>F2 &&
+       git add F2 &&
+       git rebase --continue &&
+       test_path_is_missing .git/rebase-merge/patch
+'
+
 test_expect_success '--reschedule-failed-exec' '
        test_when_finished "git rebase --abort" &&
        test_must_fail git rebase -x false --reschedule-failed-exec HEAD^ &&

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index ac5c390652f381ce7a85e37863ac17becc655734..59b5d6b6f276c367862df92f847db37906715004 100755 (executable)
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -128,14 +128,24 @@ test_expect_success 'generate correct todo list' '
 '
 
 test_expect_success '`reset` refuses to overwrite untracked files' '
-       git checkout -b refuse-to-reset &&
+       git checkout B &&
        test_commit dont-overwrite-untracked &&
-       git checkout @{-1} &&
-       : >dont-overwrite-untracked.t &&
-       echo "reset refs/tags/dont-overwrite-untracked" >script-from-scratch &&
+       cat >script-from-scratch <<-EOF &&
+       exec >dont-overwrite-untracked.t
+       pick $(git rev-parse B) B
+       reset refs/tags/dont-overwrite-untracked
+       pick $(git rev-parse C) C
+       exec cat .git/rebase-merge/done >actual
+       EOF
        test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
-       test_must_fail git rebase -ir HEAD &&
-       git rebase --abort
+       test_must_fail git rebase -ir A &&
+       test_cmp_rev HEAD B &&
+       head -n3 script-from-scratch >expect &&
+       test_cmp expect .git/rebase-merge/done &&
+       rm dont-overwrite-untracked.t &&
+       git rebase --continue &&
+       tail -n3 script-from-scratch >>expect &&
+       test_cmp expect actual
 '
 
 test_expect_success '`reset` rejects trees' '
@@ -165,12 +175,16 @@ test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' '
        test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
        test_tick &&
        test_must_fail git rebase -ir HEAD &&
+       test_cmp_rev REBASE_HEAD H^0 &&
        grep "^merge -C .* G$" .git/rebase-merge/done &&
        grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&
-       test_path_is_file .git/rebase-merge/patch &&
+       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 &&
-       rm G.t .git/rebase-merge/patch &&
        git reset --hard conflicting-G &&
        test_must_fail git rebase --continue &&
        ! grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&

diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index 5f3ff051ca2fa3bba8beabbcb1491d5e69262f34..ad7f8c6f00202c5ec844108b14a2c1301c185223 100755 (executable)
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -17,6 +17,12 @@ test_expect_success 'setup' '
        git checkout A^0 &&
        test_commit E bar E &&
        test_commit F foo F &&
+       git checkout B &&
+       git merge E &&
+       git tag merge-E &&
+       test_commit G G &&
+       test_commit H H &&
+       test_commit I I &&
        git checkout main &&
 
        test_hook --setup post-rewrite <<-EOF
@@ -173,6 +179,48 @@ test_fail_interactive_rebase () {
        )
 }
 
+test_expect_success 'git rebase with failed pick' '
+       clear_hook_input &&
+       cat >todo <<-\EOF &&
+       exec >bar
+       merge -C merge-E E
+       exec >G
+       pick G
+       exec >H 2>I
+       pick H
+       fixup I
+       EOF
+
+       (
+               set_replace_editor todo &&
+               test_must_fail git rebase -i D D 2>err
+       ) &&
+       grep "would be overwritten" err &&
+       rm bar &&
+
+       test_must_fail git rebase --continue 2>err &&
+       grep "would be overwritten" err &&
+       rm G &&
+
+       test_must_fail git rebase --continue 2>err &&
+       grep "would be overwritten" err &&
+       rm H &&
+
+       test_must_fail git rebase --continue 2>err &&
+       grep "would be overwritten" err &&
+       rm I &&
+
+       git rebase --continue &&
+       echo rebase >expected.args &&
+       cat >expected.data <<-EOF &&
+       $(git rev-parse merge-E) $(git rev-parse HEAD~2)
+       $(git rev-parse G) $(git rev-parse HEAD~1)
+       $(git rev-parse H) $(git rev-parse HEAD)
+       $(git rev-parse I) $(git rev-parse HEAD)
+       EOF
+       verify_hook_input
+'
+
 test_expect_success 'git rebase -i (unchanged)' '
        git reset --hard D &&
        clear_hook_input &&