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 15:14:12 -0800	[thread overview]
Message-ID: <xmqqzh2g9e0r.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <5fd7e77c78e06_d59852083e@natae.notmuch> (Felipe Contreras's message of "Mon, 14 Dec 2020 16:30:20 -0600")

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

>> 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.
>
> Right, so both should error out.

One of them, not both; the one we choose to implement would make it
fail, I would think.

> And what should these do?
>
>   git -c pull.rebase=no -c pull.ff=only pull --merge

There is no --merge, so let's reread with s/--merge/--no-rebase/;
the command line would be saying "I only want to fast-forward, or I
want the command to fail".  The advice code should not trigger
(i.e. we gave an explicit preference to merge and not to rebase, so
rebase_unspecified would be false), but the configured preference
that says "we take only fast-forward merges" will still be in effect.
If the history is fast-forward, the merge backend will happily
advance our history.  If not, the merge backend will happily fail
the pull.

If you want to countermand the configured preference from the
command line and allow a merge to be done when non-ff history is
given, what you'd need to override is not --merge/--no-rebase.  The
configuration pull.rebase=no already says you do not want rebase and
you want merge).  You want to override --ff-only, so I'd expect
"pull --ff" (or "pull --no-ff") to be how you override your
configured preference and merge in a non-ff history, either by fast
forwarding or creating an extra merge commit.

>   git -c advice.pullnonff=false pull --merge

Again with s/--merge/--no-rebase/; but that is showing the
preference not to rebase and to merge from the command line, so
shouldn't it just go ahead and merge without any advice?

> I'm going to answer because I think it's obvious what you would expect:
> if you pass --merge, both should succeed.
>
> Except they won't, because "git pull --ff-only --merge" fails.
>
> Correct?

See above.