Hi Alban, On Fri, 6 Dec 2019, Phillip Wood wrote: > On 05/12/2019 23:15, Alban Gruin wrote: > > > > Le 04/12/2019 à 22:51, Junio C Hamano a écrit : > > > Alban Gruin <alban.gruin@xxxxxxxxx> writes: > > > > > > > To prevent mistakes when editing a branch, rebase features a knob, > > > > rebase.missingCommitsCheck, to warn the user if a commit was dropped. > > > > Unfortunately, this check is only effective for the initial edit, which > > > > means that if you edit the todo list at a later point of the rebase and > > > > drop a commit, no warnings or errors would be issued. > > > > ... > > > > rebase-interactive.c | 57 ++++++++++++++++++++---- > > > > rebase-interactive.h | 2 + > > > > sequencer.c | 53 ++++++---------------- > > > > sequencer.h | 1 - > > > > t/t3404-rebase-interactive.sh | 83 +++++++++++++++++++++++++++++++++++ > > > > 5 files changed, 147 insertions(+), 49 deletions(-) > > > > > > This passes the self-test when tested by itself, but when merged > > > near the tip of 'pu', it breaks t3404.116, it seems. > > > > > > > After a quick investigation, it comes from > > pw/post-commit-from-sequencer. Since then, tests are expected to setup > > the editor and run the commands using it in a subshell. So the fix is > > straightforward. > > > > Perhaps I should take ag/sequencer-todo-updates, merge > > pw/post-commit-from-sequencer, rebase this series onto the result, fix > > the issue, and reroll the series? > > If the issue is just using a subshell to set the editor then (assuming you're > only adding new tests) I don't think you need to rebase - just change your > tests and it should be fine when Junio merges it into pu. I'm sorry I've not > looked at the latest version yet, I'll try and get round to it next week. Just squash this in: -- snipsnap -- Subject: [PATCH] fixup??? rebase-interactive: warn if commit is dropped with `rebase --edit-todo' This is required to appease the interaction with `pw/post-commit-from-sequencer`. Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> --- t/t3404-rebase-interactive.sh | 74 +++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index e3f64bc2a59..72718d0d839 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1453,10 +1453,12 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' ' test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' ' test_config rebase.missingCommitsCheck ignore && rebase_setup_and_clean missing-commit && - set_fake_editor && - FAKE_LINES="break 1 2 3 4 5" git rebase -i --root && - FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual && - git rebase --continue 2>actual && + ( + set_fake_editor && + FAKE_LINES="break 1 2 3 4 5" git rebase -i --root && + FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual && + git rebase --continue 2>actual + ) && test D = $(git cat-file commit HEAD | sed -ne \$p) && test_i18ngrep \ "Successfully rebased and updated refs/heads/missing-commit" \ @@ -1474,18 +1476,20 @@ test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = wa tail -n4 expect >expect.2 && test_config rebase.missingCommitsCheck warn && rebase_setup_and_clean missing-commit && - set_fake_editor && - test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \ - git rebase -i --root && - cp .git/rebase-merge/git-rebase-todo.backup orig && - FAKE_LINES="2 3 4" git rebase --edit-todo 2>actual.2 && - head -n5 actual.2 >actual && - test_i18ncmp expect actual && - cp orig .git/rebase-merge/git-rebase-todo && - FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual.2 && - head -n4 actual.2 >actual && - test_i18ncmp expect.2 actual && - git rebase --continue 2>actual && + ( + set_fake_editor && + test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \ + git rebase -i --root && + cp .git/rebase-merge/git-rebase-todo.backup orig && + FAKE_LINES="2 3 4" git rebase --edit-todo 2>actual.2 && + head -n5 actual.2 >actual && + test_i18ncmp expect actual && + cp orig .git/rebase-merge/git-rebase-todo && + FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual.2 && + head -n4 actual.2 >actual && + test_i18ncmp expect.2 actual && + git rebase --continue 2>actual + ) && test D = $(git cat-file commit HEAD | sed -ne \$p) && test_i18ngrep \ "Successfully rebased and updated refs/heads/missing-commit" \ @@ -1509,24 +1513,26 @@ test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = er tail -n10 expect >expect.2 && test_config rebase.missingCommitsCheck error && rebase_setup_and_clean missing-commit && - set_fake_editor && - test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \ - git rebase -i --root && - cp .git/rebase-merge/git-rebase-todo.backup orig && - test_must_fail env FAKE_LINES="2 3 4" \ - git rebase --edit-todo 2>actual && - test_i18ncmp expect actual && - test_must_fail git rebase --continue 2>actual && - test_i18ncmp expect actual && - cp orig .git/rebase-merge/git-rebase-todo && - test_must_fail env FAKE_LINES="1 2 3 4" \ - git rebase --edit-todo 2>actual && - test_i18ncmp expect.2 actual && - test_must_fail git rebase --continue 2>actual && - test_i18ncmp expect.2 actual && - cp orig .git/rebase-merge/git-rebase-todo && - FAKE_LINES="1 2 3 4 drop 5" git rebase --edit-todo && - git rebase --continue 2>actual && + ( + set_fake_editor && + test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \ + git rebase -i --root && + cp .git/rebase-merge/git-rebase-todo.backup orig && + test_must_fail env FAKE_LINES="2 3 4" \ + git rebase --edit-todo 2>actual && + test_i18ncmp expect actual && + test_must_fail git rebase --continue 2>actual && + test_i18ncmp expect actual && + cp orig .git/rebase-merge/git-rebase-todo && + test_must_fail env FAKE_LINES="1 2 3 4" \ + git rebase --edit-todo 2>actual && + test_i18ncmp expect.2 actual && + test_must_fail git rebase --continue 2>actual && + test_i18ncmp expect.2 actual && + cp orig .git/rebase-merge/git-rebase-todo && + FAKE_LINES="1 2 3 4 drop 5" git rebase --edit-todo && + git rebase --continue 2>actual + ) && test D = $(git cat-file commit HEAD | sed -ne \$p) && test_i18ngrep \ "Successfully rebased and updated refs/heads/missing-commit" \ -- 2.24.0.windows.2.611.ge9aced84530