Re: [PATCH 2/2] rebase: don't rebase linear topology with --fork-point

[Jeff King <peff@peff.net>]

On Thu, Feb 21, 2019 at 10:40:59PM +0100, Ævar Arnfjörð Bjarmason wrote:

> Fix a regression introduced in 4f21454b55 ("merge-base: handle
> --fork-point without reflog", 2016-10-12).
> [...]

OK, your explanation mostly makes sense to me, except for one thing.

> Then in 4f21454b55 ("merge-base: handle --fork-point without reflog",
> 2016-10-12) which introduced the regression being fixed here, a bug
> fix for "git merge-base --fork-point" being run stand-alone by proxy
> broke this use-case git-rebase.sh was relying on, since it was still
> assuming that if we didn't have divergent history we'd have no output.

I still don't quite see how 4f21454b55 is involved here, except by
returning a fork-point value when there is no reflog, and thus
triggering the bug in more cases.

In particular, imagine this case:

  git init
  for i in $(seq 1 3); do echo $i >$i; git add $i; git commit -m $i; done
  git checkout -t -b other
  for i in $(seq 4 6); do echo $i >$i; git add $i; git commit -m $i; done
  git rebase

With the current code, that will rewind and replay 4-6, and I understand
that to be a bug from your description. And it happens at 4f21454b55,
too. But it _also_ happens at 4f21454b55^.

I.e., I still think that the only thing that commit changed is that we
found a fork-point in more cases. But the bug was still demonstrably
there when you actually have a reflog entry.

With the fix you have here, that case now produces "Current branch other
is up to date".

This is splitting hairs a little (and of course I'm trying to exonerate
the commit I'm responsible for ;) ), but I just want to make sure we
understand fully what's going on. Your fix looks plausibly correct to
me, but I admit I don't quite grok all the details of that conditional.

-Peff