From: Wesley Schwengle <wesley@schwengle.net>
Date: Thu, 16 Sep 2021 19:39:04 GMT
Subject: Re: Possible git bug
Message-ID: <3b4270f9-6139-7007-301b-8a084f4336cf@schwengle.net>
In-Reply-To: <xmqqzgsctu10.fsf@gitster.g>

On 9/16/21 11:33 AM, Junio C Hamano wrote:

>> We are on a branch, we merge it into another branch.
>> We undo the merge because reasons.
>> Now we git rebase, without the upstream, because we've set it.
>> Fork-point is used now, because we haven't specified an upstream, but
>> we did set it and git merge-base decides, oh, we had those commits in
>> master but these where dropped so we drop them in this branch as well.
> 
> If you feel "It doesn't make sense to me", either
> 
>   - the behaviour does not make sense because it is simply buggy, in
>     which case, adding a sentence to the documentation and explaining
>     how not to use it is missing the point---don't you rather want it
>     to behave in a way that makes sense to you instead?
> 
> or
> 
>   - it appears as nonsense to you only because your understanding of
>     the behaviour is faulty but the feature is working correctly and
>     is not a bug, in which case, adding a sentence to the
>     documentation and explaining how not to use it is missing the
>     point---don't you rather want the existing documentation extended
>     to help you and other users to understand the behaviour better
>     first?
> 
> Between "buggy behaviour" and "bad documentation of a well-designed
> behaviour", I offhand do not know which side "--fork-point" is for
> this particular case, but I've always felt that it is a bad
> heuristic that should be used with care, and my gut feeling is it
> might be the third possibility: "bad heuristic that sometimes
> misbehave badly and that is unfixable".  If that is the case,
> perhaps the documentation should tell readers the unreliable nature
> of the option and warn them to double check the result before
> teaching them how to turn it off permanently.

I feel like it is a bad default, it caught me by surprise. Especially 
because in the reproduction path I wanted to explicit in my rebase 
action and this caused different behavior. After this was pointed out I 
read the man page because I thought `git rebase' and `git rebase master' 
was the same thing if that was configured as an upstream. It took me a 
while to figure this out, because I kept typing `git rebase' instead of 
`git rebase master' when quickly trying to find out why it wasn't 
behaving like it did earlier.

I'm clueless about "buggy behavior", "bad documentation of a well 
designed feature" or "bad heuristic that sometimes misbehave badly and 
that is unfixable". To me `git rebase' with a configured upstream should 
behave the same as `git rebase @{u}'. Only when adding --fork-point it 
should behave as it does currently. I'm not sure when I would want to 
use it, but I'm thinking people want it, otherwise it wouldn't be a default.

As for the patch. The reason why --fork-point is default I do not know, 
but how to disable it isn't documented and I think it should. It is 
hidden in the source code and the release notes of 2.31.0. It should be 
more visible. Which is the reason I submitted the patch.

Cheers,
Wesley

-- 
Wesley Schwengle
E: wesley@schwengle.net