Re: [PATCH v4 0/2] push: check pushed ref for --force-if-includes

From: Tyler Cipriani <tyler@tylercipriani.com>
Date: Sep 14, 2026, 19:27 UTC
Message-ID: <CAHLx=O=eH=7g=JUn5dOJgatv2xJVbQygK4C7zCA0Uv=BpzMkxQ@mail.gmail.com>
In-Reply-To: <CALnO6CDz8QBcBojmhjgwgWzi4oUbs+V4KVQ1h0+JgN7k0v-SYQ@mail.gmail.com>

On Mon, Sep 14, 2026 at 7:03 AM D. Ben Knoble <ben.knoble@gmail.com> wrote:

> Hi Tyler,

Hi Ben!

Show 10 quoted lines

> On Mon, Sep 14, 2026 at 12:00 AM Tyler Cipriani <tyler@tylercipriani.com> wrote:
> >
> > Changes since v3:
> >
> > - check_if_includes_upstream unconditionally resolves peer_ref with
> >   RESOLVE_REF_READING, now all non-branch ref pushes will be rejected
> >   when using --force-if-includes
> > - add test for --force-if-includes tag push 1/2
>
> This is intriguing and seems like a significant behavior change, let's read on…

It's definitely true that this is a behavior change and it'll add some friction to your process. And it's also true that the current behavior is failing to provide the guarantees it claims.

Show 18 quoted lines

> > Range-diff against v3:
> > 1:  da27c421ed ! 1:  e7912c3fd0 push: check pushed ref for --force-if-includes
> >     @@ Commit message
> >     -    Find local reflog using ref->peer_ref. When using a refspec like
> >     -    HEAD:refs/heads/main, we resolve HEAD. If HEAD is a branch, use that
> >     -    branch's reflog.
> >     +    Instead, use ref->peer_ref to locate a branch with a reflog. But if ref
> >     +    does not resolve to a branch (e.g., a detached HEAD, a tag, an oid),
> >     +    then we reject the push. The alternative would be to use HEAD's reflog,
> >     +    which is too broad to tell us if the history being pushed includes the
> >     +    tip of the remote. We need a per-branch reflog, which means that pushes
> >     +    of a ref that do not resolve to a branch are rejected. Rejecting the
> >     +    push of a ref like a detached HEAD already happens today (if the
> >     +    same-named local branch lacks the remote tip); now the detached HEAD and
> >     +    other non-branch pushes are explicitly rejected.
>
> So, we would now reject a force-push whose source is anything but a branch (with
> force-if-includes, and that presumably includes push.useForceIfIncludes)?

I should clarify, reject the force-push of any source not ultimately resolvable to a branch; e.g., HEAD will work if resolves to a branch.

Show 36 quoted lines

> >     ++test_expect_success '"--force-if-includes" should reject forced update from tag' '
> >     ++  setup_src_dup_dst &&
> >     ++  test_when_finished "rm -fr dst src dup" &&
> >     ++  (
> >     ++          cd src &&
> >     ++          git fetch &&
> >     ++          git switch main &&
> >     ++          git reset --hard origin/main &&
> >     ++          git switch -c newbranch origin/main &&
> >     ++          git checkout HEAD^ &&
> >     ++          git tag stable &&
> >     ++          test_must_fail git push --force-if-includes --force-with-lease origin stable:main
> >     ++  )
> >     ++'
>
> Which is what I think this test says.
>
> I think this would break a common thing I do at work (although this is soon to
> be deprecated, so take my anecdote with appropriate salt; I can't claim that no
> one else relies on it, of course):
>
> As I think I described in the message you linked, I have an alias "pf = push
> --force-with-lease" and push.useForceIfIncludes=true in config. Our team has a
> "main" release branch and a "hotfix" release branch for emergencies. When
> hotfixing, we first reset the hotfix branch to the last tag to go out to our
> production environment, which I typically do like this:
>
>     # validate that we won't lose any interesting commits (no regressions) with
>     # something like
>     git log --oneline --graph --boundary --cherry-mark --left-right
> origin/hotfix...<TAG>
>     # push
>     git pf origin <TAG>:hotfix
>
> (On a second pass before sending, I can't recall if this works as-is when I
> don't have a local hotfix branch tracking origin/hotfix.)

Yes, this workflow will break. And it will not work today without a local branch named "hotfix". It's broken today, insofar as this is a false pass since push.useForceIfIncludes is unable to say anything about whether you've integrated origin's hotfix branch into the <TAG>, you're pushing so it only incidentally works.

Today, git pf is actually checking that your refs/heads/hotfix's reflog has the tip of origin's refs/heads/hotfix. But it makes no promises about <TAG>. That is, you could:

    git checkout hotfix && git pull # This line is what makes it work today
    git checkout --orphan junk
    git commit -m --allow-empty 'Totally unrelated empty commit'
    git tag <TAG>
    git pf origin <TAG>:hotfix

And pf will allow that to happen since origin/hotfix's tip has been integrated with your local refs/heads/hotfix, which is what it's checking today.

Show 11 quoted lines

> If I'm reading this version right, I would now have to say
>
>     git pf --no-force-if-includes origin <TAG>:hotfix
>
> or perhaps better
>
>     git pf --no-force-if-includes --force-with-lease=hotfix[:origin/hotfix] …
>
> probably after seeing a (hopefully improved?) message after the original
> command. (Do I need to disable force-if-includes in the more-specific lease
> command?)

    git pf --force-with-lease=hotfix:origin/hotfix origin <TAG>:hotfix

Should be sufficient and as I understand your process, that's what you're after. The explicit --force-with-lease argument makes --force-if-includes a no-op, so --no-force-if-includes should be unnecessary.

Show 28 quoted lines

> Now, on the one hand, enshrining existing behavior is good for backwards
> compatibility but has earned us a bit of a reputation for not innovating in
> useful ways ;) On the other, I wonder if the description of force-if-includes
> allows some latitude to break with existing behavior here.
>
> The relevant docs say
>
>        --force-if-includes, --no-force-if-includes
>            Force an update only if the tip of the remote-tracking ref has been
>            integrated locally.
>
>            This option enables a check that verifies if the tip of the
>            remote-tracking ref is reachable from one of the "reflog" entries of
>            the local branch based in it for a rewrite. The check ensures that
>            any updates from the remote have been incorporated locally by
>            rejecting the forced update if that is not the case.
>
> It is unclear to me what "one of the 'reflog' entries of the local branch based
> in it" means! Ignoring that, the surrounding text only talks about whether the
> remote-tracking ref's tip (or "updates from the remote") have been "integrated
> locally."
>
> So I think we *could* say that, in this case, we don't have enough information
> from "<TAG>:hotfix" to check whether "origin/hotfix" has been integrated locally
> or not, and we should tighten the meaning of the check. (Perhaps when
> "--force-with-lease=hotfix" is given, though, we now have more information
> available to check---but that could be outside the scope of this series if we
> don't mind breaking backwards compatibility now.)

From my perspective, this is similar to the detached HEAD discussion from 2020[0] where "[the reflog of HEAD not attached to a branch] _does_ answer a different question from what we actually asked."

[0]: <https://lore.kernel.org/git/nycvar.QRO.7.76.6.2009161214030.56@tvgsbejvaqbjf.bet/>

I opted for a direction requiring explicit arguments to express intent, since that's the only way to ensure --force-if-includes aligns with (how I read) the documentation and the previous discussions.

Specifically, with tags:

- tags may have a reflog, but it answers a different question vs. "has
this tag integrated changes from an upstream" it answers what oid/ref
does this tag point to
- tags may incidentally point at oids referenced by branches with
reflogs, but there may also be several branches pointed to the same
oid, so which would we choose?

BUT I just realized there is existing, more fundamental breakage with --force-if-includes here that I'm making worse.

There is one case where we do have enough information to say whether <TAG> has integrated the tip of the remote-ref locally: fast-forward push. And that's actually broken today, too :)

    git --version
    git version 2.47.3
    git clone repo.git repo && cd repo
    git commit --allow-empty -m 'Normal, no-force-needed fast forward commit'
    git reflog expire --expire=all --all
    # Regular fast-forward push fails, even though it does not require
--force to begin with
    git push --force-with-lease --force-if-includes origin main
    ! [rejected]        main -> main (remote ref updated since checkout)

Checking for fast-forward happens after --force-if-includes checks the reflog. So that will need a fix…

My change makes an existing problem more acute, and probably requires a fix before other fixes can merge. Otherwise, --force-if-includes will always fail when pushing tags and detached heads, even when they're fast forward changes, adding needless friction to otherwise safe pushes (e.g., for tags that fast-forward a branch). So v5 will require a third change that touches other functions in remote.c. :/

> Thanks,
> D. Ben Knoble

Thank you for all the review and thoughts!