Phillip Wood <phillip.wood123@xxxxxxxxx> writes: > We do actually check that there is a valid HEAD before we try to fixup > a commit. Though perhaps we should still change this patch as HEAD may > be changed by another process between that check and re-reading it > here. If you try to fixup a commit without a valid HEAD you get > > error: need a HEAD to fixup > hint: Could not execute the todo command > hint: > hint: fixup faef1a5a7637ff91b3611aabd1b96541da5f5536 P > hint: > hint: It has been rescheduled; To edit the command before continuing, please > hint: edit the todo list first: > hint: > hint: git rebase --edit-todo > hint: git rebase --continue > error: could not copy '.git/rebase-merge/message-squash' to > '.git/rebase-merge/message' > > The last error message is unfortunate but we do exit in an orderly > manner rather than segfaulting. Thanks for thinking about the issue further. > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index d2f1d5bd23..4f55f0cd1c 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -67,6 +67,21 @@ test_expect_success 'setup' ' > SHELL= > export SHELL > > +test_expect_success 'fixup on orphan branch errors out' ' > + > + test_when_finished "git switch master" && > + write_script switch-branch.sh <<-\EOF && > + git symbolic-ref HEAD refs/heads/does-not-exist && > + git rm -rf . That "git rm -rf ." scares me, though. > + EOF > + ( > + set_fake_editor && > + FAKE_LINES="exec_./switch-branch.sh \ > + fixup 1" git rebase -i HEAD^ > + ) && > + test_pause > +' > + > > I think it would be useful to add something like this to the test > suite (changed to check the error message, with a better name for the > script and modified to expect failure) What do you think? So, we try an interactive rebase, try to apply a fix-up on an unborn branch and expect it to fail in a controlled way, something like ( # we are in subshell so freely export set_fake_editor && export FAKE_LINES="exec_./switch-branch.sh fixup 1" && test_must_fail git rebase -i HEAD^ 2>error && test_i18ngrep "... what we expect ..." error ) Perhaps. I do not think of a good reason why we should allow switching to another branch when "rebase -i" gives control back to the user, so in the longer run, the checked condition may not stay the same (I suspect you would catch "does-not-exist is unborn and there is nothing to 'fixup'" right now---I am envisioning that the condition that is checked and the message we would issue would be "we gave you a detached HEAD for a good reason---stay there and do not switch to any other branch") and the message expected by this test would have to be updated. Thanks.
