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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Dec 8, 2020, 03:15 UTC
Message-ID: <CAMP44s3KCoDfRXzarJw5AE7UsY-=eP6GbHzdDcdrs2rsw5tL+w@mail.gmail.com>
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:

Show 14 quoted lines

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

Show 7 quoted lines

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

Show 8 quoted lines

> 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