Re: [PATCH v2 02/14] pull: improve default warning

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 14, 2020, 21:04 UTC
Message-ID: <xmqqh7ooje03.fsf@gitster.c.googlers.com>
In-Reply-To: <CAMP44s3CcNAT4dFogyo61zV+D1pZ-0K+1rDBk_BUk-RYVUW0RQ@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> This is irrelevant.

Oh, now you are saying that you do not need a way to squelch these message to help unconfigured users choose between rebase or merge? I am confused.

> As long as it's an error I don't care if it's short or long. I'm
> against turning on an error from one version to the next.

Now you are changing your mind? Current version and past ones do not make it an error to pull non-ff history without choosing rebase or merge---we go ahead and merge anyway. I thought that in the far future agreed between two of us, it would be turnend into an error at some point. There needs one version that turns it into an error for that to happen. Puzzled.

Show 27 quoted lines

>> I too initially thought that pull.mode may be needed, but probably I
>> was wrong.  I do think this can be done without pull.mode at all, at
>> least in two ways, without adding different ways to do the same
>> thing.
>>
>>  - When pull.rebase is set to 'no' and pull.ff is set to 'only',
>>    "git pull" that sees a non-ff history should error out safely.
>>    The user is telling that their preference is to merge, but the
>>    difference between merge and rebase does not really matter
>>    because pull.ff=only would mean we forbid merges of non-ff
>>    history anyway.  The message you'd get would be "fatal: Not
>>    possible to fast-forward, aborting." though.
>>
>>  - Or with the advice that hides the latter two points, a user can
>>    unset pull.rebase and set the advice.pullNonFF to false to get
>>    the same behaviour (i.e. disable the more dangerous half of
>>    "pull") with just the "we stopped" error message.
>
> So, after your hypothetical patch, there would be no difference between:
>
>   git -c pull.rebase=no -c pull.ff=only pull
>
> and:
>
>   git -c advice.pullnonff=false pull
>
> ?

We do not have to or implement both, but either should give us the "when pull sees a non-ff history, it should stop without merging or rebasing, and the user won't be given the advice on how to choose between merge and rebase" behaviour, I would think.

Show 5 quoted lines

>> I think either of these are close enough to what you want, and I
>> think the latter gives us more flexibility in how we tone down the
>> message with advice.pullNonFF.
>
> You are missing at least two things.

I am guessing that the '?' above I just answered is one you wanted to ask me, but what's the other one?