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 [off-list ref] wrote:

Hi Tyler,

Hi Ben!

On Mon, Sep 14, 2026 at 12:00 AM Tyler Cipriani [off-list ref] wrote:

quoted

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.

quoted

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.

quoted

    ++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/ (local)>

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!