Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only

From: D. Ben Knoble <ben.knoble+github@gmail.com>
Date: Feb 5, 2025, 16:36 UTC
Message-ID: <CALnO6CA_vF4huxMx6jSS4SVjS4+EO9K16Msco-vMUDzSoYRDOg@mail.gmail.com>
In-Reply-To: <xmqqbjvgr11y.fsf@gitster.g>

On Wed, Feb 5, 2025 at 8:09 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 23 quoted lines

>
> "D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
>
> > When running "git pull" with the following configuration options, we
> > fail to merge divergent branches:
> >
> > - pull.ff=only
> > - pull.rebase (unset)
> > - branch.<current_branch>.rebase=true
> >
> > Yet it seems that the user intended to make rebase the default for the
> > current branch while using --ff-only for non-rebase pulls. Since this
> > case appears uncovered by existing tests, changing the behavior here
> > might be safe: it makes what was an error into a successful rebase.
>
> Hmph, to me it looks more like with pull.ff, the user, no matter
> what other variables say and which mode between merge and rebase a
> pull consolidates the histories, wanted to make sure they will never
> accept anything other than fast-forwarding of the history, because
> the end-user expects that they will pull only after they push out
> everything, i.e., the expectation is that the other side is a strict
> fast-forward or the user wants to examine the situation before
> making further damage to the local history.

That's certainly one way to understand --ff-only, but I can't find it supported by existing docs (though it's what current code says, excepting lack of test for interaction with branch.name.merge). For example, `git help pull`:

        --ff-only
           Only update to the new history if there is no divergent local
           history. This is the default when no method for reconciling divergent
           histories is provided (via the --rebase=* flags).

and `git help config`:

       pull.ff
           By default, Git does not create an extra merge commit when merging a
           commit that is a descendant of the current commit. Instead, the tip
           of the current branch is fast-forwarded. When set to false, this
           variable tells Git to create an extra merge commit in such a case
           (equivalent to giving the --no-ff option from the command 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.

[…]

       branch.autoSetupRebase
           When a new branch is created with git branch, git switch or git
           checkout that tracks another branch, this variable tells Git to set
           up pull to rebase instead of merge (see "branch.<name>.rebase"). When
           never, rebase is never automatically set to true. When local, rebase
           is set to true for tracked branches of other local branches. When
           remote, rebase is set to true for tracked branches of remote-tracking
           branches. When always, rebase will be set to true for all tracking
           branches. See "branch.autoSetupMerge" for details on how to set up a
           branch to track another branch. This option defaults to never.

[…]

       branch.<name>.rebase
           When true, rebase the branch <name> on top of the fetched branch,
           instead of merging the default branch from the default remote when
           "git pull" is run. See "pull.rebase" for doing this in a non
           branch-specific manner.

           [snip]

           NOTE: this is a possibly dangerous operation; do not use it unless
           you understand the implications (see git-rebase(1) for details).

So I would tend to read branch.name.rebase as "you opted in to this, you know what you're doing" and let it override --ff-only.

Granted, it's not clear just from reading the various git-config files which sections and variables override which, so I'm perhaps overly-reliant on the documentation to understand when those overrides happen (see "notes" in original post).

Show 7 quoted lines

>
> With that understanding, I am not sure "even though pull.ff tells
> us to stop unless the other side is a descendant of our history, if
> we are rebasing, it is OK if they have something we have never seen"
> is a good thing to do.
>
> So, I dunno.

Agreed that if pull.ff=only is supposed to override all other options (except those on the command-line), this might be wrong. And `git pull --rebase` works in the scenario I described.

I think that `pull.ff=only` + `branch.name.rebase=true` is a useful combination to say "unless I'm asking to rebase [via --rebase or branch settings], only permit fast-forward pulls." For example, my main or master branch is typically fast-forward only, while I want my topic branches to be rebased; preferably, all of those things happen for just "git pull."

But maybe the intended way to accomplish what I want is pull.ff=true (the default?), which doesn't prevent accidental merges in the cases I want it to without setting branch.name.mergeOptions for each branch I want to protect from accidental pull-merges. (I'm in the habit of using fetch + merge as needed and mostly use pull to shortcut things when I'm confident, and obviously I can undo the accidental merge… but not having it in the first place is nice, too.)

LMK if something in my position is not clear—my overreliance on parentheticals can be confusing.