This series fixes several bugs in the way we handle a commit cannot be picked because it would overwrite an untracked file. * after a failed pick "git rebase --continue" will happily commit any staged changes even though no commit was picked. * the commit of the failed pick is recorded as rewritten even though no commit was picked. * the "done" file used by "git status" to show the recently executed commands contains an incorrect entry. Thanks for the comments on V1, this series has now grown somewhat. Previously I was worried that refactoring would change the behavior, but having thought about it the current behavior is wrong and should be changed. Changes since V1: Rebased onto master to avoid a conflict with ab/remove-implicit-use-of-the-repository * Patches 1-3 are new preparatory changes * Patches 4 & 5 are new and fix the first two issues listed above. * Patch 6 is the old patch 1 which has been rebased and the commit message reworded. It fixes the last issues listed above. Phillip Wood (6): rebase -i: move unlink() calls rebase -i: remove patch file after conflict resolution sequencer: factor out part of pick_commits() rebase --continue: refuse to commit after failed command rebase: fix rewritten list for failed pick rebase -i: fix adding failed command to the todo list sequencer.c | 170 ++++++++++++++++++---------------- t/t3404-rebase-interactive.sh | 49 +++++++--- t/t3430-rebase-merges.sh | 35 +++++-- t/t5407-post-rewrite-hook.sh | 11 +++ 4 files changed, 165 insertions(+), 100 deletions(-) base-commit: 9c6990cca24301ae8f82bf6291049667a0aef14b Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1492%2Fphillipwood%2Frebase-dont-write-done-when-rescheduling-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1492/phillipwood/rebase-dont-write-done-when-rescheduling-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1492 Range-diff vs v1: -: ----------- > 1: 3dfb2c6903b rebase -i: move unlink() calls -: ----------- > 2: 227aea031b5 rebase -i: remove patch file after conflict resolution -: ----------- > 3: 31bb644e769 sequencer: factor out part of pick_commits() -: ----------- > 4: 9356d14b09a rebase --continue: refuse to commit after failed command -: ----------- > 5: f8e64c1b631 rebase: fix rewritten list for failed pick 1: dc51a7499bc ! 6: a836b049b90 rebase -i: do not update "done" when rescheduling command @@ Metadata Author: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> ## Commit message ## - rebase -i: do not update "done" when rescheduling command + rebase -i: fix adding failed command to the todo list - As the sequencer executes todo commands it appends them to - .git/rebase-merge/done. This file is used by "git status" to show the - recently executed commands. Unfortunately when a command is rescheduled + When rebasing commands are moved from the todo list in "git-rebase-todo" + to the "done" file (which is used by "git status" to show the recently + executed commands) just before they are executed. This means that if a + command fails because it would overwrite an untracked file it has to be + added back into the todo list before the rebase stops for the user to + fix the problem. + + Unfortunately when a failed command is added back into the todo list the command preceding it is erroneously appended to the "done" file. - This means that when rebase stops after rescheduling "pick B" the "done" + This means that when rebase stops after "pick B" fails the "done" file contains pick A @@ Commit message pick A pick B - Fix this by not updating the "done" file when adding a rescheduled - command back into the "todo" file. A couple of the existing tests are + Fix this by not updating the "done" file when adding a failed command + back into the "git-rebase-todo" file. A couple of the existing tests are modified to improve their coverage as none of them trigger this bug or check the "done" file. - Note that the rescheduled command will still be appended to the "done" - file again when it is successfully executed. Arguably it would be better - not to do that but fixing it would be more involved. - Reported-by: Stefan Haller <lists@xxxxxxxxxxxxxxxx> Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> @@ sequencer.c: static int pick_commits(struct repository *r, int check_todo = 0; - if (save_todo(todo_list, opts)) -+ if (save_todo(todo_list, opts, 0)) ++ if (save_todo(todo_list, opts, reschedule)) return -1; if (is_rebase_i(opts)) { if (item->command != TODO_COMMENT) { -@@ sequencer.c: static int pick_commits(struct repository *r, - 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, 1)) - return -1; - } - if (item->command == TODO_EDIT) { @@ sequencer.c: 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, 1)) ++ if (save_todo(todo_list, opts, reschedule)) return -1; if (item->commit) - return error_with_patch(r, + write_rebase_head(&item->commit->object.oid); ## t/t3404-rebase-interactive.sh ## @@ t/t3404-rebase-interactive.sh: test_expect_success 'todo count' ' @@ t/t3404-rebase-interactive.sh: test_expect_success 'todo count' ' + head -n3 todo >expect && + test_cmp expect .git/rebase-merge/done && + rm file2 && + test_path_is_missing .git/rebase-merge/author-script && + test_path_is_missing .git/rebase-merge/patch && + test_path_is_missing .git/MERGE_MSG && +@@ t/t3404-rebase-interactive.sh: test_expect_success 'rebase -i commits that overwrite untracked files (pick)' ' + 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 && -- gitgitgadget
