From f4b5b9f85fe6e4e002a8b935e41f3072f6f19120 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Fri, 7 Apr 2023 11:57:39 +0200 Subject: [PATCH] Fix the WPT export script - Have the WPT exporter script use the WPT_SYNC_TOKEN for checking out wpt. - Make sure the local WPT repository is unshallow when pushing. - When searching for existing PRs use the main GitHub search API, as the pull request search does not seem to properly process the "head" parameter. - When deleting branches in the downstream WPT repository, use the full URL to avoid trying to modify the upstream repository. --- .github/workflows/upstream-wpt-changes.yml | 7 ++++- etc/ci/upstream-wpt-changes/test.py | 27 +++++++++++++++---- .../wptupstreamer/github.py | 26 +++++++++++++----- .../wptupstreamer/step.py | 12 ++++++++- 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/.github/workflows/upstream-wpt-changes.yml b/.github/workflows/upstream-wpt-changes.yml index 97b29804e94..02afe64c750 100644 --- a/.github/workflows/upstream-wpt-changes.yml +++ b/.github/workflows/upstream-wpt-changes.yml @@ -25,7 +25,12 @@ jobs: with: path: wpt repository: 'web-platform-tests/wpt' - token: ${{ secrets.GITHUB_TOKEN }} + # The token here must be the token that we will use to push to the + # WPT repository and not the token used for GitHub actions, because + # the checkout action sets up an `extraheader` authorization override + # using the token specified here. + # See https://github.com/actions/checkout/issues/162. + token: ${{ secrets.WPT_SYNC_TOKEN }} - name: Install requirements run: pip install -r servo/etc/ci/upstream-wpt-changes/requirements.txt - name: Process pull request diff --git a/etc/ci/upstream-wpt-changes/test.py b/etc/ci/upstream-wpt-changes/test.py index b40af2a48ee..c56831360ba 100644 --- a/etc/ci/upstream-wpt-changes/test.py +++ b/etc/ci/upstream-wpt-changes/test.py @@ -131,12 +131,29 @@ class MockGitHubAPIServer(): return ('', 204) return ('', 404) - @self.app.route("/repos///pulls", methods=['GET']) - def get_pulls(org, repo): + @self.app.route("/search/issues", methods=['GET']) + def search(): + params = {} + param_strings = flask.request.args.get("q", "").split(" ") + for string in param_strings: + parts = string.split(":") + params[parts[0]] = parts[1] + + assert params["is"] == "pr" + assert params["state"] == "open" + assert "author" in params + assert "head" in params + head_ref = f"{params['author']}:{params['head']}" + for pull_request in self.pulls: - if pull_request.head == flask.request.args["head"]: - return json.dumps([{"number": pull_request.number}]) - return json.dumps([]) + if pull_request.head == head_ref: + return json.dumps({ + "total_count": 1, + "items": [{ + "number": pull_request.number + }] + }) + return json.dumps({"total_count": 0, "items": []}) @self.app.route("/repos///pulls", methods=['POST']) def create_pull_request(org, repo): diff --git a/etc/ci/upstream-wpt-changes/wptupstreamer/github.py b/etc/ci/upstream-wpt-changes/wptupstreamer/github.py index 4905a841fe2..31fa5bb93c3 100644 --- a/etc/ci/upstream-wpt-changes/wptupstreamer/github.py +++ b/etc/ci/upstream-wpt-changes/wptupstreamer/github.py @@ -76,18 +76,30 @@ class GithubRepository: """If this repository has an open pull request with the given source head reference targeting the master branch, return the first matching pull request, otherwise return None.""" - # Frustratingly, this is different from what you need for opening a pull request. - head = f"{branch.repo.org}:{branch.name}" - response = authenticated( - self.sync, "GET", f"{self.pulls_url}?head={head}&base=master&state=open" - ) + + params = "+".join([ + "is:pr", + "state:open", + f"repo:{self.repo}", + f"author:{branch.repo.org}", + f"head:{branch.name}", + ]) + response = authenticated(self.sync, "GET", f"search/issues?q={params}") if int(response.status_code / 100) != 2: return None json = response.json() - if not json or not isinstance(json, list): + if not isinstance(json, dict) or \ + "total_count" not in json or \ + "items" not in json: + raise ValueError( + f"Got unexpected response from GitHub search: {response.text}" + ) + + if json["total_count"] < 1: return None - return self.get_pull_request(json[0]["number"]) + + return self.get_pull_request(json["items"][0]["number"]) def open_pull_request(self, branch: GithubBranch, title: str, body: str): data = { diff --git a/etc/ci/upstream-wpt-changes/wptupstreamer/step.py b/etc/ci/upstream-wpt-changes/wptupstreamer/step.py index 3c16bf88c81..8c9d5c9a93b 100644 --- a/etc/ci/upstream-wpt-changes/wptupstreamer/step.py +++ b/etc/ci/upstream-wpt-changes/wptupstreamer/step.py @@ -172,6 +172,12 @@ class CreateOrUpdateBranchForPRStep(Step): # Push the branch upstream (forcing to overwrite any existing changes). if not run.sync.suppress_force_push: + + # In order to push to our downstream branch we need to ensure that + # the local repository isn't a shallow clone. Shallow clones are + # commonly created by GitHub actions. + run.sync.local_wpt_repo.run("fetch", "--unshallow", "origin") + user = run.sync.github_username token = run.sync.github_api_token repo = run.sync.downstream_wpt_repo @@ -198,7 +204,11 @@ class RemoveBranchForPRStep(Step): 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: - run.sync.local_wpt_repo.run("push", "origin", "--delete", + user = run.sync.github_username + token = run.sync.github_api_token + repo = run.sync.downstream_wpt_repo + remote_url = f"https://{user}:{token}@github.com/{repo}.git" + run.sync.local_wpt_repo.run("push", remote_url, "--delete", self.branch_name)