From 330d846d79399189390ef27757d060f573dd8c59 Mon Sep 17 00:00:00 2001 From: Jerry Zhang Date: Fri, 25 Sep 2026 17:47:50 -0700 Subject: [PATCH] upload: Keep a review stacked on its relative branch's tip A relative branch is someone else's work that a review is stacked on, so the review has to sit exactly on the commit it will be uploaded to, the same rule a relative series already follows. Rebase detection instead asked whether the remote base is an ancestor of the local one, which is right for a base branch (don't re-push every time master moves) but always true for a relative branch that has moved forward. Upload retargeted the pr on the forge and then skipped the push as a rebase, leaving the review based on an old tip and showing the relative branch's commits as its own. A relative branch review is now either nochange, when it is already on the tip, or pushed. This means such a review re-pushes whenever the relative branch moves, even without --rebase. --- revup/topic_stack.py | 17 +++++++++----- tests/test_upload.py | 53 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/revup/topic_stack.py b/revup/topic_stack.py index 740dc06..7c09fe0 100644 --- a/revup/topic_stack.py +++ b/revup/topic_stack.py @@ -921,11 +921,18 @@ async def mark_rebases(self, skip_rebase: bool) -> None: if topic.relative_topic is None: if not review.base_ref: raise RuntimeError("Review doesn't have a base ref!") - # For non-relative reviews, the base is correct if the remote base commit is a - # first-parent ancestor of the local remote base. - is_on_correct_base = await self.git_ctx.is_ancestor( - review.remote_commits[0].parents[0], review.base_ref - ) + remote_parent = review.remote_commits[0].parents[0] + if review.relative_branch: + # A relative branch is someone else's work that this review is stacked + # on, so the review has to sit exactly on it, same as a relative series. + # Anything else makes the forge show their commits as part of this review. + is_on_correct_base = remote_parent == review.base_ref + else: + # For non-relative reviews, the base is correct if the remote base commit + # is a first-parent ancestor of the local remote base. + is_on_correct_base = await self.git_ctx.is_ancestor( + remote_parent, review.base_ref + ) else: # For a relative series of reviews, revup will only ever upload them directly # on top of each other. If this relationship is ever broken, we always reupload diff --git a/tests/test_upload.py b/tests/test_upload.py index a0e54f0..7ac320e 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -1439,6 +1439,59 @@ async def test_base_derived_by_walking_back_from_head(self): assert g_review.is_pure_rebase assert g_review.remote_commits[0].parents[0] == b_review.remote_commits[-1].commit_id + @async_test + async def test_relative_branch_moving_forward_is_pushed(self): + """A review stacked on a relative branch follows that branch's tip.""" + async with GitTestEnvironment() as env: + await setup_repo(env) + root = await env.get_commit_hash() + await env.git_ctx.git("branch", "origin/staging", root) + await env.commit("feat\n\nTopic: alpha\nRelative-Branch: staging", {"a.txt": "a"}) + + first = await run_upload_pipeline(env) + first_review = first.topics["alpha"].reviews["origin/main"] + remote_head = first_review.new_commits[-1] + remote_num_commits = len(first_review.new_commits) + + await env.git_ctx.git("checkout", root) + await env.commit("staging moves on", {"s.txt": "s"}) + await env.git_ctx.git("branch", "origin/staging", "HEAD", "-f") + await env.git_ctx.git("checkout", "main") + + topics = await run_upload_pipeline(env) + review = topics.topics["alpha"].reviews["origin/main"] + review.pr_info = PrInfo( + baseRef="staging", + headRef=review.remote_head, + headRefOid=remote_head, + numCommits=remote_num_commits, + body="", + title="", + state="OPEN", + ) + + await topics.mark_rebases(skip_rebase=True) + + assert review.is_pure_rebase + assert review.remote_commits[0].parents[0] == root + assert review.push_status == PushStatus.PUSHED + + @async_test + async def test_relative_branch_staying_put_is_nochange(self): + """A review already on the relative branch's tip isn't pushed again.""" + async with GitTestEnvironment() as env: + await setup_repo(env) + await env.git_ctx.git("branch", "origin/staging", "HEAD") + await env.commit("feat\n\nTopic: alpha\nRelative-Branch: staging", {"a.txt": "a"}) + + topics = await run_upload_pipeline(env) + review = topics.topics["alpha"].reviews["origin/main"] + review.pr_info = make_pr_info(review, base_branch="staging") + + await topics.mark_rebases(skip_rebase=True) + + assert review.push_status == PushStatus.NOCHANGE + class TestSkipEmptyFirstCommit: @async_test