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

>
> On Mon, Sep 14, 2026 at 7:03 AM D. Ben Knoble <ben.knoble@gmail.com> wrote:
> > Hi Tyler,
>
> Hi Ben!
>
> > 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.
>
> > > 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.
>
> > >     ++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.
>
> > 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.

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