Re: fc/pull-merge-rebase, was Re: What's cooking in git.git (Dec 2020, #01; Tue, 8)

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Emily Shaffer <emilyshaffer@google.com>,
	Felipe Contreras <felipe.contreras@gmail.com>
Subject: Re: fc/pull-merge-rebase, was Re: What's cooking in git.git (Dec 2020, #01; Tue, 8)
Date: Thu, 10 Dec 2020 05:46:31 +0100 (CET)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2012100543020.25979@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.2012100507450.25979@tvgsbejvaqbjf.bet>

Hi Junio,

On Thu, 10 Dec 2020, Johannes Schindelin wrote:

> On Wed, 9 Dec 2020, Junio C Hamano wrote:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > > On Tue, 8 Dec 2020, Junio C Hamano wrote:
> > >
> > >> * fc/pull-merge-rebase (2020-12-08) 19 commits
> > >>  - future: pull: enable ff-only mode by default
> > >>  - pull: advice of future changes
> > >>  - pull: add pull.mode=ff-only
> > >>  - pull: add pull.mode
> > >>  - pull: trivial memory fix
> > >>  - test: pull-options: revert unnecessary changes
> > >>  - test: merge-pull-config: trivial cleanup
> > >>  - pull: move configurations fetches
> > >>  - rebase: add REBASE_DEFAULT
> > >>  - pull: show warning with --ff
> > >>  - pull: introduce --merge option
> > >>  - pull: trivial whitespace style fix
> > >>  - pull: display default warning only when non-ff
> > >>  - pull: move default warning
> > >>  - pull: trivial cleanup
> > >>  - pull: cleanup autostash check
> > >>  - pull: refactor fast-forward check
> > >>  - pull: improve default warning
> > >>  - doc: pull: explain what is a fast-forward
> > >>
> > >>  When a user does not tell "git pull" to use rebase or merge, the
> > >>  command gives a loud message telling a user to choose between
> > >>  rebase or merge but creates a merge anyway, forcing users who would
> > >>  want to rebase to redo the operation.  Fix this by (1) tightening
> > >>  the condition to give the message---there is no reason to stop or
> > >>  force the user to choose between rebase or merge if the history
> > >>  fast-forwards, and (2) failing the operation when the history does
> > >>  not fast-forward, instead of making a merge, in such a case.
> > >
> > > Despite what the commit message of the tip commit says, it is not "time to
> > > flip the switch and make ff-only the default" because it breaks our very
> > > own test suite left and right. See for yourself:

BTW I suspect that the test failures point to shortcomings in the patch
series that should be addressed anyway. I saw at least one obvious
omission: when `git pull -s <strategy>` is used, I find it relatively
obvious that the user wanted that to imply `--merge`. Yet t4013 fails
thusly:

	+ git pull -s ours . side
	From .
	 * branch            side       -> FETCH_HEAD
	fatal: The pull was not fast-forward, please either merge or rebase.

It is test breakages like these which make me believe even more strongly
than before that it is really reasonable for contributors to make sure
that their patches pass the CI build before asking for reviews.

Ciao,
Dscho