Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible

From: Elijah Newren <newren@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Alex Henrie" <alexhenrie24@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Ævar Arnfjörð" <avarab@gmail.com>,
	"Felipe Contreras" <felipe.contreras@gmail.com>
Subject: Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible
Date: Mon, 12 Jul 2021 16:24:12 -0700	[thread overview]
Message-ID: <CABPp-BHV4bkyKswbg0gBH8uJG7knH4fBC=fitRJK4_LCP50Tog@mail.gmail.com> (raw)
In-Reply-To: <xmqqbl772l4m.fsf@gitster.g>

On Mon, Jul 12, 2021 at 4:00 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> >> When we cannot fast-forward (i.e. we have our own development that
> >> is not in the tip of their history),
> >>
> >>  --ff-only would cause the operation fail
> >>  --ff would become no-op (as it merely allows fast-forwarding)
> >>  --no-ff would become no-op (as it merely forbids fast-forwarding)
> >>
> >> and the latter two case, we'd either merge or rebase (with possibly
> >> specified mode like --preserve-merges).  I thought the current
> >> documentation is already fairly clear on this point?
> >
> > git pull's --no-ff is documented to "create a merge commit in all
> > cases", and thus as worded, seems incompatible with rebasing to me.
>
> It smells like a "too literally to be useful" interpretation of a
> pice of documentation that has no relevance to "pull --rebase" to
> me, though.  It comes from merge-options.txt and would not be
> relevant to "git pull --rebase" to begin with.

"git pull --rebase" isn't a very interesting case -- it's missing
--no-ff, and has an explicit flag declaring user intent making both
pull.ff and pull.rebase irrelevant.  It doesn't really help us handle
the tougher cases at all.

Let me back up and see if I can explain a bit better.

1. In general, we allow command line options to countermand either
configuration options or other conflicting command line options, e.g.
--no-gpg-sign can countermand --gpg-sign=$KEY or commit.gpgSign
setting.

2. Starting with simple conflicts, git-pull has two sets of them of
the above form:
  * --rebase, --no-rebase, pull.rebase
  * --no-ff, --ff-only, --ff, pull.ff

3. git-pull *also* has conflicting options across these types.
  * --rebase[={true,merges,preserve,interactive}] and --ff-only
  * possibly also --no-ff with any of those rebase flags
  * the underlying pull.rebase and pull.ff which are meant to mirror
these flags thus also can conflict

4. The way we handle conflicting options in git is typically:
  * The last command line option countermands any earlier ones on the
command line
  * Command line options countermand anything in config
  * We do not assume any ordering in config, so if two configuration
options conflict, it's either an error or we document which one
overrides the other (e.g. diff.renames=false overrides
diff.renameLimit)

So my proposal is just to do 4 above, noting that:
  * pull.rebase set to anything other than "false" conflicts with
pull.ff set to anything other than "true" (and similarly for the
equivalent command line options)
  * If both pull.rebase and pull.ff are set (in conflicting fashion)
and no countermanding command line flags are set, I recommend throwing
an error; it's not clear that having one override the other would
match user intent.