mirror of
https://github.com/servo/servo.git
synced 2025-07-22 06:43:40 +01:00
Mach: introduce Pyrefly for Python type checking, starting with the wpt folder (#37953)
This is the first stage of adopting Pyrefly. It introduces the Python folder and focuses on fixing issues around it. Testing: *Describe how this pull request is tested or why it doesn't require tests* Fixes: *Link to an issue this pull requests fixes or remove this line if there is no issue* --------- Signed-off-by: Jerens Lensun <jerensslensun@gmail.com>
This commit is contained in:
Jerens Lensun
GitHub
parent
2366a8bf9e
commit
55fd7b862f
14 changed files with 303 additions and 154 deletions
|
|
@ -21,7 +21,7 @@ import logging
|
|||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
|
||||
from dataclasses import field
|
||||
from typing import Callable, Optional
|
||||
|
||||
from .common import (
|
||||
|
|
@ -34,7 +34,6 @@ from .common import (
|
|||
UPSTREAMABLE_PATH,
|
||||
wpt_branch_name_from_servo_pr_number,
|
||||
)
|
||||
|
||||
from .github import GithubRepository, PullRequest
|
||||
from .step import (
|
||||
AsyncValue,
|
||||
|
|
@ -49,7 +48,7 @@ from .step import (
|
|||
|
||||
|
||||
class LocalGitRepo:
|
||||
def __init__(self, path: str, sync: WPTSync):
|
||||
def __init__(self, path: str, sync: WPTSync) -> None:
|
||||
self.path = path
|
||||
self.sync = sync
|
||||
|
||||
|
|
@ -57,7 +56,9 @@ class LocalGitRepo:
|
|||
# git in advance and run the subprocess by its absolute path.
|
||||
self.git_path = shutil.which("git")
|
||||
|
||||
def run_without_encoding(self, *args, env: dict = {}):
|
||||
def run_without_encoding(self, *args, env: dict = {}) -> bytes:
|
||||
if self.git_path is None:
|
||||
raise RuntimeError("Git executable not found in PATH")
|
||||
command_line = [self.git_path] + list(args)
|
||||
logging.info(" → Execution (cwd='%s'): %s", self.path, " ".join(command_line))
|
||||
|
||||
|
|
@ -74,7 +75,7 @@ class LocalGitRepo:
|
|||
)
|
||||
raise exception
|
||||
|
||||
def run(self, *args, env: dict = {}):
|
||||
def run(self, *args, env: dict = {}) -> str:
|
||||
return self.run_without_encoding(*args, env=env).decode("utf-8", errors="surrogateescape")
|
||||
|
||||
|
||||
|
|
@ -93,11 +94,19 @@ class SyncRun:
|
|||
servo_pr=self.servo_pr,
|
||||
)
|
||||
|
||||
def add_step(self, step) -> Optional[AsyncValue]:
|
||||
def add_step(
|
||||
self,
|
||||
step: ChangePRStep
|
||||
| CommentStep
|
||||
| CreateOrUpdateBranchForPRStep
|
||||
| MergePRStep
|
||||
| OpenPRStep
|
||||
| RemoveBranchForPRStep,
|
||||
) -> Optional[AsyncValue]:
|
||||
self.steps.append(step)
|
||||
return step.provides()
|
||||
|
||||
def run(self):
|
||||
def run(self) -> None:
|
||||
# This loop always removes the first step and runs it, because
|
||||
# individual steps can modify the list of steps. For instance, if a
|
||||
# step fails, it might clear the remaining steps and replace them with
|
||||
|
|
@ -142,7 +151,13 @@ class WPTSync:
|
|||
github_name: str
|
||||
suppress_force_push: bool = False
|
||||
|
||||
def __post_init__(self):
|
||||
servo: GithubRepository = field(init=False)
|
||||
wpt: GithubRepository = field(init=False)
|
||||
downstream_wpt: GithubRepository = field(init=False)
|
||||
local_servo_repo: LocalGitRepo = field(init=False)
|
||||
local_wpt_repo: LocalGitRepo = field(init=False)
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
self.servo = GithubRepository(self, self.servo_repo, "main")
|
||||
self.wpt = GithubRepository(self, self.wpt_repo, "master")
|
||||
self.downstream_wpt = GithubRepository(self, self.downstream_wpt_repo, "master")
|
||||
|
|
@ -194,7 +209,7 @@ class WPTSync:
|
|||
logging.error(exception, exc_info=True)
|
||||
return False
|
||||
|
||||
def handle_new_pull_request_contents(self, run: SyncRun, pull_data: dict):
|
||||
def handle_new_pull_request_contents(self, run: SyncRun, pull_data: dict) -> None:
|
||||
num_commits = pull_data["commits"]
|
||||
head_sha = pull_data["head"]["sha"]
|
||||
is_upstreamable = (
|
||||
|
|
@ -243,13 +258,13 @@ class WPTSync:
|
|||
# Leave a comment to the new pull request in the original pull request.
|
||||
run.add_step(CommentStep(run.servo_pr, OPENED_NEW_UPSTREAM_PR))
|
||||
|
||||
def handle_edited_pull_request(self, run: SyncRun, pull_data: dict):
|
||||
def handle_edited_pull_request(self, run: SyncRun, pull_data: dict) -> None:
|
||||
logging.info("Changing upstream PR title")
|
||||
if run.upstream_pr.has_value():
|
||||
run.add_step(ChangePRStep(run.upstream_pr.value(), "open", pull_data["title"], pull_data["body"]))
|
||||
run.add_step(CommentStep(run.servo_pr, UPDATED_TITLE_IN_EXISTING_UPSTREAM_PR))
|
||||
|
||||
def handle_closed_pull_request(self, run: SyncRun, pull_data: dict):
|
||||
def handle_closed_pull_request(self, run: SyncRun, pull_data: dict) -> None:
|
||||
logging.info("Processing closed PR")
|
||||
if not run.upstream_pr.has_value():
|
||||
# If we don't recognize this PR, it never contained upstreamable changes.
|
||||
|
|
|
|||
|
|
@ -43,5 +43,5 @@ COULD_NOT_MERGE_CHANGES_UPSTREAM_COMMENT = (
|
|||
)
|
||||
|
||||
|
||||
def wpt_branch_name_from_servo_pr_number(servo_pr_number):
|
||||
def wpt_branch_name_from_servo_pr_number(servo_pr_number) -> str:
|
||||
return f"servo_export_{servo_pr_number}"
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ day be entirely replaced with something like PyGithub."""
|
|||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import urllib
|
||||
import urllib.parse
|
||||
|
||||
from typing import Optional, TYPE_CHECKING
|
||||
|
||||
|
|
@ -29,7 +29,7 @@ USER_AGENT = "Servo web-platform-test sync service"
|
|||
TIMEOUT = 30 # 30 seconds
|
||||
|
||||
|
||||
def authenticated(sync: WPTSync, method, url, json=None) -> requests.Response:
|
||||
def authenticated(sync: WPTSync, method: str, url: str, json=None) -> requests.Response:
|
||||
logging.info(" → Request: %s %s", method, url)
|
||||
if json:
|
||||
logging.info(" → Request JSON: %s", json)
|
||||
|
|
@ -51,14 +51,14 @@ class GithubRepository:
|
|||
This class allows interacting with a single GitHub repository.
|
||||
"""
|
||||
|
||||
def __init__(self, sync: WPTSync, repo: str, default_branch: str):
|
||||
def __init__(self, sync: WPTSync, repo: str, default_branch: str) -> None:
|
||||
self.sync = sync
|
||||
self.repo = repo
|
||||
self.default_branch = default_branch
|
||||
self.org = repo.split("/")[0]
|
||||
self.pulls_url = f"repos/{self.repo}/pulls"
|
||||
|
||||
def __str__(self):
|
||||
def __str__(self) -> str:
|
||||
return self.repo
|
||||
|
||||
def get_pull_request(self, number: int) -> PullRequest:
|
||||
|
|
@ -94,7 +94,7 @@ class GithubRepository:
|
|||
|
||||
return self.get_pull_request(json["items"][0]["number"])
|
||||
|
||||
def open_pull_request(self, branch: GithubBranch, title: str, body: str):
|
||||
def open_pull_request(self, branch: GithubBranch, title: str, body: str) -> PullRequest:
|
||||
data = {
|
||||
"title": title,
|
||||
"head": branch.get_pr_head_reference_for_repo(self),
|
||||
|
|
@ -107,11 +107,11 @@ class GithubRepository:
|
|||
|
||||
|
||||
class GithubBranch:
|
||||
def __init__(self, repo: GithubRepository, branch_name: str):
|
||||
def __init__(self, repo: GithubRepository, branch_name: str) -> None:
|
||||
self.repo = repo
|
||||
self.name = branch_name
|
||||
|
||||
def __str__(self):
|
||||
def __str__(self) -> str:
|
||||
return f"{self.repo}/{self.name}"
|
||||
|
||||
def get_pr_head_reference_for_repo(self, other_repo: GithubRepository) -> str:
|
||||
|
|
@ -128,20 +128,20 @@ class PullRequest:
|
|||
This class allows interacting with a single pull request on GitHub.
|
||||
"""
|
||||
|
||||
def __init__(self, repo: GithubRepository, number: int):
|
||||
def __init__(self, repo: GithubRepository, number: int) -> None:
|
||||
self.repo = repo
|
||||
self.context = repo.sync
|
||||
self.number = number
|
||||
self.base_url = f"repos/{self.repo.repo}/pulls/{self.number}"
|
||||
self.base_issues_url = f"repos/{self.repo.repo}/issues/{self.number}"
|
||||
|
||||
def __str__(self):
|
||||
def __str__(self) -> str:
|
||||
return f"{self.repo}#{self.number}"
|
||||
|
||||
def api(self, *args, **kwargs) -> requests.Response:
|
||||
return authenticated(self.context, *args, **kwargs)
|
||||
|
||||
def leave_comment(self, comment: str):
|
||||
def leave_comment(self, comment: str) -> requests.Response:
|
||||
return self.api("POST", f"{self.base_issues_url}/comments", json={"body": comment})
|
||||
|
||||
def change(
|
||||
|
|
@ -149,7 +149,7 @@ class PullRequest:
|
|||
state: Optional[str] = None,
|
||||
title: Optional[str] = None,
|
||||
body: Optional[str] = None,
|
||||
):
|
||||
) -> requests.Response:
|
||||
data = {}
|
||||
if title:
|
||||
data["title"] = title
|
||||
|
|
@ -159,11 +159,11 @@ class PullRequest:
|
|||
data["state"] = state
|
||||
return self.api("PATCH", self.base_url, json=data)
|
||||
|
||||
def remove_label(self, label: str):
|
||||
def remove_label(self, label: str) -> None:
|
||||
self.api("DELETE", f"{self.base_issues_url}/labels/{label}")
|
||||
|
||||
def add_labels(self, labels: list[str]):
|
||||
def add_labels(self, labels: list[str]) -> None:
|
||||
self.api("POST", f"{self.base_issues_url}/labels", json=labels)
|
||||
|
||||
def merge(self):
|
||||
def merge(self) -> None:
|
||||
self.api("PUT", f"{self.base_url}/merge", json={"merge_method": "rebase"})
|
||||
|
|
|
|||
|
|
@ -36,13 +36,13 @@ PATCH_FILE_NAME = "tmp.patch"
|
|||
|
||||
|
||||
class Step:
|
||||
def __init__(self, name):
|
||||
def __init__(self, name) -> None:
|
||||
self.name = name
|
||||
|
||||
def provides(self) -> Optional[AsyncValue]:
|
||||
return None
|
||||
|
||||
def run(self, _: SyncRun):
|
||||
def run(self, run: SyncRun) -> None:
|
||||
return
|
||||
|
||||
|
||||
|
|
@ -50,31 +50,31 @@ T = TypeVar("T")
|
|||
|
||||
|
||||
class AsyncValue(Generic[T]):
|
||||
def __init__(self, value: Optional[T] = None):
|
||||
def __init__(self, value: Optional[T] = None) -> None:
|
||||
self._value = value
|
||||
|
||||
def resolve(self, value: T):
|
||||
def resolve(self, value: Optional[T]) -> None:
|
||||
self._value = value
|
||||
|
||||
def value(self) -> T:
|
||||
assert self._value is not None
|
||||
return self._value
|
||||
|
||||
def has_value(self):
|
||||
def has_value(self) -> bool:
|
||||
return self._value is not None
|
||||
|
||||
|
||||
class CreateOrUpdateBranchForPRStep(Step):
|
||||
def __init__(self, pull_data: dict, pull_request: PullRequest):
|
||||
def __init__(self, pull_data: dict, pull_request: PullRequest) -> None:
|
||||
Step.__init__(self, "CreateOrUpdateBranchForPRStep")
|
||||
self.pull_data = pull_data
|
||||
self.pull_request = pull_request
|
||||
self.branch: AsyncValue[GithubBranch] = AsyncValue()
|
||||
|
||||
def provides(self):
|
||||
def provides(self) -> AsyncValue[GithubBranch]:
|
||||
return self.branch
|
||||
|
||||
def run(self, run: SyncRun):
|
||||
def run(self, run: SyncRun) -> None:
|
||||
try:
|
||||
commits = self._get_upstreamable_commits_from_local_servo_repo(run.sync)
|
||||
branch_name = self._create_or_update_branch_for_pr(run, commits)
|
||||
|
|
@ -128,7 +128,7 @@ class CreateOrUpdateBranchForPRStep(Step):
|
|||
]
|
||||
return filtered_commits
|
||||
|
||||
def _apply_filtered_servo_commit_to_wpt(self, run: SyncRun, commit: dict):
|
||||
def _apply_filtered_servo_commit_to_wpt(self, run: SyncRun, commit: dict) -> None:
|
||||
patch_path = os.path.join(run.sync.wpt_path, PATCH_FILE_NAME)
|
||||
strip_count = UPSTREAMABLE_PATH.count("/") + 1
|
||||
|
||||
|
|
@ -143,7 +143,7 @@ class CreateOrUpdateBranchForPRStep(Step):
|
|||
run.sync.local_wpt_repo.run("add", "--all")
|
||||
run.sync.local_wpt_repo.run("commit", "--message", commit["message"], "--author", commit["author"])
|
||||
|
||||
def _create_or_update_branch_for_pr(self, run: SyncRun, commits: list[dict], pre_commit_callback=None):
|
||||
def _create_or_update_branch_for_pr(self, run: SyncRun, commits: list[dict], pre_commit_callback=None) -> str:
|
||||
branch_name = wpt_branch_name_from_servo_pr_number(self.pull_data["number"])
|
||||
try:
|
||||
# Create a new branch with a unique name that is consistent between
|
||||
|
|
@ -169,7 +169,6 @@ class CreateOrUpdateBranchForPRStep(Step):
|
|||
remote_url = f"https://{user}:{token}@github.com/{repo}.git"
|
||||
run.sync.local_wpt_repo.run("push", "-f", remote_url, branch_name)
|
||||
|
||||
return branch_name
|
||||
finally:
|
||||
try:
|
||||
run.sync.local_wpt_repo.run("checkout", "master")
|
||||
|
|
@ -177,13 +176,15 @@ class CreateOrUpdateBranchForPRStep(Step):
|
|||
except Exception:
|
||||
pass
|
||||
|
||||
return branch_name
|
||||
|
||||
|
||||
class RemoveBranchForPRStep(Step):
|
||||
def __init__(self, pull_request):
|
||||
def __init__(self, pull_request) -> None:
|
||||
Step.__init__(self, "RemoveBranchForPRStep")
|
||||
self.branch_name = wpt_branch_name_from_servo_pr_number(pull_request["number"])
|
||||
|
||||
def run(self, run: SyncRun):
|
||||
def run(self, run: SyncRun) -> None:
|
||||
self.name += f":{run.sync.downstream_wpt.get_branch(self.branch_name)}"
|
||||
logging.info(" -> Removing branch used for upstream PR")
|
||||
if not run.sync.suppress_force_push:
|
||||
|
|
@ -201,7 +202,7 @@ class ChangePRStep(Step):
|
|||
state: str,
|
||||
title: Optional[str] = None,
|
||||
body: Optional[str] = None,
|
||||
):
|
||||
) -> None:
|
||||
name = f"ChangePRStep:{pull_request}:{state}"
|
||||
if title:
|
||||
name += f":{title}"
|
||||
|
|
@ -212,7 +213,7 @@ class ChangePRStep(Step):
|
|||
self.title = title
|
||||
self.body = body
|
||||
|
||||
def run(self, run: SyncRun):
|
||||
def run(self, run: SyncRun) -> None:
|
||||
body = self.body
|
||||
if body:
|
||||
body = run.prepare_body_text(body)
|
||||
|
|
@ -222,12 +223,12 @@ class ChangePRStep(Step):
|
|||
|
||||
|
||||
class MergePRStep(Step):
|
||||
def __init__(self, pull_request: PullRequest, labels_to_remove: list[str] = []):
|
||||
def __init__(self, pull_request: PullRequest, labels_to_remove: list[str] = []) -> None:
|
||||
Step.__init__(self, f"MergePRStep:{pull_request}")
|
||||
self.pull_request = pull_request
|
||||
self.labels_to_remove = labels_to_remove
|
||||
|
||||
def run(self, run: SyncRun):
|
||||
def run(self, run: SyncRun) -> None:
|
||||
try:
|
||||
for label in self.labels_to_remove:
|
||||
self.pull_request.remove_label(label)
|
||||
|
|
@ -250,7 +251,7 @@ class OpenPRStep(Step):
|
|||
title: str,
|
||||
body: str,
|
||||
labels: list[str],
|
||||
):
|
||||
) -> None:
|
||||
Step.__init__(self, "OpenPRStep")
|
||||
self.title = title
|
||||
self.body = body
|
||||
|
|
@ -259,10 +260,10 @@ class OpenPRStep(Step):
|
|||
self.new_pr: AsyncValue[PullRequest] = AsyncValue()
|
||||
self.labels = labels
|
||||
|
||||
def provides(self):
|
||||
def provides(self) -> AsyncValue[PullRequest]:
|
||||
return self.new_pr
|
||||
|
||||
def run(self, run: SyncRun):
|
||||
def run(self, run: SyncRun) -> None:
|
||||
pull_request = self.target_repo.open_pull_request(
|
||||
self.source_branch.value(), self.title, run.prepare_body_text(self.body)
|
||||
)
|
||||
|
|
@ -276,12 +277,12 @@ class OpenPRStep(Step):
|
|||
|
||||
|
||||
class CommentStep(Step):
|
||||
def __init__(self, pull_request: PullRequest, comment_template: str):
|
||||
def __init__(self, pull_request: PullRequest, comment_template: str) -> None:
|
||||
Step.__init__(self, "CommentStep")
|
||||
self.pull_request = pull_request
|
||||
self.comment_template = comment_template
|
||||
|
||||
def run(self, run: SyncRun):
|
||||
def run(self, run: SyncRun) -> None:
|
||||
comment = run.make_comment(self.comment_template)
|
||||
self.name += f":{self.pull_request}:{comment}"
|
||||
self.pull_request.leave_comment(comment)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue