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

[Ben Knoble <ben.knoble@gmail.com>]

> Le 4 sept. 2026 à 17:01, Tyler Cipriani <tyler@tylercipriani.com> a écrit :
> 
> "--force-if-includes" ensures, "tip of the remote-tracking ref is
> reachable from one of the 'reflog' entries of the local branch."
> 
> But check_if_includes_upstream() uses the local per-branch reflog based
> on the destination branch rather than the branch being pushed; using
> ref->name vs. ref->peer_ref->name.
> 
> This can cause confusing rejections or unintended data loss.
> 
> Using a command like:
> 
>  git push --force-if-includes --force-with-lease origin src:main
> 
> False rejections: when src is an up-to-date branch, but main is
> out-of-date or nonexistent, then the includes check will fail telling
> users the remote ref has been updated since the last checkout.
> 
> Data loss: when src is an orphan/out-dated branch, but main is
> up-to-date, then the if-includes check will allow the push, clobbering
> the remote main.

Hm. This case *could* be by design, to rewind and potentially
modify a remote branch, discarding new work I’ve already checked.

But the includes check is about reminding to do such a check.
So failing and requiring me to bypass the check seems ok.

> Find local reflog using ref->peer_ref. When using a refspec like
> HEAD:refs/heads/main, we resolve HEAD to a branch and use that reflog.
> In a detached HEAD state, the reflog cannot tell us if the history
> being pushed includes the tip of the remote, so the push is rejected.

This seems to be what I reported in the mail your cover
letter cites. So, am I reading correctly that this is no change
from current behavior?

…ah, patch 2 addresses that specifically. Which, I now
remember you said in the cover as well. Oops.

It *could* be worth clarifying in the proposed log message that we are only preserving behavior here, but that’s a very small nit.

> Skip deletions:
> 
>  git push --force-if-includes --force-with-lease origin :main
> 
> ref->deletion is set after apply_push_cas (which triggers
> check_if_includes_upstream). The ref->peer_ref name is "(delete)".
> Instead check with is_null_oid to detect and allow deletion.
> 
> Reported-by: Stefan Haller <lists@haller-berlin.de>
> Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
> Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
> ---
> remote.c            | 24 ++++++++++++++++-
> t/t5533-push-cas.sh | 65 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 88 insertions(+), 1 deletion(-)
> 
> diff --git a/remote.c b/remote.c
> index 00723b385e..326af76eeb 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2806,7 +2806,29 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
> */
> static void check_if_includes_upstream(struct ref *remote)
> {
> -    struct ref *local = get_local_ref(remote->name);
> +    struct ref *local;
> +    const char *name;
> +    int flag;
> +
> +    if (!remote->peer_ref)
> +        return;
> +
> +    /* A deletion has no local history to check against. */
> +    if (is_null_oid(&remote->peer_ref->new_oid))
> +        return;
> +
> +    name = remote->peer_ref->name;
> +    if (!strcmp(name, "HEAD")) {
> +        name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> +                           "HEAD", 0, NULL, &flag);
> +        if (!name || !(flag & REF_ISSYMREF)) {
> +            /* detached HEAD: no per-branch reflog to consult */
> +            remote->unreachable = 1;
> +            return;
> +        }
> +    }
> +
> +    local = get_local_ref(name);
>  if (!local)
>      return;
> 
> diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> index cba26a872d..0c02151747 100755
> --- a/t/t5533-push-cas.sh
> +++ b/t/t5533-push-cas.sh
> @@ -396,4 +396,69 @@ test_expect_success '"--force-if-includes" should allow deletes' '
>  )
> '
> 
> +test_expect_success '"--force-if-includes" should allow forced update when using differently named branches' '
> +    setup_src_dup_dst &&
> +    test_when_finished "rm -fr dst src dup" &&
> +    (
> +        cd src &&
> +        git fetch &&
> +        git switch -c newbranch origin/main &&
> +        git rebase HEAD --onto HEAD^ &&
> +        git push --force-if-includes --force-with-lease origin newbranch:main
> +    )
> +'
> +test_expect_success '"--force-if-includes" should allow forced update from HEAD' '
> +    setup_src_dup_dst &&
> +    test_when_finished "rm -fr dst src dup" &&
> +    (
> +        cd src &&
> +        git fetch &&
> +        git switch -c newbranch origin/main &&
> +        git rebase HEAD --onto HEAD^ &&
> +        git push --force-if-includes --force-with-lease origin HEAD:main
> +    )
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from differently named branches when local lacks remote ref' '
> +    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 --orphan orphan &&
> +        test_commit I &&
> +        test_must_fail git push --force-with-lease --force-if-includes origin orphan:main
> +    )
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from HEAD when it lacks remote ref' '
> +    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 --orphan orphan &&
> +        test_commit I &&
> +        test_must_fail git push --force-with-lease --force-if-includes origin HEAD:main
> +    )
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from detached HEAD' '
> +    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^ &&
> +        test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
> +    )
> +'
> +
> test_done
> --
> 2.47.3