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

[Junio C Hamano <gitster@pobox.com>]

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

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

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

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