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

Phillip Wood wrote:

On 11/07/2021 02:26, Alex Henrie wrote:

quoted

The warning about pulling without specifying how to reconcile divergent
branches says that after setting pull.rebase to true, --ff-only can
still be passed on the command line to require a fast-forward. Make that
actually work.

Thanks for revising this patch, I like this approach much better. I do 
however have some concerns about the interaction of pull.ff with the 
rebase config and command line options. I'd naively expect the following 
behavior (where rebase can fast-forward if possible)

   pull.ff  pull.rebase  commandline  action
    only     not false                rebase

Agreed. (pull.ff applies only for --merge)

    only     not false   --no-rebase  fast-forward only

Agreed. (--no-rebase is --merge, and pull.ff applies)

     *       not false    --ff-only   fast-forward only

Disagree. (--ff-only is for --merge)

We would need to change the documentation and the advice warning for
this to be correct.

    only     not false    --ff        merge --ff

Disagree.

This is a rebase, --ff should be ignored.

Junio already proposed --ff and other options to imply a merge [1], but
I already explained why that is problematic [2].

    only     not false    --no-ff     merge --no-ff

Disagree. (ditto)

    only       false                  fast-forward only
    only       false      --rebase    rebase
    only       false      --ff        merge --ff
    only       false      --no-ff     merge --no-ff

Agreed.

I don't think enforcing fast-forward only for rebases makes sense unless 
it is given on the command line.

But why? This is inconsistent.

Everywhere else in git the configuration is another way of specifying
the command line. This would be the first instance where it would not be
the case.

If the user gives `--rebase` `--ff-only` on the command line then we
should either error out or take the last one in which case `pull
--rebase --ff-only` would fast-forward only but `pull --ff-only
--rebase` would rebase.

Following the same logic `pull --ff-only --merge` would ignore the
previous --ff-only, wouldn't it?


This is a pretty significant semantic change and nowhere in this patch
it's explained who this is supposed to help, or what is the motivtion
behind it.

[1] https://lore.kernel.org/git/xmqqmtyf8hfm.fsf@gitster.c.googlers.com/ (local)
[2] https://lore.kernel.org/git/5fd8aa6a52e81_190cd7208c8@natae.notmuch/ (local)

-- 
Felipe Contreras