From 98287168c007f875e35125feaae9473229d6442d Mon Sep 17 00:00:00 2001 From: Jerry Zhang Date: Tue, 19 May 2026 16:43:18 -0700 Subject: [PATCH] forge: Genericize github handling into a forge abstraction Clean up and isolate the github specific code behind a forge abstraction. This makes it possible to add tests with dummy forge backends and also makes it easier to add more forges in the future. Update comments, prints, and docs where necessary. --github-oauth and --github-url remain as backward compatibility for forge-oauth and forget-url. github-username has been removed completely as it hasn't been used in a long time. --- README.md | 2 +- docs/revup.md | 29 +- revup/__init__.pyc | Bin 186 -> 0 bytes revup/__main__.py | 6 +- revup/amend.py | 2 - revup/cherry_pick.py | 17 +- revup/config.py | 19 +- revup/forge.py | 131 +++ revup/forge_utils.py | 132 +++ revup/git.py | 38 +- revup/github.py | 15 - revup/github/__init__.py | 0 revup/{github_real.py => github/endpoint.py} | 12 +- revup/github/github.py | 732 +++++++++++++++ revup/github_utils.py | 911 ------------------- revup/restack.py | 2 - revup/revup.py | 27 +- revup/topic_stack.py | 127 ++- revup/types.py | 2 +- revup/upload.py | 18 +- tests/test_tools.py | 5 +- tests/test_upload.py | 2 +- 22 files changed, 1109 insertions(+), 1120 deletions(-) delete mode 100644 revup/__init__.pyc create mode 100644 revup/forge.py create mode 100644 revup/forge_utils.py delete mode 100644 revup/github.py create mode 100644 revup/github/__init__.py rename revup/{github_real.py => github/endpoint.py} (91%) create mode 100644 revup/github/github.py delete mode 100644 revup/github_utils.py diff --git a/README.md b/README.md index 0be70e9..d3f787c 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ git clone https://github.com//revup.git && cd revup On first run, you will need to configure github credentials. Create a [GitHub Personal Access Token](https://github.com/settings/tokens/new?scopes=repo) with "repo" scope. Revup needs this in order to create and modify pull requests. Then run ```sh -revup config github_oauth +revup config forge_oauth ``` and copy and paste the oauth into the prompt. diff --git a/docs/revup.md b/docs/revup.md index 8f39658..b9ee099 100644 --- a/docs/revup.md +++ b/docs/revup.md @@ -10,7 +10,7 @@ revup - Efficient git workflow and code review toolkit Revup is a python command-line toolkit for speeding up your git workflow. It provides commit-based development support and -full github integration. +full forge integration (GitHub, with more backends planned). All revup options can be given with a shorter unambiguous prefix. @@ -25,7 +25,7 @@ line, the value of the last one will be used. : Prints out extra details for debugging. This will include the full command-line of any subprocesses that are run along with their full output, as well as the full input and output of any -graphql requests to github. +graphql requests to the forge. **--help, -h** : Show this help page. @@ -34,32 +34,29 @@ graphql requests to github. : Print the version of revup and exit. **--proxy** -: Proxy to use when making connections to GitHub +: Proxy to use when making connections to the forge. -**--github-oauth** -: The oauth token that provides login credentials to github. Revup +**--forge-oauth** +: The oauth token that provides login credentials to the forge. Revup requires full repository read/write permissions in order to create -and modify reviews. This is represented by the "repo" section of +and modify reviews. For GitHub, this is represented by the "repo" section of https://github.com/settings/tokens/new. -**--github-username** -: The user's github username for login. - -**--github-url** -: URL to use for github. Defaults to "github.com" and would only -need changed if the user is using github enterprise. +**--forge-url** +: URL to use for the forge. Defaults to "github.com" and would only +need changed if the user is using GitHub Enterprise or another host. **--remote-name** -: The name of the remote that corresponds to github. Branches on this +: The name of the remote that corresponds to the forge. Branches on this remote are also used for base branch detection. Defaults to "origin". **--fork-name** -: If specified, the name of the remote that corresponds to a github fork +: If specified, the name of the remote that corresponds to a fork that should be used to push branches to. The pull request will be created using the branch from this fork. If empty, remote-name is used for both pushing and creating the pull request. -Github does not allow base branches of pull requests to be in a different +GitHub does not allow base branches of pull requests to be in a different fork, so reviews with a Relative: label will be deferred until its base merges. Relative-Branch cannot be used across forks. @@ -114,7 +111,7 @@ cache. Can also change the commit text. given branch relative to its base branch, then cherry-pick it. **revup upload** -: Upload and push the current stack of code reviews to github. +: Upload and push the current stack of code reviews. **revup restack** : Reorder the current stack so that commits in a topic are consecutive. diff --git a/revup/__init__.pyc b/revup/__init__.pyc deleted file mode 100644 index 39123e5c016e3615e43c0e642dddcee87b51ff54..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 186 zcmZSn%*$o@l0PGv0ScIav;z1J z1~W(fm?CNb diff --git a/revup/__main__.py b/revup/__main__.py index 37debff..df40112 100644 --- a/revup/__main__.py +++ b/revup/__main__.py @@ -6,7 +6,7 @@ from revup.revup import build_parser, main from revup.types import ( RevupConflictException, - RevupGithubException, + RevupForgeException, RevupRequestException, RevupShellException, RevupUsageException, @@ -35,8 +35,8 @@ def _main() -> None: except RevupShellException as e: logging.error(str(e)) sys.exit(4) - except RevupGithubException as e: - logging.error(f"Github Exception: {e.type}: {e.message}") + except RevupForgeException as e: + logging.error(f"Forge Exception: {e.type}: {e.message}") sys.exit(5) except RevupRequestException as e: logging.error(f"Request failed with response status {e.status}") diff --git a/revup/amend.py b/revup/amend.py index 0aedadf..2c739b5 100644 --- a/revup/amend.py +++ b/revup/amend.py @@ -160,8 +160,6 @@ async def get_has_unstaged() -> bool: git_ctx, args.base_branch, args.relative_branch, - None, - None, ) if args.ref_or_topic: commit = await parse_ref_or_topic(args.ref_or_topic, args, git_ctx, topics) diff --git a/revup/cherry_pick.py b/revup/cherry_pick.py index 20bec70..bf201a2 100644 --- a/revup/cherry_pick.py +++ b/revup/cherry_pick.py @@ -3,12 +3,7 @@ from typing import Tuple from revup import config, git -from revup.github_utils import ( - RE_PR_URL, - github_connection, - parse_pull_request_url, - query_pr_by_number, -) +from revup.forge_utils import RE_PR_URL, forge_connection, parse_pull_request_url from revup.types import GitCommitHash, GitTreeHash, RevupUsageException @@ -19,13 +14,9 @@ async def resolve_pr_url( Resolve a PR URL to (head_ref, base_ref) by querying the GitHub API. """ pr_params = parse_pull_request_url(pr_url) - async with github_connection(args=args, git_ctx=git_ctx, conf=conf) as ( - github_ep, - _repo_info, - _fork_info, - ): - head_ref, base_ref = await query_pr_by_number( - github_ep, pr_params.owner, pr_params.name, pr_params.number + async with forge_connection(args=args, git_ctx=git_ctx, conf=conf) as forge: + head_ref, base_ref = await forge.query_pr_by_number( + pr_params.owner, pr_params.name, pr_params.number ) logging.info(f"Resolved PR #{pr_params.number} to branch '{head_ref}' based on '{base_ref}'") return head_ref, base_ref diff --git a/revup/config.py b/revup/config.py index deb618c..87713b6 100644 --- a/revup/config.py +++ b/revup/config.py @@ -71,9 +71,10 @@ def get_actions(self) -> Dict[str, argparse.Action]: # Ignore nonconfigurable actions (help, auto-generated negation) continue - if len(action.option_strings) > 0 and action.option_strings[0].startswith("--"): - option = action.option_strings[0][2:].replace("-", "_") - ret[option] = action + for opt in action.option_strings: + if opt.startswith("--"): + option = opt[2:].replace("-", "_") + ret.setdefault(option, action) return ret def set_option_default(self, option: str, action: argparse.Action, value: str) -> None: @@ -209,7 +210,7 @@ def config_main(conf: Config, args: argparse.Namespace, all_parsers: List[RevupA ) elif args.value: value = args.value - if command == "revup" and key == "github_oauth": + if command == "revup" and key in ("forge_oauth", "github_oauth"): logging.warning( "Prefer to omit the value on command line when entering sensitive info. " "You may want to clear your shell history." @@ -226,15 +227,7 @@ def config_main(conf: Config, args: argparse.Namespace, all_parsers: List[RevupA # (this may throw if the value is not allowed) parser.set_option_default(key, actions[key], value) - if command == "revup" and key == "github_username": - # From https://www.npmjs.com/package/github-username-regex : - # Github username may only contain alphanumeric characters or hyphens. - # Github username cannot have multiple consecutive hyphens. - # Github username cannot begin or end with a hyphen. - # Maximum is 39 characters. - if not re.match(r"^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$", value, re.I): - raise ValueError(f"{value} is not a valid GitHub username") - elif command == "revup" and key == "github_oauth": + if command == "revup" and key in ("forge_oauth", "github_oauth"): if not re.match(r"^[a-z\d_]+$", value, re.I): raise ValueError("Input string is not a valid oauth") diff --git a/revup/forge.py b/revup/forge.py new file mode 100644 index 0000000..205ba8d --- /dev/null +++ b/revup/forge.py @@ -0,0 +1,131 @@ +from abc import ABCMeta, abstractmethod +from dataclasses import dataclass, field +from typing import Dict, List, Optional, Set, Tuple + + +@dataclass +class ForgeRepoInfo: + name: str = "" + owner: str = "" + + +@dataclass +class PullRequestParams: + forge_url: str + owner: str + name: str + number: int + + +@dataclass +class PrComment: + text: str = "" + id: Optional[str] = None + + +@dataclass +class PrInfo: + baseRef: str + headRef: str + baseRefOid: Optional[str] + headRefOid: Optional[str] + body: str + title: str + id: str = "" + url: str = "" + state: str = "" + reviewers: Set[str] = field(default_factory=set) + reviewer_ids: Set[str] = field(default_factory=set) + reviewer_teams: Set[str] = field(default_factory=set) + reviewer_team_ids: Set[str] = field(default_factory=set) + assignees: Set[str] = field(default_factory=set) + assignee_ids: Set[str] = field(default_factory=set) + labels: Set[str] = field(default_factory=set) + label_ids: Set[str] = field(default_factory=set) + removed_reviewers: Set[str] = field(default_factory=set) + removed_reviewer_ids: Set[str] = field(default_factory=set) + removed_assignees: Set[str] = field(default_factory=set) + removed_assignee_ids: Set[str] = field(default_factory=set) + is_draft: bool = False + comments: List[PrComment] = field(default_factory=list) + + +@dataclass +class PrUpdate: + baseRef: Optional[str] = None + body: Optional[str] = None + title: Optional[str] = None + id: str = "" + reviewer_ids: Set[str] = field(default_factory=set) + reviewer_team_ids: Set[str] = field(default_factory=set) + assignee_ids: Set[str] = field(default_factory=set) + label_ids: Set[str] = field(default_factory=set) + is_draft: Optional[bool] = None + comments: List[PrComment] = field(default_factory=list) + + +MAX_COMMENTS_TO_QUERY = 3 + + +class Forge(metaclass=ABCMeta): + @property + def name(self) -> str: + return type(self).__name__.lower() + + @property + @abstractmethod + def repo_owner(self) -> str: + """Owner of the fork remote (or repo remote if no fork).""" + + @property + @abstractmethod + def repo_name(self) -> str: + """Name of the repository.""" + + @property + @abstractmethod + def is_fork(self) -> bool: + """True if the fork remote points to a different owner than the repo remote.""" + + @abstractmethod + async def query_everything( + self, + head_refs: List[str], + user_ids: List[str], + labels: List[str], + teams: List[Tuple[str, str]], + ) -> Tuple[ + str, + List[Optional[PrInfo]], + Dict[str, str], + Dict[str, str], + Dict[str, str], + Dict[str, str], + Dict[str, Optional[Set[str]]], + ]: + """ + Query all needed info in one request. Returns: + - Repository node id + - List of pull requests (None if not found for that ref) + - Dict of user query strings to node ids + - Dict of user query strings to full login names + - Dict of label names to node ids + - Dict of team refs ("org/slug") to node ids + - Dict of team refs to member logins (None if membership unknown) + """ + + @abstractmethod + async def create_pull_requests(self, repo_id: str, prs: List[PrInfo]) -> None: + """Create pull requests. Modifies prs in-place to set id and url.""" + + @abstractmethod + async def update_pull_requests(self, prs: List[PrUpdate]) -> None: + """Update existing pull requests.""" + + @abstractmethod + async def query_pr_by_number(self, owner: str, name: str, number: int) -> Tuple[str, str]: + """Query a pull request by number and return (headRefName, baseRefName).""" + + @abstractmethod + async def close(self) -> None: + """Clean up any connections.""" diff --git a/revup/forge_utils.py b/revup/forge_utils.py new file mode 100644 index 0000000..86a999b --- /dev/null +++ b/revup/forge_utils.py @@ -0,0 +1,132 @@ +import argparse +import logging +import os +import re +from contextlib import asynccontextmanager +from typing import AsyncGenerator + +from revup import config, git, logs +from revup.forge import Forge, ForgeRepoInfo, PullRequestParams +from revup.types import RevupUsageException + +RE_PR_URL = re.compile( + r"^https://(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/pull/(?P[0-9]+)/?$" +) + + +def parse_forge_info(remote_url: str, forge_url: str) -> ForgeRepoInfo: + """ + Parse owner and repo name from a remote URL, matching against the given forge host. + """ + owner = "" + name = "" + while True: + match = rf"^[^@]+@{forge_url}:([^/]+)/(.+)(? PullRequestParams: + m = RE_PR_URL.match(pull_request) + if not m: + raise RuntimeError("Did not understand PR argument. PR must be URL") + + forge_url = m.group("forge_url") + owner = m.group("owner") + name = m.group("name") + number = int(m.group("number")) + return PullRequestParams(forge_url=forge_url, owner=owner, name=name, number=number) + + +@asynccontextmanager +async def forge_connection( + git_ctx: git.Git, args: argparse.Namespace, conf: config.Config +) -> AsyncGenerator[Forge, None]: + remote_url = await git_ctx.get_remote_url(args.remote_name) + repo_info = parse_forge_info(remote_url, args.forge_url) + + if not repo_info.owner or not repo_info.name: + raise RevupUsageException( + f'Configured remote "{args.remote_name}" does not ' + "point to a recognized forge repository! " + "You can set it manually by running " + f"`git remote set-url {args.remote_name} git@:{{OWNER}}/{{PROJECT}}` " + f"or change the configured remote in {conf.config_path}/" + ) + + fork_info = repo_info + if args.fork_name and args.fork_name != args.remote_name: + fork_url = await git_ctx.get_remote_url(args.fork_name) + fork_info = parse_forge_info(fork_url, args.forge_url) + + if not fork_info.owner or not fork_info.name: + raise RevupUsageException( + f'Configured remote fork "{args.fork_info}" does not ' + "point to a recognized forge repository! " + "You can set it manually by running " + f"`git remote set-url {args.fork_info} git@:{{OWNER}}/{{PROJECT}}` " + f"or change the configured remote in {conf.config_path}." + ) + + if repo_info.name != fork_info.name: + raise RevupUsageException( + f'Configured remote fork "{args.fork_info}" is not ' + f"the same repo as the remote {args.remote_info}." + ) + + if not args.forge_oauth: + # Try environment variables first + args.forge_oauth = os.environ.get("GITHUB_TOKEN") + if args.forge_oauth: + logs.redact({args.forge_oauth: ""}) + logging.debug("Used token from environment variable") + else: + # Fall back to git credential helper + args.forge_oauth = await git_ctx.credential( + protocol="https", + host=args.forge_url, + path=f"{fork_info.owner}/{fork_info.name}.git", + ) + if args.forge_oauth: + logs.redact({args.forge_oauth: ""}) + logging.debug("Used credential from git-credential") + + if not args.forge_oauth: + raise RevupUsageException( + "No OAuth token found! " + "Set the GITHUB_TOKEN environment variable, " + "login with 'gh auth login', " + "or make one at https://github.com/settings/tokens/new " + "(revup needs full repo permissions) " + "then set it with `revup config forge_oauth`." + ) + + if "github" in args.forge_url: + from revup.github.endpoint import GitHubEndpoint + from revup.github.github import Github + + endpoint = GitHubEndpoint( + oauth_token=args.forge_oauth, proxy=args.proxy, github_url=args.forge_url + ) + forge = Github(endpoint=endpoint, repo_info=repo_info, fork_info=fork_info) + try: + yield forge + finally: + await forge.close() + else: + raise RevupUsageException( + f'Unrecognized forge URL "{args.forge_url}". ' "Currently only GitHub is supported." + ) diff --git a/revup/git.py b/revup/git.py index 6c4d88c..3fadf2a 100644 --- a/revup/git.py +++ b/revup/git.py @@ -5,7 +5,7 @@ import re import shutil import tempfile -from typing import Any, Dict, List, NamedTuple, Optional, Pattern, Tuple +from typing import Any, Dict, List, Optional, Pattern, Tuple from async_lru import alru_cache as lru_cache @@ -53,12 +53,6 @@ COMMON_MAIN_BRANCHES = ["main", "master"] # Below logic assumes 2 values here -GitHubRepoInfo = NamedTuple( - "GitHubRepoInfo", - [("name", str), ("owner", str)], -) - - def parse_commit_header(raw_header: str) -> CommitHeader: def _search_group(raw_header: str, regex: Pattern[str], group: str) -> str: m = regex.search(raw_header) @@ -304,34 +298,14 @@ async def git_return_code(self, *args: str, **kwargs: Any) -> int: async def git_stdout(self, *args: str, **kwargs: Any) -> str: return (await self.git(*args, **kwargs))[1] - async def get_github_repo_info(self, github_url: str, remote_name: str) -> GitHubRepoInfo: + async def get_remote_url(self, remote_name: str) -> str: """ - Return github repo's name and owner. + Return the URL configured for the given remote, or empty string if not found. """ - owner = "" - name = "" ret = await self.git("config", "--get", f"remote.{remote_name}.url", raiseonerror=False) if ret[0] != 0: - return GitHubRepoInfo(owner=owner, name=name) - remote_url = ret[1] - while True: - match = rf"^[^@]+@{github_url}:([^/]+)/(.+)(? Any: - """ - Args: - query: string GraphQL query to execute - **kwargs: values for variables in the graphql query - - Returns: parsed JSON response - """ - pass diff --git a/revup/github/__init__.py b/revup/github/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/revup/github_real.py b/revup/github/endpoint.py similarity index 91% rename from revup/github_real.py rename to revup/github/endpoint.py index f6989c9..2cbb9e5 100644 --- a/revup/github_real.py +++ b/revup/github/endpoint.py @@ -7,14 +7,13 @@ from aiohttp import ClientSession, ContentTypeError -from revup import github -from revup.types import RevupGithubException, RevupRequestException +from revup.types import RevupForgeException, RevupRequestException TRANSIENT_STATUSES = frozenset({500, 502, 503, 504}) RETRYABLE_GRAPHQL_ERRORS = frozenset({"RESOURCE_LIMITS_EXCEEDED"}) -class RealGitHubEndpoint(github.GitHubEndpoint): +class GitHubEndpoint: """ A class representing a GitHub endpoint we can send queries to. It supports both GraphQL and REST interfaces. @@ -123,7 +122,7 @@ async def _graphql_once(self, query: str, **kwargs: Any) -> Any: logging.debug("Response JSON:\n{}".format(pretty_json)) if "errors" in r: - raise RevupGithubException(r["errors"]) + raise RevupForgeException(r["errors"]) return r @@ -139,13 +138,10 @@ async def graphql( msg = "GitHub returned {}".format(e.status) if not await self._should_retry(attempt, max_retries, base_delay, msg): raise - except RevupGithubException as e: + except RevupForgeException as e: retryable = set(e.types) & RETRYABLE_GRAPHQL_ERRORS if not retryable: raise - # TODO: For RESOURCE_LIMITS_EXCEEDED, use x-ratelimit-reset header - # instead of exponential backoff - either wait until reset time or - # fail immediately if the wait would be too long. msg = "GitHub GraphQL error ({})".format(", ".join(retryable)) if not await self._should_retry(attempt, max_retries, base_delay, msg): raise diff --git a/revup/github/github.py b/revup/github/github.py new file mode 100644 index 0000000..b82d222 --- /dev/null +++ b/revup/github/github.py @@ -0,0 +1,732 @@ +import logging +from typing import Any, Dict, Iterable, List, Optional, Set, Tuple + +from revup.forge import ( + MAX_COMMENTS_TO_QUERY, + Forge, + ForgeRepoInfo, + PrComment, + PrInfo, + PrUpdate, +) +from revup.github.endpoint import GitHubEndpoint +from revup.types import RevupForgeException + + +def _get_args_dict(args: List[Any], prefix: str) -> Dict[str, Any]: + return {f"{prefix}{n}": arg for n, arg in enumerate(args)} + + +def _get_args_declaration(args: Dict[str, Any], typ: str) -> List[str]: + return [f"${var}: {typ}" for var in args] + + +def _get_result_args(num: int, prefix: str) -> List[str]: + return [f"{prefix}{n}" for n in range(num)] + + +def _zip_and_flatten(l1: Iterable[str], l2: Iterable[str]) -> List[str]: + ret: List[str] = [] + iter1 = iter(l1) + iter2 = iter(l2) + while True: + try: + ret.append(next(iter1)) + ret.append(next(iter2)) + except StopIteration: + break + return ret + + +class Github(Forge): + def __init__( + self, + endpoint: GitHubEndpoint, + repo_info: ForgeRepoInfo, + fork_info: ForgeRepoInfo, + ): + self.endpoint = endpoint + self.repo_info = repo_info + self.fork_info = fork_info + + @property + def repo_owner(self) -> str: + return self.fork_info.owner + + @property + def repo_name(self) -> str: + return self.repo_info.name + + @property + def is_fork(self) -> bool: + return self.fork_info.owner != self.repo_info.owner + + async def close(self) -> None: + await self.endpoint.close() + + async def query_everything( + self, + head_refs: List[str], + user_ids: List[str], + labels: List[str], + teams: List[Tuple[str, str]], + ) -> Tuple[ + str, + List[Optional[PrInfo]], + Dict[str, str], + Dict[str, str], + Dict[str, str], + Dict[str, str], + Dict[str, Optional[Set[str]]], + ]: + head_refs_args = _get_args_dict(head_refs, "pr") + user_id_args = _get_args_dict(user_ids, "user") + label_args = _get_args_dict(labels, "label") + team_org_args = _get_args_dict([t[0] for t in teams], "team_org") + team_slug_args = _get_args_dict([t[1] for t in teams], "team_slug") + + prs_out = _get_result_args(len(head_refs), "pr_out") + user_id_out = _get_result_args(len(user_ids), "user_out") + label_out = _get_result_args(len(labels), "label_out") + team_out = _get_result_args(len(teams), "team_out") + + arg_str = ", ".join( + _get_args_declaration(head_refs_args, "String!") + + _get_args_declaration(user_id_args, "String!") + + _get_args_declaration(label_args, "String!") + + _get_args_declaration(team_org_args, "String!") + + _get_args_declaration(team_slug_args, "String!") + ) + + # NOTE: There are possible limitations here because we depend on PRs being + # returned in order of OPEN prs, followed by MERGED prs in the order that + # they merged. github doesn't offer these options and it is excessively + # expensive to always fetch multiple prs and order them on this side. For now + # we hope that the most relevant PR will have the most recent update time. + request_str = "".join( + len(head_refs) + * [ + "{}: pullRequests (headRefName: ${}, states: [OPEN, MERGED], first: 1, " + "orderBy: {{direction: DESC, field:UPDATED_AT}}) {{" + "...PrResult" + "}}," + ] + ) + request_str = request_str.format(*_zip_and_flatten(prs_out, head_refs_args.keys())) + + user_str = "".join( + len(user_ids) * ["{}: assignableUsers (query: ${}, first: 25) {{...UserResult}},"] + ) + user_str = user_str.format(*_zip_and_flatten(user_id_out, user_id_args.keys())) + + label_str = "".join(len(labels) * ["{}: label (name: ${}) {{...LabelResult}},"]) + label_str = label_str.format(*_zip_and_flatten(label_out, label_args.keys())) + + team_str = "" + for i in range(len(teams)): + team_str += ( + f"{team_out[i]}: organization(login: ${list(team_org_args.keys())[i]}) " + f"{{team(slug: ${list(team_slug_args.keys())[i]}) " + f"{{id, members(first: 100) {{nodes {{login}}, totalCount}}}}}}," + ) + + multi_query_str = f""" + query GetPrResults($owner: String!, $name: String!, {arg_str}) {{ + repository(name: $name, owner: $owner) {{ + id + {request_str}{user_str}{label_str} + }} + {team_str} + }}""" + if user_str: + multi_query_str += """ + fragment UserResult on UserConnection { + nodes { + login + id + } + totalCount + }""" + if label_str: + multi_query_str += """ + fragment LabelResult on Label { + id + name + }""" + if request_str: + multi_query_str += f""" + fragment PrResult on PullRequestConnection {{ + nodes {{ + id + state + url + baseRefName + body + title + isDraft + baseCommit: commits(first: 1) {{ + nodes {{ + commit {{ + parents (first: 1) {{ + nodes {{ + oid + }} + }} + }} + }} + }} + headCommit: commits(last: 1) {{ + nodes {{ + commit {{ + oid + }} + }} + }} + reviewRequests (first: 25) {{ + nodes {{ + requestedReviewer {{ + ... on User {{ + login + id + }} + ... on Team {{ + slug + id + organization {{ + login + }} + }} + }} + }} + }} + timelineItems( + itemTypes: [REVIEW_REQUEST_REMOVED_EVENT, UNASSIGNED_EVENT] + first: 50 + ) {{ + nodes {{ + ... on ReviewRequestRemovedEvent {{ + requestedReviewer {{ + ... on User {{ + login + id + }} + }} + }} + ... on UnassignedEvent {{ + assignee {{ + ... on User {{ + login + id + }} + }} + }} + }} + }} + latestReviews (first: 25) {{ + nodes {{ + author {{ + ... on User {{ + login + id + }} + }} + viewerDidAuthor + }} + }} + assignees (first: 25) {{ + nodes {{ + ... on User {{ + login + id + }} + }} + }} + labels (first: 25) {{ + nodes {{ + name + id + }} + }} + comments (first: {MAX_COMMENTS_TO_QUERY}) {{ + nodes {{ + body + id + }} + }} + }} + totalCount + }}""" + + pr_result = await self.endpoint.graphql( + multi_query_str, + owner=self.repo_info.owner, + name=self.repo_info.name, + **head_refs_args, + **user_id_args, + **label_args, + **team_org_args, + **team_slug_args, + ) + + prs: List[Optional[PrInfo]] = [] + for i, branch_name in enumerate(head_refs): + this_node = pr_result["data"]["repository"][prs_out[i]] + if len(this_node["nodes"]) == 1: + this_node = this_node["nodes"][0] + pr_labels: Set[str] = set() + pr_label_ids: Set[str] = set() + reviewers: Set[str] = set() + reviewer_ids: Set[str] = set() + reviewer_teams: Set[str] = set() + reviewer_team_ids: Set[str] = set() + assignees: Set[str] = set() + assignee_ids: Set[str] = set() + for label in this_node["labels"]["nodes"]: + pr_labels.add(label["name"]) + pr_label_ids.add(label["id"]) + for revs in this_node["reviewRequests"]["nodes"]: + requested = revs["requestedReviewer"] + if not requested: + continue + elif "slug" in requested: + reviewer_teams.add( + f"{requested['organization']['login']}/{requested['slug']}" + ) + reviewer_team_ids.add(requested["id"]) + elif "login" in requested: + reviewers.add(requested["login"]) + reviewer_ids.add(requested["id"]) + for revs in this_node["latestReviews"]["nodes"]: + # Ignore self reviews and bot reviews (without a login) + if not revs["viewerDidAuthor"] and "login" in revs["author"]: + reviewers.add(revs["author"]["login"]) + reviewer_ids.add(revs["author"]["id"]) + for user in this_node["assignees"]["nodes"]: + assignees.add(user["login"]) + assignee_ids.add(user["id"]) + + # The plain headRef and baseRef fields return the latest commit id associated with + # that branch name which may be newer than the PR itself if it was merged. We want + # the ids of the commits actually last associated with the PR, which we query from + # the commit list. This can also mean they are None if the PR has 0 commits. + headRefOid = ( + this_node["headCommit"]["nodes"][0]["commit"]["oid"] + if this_node["headCommit"]["nodes"] + else None + ) + baseRefOid = ( + this_node["baseCommit"]["nodes"][0]["commit"]["parents"]["nodes"][0]["oid"] + if this_node["baseCommit"]["nodes"] + else None + ) + + comments = [] + for c in this_node["comments"]["nodes"]: + comments.append(PrComment(c["body"], c["id"])) + + removed_reviewers: Set[str] = set() + removed_reviewer_ids: Set[str] = set() + removed_assignees: Set[str] = set() + removed_assignee_ids: Set[str] = set() + for event in this_node["timelineItems"]["nodes"]: + rr = event.get("requestedReviewer") + if rr and "login" in rr and rr["login"] not in reviewers: + removed_reviewers.add(rr["login"]) + removed_reviewer_ids.add(rr["id"]) + assignee = event.get("assignee") + if assignee and "login" in assignee and assignee["login"] not in assignees: + removed_assignees.add(assignee["login"]) + removed_assignee_ids.add(assignee["id"]) + + prs.append( + PrInfo( + id=this_node["id"], + url=this_node["url"], + baseRef=this_node["baseRefName"], + headRef=branch_name, + baseRefOid=baseRefOid, + headRefOid=headRefOid, + body=this_node["body"], + title=this_node["title"], + reviewers=reviewers, + reviewer_ids=reviewer_ids, + reviewer_teams=reviewer_teams, + reviewer_team_ids=reviewer_team_ids, + assignees=assignees, + assignee_ids=assignee_ids, + labels=pr_labels, + label_ids=pr_label_ids, + removed_reviewers=removed_reviewers, + removed_reviewer_ids=removed_reviewer_ids, + removed_assignees=removed_assignees, + removed_assignee_ids=removed_assignee_ids, + is_draft=this_node["isDraft"], + state=this_node["state"], + comments=comments, + ) + ) + else: + prs.append(None) + + names_to_ids: Dict[str, str] = {} + names_to_logins: Dict[str, str] = {} + for i, user_id in enumerate(user_ids): + this_node = pr_result["data"]["repository"][user_id_out[i]] + if len(this_node["nodes"]) == 0: + logging.warning("No matching user found for {}".format(user_id)) + else: + if this_node["totalCount"] > len(this_node["nodes"]): + logging.warning( + "Too many matching users found for {}, try being more specific".format( + user_id + ) + ) + shortest_name = this_node["nodes"][0]["login"] + names_to_ids[user_id] = this_node["nodes"][0]["id"] + found_match = False + for user in this_node["nodes"]: + if len(user["login"]) <= len(shortest_name) and user["login"].startswith( + user_id + ): + shortest_name = user["login"] + names_to_ids[user_id] = user["id"] + names_to_logins[user_id] = user["login"] + found_match = True + if not found_match: + logging.warning( + "Couldn't find a prefixed match for {}, going with {} instead".format( + user_id, shortest_name + ) + ) + + labels_to_ids: Dict[str, str] = {} + for i, label in enumerate(labels): + this_node = pr_result["data"]["repository"][label_out[i]] + if this_node is not None: + labels_to_ids[label] = this_node["id"] + else: + logging.warning("Couldn't find an existing label named {}".format(label)) + + teams_to_ids: Dict[str, str] = {} + teams_to_members: Dict[str, Optional[Set[str]]] = {} + for i, (org, slug) in enumerate(teams): + team_node = pr_result["data"][team_out[i]] + if team_node is not None and team_node["team"] is not None: + team_ref = f"{org}/{slug}" + teams_to_ids[team_ref] = team_node["team"]["id"] + members_node = team_node["team"]["members"] + member_logins = {m["login"] for m in members_node["nodes"]} + if members_node["totalCount"] > len(members_node["nodes"]): + # Team has more members than we fetched; we can't check membership reliably. + teams_to_members[team_ref] = None + else: + teams_to_members[team_ref] = member_logins + else: + logging.warning("Couldn't find a team matching {}/{}".format(org, slug)) + + return ( + pr_result["data"]["repository"]["id"], + prs, + names_to_ids, + names_to_logins, + labels_to_ids, + teams_to_ids, + teams_to_members, + ) + + async def create_pull_requests(self, repo_id: str, prs: List[PrInfo]) -> None: + inputs = [] + for pr in prs: + headRef = ( + pr.headRef + if self.fork_info.owner == self.repo_info.owner + else f"{self.fork_info.owner}:{pr.headRef}" + ) + inputs.append({ + "baseRefName": pr.baseRef, + "body": pr.body, + "clientMutationId": "revup", + "headRefName": headRef, + "repositoryId": repo_id, + "title": pr.title, + "draft": pr.is_draft, + }) + inputs_args = _get_args_dict(inputs, "pr") + prs_out = _get_result_args(len(inputs), "pr_out") + + arg_str = ", ".join(_get_args_declaration(inputs_args, "CreatePullRequestInput!")) + + request_str = "".join( + len(inputs) + * [ + """ + {}: createPullRequest(input: ${}) {{ + pullRequest {{ + id + url + }} + }},""" + ] + ) + request_str = request_str.format(*_zip_and_flatten(prs_out, inputs_args.keys())) + + mutation_str = f""" + mutation ({arg_str}) {{ + {request_str} + }}""" + + # Creating a pull request can fail if the branch is already merged. + pr_results = await self.endpoint.graphql(mutation_str, require_success=False, **inputs_args) + for i, pr in enumerate(prs): + result = pr_results["data"][prs_out[i]]["pullRequest"] + if result is not None: + pr.id = result["id"] + pr.url = result["url"] + + async def update_pull_requests(self, prs: List[PrUpdate]) -> None: + inputs = [] + labels = [] + reviewers = [] + assignees = [] + convert_to_draft = [] + convert_from_draft = [] + comments = [] + edit_comments = [] + for pr in prs: + update_dict: Dict[str, Any] = { + "clientMutationId": "revup", + "pullRequestId": pr.id, + } + if pr.baseRef is not None: + update_dict["baseRefName"] = pr.baseRef + if pr.body is not None: + update_dict["body"] = pr.body + if pr.title is not None: + update_dict["title"] = pr.title + inputs.append(update_dict) + + if pr.label_ids: + labels.append({ + "labelIds": list(pr.label_ids), + "clientMutationId": "revup", + "labelableId": pr.id, + }) + + if pr.reviewer_ids or pr.reviewer_team_ids: + reviewers.append({ + "userIds": list(pr.reviewer_ids), + "teamIds": list(pr.reviewer_team_ids), + "clientMutationId": "revup", + "pullRequestId": pr.id, + "union": True, + }) + if pr.assignee_ids: + assignees.append({ + "assigneeIds": list(pr.assignee_ids), + "clientMutationId": "revup", + "assignableId": pr.id, + }) + + if pr.is_draft is not None: + if pr.is_draft: + convert_to_draft.append({ + "clientMutationId": "revup", + "pullRequestId": pr.id, + }) + else: + convert_from_draft.append({ + "clientMutationId": "revup", + "pullRequestId": pr.id, + }) + + for c in pr.comments: + if c.id: + edit_comments.append({ + "body": c.text, + "clientMutationId": "revup", + "id": c.id, + }) + else: + comments.append({ + "body": c.text, + "clientMutationId": "revup", + "subjectId": pr.id, + }) + + inputs_args = _get_args_dict(inputs, "pr") + prs_out = _get_result_args(len(inputs), "pr_out") + + labels_args = _get_args_dict(labels, "label") + labels_out = _get_result_args(len(labels), "label_out") + + reviewers_args = _get_args_dict(reviewers, "rev") + reviewers_out = _get_result_args(len(reviewers), "rev_out") + + assignees_args = _get_args_dict(assignees, "asn") + assignees_out = _get_result_args(len(assignees), "asn_out") + + to_draft_args = _get_args_dict(convert_to_draft, "to_d") + to_draft_out = _get_result_args(len(convert_to_draft), "to_d_out") + + from_draft_args = _get_args_dict(convert_from_draft, "from_d") + from_draft_out = _get_result_args(len(convert_from_draft), "from_d_out") + + comments_args = _get_args_dict(comments, "com") + comments_out = _get_result_args(len(comments), "com_out") + + edit_comments_args = _get_args_dict(edit_comments, "edit_com") + edit_comments_out = _get_result_args(len(edit_comments), "edit_com_out") + + arg_str = ", ".join( + _get_args_declaration(inputs_args, "UpdatePullRequestInput!") + + _get_args_declaration(labels_args, "AddLabelsToLabelableInput!") + + _get_args_declaration(reviewers_args, "RequestReviewsInput!") + + _get_args_declaration(assignees_args, "AddAssigneesToAssignableInput!") + + _get_args_declaration(to_draft_args, "ConvertPullRequestToDraftInput!") + + _get_args_declaration(from_draft_args, "MarkPullRequestReadyForReviewInput!") + + _get_args_declaration(comments_args, "AddCommentInput!") + + _get_args_declaration(edit_comments_args, "UpdateIssueCommentInput!") + ) + + update_str = "".join( + len(inputs) + * [ + """ + {}: updatePullRequest(input: ${}) {{ + clientMutationId + }},""" + ] + ) + update_str = update_str.format(*_zip_and_flatten(prs_out, inputs_args.keys())) + + request_reviewers_str = "".join( + len(reviewers_args) + * [ + """ + {}: requestReviews(input: ${}) {{ + clientMutationId + }},""" + ] + ) + request_reviewers_str = request_reviewers_str.format( + *_zip_and_flatten(reviewers_out, reviewers_args.keys()) + ) + assignees_str = "".join( + len(assignees_args) + * [ + """ + {}: addAssigneesToAssignable(input: ${}) {{ + clientMutationId + }},""" + ] + ) + assignees_str = assignees_str.format( + *_zip_and_flatten(assignees_out, assignees_args.keys()) + ) + + add_labels_str = "".join( + len(labels_args) + * [ + """ + {}: addLabelsToLabelable(input: ${}) {{ + clientMutationId + }},""" + ] + ) + add_labels_str = add_labels_str.format(*_zip_and_flatten(labels_out, labels_args.keys())) + + to_draft_str = "".join( + len(convert_to_draft) + * [ + """ + {}: convertPullRequestToDraft(input: ${}) {{ + clientMutationId + }},""" + ] + ) + to_draft_str = to_draft_str.format(*_zip_and_flatten(to_draft_out, to_draft_args.keys())) + + from_draft_str = "".join( + len(convert_from_draft) + * [ + """ + {}: markPullRequestReadyForReview(input: ${}) {{ + clientMutationId + }},""" + ] + ) + from_draft_str = from_draft_str.format( + *_zip_and_flatten(from_draft_out, from_draft_args.keys()) + ) + + add_comments_str = "".join( + len(comments_args) + * [ + """ + {}: addComment(input: ${}) {{ + clientMutationId + }},""" + ] + ) + add_comments_str = add_comments_str.format( + *_zip_and_flatten(comments_out, comments_args.keys()) + ) + + edit_comments_str = "".join( + len(edit_comments_args) + * [ + """ + {}: updateIssueComment(input: ${}) {{ + clientMutationId + }},""" + ] + ) + edit_comments_str = edit_comments_str.format( + *_zip_and_flatten(edit_comments_out, edit_comments_args.keys()) + ) + + # Add comment mutations first to ensure comments are at the top of the PR + mutation_str = f""" + mutation ({arg_str}) {{ + {add_comments_str}{update_str}{request_reviewers_str}{assignees_str}{add_labels_str}\ +{to_draft_str}{from_draft_str}{edit_comments_str} + }}""" + + try: + await self.endpoint.graphql( + mutation_str, + **comments_args, + **inputs_args, + **reviewers_args, + **assignees_args, + **labels_args, + **to_draft_args, + **from_draft_args, + **edit_comments_args, + ) + except RevupForgeException as e: + if "timeout" in e.message: + logging.warning( + "Github update request timed out! Most likely this is a false alarm and changes" + " actually succeeded. You may want to rerun this command to verify." + ) + else: + raise e + + async def query_pr_by_number(self, owner: str, name: str, number: int) -> Tuple[str, str]: + result = await self.endpoint.graphql( + query="""\ +query ($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + headRefName + baseRefName + } + } +}""", + owner=owner, + name=name, + number=number, + ) + pr = result["data"]["repository"]["pullRequest"] + return pr["headRefName"], pr["baseRefName"] diff --git a/revup/github_utils.py b/revup/github_utils.py deleted file mode 100644 index 33b976c..0000000 --- a/revup/github_utils.py +++ /dev/null @@ -1,911 +0,0 @@ -import argparse -import logging -import os -import re -from contextlib import asynccontextmanager -from dataclasses import dataclass, field -from typing import ( - Any, - AsyncGenerator, - Dict, - Iterable, - List, - NamedTuple, - Optional, - Set, - Tuple, -) - -from revup import config, git, github, logs -from revup.git import GitHubRepoInfo -from revup.types import GitCommitHash, RevupGithubException, RevupUsageException - -MAX_COMMENTS_TO_QUERY = 3 - - -@dataclass -class PrComment: - text: str = "" - id: Optional[str] = None - - -@dataclass -class PrInfo: - """ - Represents a Github pull request. - """ - - baseRef: str - headRef: str - baseRefOid: Optional[GitCommitHash] - headRefOid: Optional[GitCommitHash] - body: str - title: str - id: str = "" - url: str = "" - state: str = "" - reviewers: Set[str] = field(default_factory=set) - reviewer_ids: Set[str] = field(default_factory=set) - reviewer_teams: Set[str] = field(default_factory=set) - reviewer_team_ids: Set[str] = field(default_factory=set) - assignees: Set[str] = field(default_factory=set) - assignee_ids: Set[str] = field(default_factory=set) - labels: Set[str] = field(default_factory=set) - label_ids: Set[str] = field(default_factory=set) - removed_reviewers: Set[str] = field(default_factory=set) - removed_reviewer_ids: Set[str] = field(default_factory=set) - removed_assignees: Set[str] = field(default_factory=set) - removed_assignee_ids: Set[str] = field(default_factory=set) - is_draft: bool = False - comments: List[PrComment] = field(default_factory=list) - - -@dataclass -class PrUpdate: - """ - Represents a Github pull request update with the same fields as - a pull request, except some are optional. - """ - - baseRef: Optional[str] = None - body: Optional[str] = None - title: Optional[str] = None - id: str = "" - reviewer_ids: Set[str] = field(default_factory=set) - reviewer_team_ids: Set[str] = field(default_factory=set) - assignee_ids: Set[str] = field(default_factory=set) - label_ids: Set[str] = field(default_factory=set) - is_draft: Optional[bool] = None - comments: List[PrComment] = field(default_factory=list) - - -def get_args_dict(args: List[Any], prefix: str) -> Dict[str, Any]: - """ - Return a dictionary of argument names to argument values, for use in graphql. - """ - return {f"{prefix}{n}": arg for n, arg in enumerate(args)} - - -def get_args_declaration(args: Dict[str, Any], typ: str) -> List[str]: - """ - Return a list of args with their type declaration. - """ - return [f"${var}: {typ}" for var in args] - - -def get_result_args(num: int, prefix: str) -> List[str]: - """ - Return a list of result variable names. - """ - return [f"{prefix}{n}" for n in range(num)] - - -def zip_and_flatten(l1: Iterable[str], l2: Iterable[str]) -> List[str]: - """ - Return a list of l1 and l2 interleaved. - """ - ret: List[str] = [] - iter1 = iter(l1) - iter2 = iter(l2) - while True: - try: - ret.append(next(iter1)) - ret.append(next(iter2)) - except StopIteration: - break - return ret - - -async def query_everything( - github_ep: github.GitHubEndpoint, - repo_info: GitHubRepoInfo, - head_refs: List[str], - user_ids: List[str], - labels: List[str], - teams: List[Tuple[str, str]], -) -> Tuple[ - str, - List[Optional[PrInfo]], - Dict[str, str], - Dict[str, str], - Dict[str, str], - Dict[str, str], - Dict[str, Optional[Set[str]]], -]: - """ - This function does all necessary graphql querying in one request. This dramatically reduces the - amount of time spent on querying. - - Returns a tuple of: - - Repository node id - - List of pull requests, one for each ref in head_refs. None if a pr wasn't found for that ref - - Dict of user_ids as given to graphql node ids - - Dict of user_ids as given to their full login name - - Dict of labels to their graphql node ids - - Dict of "org/slug" team refs to graphql node ids - - Dict of "org/slug" team refs to their member logins. None if the team has more members - than we fetched (meaning membership is unknown / incomplete). - """ - head_refs_args = get_args_dict(head_refs, "pr") - user_id_args = get_args_dict(user_ids, "user") - label_args = get_args_dict(labels, "label") - team_org_args = get_args_dict([t[0] for t in teams], "team_org") - team_slug_args = get_args_dict([t[1] for t in teams], "team_slug") - - prs_out = get_result_args(len(head_refs), "pr_out") - user_id_out = get_result_args(len(user_ids), "user_out") - label_out = get_result_args(len(labels), "label_out") - team_out = get_result_args(len(teams), "team_out") - - arg_str = ", ".join( - get_args_declaration(head_refs_args, "String!") - + get_args_declaration(user_id_args, "String!") - + get_args_declaration(label_args, "String!") - + get_args_declaration(team_org_args, "String!") - + get_args_declaration(team_slug_args, "String!") - ) - - # NOTE: There are possible limitations here because we depend on PRs being returned in order of - # OPEN prs, followed by MERGED prs in the order that they merged. github doesn't offer these - # options and it is excessively expensive to always fetch multiple prs and order them on this - # side. For now we hope that the most relevant PR will have the most recent update time. - request_str = "".join( - len(head_refs) - * [ - "{}: pullRequests (headRefName: ${}, states: [OPEN, MERGED], first: 1, " - "orderBy: {{direction: DESC, field:UPDATED_AT}}) {{" - "...PrResult" - "}}," - ] - ) - request_str = request_str.format(*zip_and_flatten(prs_out, head_refs_args.keys())) - - user_str = "".join( - len(user_ids) * ["{}: assignableUsers (query: ${}, first: 25) {{...UserResult}},"] - ) - user_str = user_str.format(*zip_and_flatten(user_id_out, user_id_args.keys())) - - label_str = "".join(len(labels) * ["{}: label (name: ${}) {{...LabelResult}},"]) - label_str = label_str.format(*zip_and_flatten(label_out, label_args.keys())) - - team_str = "" - for i in range(len(teams)): - team_str += ( - f"{team_out[i]}: organization(login: ${list(team_org_args.keys())[i]}) " - f"{{team(slug: ${list(team_slug_args.keys())[i]}) " - f"{{id, members(first: 100) {{nodes {{login}}, totalCount}}}}}}," - ) - - multi_query_str = f""" - query GetPrResults($owner: String!, $name: String!, {arg_str}) {{ - repository(name: $name, owner: $owner) {{ - id - {request_str}{user_str}{label_str} - }} - {team_str} - }}""" - if user_str: - multi_query_str += """ - fragment UserResult on UserConnection { - nodes { - login - id - } - totalCount - }""" - if label_str: - multi_query_str += """ - fragment LabelResult on Label { - id - name - }""" - if request_str: - multi_query_str += f""" - fragment PrResult on PullRequestConnection {{ - nodes {{ - id - state - url - baseRefName - body - title - isDraft - baseCommit: commits(first: 1) {{ - nodes {{ - commit {{ - parents (first: 1) {{ - nodes {{ - oid - }} - }} - }} - }} - }} - headCommit: commits(last: 1) {{ - nodes {{ - commit {{ - oid - }} - }} - }} - reviewRequests (first: 25) {{ - nodes {{ - requestedReviewer {{ - ... on User {{ - login - id - }} - ... on Team {{ - slug - id - organization {{ - login - }} - }} - }} - }} - }} - timelineItems( - itemTypes: [REVIEW_REQUEST_REMOVED_EVENT, UNASSIGNED_EVENT] - first: 50 - ) {{ - nodes {{ - ... on ReviewRequestRemovedEvent {{ - requestedReviewer {{ - ... on User {{ - login - id - }} - }} - }} - ... on UnassignedEvent {{ - assignee {{ - ... on User {{ - login - id - }} - }} - }} - }} - }} - latestReviews (first: 25) {{ - nodes {{ - author {{ - ... on User {{ - login - id - }} - }} - viewerDidAuthor - }} - }} - assignees (first: 25) {{ - nodes {{ - ... on User {{ - login - id - }} - }} - }} - labels (first: 25) {{ - nodes {{ - name - id - }} - }} - comments (first: {MAX_COMMENTS_TO_QUERY}) {{ - nodes {{ - body - id - }} - }} - }} - totalCount - }}""" - - pr_result = await github_ep.graphql( - multi_query_str, - owner=repo_info.owner, - name=repo_info.name, - **head_refs_args, - **user_id_args, - **label_args, - **team_org_args, - **team_slug_args, - ) - - prs: List[Optional[PrInfo]] = [] - for i, branch_name in enumerate(head_refs): - this_node = pr_result["data"]["repository"][prs_out[i]] - if len(this_node["nodes"]) == 1: - this_node = this_node["nodes"][0] - pr_labels = set() - pr_label_ids = set() - reviewers = set() - reviewer_ids = set() - reviewer_teams = set() - reviewer_team_ids = set() - assignees = set() - assignee_ids = set() - for label in this_node["labels"]["nodes"]: - pr_labels.add(label["name"]) - pr_label_ids.add(label["id"]) - for revs in this_node["reviewRequests"]["nodes"]: - requested = revs["requestedReviewer"] - if not requested: - continue - elif "slug" in requested: - reviewer_teams.add(f"{requested['organization']['login']}/{requested['slug']}") - reviewer_team_ids.add(requested["id"]) - elif "login" in requested: - reviewers.add(requested["login"]) - reviewer_ids.add(requested["id"]) - for revs in this_node["latestReviews"]["nodes"]: - # Ignore self reviews and bot reviews (without a login) - if not revs["viewerDidAuthor"] and "login" in revs["author"]: - reviewers.add(revs["author"]["login"]) - reviewer_ids.add(revs["author"]["id"]) - for user in this_node["assignees"]["nodes"]: - assignees.add(user["login"]) - assignee_ids.add(user["id"]) - - # The plain headRef and baseRef fields return the latest commit id associated with - # that branch name which may be newer than the PR itself if it was merged. We want - # the ids of the commits actually last associated with the PR, which we query from - # the commit list. This can also mean they are None if the PR has 0 commits. - headRefOid = ( - this_node["headCommit"]["nodes"][0]["commit"]["oid"] - if this_node["headCommit"]["nodes"] - else None - ) - baseRefOid = ( - this_node["baseCommit"]["nodes"][0]["commit"]["parents"]["nodes"][0]["oid"] - if this_node["baseCommit"]["nodes"] - else None - ) - - comments = [] - for c in this_node["comments"]["nodes"]: - comments.append(PrComment(c["body"], c["id"])) - - removed_reviewers: Set[str] = set() - removed_reviewer_ids: Set[str] = set() - removed_assignees: Set[str] = set() - removed_assignee_ids: Set[str] = set() - for event in this_node["timelineItems"]["nodes"]: - rr = event.get("requestedReviewer") - if rr and "login" in rr and rr["login"] not in reviewers: - removed_reviewers.add(rr["login"]) - removed_reviewer_ids.add(rr["id"]) - assignee = event.get("assignee") - if assignee and "login" in assignee and assignee["login"] not in assignees: - removed_assignees.add(assignee["login"]) - removed_assignee_ids.add(assignee["id"]) - - prs.append( - PrInfo( - id=this_node["id"], - url=this_node["url"], - baseRef=this_node["baseRefName"], - headRef=branch_name, - baseRefOid=baseRefOid, - headRefOid=headRefOid, - body=this_node["body"], - title=this_node["title"], - reviewers=reviewers, - reviewer_ids=reviewer_ids, - reviewer_teams=reviewer_teams, - reviewer_team_ids=reviewer_team_ids, - assignees=assignees, - assignee_ids=assignee_ids, - labels=pr_labels, - label_ids=pr_label_ids, - removed_reviewers=removed_reviewers, - removed_reviewer_ids=removed_reviewer_ids, - removed_assignees=removed_assignees, - removed_assignee_ids=removed_assignee_ids, - is_draft=this_node["isDraft"], - state=this_node["state"], - comments=comments, - ) - ) - else: - prs.append(None) - - names_to_ids = {} - names_to_logins = {} - for i, user_id in enumerate(user_ids): - this_node = pr_result["data"]["repository"][user_id_out[i]] - if len(this_node["nodes"]) == 0: - logging.warning("No matching user found for {}".format(user_id)) - else: - if this_node["totalCount"] > len(this_node["nodes"]): - logging.warning( - "Too many matching users found for {}, try being more specific".format(user_id) - ) - shortest_name = this_node["nodes"][0]["login"] - names_to_ids[user_id] = this_node["nodes"][0]["id"] - found_match = False - for user in this_node["nodes"]: - if len(user["login"]) <= len(shortest_name) and user["login"].startswith(user_id): - shortest_name = user["login"] - names_to_ids[user_id] = user["id"] - names_to_logins[user_id] = user["login"] - found_match = True - if not found_match: - logging.warning( - "Couldn't find a prefixed match for {}, going with {} instead".format( - user_id, shortest_name - ) - ) - - labels_to_ids = {} - for i, label in enumerate(labels): - this_node = pr_result["data"]["repository"][label_out[i]] - if this_node is not None: - labels_to_ids[label] = this_node["id"] - else: - logging.warning("Couldn't find an existing label named {}".format(label)) - - teams_to_ids = {} - teams_to_members: Dict[str, Optional[Set[str]]] = {} - for i, (org, slug) in enumerate(teams): - team_node = pr_result["data"][team_out[i]] - if team_node is not None and team_node["team"] is not None: - team_ref = f"{org}/{slug}" - teams_to_ids[team_ref] = team_node["team"]["id"] - members_node = team_node["team"]["members"] - member_logins = {m["login"] for m in members_node["nodes"]} - if members_node["totalCount"] > len(members_node["nodes"]): - # Team has more members than we fetched; we can't check membership reliably. - teams_to_members[team_ref] = None - else: - teams_to_members[team_ref] = member_logins - else: - logging.warning("Couldn't find a team matching {}/{}".format(org, slug)) - - return ( - pr_result["data"]["repository"]["id"], - prs, - names_to_ids, - names_to_logins, - labels_to_ids, - teams_to_ids, - teams_to_members, - ) - - -async def create_pull_requests( - github_ep: github.GitHubEndpoint, - repo_id: str, - repo_info: GitHubRepoInfo, - fork_info: GitHubRepoInfo, - prs: List[PrInfo], -) -> None: - """ - Create all pull requests given in prs and modify them to add the new pr node id and URL. - """ - inputs = [] - for pr in prs: - headRef = ( - pr.headRef if fork_info.owner == repo_info.owner else f"{fork_info.owner}:{pr.headRef}" - ) - inputs.append({ - "baseRefName": pr.baseRef, - "body": pr.body, - "clientMutationId": "revup", - "headRefName": headRef, - "repositoryId": repo_id, - "title": pr.title, - "draft": pr.is_draft, - }) - inputs_args = get_args_dict(inputs, "pr") - prs_out = get_result_args(len(inputs), "pr_out") - - arg_str = ", ".join(get_args_declaration(inputs_args, "CreatePullRequestInput!")) - - request_str = "".join( - len(inputs) - * [ - """ - {}: createPullRequest(input: ${}) {{ - pullRequest {{ - id - url - }} - }},""" - ] - ) - request_str = request_str.format(*zip_and_flatten(prs_out, inputs_args.keys())) - - mutation_str = f""" - mutation ({arg_str}) {{ - {request_str} - }}""" - - # Creating a pull request can fail if the branch is already merged. - pr_results = await github_ep.graphql(mutation_str, require_success=False, **inputs_args) - for i, pr in enumerate(prs): - result = pr_results["data"][prs_out[i]]["pullRequest"] - if result is not None: - pr.id = result["id"] - pr.url = result["url"] - - -async def update_pull_requests(github_ep: github.GitHubEndpoint, prs: List[PrUpdate]) -> None: - """ - Update the given pull request contents, and also add reviewers and labels. - """ - inputs = [] - labels = [] - reviewers = [] - assignees = [] - convert_to_draft = [] - convert_from_draft = [] - comments = [] - edit_comments = [] - for pr in prs: - update_dict = { - "clientMutationId": "revup", - "pullRequestId": pr.id, - } - if pr.baseRef is not None: - update_dict["baseRefName"] = pr.baseRef - if pr.body is not None: - update_dict["body"] = pr.body - if pr.title is not None: - update_dict["title"] = pr.title - inputs.append(update_dict) - - if pr.label_ids: - labels.append({ - "labelIds": list(pr.label_ids), - "clientMutationId": "revup", - "labelableId": pr.id, - }) - - if pr.reviewer_ids or pr.reviewer_team_ids: - reviewers.append({ - "userIds": list(pr.reviewer_ids), - "teamIds": list(pr.reviewer_team_ids), - "clientMutationId": "revup", - "pullRequestId": pr.id, - "union": True, - }) - if pr.assignee_ids: - assignees.append({ - "assigneeIds": list(pr.assignee_ids), - "clientMutationId": "revup", - "assignableId": pr.id, - }) - - if pr.is_draft is not None: - if pr.is_draft: - convert_to_draft.append({ - "clientMutationId": "revup", - "pullRequestId": pr.id, - }) - else: - convert_from_draft.append({ - "clientMutationId": "revup", - "pullRequestId": pr.id, - }) - - for c in pr.comments: - if c.id: - edit_comments.append({ - "body": c.text, - "clientMutationId": "revup", - "id": c.id, - }) - else: - comments.append({ - "body": c.text, - "clientMutationId": "revup", - "subjectId": pr.id, - }) - - inputs_args = get_args_dict(inputs, "pr") - prs_out = get_result_args(len(inputs), "pr_out") - - labels_args = get_args_dict(labels, "label") - labels_out = get_result_args(len(labels), "label_out") - - reviewers_args = get_args_dict(reviewers, "rev") - reviewers_out = get_result_args(len(reviewers), "rev_out") - - assignees_args = get_args_dict(assignees, "asn") - assignees_out = get_result_args(len(assignees), "asn_out") - - to_draft_args = get_args_dict(convert_to_draft, "to_d") - to_draft_out = get_result_args(len(convert_to_draft), "to_d_out") - - from_draft_args = get_args_dict(convert_from_draft, "from_d") - from_draft_out = get_result_args(len(convert_from_draft), "from_d_out") - - comments_args = get_args_dict(comments, "com") - comments_out = get_result_args(len(comments), "com_out") - - edit_comments_args = get_args_dict(edit_comments, "edit_com") - edit_comments_out = get_result_args(len(edit_comments), "edit_com_out") - - arg_str = ", ".join( - get_args_declaration(inputs_args, "UpdatePullRequestInput!") - + get_args_declaration(labels_args, "AddLabelsToLabelableInput!") - + get_args_declaration(reviewers_args, "RequestReviewsInput!") - + get_args_declaration(assignees_args, "AddAssigneesToAssignableInput!") - + get_args_declaration(to_draft_args, "ConvertPullRequestToDraftInput!") - + get_args_declaration(from_draft_args, "MarkPullRequestReadyForReviewInput!") - + get_args_declaration(comments_args, "AddCommentInput!") - + get_args_declaration(edit_comments_args, "UpdateIssueCommentInput!") - ) - - update_str = "".join( - len(inputs) - * [ - """ - {}: updatePullRequest(input: ${}) {{ - clientMutationId - }},""" - ] - ) - update_str = update_str.format(*zip_and_flatten(prs_out, inputs_args.keys())) - - request_reviewers_str = "".join( - len(reviewers_args) - * [ - """ - {}: requestReviews(input: ${}) {{ - clientMutationId - }},""" - ] - ) - request_reviewers_str = request_reviewers_str.format( - *zip_and_flatten(reviewers_out, reviewers_args.keys()) - ) - assignees_str = "".join( - len(assignees_args) - * [ - """ - {}: addAssigneesToAssignable(input: ${}) {{ - clientMutationId - }},""" - ] - ) - assignees_str = assignees_str.format(*zip_and_flatten(assignees_out, assignees_args.keys())) - - add_labels_str = "".join( - len(labels_args) - * [ - """ - {}: addLabelsToLabelable(input: ${}) {{ - clientMutationId - }},""" - ] - ) - add_labels_str = add_labels_str.format(*zip_and_flatten(labels_out, labels_args.keys())) - - to_draft_str = "".join( - len(convert_to_draft) - * [ - """ - {}: convertPullRequestToDraft(input: ${}) {{ - clientMutationId - }},""" - ] - ) - to_draft_str = to_draft_str.format(*zip_and_flatten(to_draft_out, to_draft_args.keys())) - - from_draft_str = "".join( - len(convert_from_draft) - * [ - """ - {}: markPullRequestReadyForReview(input: ${}) {{ - clientMutationId - }},""" - ] - ) - from_draft_str = from_draft_str.format(*zip_and_flatten(from_draft_out, from_draft_args.keys())) - - add_comments_str = "".join( - len(comments_args) - * [ - """ - {}: addComment(input: ${}) {{ - clientMutationId - }},""" - ] - ) - add_comments_str = add_comments_str.format(*zip_and_flatten(comments_out, comments_args.keys())) - - edit_comments_str = "".join( - len(edit_comments_args) - * [ - """ - {}: updateIssueComment(input: ${}) {{ - clientMutationId - }},""" - ] - ) - edit_comments_str = edit_comments_str.format( - *zip_and_flatten(edit_comments_out, edit_comments_args.keys()) - ) - - # Have any add comment mutations first in order to ensure that comments are at the top of the PR - mutation_str = f""" - mutation ({arg_str}) {{ - {add_comments_str}{update_str}{request_reviewers_str}{assignees_str}{add_labels_str}\ -{to_draft_str}{from_draft_str}{edit_comments_str} - }}""" - - try: - await github_ep.graphql( - mutation_str, - **comments_args, - **inputs_args, - **reviewers_args, - **assignees_args, - **labels_args, - **to_draft_args, - **from_draft_args, - **edit_comments_args, - ) - except RevupGithubException as e: - if "timeout" in e.message: - logging.warning( - "Github update request timed out! Most likely this is a false alarm and changes" - " actually succeeded. You may want to rerun this command to verify." - ) - else: - raise e - - -RE_PR_URL = re.compile( - r"^https://(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/pull/(?P[0-9]+)/?$" -) - - -GitHubPullRequestParams = NamedTuple( - "GitHubPullRequestParams", - [ - ("github_url", str), - ("owner", str), - ("name", str), - ("number", int), - ], -) - - -def parse_pull_request_url(pull_request: str) -> GitHubPullRequestParams: - m = RE_PR_URL.match(pull_request) - if not m: - raise RuntimeError("Did not understand PR argument. PR must be URL") - - github_url = m.group("github_url") - owner = m.group("owner") - name = m.group("name") - number = int(m.group("number")) - return GitHubPullRequestParams(github_url=github_url, owner=owner, name=name, number=number) - - -@asynccontextmanager -async def github_connection( - git_ctx: git.Git, args: argparse.Namespace, conf: config.Config -) -> AsyncGenerator[Tuple, None]: - from revup import github_real - - repo_info = await git_ctx.get_github_repo_info( - github_url=args.github_url, remote_name=args.remote_name - ) - - if not repo_info.owner or not repo_info.name: - raise RevupUsageException( - f'Configured remote "{args.remote_name}" does not ' - "point to the a github repository! " - "You can set it manually by running " - f"`git remote set-url {args.remote_name} git@github.com:{{OWNER}}/{{PROJECT}}` " - f"or change the configured remote in {conf.config_path}/" - ) - - fork_info = repo_info - if args.fork_name and args.fork_name != args.remote_name: - fork_info = await git_ctx.get_github_repo_info( - github_url=args.github_url, remote_name=args.fork_name - ) - - if not fork_info.owner or not fork_info.name: - raise RevupUsageException( - f'Configured remote fork "{args.fork_info}" does not ' - "point to the a github repository! " - "You can set it manually by running " - f"`git remote set-url {args.fork_info} git@github.com:{{OWNER}}/{{PROJECT}}` " - f"or change the configured remote in {conf.config_path}." - ) - - if repo_info.name != fork_info.name: - raise RevupUsageException( - f'Configured remote fork "{args.fork_info}" is not ' - f"the same repo as the remote {args.remote_info}." - ) - - if not args.github_oauth: - # Try environment variables first - args.github_oauth = os.environ.get("GITHUB_TOKEN") - if args.github_oauth: - logs.redact({args.github_oauth: ""}) - logging.debug("Used GitHub token from environment variable") - else: - # Fall back to git credential helper - args.github_oauth = await git_ctx.credential( - protocol="https", - host=args.github_url, - path=f"{fork_info.owner}/{fork_info.name}.git", - ) - if args.github_oauth: - logs.redact({args.github_oauth: ""}) - logging.debug("Used credential from git-credential") - - if not args.github_oauth: - raise RevupUsageException( - "No Github OAuth token found! " - "Set the GITHUB_TOKEN environment variable, " - "login with 'gh auth login', " - "or make one at https://github.com/settings/tokens/new " - "(revup needs full repo permissions) " - "then set it with `revup config github_oauth`." - ) - - github_ep = github_real.RealGitHubEndpoint( - oauth_token=args.github_oauth, proxy=args.proxy, github_url=args.github_url - ) - try: - yield github_ep, repo_info, fork_info - finally: - await github_ep.close() - - -async def query_pr_by_number( - github_ep: github.GitHubEndpoint, - owner: str, - name: str, - number: int, -) -> Tuple[str, str]: - """ - Query a pull request by number and return (headRefName, baseRefName). - """ - result = await github_ep.graphql( - query="""\ -query ($owner: String!, $name: String!, $number: Int!) { - repository(owner: $owner, name: $name) { - pullRequest(number: $number) { - headRefName - baseRefName - } - } -}""", - owner=owner, - name=name, - number=number, - ) - pr = result["data"]["repository"]["pullRequest"] - return pr["headRefName"], pr["baseRefName"] diff --git a/revup/restack.py b/revup/restack.py index 85408bc..654c9be 100644 --- a/revup/restack.py +++ b/revup/restack.py @@ -167,8 +167,6 @@ async def main(args: argparse.Namespace, git_ctx: git.Git) -> int: git_ctx, args.base_branch, args.relative_branch, - None, - None, ) await topics.populate_topics() diff --git a/revup/revup.py b/revup/revup.py index 21c5bf3..aebce7f 100755 --- a/revup/revup.py +++ b/revup/revup.py @@ -18,6 +18,7 @@ topic_completer, ) from revup.config import RevupArgParser +from revup.forge_utils import parse_forge_info from revup.topic_stack import PrBodySource from revup.types import RevupUsageException @@ -50,9 +51,8 @@ def make_toplevel_parser() -> RevupArgParser: "--version", action="version", version=f"%(prog)s {revup.__version__}" ) revup_parser.add_argument("--proxy") - revup_parser.add_argument("--github-oauth") - revup_parser.add_argument("--github-username") - revup_parser.add_argument("--github-url", default="github.com") + revup_parser.add_argument("--forge-oauth", "--github-oauth") + revup_parser.add_argument("--forge-url", "--github-url", default="github.com") revup_parser.add_argument("--remote-name", default="origin") revup_parser.add_argument("--fork-name", default="") revup_parser.add_argument("--editor") @@ -319,7 +319,7 @@ async def main(revup_parser: RevupArgParser, all_parsers: List[RevupArgParser]) # So users don't accidentally leak their oauth when sharing logs logs.configure_logger( debug=args.verbose, - redactions={args.github_oauth: ""} if args.github_oauth else {}, + redactions={args.forge_oauth: ""} if args.forge_oauth else {}, ) if args.cmd != "toolkit": @@ -346,12 +346,11 @@ async def main(revup_parser: RevupArgParser, all_parsers: List[RevupArgParser]) # "commit" is an alias of "amend --insert" args.insert = args.cmd == "commit" or args.insert - repo_info = await git_ctx.get_github_repo_info( - github_url=args.github_url, remote_name=args.remote_name - ) + remote_url = await git_ctx.get_remote_url(args.remote_name) + repo_info = parse_forge_info(remote_url, args.forge_url) if not repo_info.owner or not repo_info.name: - # Don't try to get topics for repos that are not in use with github + # Don't try to get topics for repos that are not in use with a forge args.parse_topics = False return await amend.main(args=args, git_ctx=git_ctx) @@ -361,22 +360,16 @@ async def main(revup_parser: RevupArgParser, all_parsers: List[RevupArgParser]) return await restack.main(args=args, git_ctx=git_ctx) - from revup.github_utils import github_connection + from revup.forge_utils import forge_connection - async with github_connection(args=args, git_ctx=git_ctx, conf=conf) as ( - github_ep, - repo_info, - fork_info, - ): + async with forge_connection(args=args, git_ctx=git_ctx, conf=conf) as forge: if args.cmd == "upload": from revup import upload return await upload.main( args=args, git_ctx=git_ctx, - github_ep=github_ep, - repo_info=repo_info, - fork_info=fork_info, + forge=forge, ) return 1 diff --git a/revup/topic_stack.py b/revup/topic_stack.py index 209e859..06455c1 100644 --- a/revup/topic_stack.py +++ b/revup/topic_stack.py @@ -14,8 +14,8 @@ from rich import get_console from rich.markup import escape -from revup import git, github, github_utils -from revup.github_utils import PrComment, PrInfo, PrUpdate +from revup import git +from revup.forge import MAX_COMMENTS_TO_QUERY, Forge, PrComment, PrInfo, PrUpdate from revup.types import ( GitCommitHash, GitConflictException, @@ -25,7 +25,7 @@ ) # Since topic name is incorporated into the branch name, we must ensure that it matches -# the character set that github supports. It's possible to have other characters if they're +# the character set that forges support. It's possible to have other characters if they're # properly escaped but we don't want to deal with that for now. # 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_\.\/-]+$") @@ -112,11 +112,11 @@ def __str__(self) -> str: return self.value -# The current state of each review within github. +# The current state of each review on the forge. class PrStatus(Enum): NEW = "new" # needs to be created, or was just created - UPDATED = "updated" # github data needs to be modified (title, reviewers, labels, etc) - NOCHANGE = "no change" # no github mutations are necessary + UPDATED = "updated" # forge data needs to be modified (title, reviewers, labels, etc) + NOCHANGE = "no change" # no forge mutations are necessary MERGED = "already merged" # change has already merged (and no mutations are possible) @@ -130,7 +130,7 @@ class PushStatus(Enum): @dataclass class Review: """ - Represents a single github pull request. Uniquely keyed by topic name and base branch. + Represents a single pull request. Uniquely keyed by topic name and base branch. """ # Reference to the enclosing topic object @@ -234,10 +234,8 @@ class TopicStack: # Branch / review that current reviews are relative to relative_branch: str - # Github access info - github_ep: Optional[github.GitHubEndpoint] = None - repo_info: Optional[github_utils.GitHubRepoInfo] = None - fork_info: Optional[github_utils.GitHubRepoInfo] = None + # Forge interface for PR operations + forge: Optional[Forge] = None # Commit at the head of the branch to be uploaded head: str = "HEAD" @@ -248,19 +246,19 @@ class TopicStack: # Topic names to topic info topics: Dict[str, Topic] = field(default_factory=dict) - # Github node id of the repo + # Forge node id of the repo repo_id: Optional[str] = None - # Github node ids of users (reviewer/assignee) + # Forge node ids of users (reviewer/assignee) names_to_ids: Optional[Dict[str, str]] = None - # Github full login names of users + # Full login names of users names_to_logins: Optional[Dict[str, str]] = None - # Github node ids of labels + # Forge node ids of labels labels_to_ids: Optional[Dict[str, str]] = None - # Github node ids of teams, keyed by "org/slug" + # Forge node ids of teams, keyed by "org/slug" teams_to_ids: Optional[Dict[str, str]] = None # Team member logins keyed by "org/slug". None means the team is too large to @@ -276,7 +274,7 @@ class TopicStack: # Whether populate() was successfully called populated: bool = False - # Whether to work around github issues with reordering by pushing a dummy commit + # Whether to work around forge issues with reordering by pushing a dummy commit use_reordering_workaround = False def all_reviews_iter(self) -> Iterator[Tuple[str, Topic, str, Review]]: @@ -344,8 +342,7 @@ async def create_patchsets_comment( if ( review.push_status != PushStatus.PUSHED or review.status == PrStatus.MERGED - or not self.repo_info - or not self.fork_info + or not self.forge or review.pr_info is None or review.pr_info.headRefOid is None or review.pr_info.baseRefOid is None @@ -380,8 +377,8 @@ async def create_patchsets_comment( if self.last_virtual_diff_target is None: self.last_virtual_diff_target = GitCommitHash(self.base_branch) self.last_virtual_diff_target = await self.git_ctx.make_virtual_diff_target( - review.pr_info.baseRefOid, - review.pr_info.headRefOid, + GitCommitHash(review.pr_info.baseRefOid), + GitCommitHash(review.pr_info.headRefOid), review.base_ref, review.new_commits[-1], self.last_virtual_diff_target, @@ -389,9 +386,9 @@ async def create_patchsets_comment( diff_base = self.last_virtual_diff_target else: # Non rebase push, diff against previous version of the branch - diff_base = review.pr_info.headRefOid + diff_base = GitCommitHash(review.pr_info.headRefOid) diff = ( - f"[diff](/{self.fork_info.owner}/{self.repo_info.name}/compare/" + f"[diff](/{self.forge.repo_owner}/{self.forge.repo_name}/compare/" f"{diff_base}..{review.new_commits[-1]})" ) summary = await self.git_ctx.get_diff_summary(diff_base, review.new_commits[-1]) @@ -405,9 +402,9 @@ async def create_patchsets_comment( d = datetime.now() ret += ( f"\r\n| {number} | [{review.new_commits[-1][:8]}]" - f"(/{self.fork_info.owner}/{self.repo_info.name}/commit/{review.new_commits[-1]}) " + f"(/{self.forge.repo_owner}/{self.forge.repo_name}/commit/{review.new_commits[-1]}) " f"| [{review.base_ref[:8]}]" - f"(/{self.fork_info.owner}/{self.repo_info.name}/commit/{review.base_ref}) " + f"(/{self.forge.repo_owner}/{self.forge.repo_name}/commit/{review.base_ref}) " f"| {diff} | {d:%b} {d.day} {d.hour}:{d.minute:02} {d:%p} | {summary} |" ) return PrComment(ret, orig.id if orig else None) @@ -570,14 +567,14 @@ async def populate_reviews( ) relative_topic = "" - if self.repo_info and self.fork_info and self.fork_info.owner != self.repo_info.owner: + if self.forge and self.forge.is_fork: if len(topic.tags[TAG_RELATIVE_BRANCH]) > 1: raise RevupUsageException( - "Can't use 'Relative-Branch' across forks due to github limitations!" + "Can't use 'Relative-Branch' across forks due to forge limitations!" ) if relative_topic: logging.warning( - f"Skipping topic '{name}' since github does not allow relative reviews" + f"Skipping topic '{name}' since the forge does not allow relative reviews" f" across forks. It will be uploaded when '{relative_topic}' merges." ) del self.topics[name] @@ -612,13 +609,13 @@ async def populate_reviews( if auto_add_users in ("a2r", "both"): topic.tags[TAG_REVIEWER].update(topic.tags[TAG_ASSIGNEE]) - # Github does not support teams as assignees, so reject any team-style entries + # Teams are not supported as assignees, so reject any team-style entries # up front rather than silently ignoring them. team_assignees = {a for a in topic.tags[TAG_ASSIGNEE] if is_team(a)} if team_assignees: raise RevupUsageException( - f"Topic '{name}' has team(s) listed as assignees, but Github only allows" - f" users as assignees: {sorted(team_assignees)}" + f"Topic '{name}' has team(s) listed as assignees, but only users are allowed:" + f" {sorted(team_assignees)}" ) # Track the last actually used topic for the relative-chain feature @@ -917,8 +914,8 @@ async def mark_rebases(self, skip_rebase: bool) -> None: if review.push_status == PushStatus.PUSHED: # If this change must be pushed, then all changes it depends on cannot be - # skipped due to rebase (although can be due to nochange), otherwise github - # will show the wrong commit diff between the two reviews + # skipped due to rebase (although can be due to nochange), otherwise the + # forge will show the wrong commit diff between the two reviews cur_topic = topic.relative_topic while cur_topic is not None: cur_review = cur_topic.reviews[base_branch] @@ -1021,8 +1018,7 @@ async def fetch_git_refs(self) -> None: These would normally exist locally, but might not due to running a git gc, or user switching to a different machine. """ - if not self.github_ep or not self.repo_info: - raise RuntimeError("Can't fetch without github info") + assert self.forge to_fetch = set() for _, _, _, review in self.all_reviews_iter(): @@ -1049,10 +1045,10 @@ async def retarget_orphaned_prs(self) -> None: Detect PRs whose previous target branch is no longer being pushed in this run. Retarget them to the base branch as a safe intermediate before pushing. This works around an issue where orphaning a change and rebasing will temporarily - cause github to see the entire upstream history in the PR diff, and mistakenly thrashes + cause the forge to see the entire upstream history in the PR diff, and mistakenly thrashes a bunch of stuff like CODEOWNERS. """ - if not self.github_ep or not self.repo_info: + if not self.forge: return # Collect the set of branches we're pushing this run @@ -1086,14 +1082,13 @@ async def retarget_orphaned_prs(self) -> None: review.pr_info.baseRef = base if orphaned_updates: - await github_utils.update_pull_requests(self.github_ep, orphaned_updates) + await self.forge.update_pull_requests(orphaned_updates) async def push_git_refs(self, uploader: str, create_local_branches: bool) -> None: """ Push all refs to their branch on the remote. """ - if not self.github_ep or not self.repo_info: - raise RuntimeError("Can't push without github info") + assert self.forge push_targets = [] for _, _, _, review in self.all_reviews_iter(): @@ -1102,11 +1097,11 @@ async def push_git_refs(self, uploader: str, create_local_branches: bool) -> Non commit_to_push = review.new_commits[-1] if self.use_reordering_workaround: - # When reordering a relative series of PRs, github isn't able to handle the push, - # which happens via git, atomically with the api update, which happens through - # http. As a result github always sees the push happen first with the old relative - # structure. When reordering, a PR that is being moved forward might briefly look - # like it contains no new commits, causing github to either mark it merged or + # When reordering a relative series of PRs, the forge isn't able to handle the + # push, which happens via git, atomically with the api update, which happens + # through http. As a result the forge always sees the push happen first with the + # old relative structure. When reordering, a PR that is being moved forward might + # briefly look like it contains no new commits, causing it to be marked merged or # closed. To prevent this, we add an empty dummy commit onto the branch before # pushing, do the update, then push once more to remove the dummy commit. This # only happens when we detect that it is needed, so does not add much overhead. @@ -1152,14 +1147,13 @@ async def push_git_refs(self, uploader: str, create_local_branches: bool) -> Non ), ) - async def query_github(self) -> None: + async def query(self) -> None: """ - Query pr and reviewer/label info from github + Query pr and reviewer/label info from the forge """ if not self.topics: return - if not self.github_ep or not self.repo_info: - raise RuntimeError("Can't query without github info") + assert self.forge pr_targets = [] user_ids = set() @@ -1199,9 +1193,7 @@ async def query_github(self) -> None: self.labels_to_ids, self.teams_to_ids, self.teams_to_members, - ) = await github_utils.query_everything( - self.github_ep, - self.repo_info, + ) = await self.forge.query_everything( pr_targets, list(user_ids), list(labels), @@ -1261,7 +1253,7 @@ def populate_update_info( pr_body_source: PrBodySource = PrBodySource.FIRST_COMMIT, ) -> None: """ - Populate information necessary to do PR creation / update in github. + Populate information necessary to do PR creation / update on the forge. """ if not self.topics: return @@ -1296,7 +1288,7 @@ def populate_update_info( if not review.pr_info or review.status == PrStatus.MERGED: continue - for i in range(github_utils.MAX_COMMENTS_TO_QUERY): + for i in range(MAX_COMMENTS_TO_QUERY): # Match comment indexes for various features (they will be populated later) if i >= len(review.pr_info.comments): if review.review_graph_index is None: @@ -1330,7 +1322,7 @@ def populate_update_info( user_reviewer_tags, self.names_to_logins ).difference(review.pr_info.reviewers) - # A team may have been "resolved" by Github's code review assignment (the team + # A team may have been "resolved" by code review auto-assignment (the team # gets dropped from reviewRequests and individuals get added). Re-requesting # the team would trigger auto-assignment again, so skip teams whose members # are already among the existing reviewers. @@ -1348,7 +1340,7 @@ def populate_update_info( f"Not re-requesting team '{team_ref}' on " f"{review.pr_info.url or review.remote_head} because existing " f"reviewer(s) {overlap} are members of it. Re-requesting would trigger" - " Github code review auto-assignment again." + " code review auto-assignment again." ) continue teams_to_request.add(team_ref) @@ -1498,12 +1490,11 @@ async def populate_patchsets(self) -> None: async def create_prs(self) -> None: """ - Actually perform the github graphql PR creation + Actually perform the PR creation on the forge """ if not self.topics: return - if not self.github_ep or not self.repo_info or not self.fork_info or not self.repo_id: - raise RuntimeError("Can't update without github info") + assert self.forge and self.repo_id prs_to_create = [] for _, _, _, review in self.all_reviews_iter(): @@ -1513,22 +1504,15 @@ async def create_prs(self) -> None: if prs_to_create: # Create all prs in one request. These will most likely end up being modified # later since its not possible to add labels or reviewers at creation time. - await github_utils.create_pull_requests( - self.github_ep, - self.repo_id, - self.repo_info, - self.fork_info, - prs_to_create, - ) + await self.forge.create_pull_requests(self.repo_id, prs_to_create) async def update_prs(self) -> None: """ - Actually perform the github graphql PR updates + Actually perform the PR updates on the forge """ if not self.topics: return - if not self.github_ep or not self.repo_info or not self.fork_info or not self.repo_id: - raise RuntimeError("Can't update without github info") + assert self.forge and self.repo_id prs_to_update = [] for _, _, _, review in self.all_reviews_iter(): @@ -1551,7 +1535,7 @@ async def update_prs(self) -> None: review.status = PrStatus.UPDATED if prs_to_update: - await github_utils.update_pull_requests(self.github_ep, prs_to_update) + await self.forge.update_pull_requests(prs_to_update) def num_reviews_changed(self) -> int: """ @@ -1636,5 +1620,6 @@ def print(self, skip_empty: bool) -> None: if review.push_status != PushStatus.NOCHANGE: # Push status is redundant if there's no change. status_str += f" ({review.push_status.value})" - get_console().print("[green]Github URL:[/]") + forge_name = self.forge.name if self.forge else "PR" + get_console().print(f"[green]{forge_name} URL:[/]") get_console().print(f" [underline]{review.pr_info.url}[/] {status_str}") diff --git a/revup/types.py b/revup/types.py index 4114d13..10c81d0 100644 --- a/revup/types.py +++ b/revup/types.py @@ -69,7 +69,7 @@ class RevupShellException(Exception): pass -class RevupGithubException(Exception): +class RevupForgeException(Exception): def __init__(self, error_json: Dict): super().__init__() self.error_json = error_json diff --git a/revup/upload.py b/revup/upload.py index 5020985..ca80a45 100644 --- a/revup/upload.py +++ b/revup/upload.py @@ -1,19 +1,17 @@ import argparse import subprocess -from typing import Optional from rich import get_console -from revup import git, github, github_utils, topic_stack +from revup import git, topic_stack +from revup.forge import Forge from revup.types import RevupShellException async def main( args: argparse.Namespace, git_ctx: git.Git, - github_ep: Optional[github.GitHubEndpoint] = None, - repo_info: Optional[github_utils.GitHubRepoInfo] = None, - fork_info: Optional[github_utils.GitHubRepoInfo] = None, + forge: Forge, ) -> int: """ Handles the "upload" command. @@ -22,9 +20,7 @@ async def main( git_ctx, args.base_branch, args.relative_branch, - github_ep, - repo_info, - fork_info, + forge, args.head, ) with get_console().status("Finding topics…"): @@ -47,8 +43,8 @@ async def main( ) if not args.dry_run and not args.push_only: - with get_console().status("Querying github…"): - await topics.query_github() + with get_console().status(f"Querying {forge.name}…"): + await topics.query() # Fetch uses the oid results from the query await topics.fetch_git_refs() @@ -106,7 +102,7 @@ async def main( try: # Must create PRs after refs are pushed, and must update PRs after creating them. - with get_console().status("Updating github PRs…"): + with get_console().status(f"Updating {forge.name} PRs…"): await topics.create_prs() if args.review_graph: # Review graph requires populated PR urls from creation diff --git a/tests/test_tools.py b/tests/test_tools.py index 2e14ab8..879b8bb 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -14,9 +14,8 @@ def mock_revup(args, user_input) -> str: # The first arg is always the program path. args = ["revup"] + args - # We want to ensure that no connection to github is established (as we are - # not actually githubbing)! - mock.patch("revup.revup.github_connection") + # We want to ensure that no connection to the forge is established + mock.patch("revup.revup.forge_connection") # User input mocks the user typing things into the terminal. user_input = list(user_input) if isinstance(user_input, list) else [user_input] with mock.patch.object(sys, "argv", args): diff --git a/tests/test_upload.py b/tests/test_upload.py index 22a8762..ae707ae 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -3,7 +3,7 @@ import pytest from git_env import GitTestEnvironment, async_test -from revup.github_utils import PrInfo +from revup.forge import PrInfo from revup.topic_stack import ( PrBodySource, PrStatus,