Auto merge of #29299 - mrobinson:fix-upstream-wpt-changes, r=jdm

Fix upstreaming of WPT changes

The GitHub Action needs access to Servo repository secrets, so switch to using the 'pull_request_target' event. Since these PRs have more complete access to the Servo repository, do not execute the version of the upstream script that comes with the PR. Instead, simply fetch the changes. To make this work, the script no longer expects the PR commit to be checked out, merely that they exist in the repository somewhere.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

.github/workflows/upstream-wpt-changes.yml

@@ -1,6 +1,6 @@
 name: WPT export
 on:
-  pull_request:
+  pull_request_target:
     types: ['opened', 'synchronize', 'reopened', 'edited', 'closed']
 
 jobs:
@@ -16,7 +16,8 @@ jobs:
           git init -b main
           git remote add origin ${{ github.event.repository.clone_url}}
           git fetch origin pull/${{ github.event.pull_request.number}}/head:pr --depth ${{ env.PR_FETCH_DEPTH }}
-          git checkout pr
+          git fetch origin master:master --depth 1
+          git checkout master
       - name: Check out wpt
         uses: actions/checkout@v3
         with:

etc/ci/upstream-wpt-changes/test.py

@@ -268,9 +268,12 @@ class TestFullSyncRun(unittest.TestCase):
 
     @classmethod
     def tearDownClass(cls):
+        assert cls.server is not None
         cls.server.shutdown()
 
     def tearDown(self):
+        assert SYNC is not None
+
         # Clean up any old files.
         first_commit_hash = SYNC.local_servo_repo.run("rev-list", "HEAD").splitlines()[
             -1
@@ -286,9 +289,9 @@ class TestFullSyncRun(unittest.TestCase):
                 return [diff, "tmp author", "tmp@tmp.com", "tmp commit message"]
             return diff
 
-        commits = [make_commit_data(diff) for diff in diffs]
-
         # Apply each commit to the repository.
+        orig_sha = SYNC.local_servo_repo.run("rev-parse", "HEAD").strip()
+        commits = [make_commit_data(diff) for diff in diffs]
         for commit in commits:
             patch_file, author, email, message = commit
             SYNC.local_servo_repo.run("apply", os.path.join(TESTS_DIR, patch_file))
@@ -306,6 +309,12 @@ class TestFullSyncRun(unittest.TestCase):
                 },
             )
 
+        # Reset the repository to the original hash, but the commits are still
+        # available until the next `git gc`.
+        last_commit_sha = SYNC.local_servo_repo.run("rev-parse", "HEAD").strip()
+        SYNC.local_servo_repo.run("reset", "--hard", orig_sha)
+        return last_commit_sha
+
     def run_test(
         self, payload_file: str, diffs: list, existing_prs: list[MockPullRequest] = []
     ):
@@ -313,7 +322,8 @@ class TestFullSyncRun(unittest.TestCase):
             payload = json.loads(file.read())
 
         logging.info("Mocking application of PR to servo.")
-        self.mock_servo_repository_state(diffs)
+        last_commit_sha = self.mock_servo_repository_state(diffs)
+        payload["pull_request"]["head"]["sha"] = last_commit_sha
 
         logging.info("Resetting server state")
         assert self.server is not None
@@ -618,4 +628,7 @@ def tearDownModule():
 
 
 if __name__ == "__main__":
+    # Uncomment this line to enable verbose logging.
+    # logging.getLogger().setLevel(logging.INFO)
+
     unittest.main()

etc/ci/upstream-wpt-changes/wptupstreamer/__init__.py

@@ -169,7 +169,8 @@ class WPTSync:
                 logging.info(
                     "  → Detected existing upstream PR %s", upstream_pr)
 
-            run = SyncRun(self, servo_pr, AsyncValue(upstream_pr), step_callback)
+            run = SyncRun(self, servo_pr, AsyncValue(
+                upstream_pr), step_callback)
 
             pull_data = payload["pull_request"]
             if payload["action"] in ["opened", "synchronize", "reopened"]:
@@ -189,10 +190,12 @@ class WPTSync:
             return False
 
     def handle_new_pull_request_contents(self, run: SyncRun, pull_data: dict):
+        num_commits = pull_data["commits"]
+        head_sha = pull_data["head"]["sha"]
         is_upstreamable = (
             len(
                 self.local_servo_repo.run(
-                    "diff", f"HEAD~{pull_data['commits']}", "--", UPSTREAMABLE_PATH
+                    "diff", head_sha, f"{head_sha}~{num_commits}", "--", UPSTREAMABLE_PATH
                 )
             )
             > 0
@@ -245,7 +248,8 @@ class WPTSync:
         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.upstream_pr.value(
+                ), "open", pull_data["title"], pull_data["body"]
             ))
             run.add_step(CommentStep(
                 run.servo_pr, UPDATED_TITLE_IN_EXISTING_UPSTREAM_PR))
@@ -258,7 +262,8 @@ class WPTSync:
         if pull_data["merged"]:
             # Since the upstreamable changes have now been merged locally, merge the
             # corresponding upstream PR.
-            run.add_step(MergePRStep(run.upstream_pr.value(), ["do not merge yet"]))
+            run.add_step(MergePRStep(
+                run.upstream_pr.value(), ["do not merge yet"]))
         else:
             # If a PR with upstreamable changes is closed without being merged, we
             # don't want to merge the changes upstream either.

etc/ci/upstream-wpt-changes/wptupstreamer/step.py

@@ -99,8 +99,9 @@ class CreateOrUpdateBranchForPRStep(Step):
     def _get_upstreamable_commits_from_local_servo_repo(self, sync: WPTSync):
         local_servo_repo = sync.local_servo_repo
         number_of_commits = self.pull_data["commits"]
+        pr_head = self.pull_data["head"]["sha"]
         commit_shas = local_servo_repo.run(
-            "log", "--pretty=%H", f"-{number_of_commits}"
+            "log", "--pretty=%H", pr_head, f"-{number_of_commits}"
         ).splitlines()
 
         filtered_commits = []