Confusing error message in rebase when commit becomes empty

Hi!

I am rebasing a branch to combine a couple of commits. One is a revert of a previous commit. Since there are commits in-between, I do "squash" to make sure I get everything, and then add the actual change on top of that. The problem is that rebase stops with a confusing error message (from commit, presumably):

   $ git rebase --interactive
   [...]
   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^".
   rebase in progress; onto 342b22f
   You are currently rebasing branch 'mybranch' on '342b22f'.

   No changes

   Could not apply 4682a1f20f6ac29546536921bc6ea0386441e23e... Revert "something"

OK, so I should retry the command with --allow-empty, then:

   $ git rebase --interactive --allow-empty
   error: unknown option `allow-empty'

Nope, that's not quite right.

Running "git rebase --continue" does work as expected, but perhaps it just shouldn't stop in this case?

On Wed, Jun 11, 2014 at 8:49 AM, Peter Krefting <peter@softwolves.pp.se> wrote:

The correct switch for rebase is --keep-empty, but it is too late to choose it once the interactive rebase is underway. I think the correct advice might be something like this:

  You asked to squash this commit and its parent, but doing so would make
  it empty. You can drop this empty commit with "git reset HEAD^" , or you can
  keep it with "git commit --amend --allow-empty".

But I have not tested this.

What does it mean when you say it worked as expected? Did it leave the empty commit, omit the empty commit, or leave some un-squashed commit? It's not clear to me what --continue _should_ do in this case, but it does seem like the two options here should be

 1. keep the empty commit
 2. drop the empty commit

I would expect "git rebase --skip" to drop the empty commit, but maybe it will "skip" the squash instead. I don't know. Better advice here is certainly needed.

Phil Hord:

Actually, it did not work as expected I noted afterward, it just dropped the reversion commit, and did not squash the next commit into it as I had asked, so from three commits, "change", "revert", "new-change" I had two, "change", "new-change" with the end result being the same (i.e., instead of squashing all three into one "new-change", I had "change" and "revert" + "new-change").

I sort of expect a squashed commit of "change" + "revert" to be an empty commit, and of "change" + "revert" + "new-change" to be a commit of "new-change".

On Wed, Jun 11, 2014 at 1:57 PM, Peter Krefting <peter@softwolves.pp.se> wrote:

Did you have a series of three commits being squashed in your to-do list? I mean, did you have a list like this:

   pick ...  do foo
   squash ...  revert "do foo"
   squash ...  What I really meant to do.

I suppose the rebase stopped after the first squash failed due to the emptiness of the proposed result. Then rebase --continue proceeded, having decided that you were finished with the 'revert' commit. Then ... I would expect the next commit would actually be squashed, but I can only speculate at the reasons it might have decided not to after your continue.

This actually sounds like another case of a bug I reported a few weeks ago[1] and which Fabian Ruch was kindly investigating[2] and trying to fix. I don't think his fix would have helped in this case, but I do think it is worthy of consideration for that same patch series.

Yes, but empty commits are discouraged on some projects. If you want your "change + revert = empty" commit to appear after the squash, I would expect you would want to use --keep-empty on your inital rebase command. But I'm not sure that will do what you expected either; it may only keep previously-empty commits during the rebase.

[1] http://article.gmane.org/gmane.comp.version-control.git/245688 [2] http://www.mail-archive.com/git%40vger.kernel.org/msg51703.html

Phil Hord:

Yes, that is exactly what I had. Plus an extra commit that I moved to the end, which was originally placed between the "do foo" and "revert do foo" commits (which is why I wasn't 110% sure the combination of the two would produce an empty commit).

The thing is that I wasn't expecting it to come out empty, as I had another commit to squash into it. That the interim throw-away squashed commit was empty should have been an internal matter to rebase, IMHO.

On Fri, Jun 13, 2014 at 08:25:43AM +0100, Peter Krefting wrote:

That's a good point that I neglected in my other response. Maybe the right solution is for "rebase --interactive" to always pass "--allow-empty" when doing a squash. And then either:

  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.

-Peff

On Wed, Jun 11, 2014 at 01:49:04PM +0100, Peter Krefting wrote:

Yeah, that message comes from "commit --amend", which is called by rebase to handle the squash. The "repeat your command" part is confusing. The right thing to do here is:

  git commit --amend --allow-empty

if you want to have an empty commit, or:

  git reset HEAD^

if you want to have nothing.

Of course the first one would never occur to you, because it is not "your command" in the first place. :)

We could change it to say "use git commit --amend --allow-empty", though that is slightly incomplete for other cases (e.g., you might have actually said "git commit --amend -a", and the right advice is to include that "-a".

Commit understands a "whence" flag that could let it customize the message for the case of rebase. But I think you would have to teach determine_whence to figure out that we are in a rebase.

As you noticed later in the thread, doing "--continue" omits the revert. That's because it is telling rebase "OK, I've fixed this up, we can keep going". But of course it wasn't fixed.