Re: [BUG] rebase no longer omits local commits

[John Keeping <john@keeping.me.uk>]

On Mon, Jul 07, 2014 at 10:56:23AM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > Perhaps we shuld do something like this (which passes the test suite):
> >
> > -- >8 --
> > diff --git a/git-rebase.sh b/git-rebase.sh
> > index 06c810b..0c6c5d3 100755
> > --- a/git-rebase.sh
> > +++ b/git-rebase.sh
> > @@ -544,7 +544,8 @@ if test "$fork_point" = t
> >  then
> >  	new_upstream=$(git merge-base --fork-point "$upstream_name" \
> >  			"${switch_to:-HEAD}")
> > -	if test -n "$new_upstream"
> > +	if test -n "$new_upstream" &&
> > +	   ! git merge-base --is-ancestor "$new_upstream" "$upstream_name"
> >  	then
> >  		upstream=$new_upstream
> >  	fi
> > -- 8< --
> >
> > Since the intent of `--fork-point` is to find the best starting point
> > for the "$upstream...$orig_head" range, if the fork point is behind the
> > new location of the upstream then should we leave the upstream as it
> > was?
> 
> Probably; but the check to avoid giving worse fork-point should be
> in the implementation of "merge-base --fork-point" itself, so that
> we do not have to do the above to both "rebase" and "pull --rebase",
> no?

I don't think so, since in that case we're not actually finding the fork
point as defined in the documentation, we're finding the upstream rebase
wants.

Having played with this a bit, I think we shouldn't be replacing the
upstream with the fork point but should instead add the fork point as an
additional negative ref:

	$upstream...$orig_head ^$fork_point

Here's a script that creates a repository showing this:

-- >8 --
#!/bin/sh
git init rebase-test &&
cd rebase-test &&
echo one >file &&
git add file &&
git commit -m one &&
echo frist >file2 &&
git add file2 &&
git commit -m first &&
git branch --track dev &&
echo first >file2 &&
git commit -a --amend --no-edit &&
echo two >file &&
git commit -a -m two &&
echo three >file &&
git commit -a -m three &&
echo second >file2 &&
git commit -a -m second &&
git checkout dev &&
git cherry-pick -2 master &&
echo four >file &&
git commit -a -m four &&
printf '\nWithout fork point (old behaviour)\n' &&
git rev-list --oneline --cherry @{u}... &&
printf '\nFork point as upstream (current behaviour)\n' &&
git rev-list --oneline --cherry $(git merge-base --fork-point master HEAD)... &&
printf '\nWith fork point\n' &&
git rev-list --oneline --cherry @{u}... ^$(git merge-base --fork-point master HEAD)
-- 8< --

In this case the rebase should be clean since the only applicable patch
changes "three" to "four" in "file", but the current rebase code fails
both with `--fork-point` and with `--no-fork-point`.

With `--fork-point` we try to apply "two" and "three" which have already
been cherry-picked across (as Ted originally reported) and with
`--no-fork-point`, we try to apply "first" which conflicts because we
have the version prior to it being fixed up on master.

I hacked up git-rebase to test this and the change to use the fork point
as in the last line of the script above does indeed make the rebase go
through cleanly, but I have not yet looked at how to cleanly patch in
that behaviour.

I haven't tested git-pull, but it looks like it has always (since 2009)
behaved in the way `git rebase --fork-point` does now, failing to detect
cherry-picked commits that are now in the upstream.