[PATCH] rebase--interactive: count squash commits above 10 correctly

On Fri, Jan 06, 2017 at 01:04:05AM -0800, Brandon Tolsch wrote:

Looks like a regression in v2.10. Here's the fix.

-- >8 --
Subject: rebase--interactive: count squash commits above 10 correctly

We generate the squash commit message incrementally running a sed script once for each commit. It parses "This is a combination of <N> commits" from the first line of the existing message, adds one to <N>, and uses the result as the number of our current message.

Since f2d17068fd (i18n: rebase-interactive: mark comments of squash for translation, 2016-06-17), the first line may be localized, and sed uses a pretty liberal regex, looking for:

  /^#.*([0-9][0-9]*)/

The "[0-9][0-9]*" tries to match double digits, but it doesn't quite work. The first ".*" is greedy, so if you have:

  This is a combination of 10 commits.

it will eat up "This is a combination of 1", leaving "0" to match the first "[0-9]" digit, and then skipping the optional match of "[0-9]*".

As a result, the count resets every 10 commits, and a 15-commit squash would end up as:

  # This is a combination of 5 commits.
  # This is the 1st commit message:
  ...
  # This is the commit message #2:
  ... and so on ..
  # This is the commit message #10:
  ...
  # This is the commit message #1:
  ...
  # This is the commit message #2:
  ... etc, up to 5 ...

We can fix this by making the ".*" less greedy. Instead of depending on ".*?" working portably, we can just limit the match to non-digit characters, which accomplishes the same thing.

Reported-by: Brandon Tolsch <btolsch@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---

I didn't include a test here, mostly because this bug is so weirdly specific that it seems unlikely to happen again. Especially in light of this code going away in favor of the C rebase helper, which Dscho already confirmed is not buggy (and I did not look at the code, but it cannot possibly do anything as gross as these repeated sed invocations).

It also is a little tricky to automatically extract the comments (you have to override GIT_EDITOR, but we also invoke GIT_EDITOR for the insn sheet).

I was able to replicate and confirm the fix manually with:

  git commit -q --allow-empty -m base
  git commit -q --allow-empty -m squash
  git tag base
  for i in $(seq 15); do
    echo $i >$i
    git add $i
    git commit -qm "squash! squash"
  done
  git rebase --autosquash -i base

I'd also suggest that "the commit message #4" is grammatically questionable. Probably "This is commit message #4" would be fine.

 git-rebase--interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)