Allow empty commits during rebase -i
If you squash two commits into a previous commit, where the first squash reverts the previous commit and the second redoes the change correctly, rebase -i would fail during the first squash because it generates an empty commit. This patch allows the rebase to succeed.
This also introduces the possibility that you might accidentally create an empty commit with a squash, but I expect that will happen less often than the scenario this is intended to address.
---
This arose for me recently; I used "git revert" to undo a commit several changes back, and then reworked and committed anew. The first commit that I was redoing had a thorough commit message, while my new commit had a message like "do it right this time". I squashed the three commits into one with rebase -i, but git choked on the intermediate empty commit.
I could have simply removed the first two commits I was squashing (the initial version and its revert), but then would have lost the well-written commit message that went with the first version.
I imagine an ideal version of this fix would make it so the use case I presented here would work, but rebase -i would still prevent introducing a new empty commit, or at least warn when it was introducing one. In the absence of that ideal fix, I think this behavior is better than failing to handle this case.
git-rebase--interactive.sh | 2 +- t/t3404-rebase-interactive.sh | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletions(-)
git-rebase--interactive.sh +1 -1
@@ -403,7 +403,7 @@ do_next () { GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \ GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \ GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \ - $USE_OUTPUT git commit --no-verify \ + $USE_OUTPUT git commit --no-verify --allow-empty \ $MSG_OPT "$EDIT_OR_FILE" || failed=t fi if test $failed = t
t/t3404-rebase-interactive.sh +9 -0
@@ -484,4 +484,13 @@ test_expect_success 'reword' ' git show HEAD~2 | grep "C changed" ' +test_expect_success 'squash including empty' ' + test_commit Initial_emptysquash emptysquash abc && + test_commit first_mod emptysquash abd && + test_tick && + git revert --no-edit HEAD && + test_commit second_mod emptysquash abe && + FAKE_LINES="1 squash 2 squash 3" git rebase -i Initial_emptysquash +' + test_done
Signature · 1 line
1.6.6.196.g1f735
Quoted 5 lines · Pete Harlan
I imagine an ideal version of this fix would make it so the use case I presented here would work, but rebase -i would still prevent introducing a new empty commit, or at least warn when it was introducing one. In the absence of that ideal fix, I think this behavior is better than failing to handle this case.
Sorry, I actually tend to think that in the absense of that fix, your version introduces risky behaviour that only a corner-case use case benefits, and pros-and-cons doesn't look attractive enough.
Why not do something like:
pick X a crap tree with a good message
pick Y revert X
pick Z a good tree with a crap message
-->
# drop X
# drop Y
edit Zand then run "git commit --amend -C X" when it is Z's turn to be processed?
Quoted 18 lines · Junio C Hamano
Quoted 5 lines · Pete Harlan
I imagine an ideal version of this fix would make it so the use case I presented here would work, but rebase -i would still prevent introducing a new empty commit, or at least warn when it was introducing one. In the absence of that ideal fix, I think this behavior is better than failing to handle this case.
Sorry, I actually tend to think that in the absense of that fix, your version introduces risky behaviour that only a corner-case use case benefits, and pros-and-cons doesn't look attractive enough.
Why not do something like:
pick X a crap tree with a good message
pick Y revert X
pick Z a good tree with a crap message
Quoted 8 lines
-->
# drop X # drop Y edit Zand then run "git commit --amend -C X" when it is Z's turn to be processed?
That is another way to accomplish the same thing, but doesn't prevent the current behavior from being confusing.
Part of the problem is that with the current behavior the user is sent to the command line with:
# Not currently on any branch.
nothing to commit (working directory clean)
Could not apply a0b17c5... Revert "Crap tree good message"
with HEAD pointed to X^. Unsure of how to proceed from here, I --aborted the rebase and copy/pasted the commit message I wanted and resolved to track this down and fix it when I got a chance.
As it happens, "git rebase --continue" does exactly what I would have wanted to happen, including putting me in an editor with all three commit messages and succeeding when I exit the editor. But without a better message from git I don't expect a user to discover that. And, when rebase can continue just by being told so it would be nice if it didn't require that user intervention.
If the introduction of empty commits that the user has asked for (perhaps inadvertently) is considered too undesirable, then perhaps my fix is too simple. I'll think about how to do something more sophisticated.
Thanks for your feedback,
--Pete
Quoted 4 lines · Pete Harlan
..., "git rebase --continue" does exactly what I would have wanted to happen, including putting me in an editor with all three commit messages and succeeding when I exit the editor. But without a better message from git I don't expect a user to discover that.
There seems to be an idea for a good improvement ;-) CC'ing Michael as he has been most active in this area for the past few weeks.
Hi,
Quoted 4 lines · Pete Harlan
If the introduction of empty commits that the user has asked for (perhaps inadvertently) is considered too undesirable, then perhaps my fix is too simple. I'll think about how to do something more sophisticated.
How about something less sophisticated instead? Namely, check for the condition that nothing was changed, and tell the user that the commit blablabla seems to introduce changes that are already present in HEAD. Maybe even mention that this may be due to the commit being applied already and saying that --continue is safe in that case, but please check.
Hmm?
Ciao,
Dscho