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 39123e5..0000000 Binary files a/revup/__init__.pyc and /dev/null differ 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,