Re: [RFC 2/2] pull: default pull.ff to "only" when pull.rebase is not set either

[Felipe Contreras <felipe.contreras@gmail.com>]

On Thu, Dec 3, 2020 at 8:06 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> >>> That would require changing the semantics of --ff-only, so that "git
> >>> pull --no-rebase --ff-only" doesn't make sense (as --ff-only is
> >>> overridden by --no-rebase).
> >>
> >> I do not think such a conclusion follows from "we do not want to use
> >> the 'by default force the --ff-only' when the user chooses between
> >> merge and rebase".  Specifically, I do not agree with "as --ff-only
> >> is overridden" in your statement.
> >
> > Ah, sorry, I mis-read your three lines above.
> > ...
> > And if we introduce a third-way, i.e. "we do not handle the case
> > where you have your own development at all, this is only to maintain
> > pristine copy from your upstream", and repurpose "--ff-only" for
> > that purpose, yes, what you said above does make sense.  At that
> > point, there is no reason to disagree with "as --ff-only is
> > overridden" part of your statement---in your new world, "--ff-only"
> > is redesigned to act that way.
>
> Just to avoid misunderstanding, I only meant to say that the first
> three lines I quoted is internally consistent (unlike the message I
> was responding to, in which I said "I disagree---the conclusion does
> not follow").  It no way means I think such a re-definition of what
> "--ff-only" means is a great design.

Yes, that's what I parsed.

> What we want to see can be done without such backward incompatible
> changes, e.g. declaring the combination of "--ff-only" and
> "--[no-]rebase" incompatible and/or the last one wins, I would say,
> and I suspect Alex's RFC was an attempt to make the first step in
> that direction.

It's debatable whether or not that is "backwards incompatible".

Currently "--no-rebase --ff-only" fails if the merge is
non-fast-forward. With the proposed change of semantics it would work.
That's a change.

> What is most missing in the current system is a fix for the way in
> which "--rebase" interacts with "--ff-only".  Immediately after
> fetching, if our current branch is not a subset of what we fetched
> from the other side, the command should die.  Otherwise, it should
> just rebase what we have (which is nothing) on top of the tip of the
> history of the other side (which is to fast-forward our tip and the
> working tree to their tip).

Keep in mind the whole point of the changes: to make --ff-only the
default. If you make "git pull --rebase --ff-only" fail if
not-fast-forward, then "git pull --rebase" will fail if you make
--ff-only the default.

I don't know if I'm not doing a good job of stating that --rebase
should override (and ignore) --ff-only, that's the way it should
interact.

Here it goes again. In short, this is the target behavior:

* git pull --ff-only # fail unless fast-forward
* git pull --merge --ff-only # ignore --ff-only
* git pull --rebase --ff-only # ignore --ff-only

> Another thing we would want to change is to make the "you must
> choose between rebase and merge" trigger a lot more lazily.  If our
> side does not have our own development and their history is a
> descendent of what we have, the user shouldn't have to choose.  Only
> when the history they have does not fast-forward, we have to abort
> giving the "you must choose between the two" warning message.

Yes, I already sent patches for that [1].

> When the user tells us to do rebase or merge, nothing (except that
> "--ff-only" should prevent the rebase from going forward) should
> have to be changed.

If --ff-only prevents the rebase and the merge from going forward,
then it cannot be the default.

Maybe my last patch series can make things clearer [2].

Cheers.

[1] https://lore.kernel.org/git/20201204061623.1170745-11-felipe.contreras@gmail.com/
[2] https://lore.kernel.org/git/20201204061623.1170745-1-felipe.contreras@gmail.com/T/

-- 
Felipe Contreras