Skip to content

clean-msg-after-fixup-continue-v4

Eric Sunshine pointed out that I had such a commit message in
https://public-inbox.org/git/CAPig+cRrS0_nYJJY=O6cboV630sNQHPV5QGrQdD8MW-sYzNFGQ@mail.gmail.com/
and I went on a hunt to figure out how the heck this happened.

Turns out that if there is a fixup/squash chain where the *last* command
fails with merge conflicts, and we either --skip ahead or resolve the
conflict to a clean tree and then --continue, our code does not do a
final cleanup.

Contrary to my initial gut feeling, this bug was not introduced by my
rewrite in C of the core parts of rebase -i, but it looks to me as if
that bug was with us for a very long time (at least the --skip part).

The developer (read: user of rebase -i) in me says that we would want to
fast-track this, but the author of rebase -i in me says that we should
be cautious and cook this in `next` for a while.

Fixes since v3 (thanks, Phillip, for the really fruitful discussion!):

- We now avoid using the commit message prepared for the skipped
  fixup/squash.

- Replaced the "rebase -i: Handle "combination of <n> commits" with
  GETTEXT_POISON" patch by a *real* fix instead of a work-around: Instead
  of parsing the first line of the commit message and punting when it is
  missing an ASCII-encoded number, we determine <n> separately
  (independent from any localized text).

- Fixed quite a couple more corner cases, using the `current-fixups`
  file introduced for the GETTEXT_POISON fix:

  * we only need to re-commit if this was the final fixup/squash in the
    fixup/squash chain,

  * we only need to commit interactively if there was *any* non-skipped
    squash,

  * if the fixup/squash chain continues, the <N> was incorrect in the
    "This is a combination of <N> commits" comment in the intermediate
    commit message (it included the now-skipped commits), and

  * even if a filed fixup/squash in the middle of a fixup/squash chain
    failed, and its merge conflicts were resolved and committed, the
    "This is a combination of <N> commits" comment was incorrect: we
    had already deleted message-fixup and message-squash, so the next
    update_squash_message() would mistakenly assume that we were
    starting afresh. Worse: if only fixup commands were remaining, but
    there had been a squash command, we would retain the "squash!" line
    in the commit message and not give the user a chance to clean things
    up in the final fixup!

Johannes Schindelin (4):
  rebase -i: demonstrate bugs with fixup!/squash! commit messages
  rebase -i: Handle "combination of <n> commits" with GETTEXT_POISON
  sequencer: always commit without editing when asked for
  rebase --skip: clean up commit message after a failed fixup/squash

 sequencer.c                | 193 ++++++++++++++++++++++++++++---------
 sequencer.h                |   6 +-
 t/t3418-rebase-continue.sh |  49 ++++++++++
 3 files changed, 200 insertions(+), 48 deletions(-)

base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d

Submitted-As: https://public-inbox.org/git/cover.1524862093.git.johannes.schindelin@gmx.de
In-Reply-To: https://public-inbox.org/git/CAPig+cRrS0_nYJJY=O6cboV630sNQHPV5QGrQdD8MW-sYzNFGQ@mail.gmail.com
In-Reply-To: https://public-inbox.org/git/cover.1524258351.git.johannes.schindelin@gmx.de
In-Reply-To: https://public-inbox.org/git/cover.1524226637.git.johannes.schindelin@gmx.de
In-Reply-To: https://public-inbox.org/git/CAPig+cRrS0_nYJJY=O6cboV630sNQHPV5QGrQdD8MW-sYzNFGQ@mail.gmail.com
In-Reply-To: https://public-inbox.org/git/cover.1524296064.git.johannes.schindelin@gmx.de
In-Reply-To: https://public-inbox.org/git/CAPig+cRrS0_nYJJY=O6cboV630sNQHPV5QGrQdD8MW-sYzNFGQ@mail.gmail.com
Assets 2