Re: [PATCH 3/5] pull: handle conflicting rebase/merge options via last option wins

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 15, 2021, 17:33 UTC
Message-ID: <xmqq7dhrtrc2.fsf@gitster.g>
In-Reply-To: <3c07ce978caa832b08c6bef1c48c061e41a6fd0b.1626316849.git.gitgitgadget@gmail.com>

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 9 quoted lines

> diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
> index 54048306095..e70ed99e408 100644
> --- a/Documentation/config/pull.txt
> +++ b/Documentation/config/pull.txt
> @@ -7,12 +7,13 @@ pull.ff::
>  	line). When set to `only`, only such fast-forward merges are
>  	allowed (equivalent to giving the `--ff-only` option from the
>  	command line). This setting overrides `merge.ff` when pulling.
> +	Incompatible with pull.rebase.

This update may mean well, but I sense that the description needs to be tightened up a bit more. Unless you mean to say that I am not allowed to say "[pull] rebase=no" when I set "[pull] ff", that is.

Or do you mean pull.ff=only is incompatible with any setting of pull.rebase?

Or do you mean pull.ff=only is incompatible with any setting of pull.rebase other than false?

Or do you mean any setting of pull.ff is imcompatible with any setting of pull.rebase other than false?

(You do not have to answer---the above questions just demonstrate that the description is unnecessarily loose).

Show 6 quoted lines

>  pull.rebase::
>  	When true, rebase branches on top of the fetched branch, instead
>  	of merging the default branch from the default remote when "git
>  	pull" is run. See "branch.<name>.rebase" for setting this on a
> -	per-branch basis.
> +	per-branch basis.  Incompatible with pull.ff.

Likewise.

I think it technically is OK to say "only ff update is allowed" or "ff update is allowed when able" while saying pull.rebase=yes.

 - For those who say ff=only, the actual value of pull.rebase would
   not matter (i.e. the integration immediately finishes by updating
   to what was obtained from the upstream because there is no new
   development on our own on top of theirs to either replay or
   merge).

 - For those who merely allow ff, an update may fast-forward even
   when pull.rebase is set to true (or any non-false value), while
   we'll replay our own development on top of theirs when their
   history is not descendent of ours.

So I do not see need to declare these combinations "incompatible". But perhaps I am missing some subtle cases?

Show 11 quoted lines

> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index 5c3fb67c014..03e8694e146 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -121,6 +121,9 @@ When false, merge the current branch into the upstream branch.
>  +
>  When `interactive`, enable the interactive mode of rebase.
>  +
> +Note that these flags are incompatible with --no-ff and --ff-only; if
> +such incompatible flags are given, the last one will take precedence.
> ++

I am not sure about these, either. Again, --ff-only (whether it is combined with --rebase or --rebase=no) would mean that the operation would fail when we have our own development on top of their history, and we repoint the tip of our history to theirs when we do not.

We see "--no-ff" is explained to "always create an unnecessary extra merge", bit I am not sure it is the right mental model to apply when the user prefers rebasing. The merge workflow is to treat our history as the primary and merging theirs in, so when theirs is a descendant (i.e. we have no new development of our own), some people may want to mark "we were there before we updated from them" with an extra merge. Without --no-ff, the resulting history would be quite different in the merge workflow if you have or do not have your own development. But the rebase workflow is to treat their history as the primary and replay our own development on top of theirs, and the shape of the resulting history would be the same whether you have your own development on top of theirs. We start from their tip and then replay our own development on top (which could be an empty set), and there is nothing "--no-ff" would need to do differently to keep the resulting history similar to cases where we do have something of our own. IOW, "--no-ff" becoming a no-op in a "rebase" workflow is a natural consequence, and there is no strong reason to say it is incompatible.