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

From: Felipe Contreras <felipe.contreras@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Jacob Keller" <jacob.keller@gmail.com>,
	"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>,
	"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 21:15:05 -0600	[thread overview]
Message-ID: <CAMP44s3KCoDfRXzarJw5AE7UsY-=eP6GbHzdDcdrs2rsw5tL+w@mail.gmail.com> (raw)
In-Reply-To: <xmqq360h8286.fsf@gitster.c.googlers.com>

On Mon, Dec 7, 2020 at 8:23 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> > I think the key point is that this "in the future" is referring to "a
> > future version of git will make this an error"
> >
> > This might be better if it said something like "The pull was not
> > fast-forward. In a future version of git you will need to specify
> > whether to merge or rebase, using pull.mode"
>
> Oh, I've actually been assuming that the current "warn but go ahead
> anyway asssuming the preference is to merge" can just be declared as
> a bug (iow, there is no need to say 'in the future'---we'd fix the
> bug right away).

It is a bug, in my opinion. If we were not planning on changing the
default, I would say drop the warning altogether.

*But*, if we are going to change the default, the warning can be
repurposed to say "in the future this will fail". That requires other
changes though.

> > or something similar. In theory, this warning will go away once that
> > future version of git changes so that pull.mode defaults to ff-only.
> >
> > The difference being that a warning will allow the command to continue
> > doing the default of today (merging), where as an error will stop the
> > command essentially just after the fetch portion finishes, without
> > changing the branch.

Exactly. And we can choose between those behaviors with a
configuration, like it happened with push.default.

> Yup.  If we want to take things slow, that is fine by me as well,
> but I am not sure if that is even necessary, given how annoying the
> existing "loudly warn but still go ahead" behaviour is, and how easy
> for existing users to have squelched the annoyance by choosing
> between rebase and merge already.  I've always assumed that any
> existing users who started using Git in the past several years have
> already set pull.rebase to one or the other value and they won't be
> affected by fixing "git pull" to just error out.

Existing users that are using up-to-date distributions, maybe. Debian
stable is using v2.20.1. In general it's not a good idea to assume
anything about our users. Recently a user of Oh My Zsh reported an
issue with the backwards compatibility code of git-completion; he was
using v2.17 I think.

That is exemplified by the fact that this whole thread started from a
user that refused to configure pull.rebase and expected the Git
project to just do the right thing (which is basically choosing a
useful default).

Cheers.

-- 
Felipe Contreras