Re: [PATCH v3 1/2] push: check pushed ref for --force-if-includes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 11, 2026, 15:31 UTC
- Message-ID
- <xmqq4ifverdh.fsf@gitster.g>
- In-Reply-To
- <20260910230506.1631656-2-tyler@tylercipriani.com>
Tyler Cipriani <tyler@tylercipriani.com> writes:
Show 9 quoted lines
> 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;This function signals its displeasure by setting remote->unreachble to true, so any early return means it is OK to force the push, right?
What is the significance of remote not having peer_ref? Is it a usage error (i.e., push is not updating anything over there, and it makes me wonder what the command line to do so looks like)? Is it a programming error (i.e., if we are pushing to update no remote ref, this function should never be called)? If the latter, I wonder if BUG() is more appropriate.
> + /* A deletion has no local history to check against. */ > + if (is_null_oid(&remote->peer_ref->new_oid)) > + return;
The comment for this condition is clear. If we are pushing to delete, checking if our side once used to build on top of theirs does not guarantee us anything, so we accept the loss of history.
Show 14 quoted lines
> + 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;The same question here.
Are any of these silent "punt" returns tested below? It does not seem to add a new test about pushing-to-delete.
Thanks.
Show 74 quoted lines
> 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