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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 5, 2021, 09:58 UTC
Message-ID: <xmqq8s2lnkta.fsf@gitster.g>
In-Reply-To: <349748b4-3c48-7ca7-eb0f-e859a15cab0f@gmail.com>

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

Show 13 quoted lines

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