Re: [PATCH RFC] rebase: respect --ff-only option

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> As I understand it the motivation for this change is to have 'git -c
> pull.rebase=true pull --ff-only' actually fast forward. Why cant we
> just change pull not to rebase in that case?
> ...
> Is there a use for this outside of 'git pull --ff-only'? I'm far from
> convinced we want this new option but if we do end up adding it I
> think it should error out in combination with '-i' or '-x' as '-i'
> implies the user wants to change the existing commits and '-x' can end
> up changing them as well.
>
> I think this patch addresses a valid problem but it seems to me that
> the approach taken pushes complexity into rebase to handle a case when
> pull does not need to invoke rebase in the first place.

I share the sentiment, but my conclusion would be different.

Even though we explain that "pull" is _like_ "fetch" followed by
"merge" (or "rebase"), at the conceptual level, "pull --ff-only"
should not have to invoke merge or rebase backend.  If the branch
fast-fowards to the fetched tip, "pull" should be able to just
update the branch and check it out, and if it doesn't, "pull" should
just be able to fail.

Similarly, "pull --ff" (i.e. fast-forwading allowed) should be able
to just update the branch and check it out when the current tip of
the branch can be fast-forwarded to the fetched tip.

But as you said, pull.rebase=interactive will break such a clean
separation of concerns.  You can leave "pull" oblivious of what
"rebase -i" wants to do when seeing a fast-forwardable history by
teaching "rebase" (and "merge") backend to take "--ff-only", "--ff",
and "--no-ff" options and having "pull" pass them through.

We can teach "pull" that certain backends (namely "rebase -i" in
this case) want to handle "ff logic" [*] themselves, and other
backends (i.e. "rebase" and "merge") do not mind if "pull" handles
"ff logic" for them, but that will break the abstraction (e.g. do we
really want to teach "pull" that "rebase -i" wants to rewrite all
the commits even when the history fast-forwards?) and will become a
source of duplicated logic.

Another thing to worry about are hooks.  post-rebase or post-merge
operations would want to be carried out even when the history would
fast-forward, and making "pull" to perform the fast-forwarding and
know which hooks should be called with what parameter so that we
could pretend as if the "merge" or "rebase" backend was indeed ran,
breaks the abstraction.

So, even though I wish that the world was simpler and we could
handle "ff logic" inside "pull", I am not sure if it is a realistic
wish.


[Footnote]

* By "ff logic", I am referring to what should happen in the 3x2=6
  matrix when one of ("--ff", "--ff-only", or "--no-ff") is given
  and the history (does or does not) fast-forward.