Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
Hi Eric,
Eric Sunshine writes:
Show 26 quoted lines
> On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch <bafain@gmail.com> wrote:
>> The to-do list commands `squash` and `fixup` apply the changes
>> introduced by the named commit to the tree but instead of creating
>> a new commit on top of the current head it replaces the previous
>> commit with a new commit that records the updated tree. If the
>> result is an empty commit git-rebase stops with the error message
>>
>> You asked to amend the most recent commit, but doing so would make
>> it empty. You can repeat your command with --allow-empty, or you can
>> remove the commit entirely with "git reset HEAD^".
>>
>> This message is not very helpful because neither does git-rebase
>> support an option `--allow-empty` nor does the messages say how to
>> resume the rebase. Firstly, change the error message to
>>
>> The squash result is empty and --keep-empty was not specified.
>>
>> You can remove the squash commit now with
>>
>> git reset HEAD^
>>
>> Once you are down, run
>
> I guess you meant: s/down/done
>
> Same issue with the actually message in the code (below).
Show 97 quoted lines
>> git rebase --continue
>>
>> If the user wishes to squash a sequence of commits into one
>> commit, f. i.
>>
>> pick A
>> squash Revert "A"
>> squash A'
>>
>> , it does not matter for the end result that the first squash
>> result, or any sub-sequence in general, is going to be empty. The
>> squash message is not affected at all by which commits are created
>> and only the commit created by the last line in the sequence will
>> end up in the final history. Secondly, print the error message
>> only if the whole squash sequence produced an empty commit.
>>
>> Lastly, since an empty squash commit is not a failure to rewrite
>> the history as planned, issue the message above as a mere warning
>> and interrupt the rebase with the return value zero. The
>> interruption should be considered as a notification with the
>> chance to undo it on the spot. Specifying the `--keep-empty`
>> option tells git-rebase to keep empty squash commits in the
>> rebased history without notification.
>>
>> Add tests.
>>
>> Reported-by: Peter Krefting <peter@softwolves.pp.se>
>> Signed-off-by: Fabian Ruch <bafain@gmail.com>
>> ---
>> Hi,
>>
>> Peter Krefting is cc'd as the author of the bug report "Confusing
>> error message in rebase when commit becomes empty" discussed on the
>> mailing list in June. Phil Hord and Jeff King both participated in
>> the problem discussion which ended with two proposals by Jeff.
>>
>> Jeff King writes:
>>> 1. Always keep such empty commits. A user who is surprised by them
>>> being empty can then revisit them. Or drop them by doing another
>>> rebase without --keep-empty.
>>>
>>> 2. Notice ourselves that the end-result of the whole squash is an
>>> empty commit, and stop to let the user deal with it.
>>
>> This patch chooses the second alternative. Either way seems OK. The
>> crucial consensus of the discussion was to silently throw away empty
>> interim commits.
>>
>> Fabian
>>
>> git-rebase--interactive.sh | 20 +++++++++++---
>> t/t3404-rebase-interactive.sh | 62 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 79 insertions(+), 3 deletions(-)
>>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index 3222bf6..8820eac 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -549,7 +549,7 @@ do_next () {
>> squash|s|fixup|f)
>> # This is an intermediate commit; its message will only be
>> # used in case of trouble. So use the long version:
>> - do_with_author output git commit --allow-empty-message \
>> + do_with_author output git commit --allow-empty-message --allow-empty \
>> --amend --no-verify -F "$squash_msg" \
>> ${gpg_sign_opt:+"$gpg_sign_opt"} ||
>> die_failed_squash $sha1 "$rest"
>> @@ -558,18 +558,32 @@ do_next () {
>> # This is the final command of this squash/fixup group
>> if test -f "$fixup_msg"
>> then
>> - do_with_author git commit --allow-empty-message \
>> + do_with_author git commit --allow-empty-message --allow-empty \
>> --amend --no-verify -F "$fixup_msg" \
>> ${gpg_sign_opt:+"$gpg_sign_opt"} ||
>> die_failed_squash $sha1 "$rest"
>> else
>> cp "$squash_msg" "$GIT_DIR"/SQUASH_MSG || exit
>> rm -f "$GIT_DIR"/MERGE_MSG
>> - do_with_author git commit --amend --no-verify -F "$GIT_DIR"/SQUASH_MSG -e \
>> + do_with_author git commit --allow-empty --amend --no-verify -F "$GIT_DIR"/SQUASH_MSG -e \
>> ${gpg_sign_opt:+"$gpg_sign_opt"} ||
>> die_failed_squash $sha1 "$rest"
>> fi
>> rm -f "$squash_msg" "$fixup_msg"
>> + if test -z "$keep_empty" && is_empty_commit HEAD
>> + then
>> + echo "$sha1" >"$state_dir"/stopped-sha
>> + warn "The squash result is empty and --keep-empty was not specified."
>> + warn
>> + warn "You can remove the squash commit now with"
>> + warn
>> + warn " git reset HEAD^"
>> + warn
>> + warn "Once you are down, run"
>
> s/down/done/Thanks for the thorough reading.
Fabian