git-rebase -i will complain and halt if it ever finds itself producing
an empty commit when squashing.  I encountered this when attempting to
fix a branch full of "do X, do Y, undo X, undo Y, do Z instead" junk.


Here's a simple demonstration:

  $ git init

  $ echo foo > foo
  $ git add foo
  $ git commit -m foo

  $ echo bar >> foo
  $ git commit -a -m bar

  # This undoes the previous commit
  $ echo foo > foo
  $ git commit -a -m foo2

  $ echo baz >> foo
  $ git commit -a -m baz

  $ git rebase -i HEAD~3
  # Squash all three commits into one

  Could not apply 06c1a34... foo2

Hi Frédéric,

Frédéric Brière wrote:

Thanks for the report.  I see two problems here:

First, when there are no changes to apply, rebase -i and am produce
confusing messages and abort without giving the user much guidance.
They should let the user know that one can use <driver> --skip to
continue.

Second, perhaps an empty patch is not worth stopping for anyway.

Were you expecting rebase -i to produce an empty squashed commit or
to skip the empty change?

Jonathan

I'd argue for the empty commit, since I didn't actually remove anything
from the rebase list.  I can always go back and purge it later if I so
wish.

In this case, you'll notice that the final version of that commit is not
an empty one.  I would expect a squash to retain all messages, or a
fixup to retain the message of the first commit, regardless of what
happens in-between.  Be sure to keep that in mind when coming up with a
fix.  <g>

Fri, Feb 19, 2010 at 07:59:35PM -0500, Frédéric Brière wrote:
with rebase or am, if one of my patches is already included upstream, I
would want it excluded, not replaced with an empty commit.  Indeed,
that is something rebase already does by using rev-list --cherry-pick
--left-right to generate the list of commits to apply.

      apply A --- E --- apply C [upstream]
     /
 base --- A --- B --- C --- D [master]

should become and does become

 base --- apply A --- E --- apply C [upstream] --- B --- D [master]

On the other hand, if one of my patches was already an empty commit,
or in your case where the effect of commits to be squashed cancel each
other out, it makes sense to say the intention is to create an empty
patch, I agree.  If I had wanted to remove the patch, I could always
explicitly remove it from the list.

The problem comes in a third case: what if some of my patches were
included upstream, with some other changes squashed in?

      apply A+C --- E [upstream]
     /
 base --- A --- B --- C --- D [master]

rev-list --cherry-pick will correctly note that my patch is not among
those included upstream, so rebase will try to apply it.  If the
squashed-in change doesn’t conflict with my change, that will succeed
without conflicts, resulting in an empty patch.  I would like that
patch to be omitted from the resulting history.

 base --- apply A+C --- E [upstream] --- B --- D [master]

How to achieve both?  One could (and maybe should) use heuristics
like “empty commits should be forward-ported without complaint”.
In more complicated situations like squashes, I think the best thing
is to ask the human driver which situation we are in: is this an empty
marker, with an important commit message, or a superseded patch to
discard?

I will look into improving the error message.  If you have other ideas
for improving the behavior and a little time, I encourage you to patch
git-rebase--interactive.sh to try them out and send the results to
git@vger.kernel.org.

Jonathan

Good point.  I guess that's what we get for using one single tool for
two completely different tasks.  :)

Actually, I don't personally mind if empty squished commits get dropped
on the floor, so I'll leave that into your hands.  My only concern is
with squishing commits where the end result is non-empty, but there
exists a subset that amounts to an empty commit.  I'd expect this to
work (obviously), and to not drop commit messages in the process.

Thanks for your hard work!

clone 570595 -1
retitle -1 rebase -i: (optionally) skip empty patches without prompting
severity -1 wishlist
tags 570595 + patch
forwarded 570595 http://thread.gmane.org/gmane.comp.version-control.git/148051
quit

Jonathan Nieder wrote:

So I get distracted for a few months, and poof, someone has to go
and fix it independently upstream. :)  I can’t complain, really.

I still want to see this change happen.  Cloning the bug.

Thanks again,
Jonathan