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

From: Junio C Hamano <gitster@pobox.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: "Elijah Newren" <newren@gmail.com>,
	"Jacob Keller" <jacob.keller@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Vít Ondruch" <vondruch@redhat.com>,
	"Alex Henrie" <alexhenrie24@gmail.com>,
	"Jeff King" <peff@peff.net>,
	"Philip Oakley" <philipoakley@iee.email>,
	"Theodore Y . Ts'o" <tytso@mit.edu>,
	"John Keeping" <john@keeping.me.uk>,
	"Richard Hansen" <rhansen@rhansen.org>,
	"Brian M. Carlson" <sandals@crustytoothpaste.net>,
	"W. Trevor King" <wking@tremily.us>
Subject: Re: [PATCH v2 02/14] pull: improve default warning
Date: Mon, 14 Dec 2020 13:04:28 -0800	[thread overview]
Message-ID: <xmqqh7ooje03.fsf@gitster.c.googlers.com> (raw)
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.

>> 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.

>> 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?