Re: [PATCH 1/2] builtin/rebase.c: Emit warning when rebasing without a forkpoint

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Wesley

On 19/08/2023 21:34, Wesley Schwengle wrote:
> When commit d1e894c6d7 (Document `rebase.forkpoint` in rebase man page,
> 2021-09-16) was submitted there was a discussion on if the forkpoint
> behaviour of `git rebase' was sane. In my experience this wasn't sane.
> Git rebase doesn't work if you don't have an upstream branch configured
> (or something that says `merge = refs/heads/master' in the git config).
> The behaviour of `git rebase' was that if you supply an upstream on the
> command line that it behaves as if `--no-forkpoint' was supplied and if
> you don't supply an upstream, it behaves as if `--forkpoint' was
> supplied. This can result in a loss of commits if you don't know that
> and if you don't know about `git reflog' or have other copies of your
> changes. This can be seen with the following reproduction path:
> 
>      mkdir reproduction
>      cd reproduction
>      git init .
>      echo "commit a" > file.txt
>      git add file.txt
>      git commit -m "First commit" file.txt
>      echo "commit b" >> file.txt
>      git commit -m "Second commit" file.txt
> 
>      git switch -c foo
>      echo "commit c" >> file.txt"
>      git commit -m "Third commit" file.txt
>      git branch --set-upstream-to=master
> 
>      git status
>      On branch foo
>      Your branch is ahead of 'master' by 1 commit.
> 
>      git switch master
>      git merge foo

Here "git merge" fast-forwards I think, if instead it created a merge 
commit there would be no problem as the tip of branch "foo" would not 
end up in master's reflog.

>      git reset --hard HEAD^
>      git switch foo
>      Switched to branch 'foo'
>      Your branch is ahead of 'master' by 1 commit.
> 
>      git log --format='%C(yellow)%h%Creset %Cgreen%s%Creset'

For a reproduction recipe I think "git log --oneline" would suffice.

>      5f427e3 Third commit
>      03ad791 Second commit
>      411e6d4 First commit
> 
>      git rebase
>      git status
>      On branch foo
>      Your branch is up to date with 'master'.
> 
>      git log --format='%C(yellow)%h%Creset %Cgreen%s%Creset'
>      03ad791 Second commit
>      411e6d4 First commit

Thanks for the detailed reproduction recipe, I think it would be helpful 
to summarize what's happening in the commit message, especially as it 
seems to depend on "git merge" fast-forwarding. Do you often merge a 
branch into it's upstream and then reset the upstream branch?

I tend to agree with Junio that the current default is pretty 
reasonable. Looking through the links from the cover letter it seems 
that the current behavior came from a desire for

	git fetch && git rebase

to behave like

	git pull --rebase

I think the commit message for any change to the default should address 
why that is undesirable. Also we should consider what problems may arise 
from not defaulting to --fork-point when rebasing on an upstream branch 
that has itself been rebased or rewound.

Best Wishes

Phillip