From 3730eed65bc0b1b6e9e32ff691be58f38c7a8198 Mon Sep 17 00:00:00 2001 From: Jerry Zhang Date: Wed, 13 Mar 2024 13:22:17 -0700 Subject: [PATCH] upload: Add "branch-format" config option and commit tag Branch-format will let you control how branches are named, which has an effect on when branches collide. This can be set on cmdline, from config file, or on a topic-by-topic basis inside the commit text. user+branch is the default today and will remain the default. Branch names will never collide if using this, but you cannot retarget to other base branches, and you must use Uploader: if another user wants to push to this branch. user allows retargeting prs to other base branches, but will not allow multiple base-branches to be specified. if they are, it will warn to specify user+branch instead. branch will allow others to upload to the same pr, but uploader: cannot be used. none includes both limitations and features of user and branch. I don't really believe there's good use cases for branch and none, but i'm implementing them to be complete. --- docs/upload.md | 13 +++++++++++-- revup/revup.py | 3 +++ revup/topic_stack.py | 41 ++++++++++++++++++++++++++++++++++++----- revup/upload.py | 1 + 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/docs/upload.md b/docs/upload.md index 9850f7c..d10412d 100644 --- a/docs/upload.md +++ b/docs/upload.md @@ -74,6 +74,12 @@ multiple users to collaborate on the same PR. If uploader is specified, all relative topics must specify the same uploader. However a topic without a specified uploader can still be relative to a topic with one. +**Branch-Format:** +: Specifies how the remote branches get named, which mainly affects how names +conflict. Default is "user+branch", which never conflicts, but does not allow +retargeting a PR to a different base branch. "user" will allow retargeting, but +will not allow multiple base branches. "branch" and "none" are also supported.This tag takes precedence over the config option. + **Relative-Branch:** : Optionally specifies a relative branch that this review is targeted against. A relative branch is one that represents another user's work or PR, and is @@ -114,10 +120,13 @@ branch as the base. See above section for definition of a base branch. definition of a relative branch. **--uploader** -: Used as the username for naming remote branches. The branch syntax -is {uploader}/revup/{basebranch}/{topic}. If not set value is taken +: Used as the username for naming remote branches. If not set value is taken from the portion of "git config user.email" before the "@". +**--branch-format** +: Specify how branches are named. See the Branch-Format: tag section for +options and their meaning. + **--rebase, -r** : By default revup will not push changes if local commits are a pure rebase of the remote changes. This flag overrides that behavior and causes diff --git a/revup/revup.py b/revup/revup.py index da91dea..098e210 100755 --- a/revup/revup.py +++ b/revup/revup.py @@ -272,6 +272,9 @@ async def main() -> int: "--user-aliases", ) upload_parser.add_argument("--uploader") + upload_parser.add_argument( + "--branch_format", choices=["user+branch", "user", "branch", "none"], default="user+branch" + ) upload_parser.add_argument("--pre-upload", "-p") upload_parser.add_argument("--relative-chain", "-c", action="store_true") upload_parser.add_argument("--auto-topic", "-a", action="store_true") diff --git a/revup/topic_stack.py b/revup/topic_stack.py index c0dd24e..65a56a5 100644 --- a/revup/topic_stack.py +++ b/revup/topic_stack.py @@ -28,14 +28,23 @@ # https://docs.github.com/en/get-started/using-git/dealing-with-special-characters-in-branch-and-tag-names RE_BRANCH_ALLOWED = re.compile(r"^[\w\d_\.\/-]+$") +BRANCH_FORMAT_STRATEGIES = { + "user+branch": "{uploader}/revup/{base_branch}/{topic}", + "user": "{uploader}/revup/{topic}", + "branch": "revup/{base_branch}/{topic}", + "none": "revup/{topic}", +} + -def format_remote_branch(uploader: str, base_branch: str, topic: str) -> str: +def format_remote_branch(uploader: str, base_branch: str, topic: str, branch_format: str) -> str: """ Branches are named so that it is clear that they are made by revup and can be force pushed at any time, and to minimize collision with manually created branches. """ - return f"{uploader}/revup/{base_branch}/{topic}" + return BRANCH_FORMAT_STRATEGIES[branch_format].format( + uploader=uploader, base_branch=base_branch, topic=topic + ) RE_TAGS = re.compile(r"^(?P[a-zA-Z\-]+):(?P.*)$", re.MULTILINE) @@ -49,6 +58,7 @@ def format_remote_branch(uploader: str, base_branch: str, topic: str) -> str: TAG_RELATIVE_BRANCH = "relative-branch" TAG_UPLOADER = "uploader" TAG_UPDATE_PR_BODY = "update-pr-body" +TAG_BRANCH_FORMAT = "branch-format" VALID_TAGS = { TAG_BRANCH, TAG_LABEL, @@ -59,6 +69,7 @@ def format_remote_branch(uploader: str, base_branch: str, topic: str) -> str: TAG_TOPIC, TAG_UPLOADER, TAG_UPDATE_PR_BODY, + TAG_BRANCH_FORMAT, } RE_COMMIT_LABEL = re.compile(r"^(?P[a-zA-Z\-_0-9]+):.*|^\[(?P[a-zA-Z\-_0-9]+)\].*") @@ -504,6 +515,15 @@ async def populate_reviews( f"Invalid tags for update-pr-body: {topic.tags[TAG_UPDATE_PR_BODY]}" ) + if TAG_BRANCH_FORMAT in topic.tags: + if ( + len(topic.tags[TAG_BRANCH_FORMAT]) > 1 + or min(topic.tags[TAG_BRANCH_FORMAT]).lower() not in BRANCH_FORMAT_STRATEGIES + ): + raise RevupUsageException( + f"Invalid tags for branch-format: {topic.tags[TAG_BRANCH_FORMAT]}" + ) + relative_topic = "" if force_relative_chain and last_topic is not None: relative_topic = last_topic @@ -573,7 +593,7 @@ async def populate_reviews( if name not in self.topics: logging.warning(f"Couldn't find any topic named {name}") - async def populate_relative_reviews(self, uploader: str) -> None: + async def populate_relative_reviews(self, uploader: str, branch_format: str) -> None: for name, topic in self.topological_topics(): if topic.relative_topic: if len(topic.tags[TAG_BRANCH]) == 0: @@ -638,7 +658,16 @@ async def populate_relative_reviews(self, uploader: str) -> None: f" {topic.relative_topic.tags[TAG_UPLOADER] or '{}'}" ) topic_uploader = min(topic.tags[TAG_UPLOADER]) if topic.tags[TAG_UPLOADER] else uploader - + topic_branch_format = ( + min(topic.tags[TAG_BRANCH_FORMAT]).lower() + if TAG_BRANCH_FORMAT in topic.tags + else branch_format + ) + if "branch" not in topic_branch_format and len(topic.tags[TAG_BRANCH]) > 1: + raise RevupUsageException( + "Cannot use multiple branches if Branch-Format: does not " + 'include "branch", try overriding it for this topic' + ) for branch in topic.tags[TAG_BRANCH]: review = Review(topic) # Track whether we need to query for the relative pr @@ -663,7 +692,9 @@ async def populate_relative_reviews(self, uploader: str) -> None: review.base_ref = await self.git_ctx.to_commit_hash(relative_branch) review.remote_base = self.git_ctx.remove_branch_prefix(relative_branch) - review.remote_head = format_remote_branch(topic_uploader, base_branch, name) + review.remote_head = format_remote_branch( + topic_uploader, base_branch, name, topic_branch_format + ) topic.reviews[branch] = review diff --git a/revup/upload.py b/revup/upload.py index 093c944..7aa9bd5 100644 --- a/revup/upload.py +++ b/revup/upload.py @@ -43,6 +43,7 @@ async def main( ) await topics.populate_relative_reviews( args.uploader if args.uploader else git_ctx.author, + branch_format=args.branch_format, ) if not args.dry_run: