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

Hi Tyler,

On Mon, Sep 14, 2026 at 12:00 AM Tyler Cipriani [off-list ref] 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…

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)?

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

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?)

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

Thanks,
D. Ben Knoble