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

From: Felipe Contreras <felipe.contreras@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Elijah Newren" <newren@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>,
	"Jacob Keller" <jacob.keller@gmail.com>,
	"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, 7 Dec 2020 13:09:22 -0600	[thread overview]
Message-ID: <CAMP44s2nmVnXiBA8S=vHBZznuRNKKe=xGOEBJ80MYhA_XCqNkg@mail.gmail.com> (raw)
In-Reply-To: <xmqqpn3lbhxn.fsf@gitster.c.googlers.com>

On Mon, Dec 7, 2020 at 12:16 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > When the user has specified "pull.mode=ff-only", the command dies with:
> >
> >   The pull was not fast-forward, please either merge or rebase.
> >
> > When the user has not specified any mode (by default), the command
> > would output a warning:
> >
> >   The pull was not fast-forward, in the future you will have to choose a
> >   merge, or a rebase.
>
> I quite don't get it.  They say the same thing.  At that point the
> user needs to choose between merge and rebase to move forward and
> ff-only would not be a sensible choice to offer the user.

The error happens *only* when the user has explicitly set
"pull.mode=ff-only", and the pull dies. At that point; yes, the user
must choose between one or the other.

But the warning happens by default, when the user has not chosen any
mode, and the pull succeeds. The user doesn't need to choose; the pull
continues as if the user chose the "merge" mode, which is the historic
behavior.

They start by saying the same thing. But one errors out and says the
user must choose, and the other warns that in the future the user must
choose.

> > Just to put this series in context: it's only part 1; it does not
> > introduce pull.mode, and it doesn't make --ff-only the default.
>
> I'd view the "in a non-fast-forward situation, the warning kicks in
> to those who haven't chosen between merge and rebase (i.e. no
> pull.rebase set to either true or false, and pull.ff not set to
> only), which is a bit more gentle than the current situtation" a
> good stopping point.  That state is already making ff-only the
> default for unconfigured users, or you can view it as shipping "git
> pull" in a shape that has the more dangerous half of its feature
> disabled to avoid hurting users.  So I am not sure why you keep
> saying you do not have --ff-only as the default.

The warning doesn't make the pull fail, ff-only does.

On the current master this works:

  test_expect_success 'non-fast-forward (default)' '
    setup_non_ff &&
    git pull
  '

And after my patch series (part 1) it still does. The default behavior
has not changed.

I'm not sure what it is that I'm not explaining correctly.

-- 
Felipe Contreras