Re: Bug with fixup and autosquash

[Junio C Hamano <gitster@pobox.com>]

Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:

> I have been using git rebase heavily these days and seem to have found a bug.
>
> If there are two commit messages which have same prefix e.g.
> yyyyyy This is prefix
> xxxxxx This is prefix and message
>
> xxxxxx comitted before yyyyyy
>
> Now I commit a fixup to yyyyyy using git commit --fixup yyyyyy
> zzzzzz fixup! This is prefix
>
> When I run git rebase -i --autosquash, the script it shows me looks like
> pick xxxxxx This is prefix and message
> fixup zzzzzz fixup! This is prefix
> pick yyyyyy This is prefix
>
> I think the correct order is
> pick xxxxxx This is prefix and message
> pick yyyyyy This is prefix
> fixup zzzzzz fixup! This is prefix
>
> Is that right?

Because "commit" pretends as if it took the exact commit object name
to be fixed up (after all, it accepts "yyyyyy" that is a name of the
commit object), it would be nice if the fixup is applied to that
exact commit, even if you had many commits that share exactly the
same title (i.e. not just shared prefix).

Unfortunately, "rebase -i --autosquash" reorders the entries by
identifying the commit by its title, and it goes with prefix match
so that fix-up commits created without using --fixup option but
manually records the title's prefix substring can also work.  

We could argue that the logic should notice that there is one exact
match and another non-exact prefix match and favor the former, and
certainly such a change would make your made-up example (you didn't
actually have a commit whose title is literally "This is prefix")
above work better.

But I am not sure if adding such heuristics would really help in
general.  It would not help those whose commits are mostly titled
ultra-vaguely, like "fix", "bugfix", "docfix", etc.

Another possibility is to teach "commit --fixup" to cast exact
commit object name in stone.  That certainly would solve your
immediate problem, but it has a grave negative impact when the user
rebases the same topic many times (which happens often).

For example, I may have a series of commits A and B, notice that A
could be done a bit better and have "fixup A" on top, build a new
commit C on it, and then realize that the next step (i.e. D) would
need support from a newer codebase than where I started (i.e. A^).

At that point, I would have a 4-commit series (A, B, "fixup A", and
C), and I would rebase them on top of something newer.  I am
undecided if that "fixup A" is really an improvement or unnecessary,
when I do this, but I do know that I want to build the series on top
of a different commit.  So I do rebase without --autosquash (I would
probably rebase without --interactive for this one).

Then I keep working and add a new commit D on top.  After all that,
I would have a more-or-less completed series and would be ready to
re-assess the whole series.  I perhaps decide that "fixup A" is
really an improvement.  And then I would "rebase -i" to squash the
fix-up into A.

But notice that at this point, what we are calling A has different
object name than the original A the fixup was written for, because
we rebased once on top of a newer codebase.  That commit can still
be identified by its title, but not with its original commit object
name.

I think that is why "commit --fixup <commit>" turns the commit
object name (your "yyyyyy") into a string (your "This is prefix")
and that is a reasonable design decision [*1*].

So from that point of view, if we were to address your issue, it
should happen in "rebase -i --autosquash" side, not "commit --fixup"
side.

Let's hear from some of those (Cc'ed) who were involved in an
earlier --autosquash thread.

https://public-inbox.org/git/cover.1259934977.git.mhagger@alum.mit.edu/


[Footnote]

*1* "rebase -i --autosquash" does understand "fixup! yyyyyy", so if
    you are willing to accept the consequence of not being able to
    rebase twice, you could instead do

	$ git commit -m "fixup! yyyyyy"

    I would think.