Re: Fixup of a fixup not working right

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 4, 2016, 07:36 UTC
Message-ID: <alpine.DEB.2.20.1609040923390.129229@virtualbox>
In-Reply-To: <xmqq60qdsoxj.fsf@gitster.mtv.corp.google.com>

Hi Junio & Philip,

On Fri, 2 Sep 2016, Junio C Hamano wrote:

Show 38 quoted lines

> "Philip Oakley" <philipoakley@iee.org> writes:
> 
> > As I understand this it's implied by design. The issue is that the
> > rebase is looking for that named commit within its current rebase
> > range, and can't find it, so ignores it.
> >
> > There is a separate issue that all the fixup! fixup! messages are
> > essentially treated as being concatenations of the original fixup!, no
> > matter how many time the fiup is present.
> 
> They can be handled separately, but they come from the same "design"
> that could be improved.  When the "original" is not in the range to
> be rebased for whatever reason (including the most likely one, i.e.
> it has already graduated to become part of the public history), the
> best thing the user could do at that point may be, as you suggested
> to Robert in your message, to turn the "fixup! original" that did
> not make in time before "original" hit the public record into a
> standalone "fix original" follow-up change, and then to squash
> subsequent "fixup! fixup! original" (and other "fixup! original",
> too) into that commit.  And a good direction forward may be to see
> if "rebase -i" can be taught to be more helpful for the user who
> wants to do that.
> 
> Perhaps a change like this to "rebase -i":
> 
>  - The search for "original" when handling "pick fixup! original",
>    when it does not find "original", could turn it into "reword
>    fixup! original" without changing its position in the instruction
>    sequence.
> 
>  - The search for "original" when handling "pick fixup! fixup!
>    original", could be (probably unconditionally) changed to look
>    for "fixup! original" to amend, instead of looking for "original"
>    as the current code (this is your "separate issue").  The same
>    "if the commit to be amended is not found, turn it into reword"
>    rule from the above applies to this one, too.
> 
> may be an improvement?

I would be *very* careful with such a change.

The point is that fixup! messages are really special, and are always intended to be squashed into the referenced commit *before* the latter hits `master`.

The entire design of the fixup! feature (using the commit subject as identifier, which is only "unique enough" in a topic branch that is still being developed) points to that.

I am fairly certain that we would run into tons of problems if we diluted the concept of fixup! commits by changing the design so that fixup! commits all of a sudden become their own, "real" commits that can be fixed up themselves, as much of the current code simply does not expect that.

In short, I am opposed to this change.

And even if I am overruled, I would strongly suggest to implement this on top of my rebase-i-extra branch (i.e. in the rebase--helper instead of the shell script) to avoid double churn.

Ciao, Johannes