Pin Chromium revision for binary installs #33855

DanielRyanSmith · 2022-04-28T23:45:42Z

This change allows the pinning of a Chromium revision that has been tested against the WPT GitHub check suite.

Cloud Functions are run on WPT's GCP account that identifies the most recent Chromium revision that is mutually available to all major platforms (Linux, Mac, Windows). This revision is updated as an external artifact, and a test run is initiated on this draft PR. If all checks are successful, the revision is pinned as an an additional external artifact. When a user installs Chromium via ./wpt install, The revision installed will use this revision unless specified otherwise.

The --revision flag is now available for invocations of ./wpt install. This will allow the user to specify which Chromium revision to install. This flag allows for pinned and latest arguments, which install the pinned revision or latest available revision respectively.

Chromium binaries are now installed noting the revision they are associated with. If a webdriver is installed to match a Chromium binary, or vice versa, this revision will be referenced and used to match.

This change does NOT switch any testing to use Chromium instead of Chrome. The code for installing Chromium has been added to tools/ci/run_tc.py but should not be part of the CI process.

foolip · 2022-04-29T12:52:43Z

@DanielRyanSmith what I suggested was based on my understanding of what https://github.com/puppeteer/puppeteer/blob/main/utils/check_availability.js does.

Running node utils/check_availability.js 997451 996554 shows that 996554 has all the binaries, but also reveals how sparse the builds are. I had assumed that we build each revision and that binaries are only rarely missing due to a build error or infra problem, but that isn't at all the case. Rather, it looks like we have continuous builders that always build the latest revision when they become free, with no coordination between them.

To help judge if this is good enough, could you list the mutually available revisions in the past week? Does it happen every day, or less often?

tools/wpt/update_chromium_revision.py

.github/workflows/update_chromium_revision.yml

tools/wpt/update_chromium_revision.py

DanielRyanSmith · 2022-05-02T04:57:48Z

@foolip

To help judge if this is good enough, could you list the mutually available revisions in the past week? Does it happen every day, or less often?

I checked all revisions that were added in a roughly 1-week span from April 25th to March 2nd (995555-998216) and found 33 revisions that had a download available for every platform. That's a larger number than I expected, and perhaps the previous period we tested for had an unusually low number of viable revisions. If it is the case that we cannot find a new revision for a few days, will that be a significant problem?

Also, if it's of interest, these are the numbers that were found to be viable:

foolip · 2022-05-02T18:04:59Z

tools/ci/run_tc.py

+    revisions_path = os.path.join(os.getcwd(), "tools", "wpt", "latest_chromium_revision.txt")
+    with open(revisions_path) as f:
+        revision = f.read().strip()
+        dest = os.path.join("/tmp")


Isn't this the same as just "/tmp"?

foolip · 2022-05-02T18:08:10Z

tools/ci/run_tc.py

+               f"{revision}/chrome-linux.zip")
+        resp = get(url)
+        with open(installer_path, "wb") as f:
+            f.write(resp.content)


Does this end up reading the whole file into memory and then to disk? Can it be streamed to disk, or unzipped directly from the network? Given how big the archives are it seems like it could make a measurable difference. Worth timing this before doing too much work optimizing it.

I'll take a look into it. Additionally, I think it might be worth it to remove this CI installation change entirely from this PR so that we can implement everything in a separate PR involved in changing to using Chromium for the CI.

jgraham · 2022-05-03T10:28:18Z

Is the plan here to get daily PRs to update Chromium? Unless those are merged automatically it feels like it's going to be quite spammy.

foolip · 2022-05-03T10:35:28Z

@jgraham Yes, we'll start with merging them manually and then look at making wpt-pr-bot approve them and merge if CI passes. Would it address the spamminess if we make wpt-pr-bot not assign any human reviewers for these PRs?

foolip · 2022-05-03T10:39:47Z

.github/workflows/update_chromium_revision.yml

+      with:
+        token: ${{ secrets.GITHUB_TOKEN }}
+        author: wpt-pr-bot <[email protected]>
+        title: "Update latest available Chromium revisions"


Can you update the commit/PR wording to match recent changes? For this line, maybe:

Suggested change

title: "Update latest available Chromium revisions"

title: "Update pinned Chromium revision"

I think the commit message should also be the same as the PR title.

jgraham · 2022-05-04T18:46:55Z

Maybe? I expect I'll see it a bit anyway, and it creates a bunch of extra churn for syncs (that we also get from frequently updated deps like hypothesis, but still).

In theory it could be avoidable if you could instead get the latest revision written to an external artifact at a knowable URL (I assume the GH API enables this kind of thing, but I haven't looked in detail). The disadvantage would be that there's a race where different CI systems could test the same revision with different versions of Chromium (that's solveable within a specific CI system by making the testrunning jobs depend on a revision-fetching job, like we do for Firefox in taskcuster).

Assuming we go ahead with this approach, what's the tradeoff between the pr-bot merging the PR and the GH action itself merging the PR? It seems like the latter would be simpler, but maybe has worse security properties?

DanielRyanSmith · 2022-05-04T20:38:59Z

@jgraham, @foolip and I were speaking about this earlier and explicitly brought up the idea of keeping this revision as an external artifact like you mentioned. The initial hesitance there is that we want to ensure that the revision chosen will pass the CI before being changed. I have not looked into the possibility of having the GH action merge the PR, but if we can ensure there will be no issues with the CI when this revision is changed, then that is a probable approach.

DanielRyanSmith · 2022-07-06T15:40:55Z

Hey all, I dropped work on this change but now I've updated it: I've implemented this to reference an external artifact in GCP. It uses cloud functions to check for revisions daily and run the new revisions against the CI in this draft PR using the GitHub API. The new revision will be pinned if everything looks good. The draft PR is just for the tests to run and there should be no need anymore for daily PRs.

foolip

If we could have a --revision=latest that uses the current behavior, I think this is good to go.

The pinned Chromium revision now lives in cloud storage. Updates will be checked for it daily via a cloud function.

DanielRyanSmith marked this pull request as ready for review April 29, 2022 06:09

wpt-pr-bot added ci GitHub Actions infra wpt labels Apr 29, 2022

wpt-pr-bot assigned foolip Apr 29, 2022

foolip mentioned this pull request Apr 29, 2022

fix: apparent typo SUPPORTER_PLATFORMS puppeteer/puppeteer#8294

Merged

foolip reviewed Apr 29, 2022

View reviewed changes

DanielRyanSmith force-pushed the pin-chromium branch from 662146a to f536b64 Compare May 2, 2022 02:51

DanielRyanSmith force-pushed the pin-chromium branch from 090b799 to c87408e Compare May 2, 2022 17:13

wpt-pr-bot requested a review from jgraham May 2, 2022 17:13

web-platform-tests deleted a comment May 2, 2022

foolip reviewed May 2, 2022

View reviewed changes

DanielRyanSmith force-pushed the pin-chromium branch from 894e5bd to 641f0c1 Compare May 2, 2022 19:45

foolip reviewed May 3, 2022

View reviewed changes

DanielRyanSmith force-pushed the pin-chromium branch from 641f0c1 to 3e41678 Compare May 4, 2022 02:09

DanielRyanSmith force-pushed the pin-chromium branch 2 times, most recently from 8efe8c9 to 597f808 Compare May 4, 2022 23:16

DanielRyanSmith force-pushed the pin-chromium branch 3 times, most recently from 9532c3d to 68c2a06 Compare June 29, 2022 21:40

foolip approved these changes Jul 19, 2022

View reviewed changes

Pin Chromium revision for install

c1e34e7

DanielRyanSmith added 12 commits July 20, 2022 12:02

Apply suggestions from @foolip code review

b7fe734

Add revision flag for installation

5081ee8

docstring description change

219254f

Pinned revision is now external artifact

59a06ec

The pinned Chromium revision now lives in cloud storage. Updates will be checked for it daily via a cloud function.

Use pinned revision if a revision isn't detected

de2989c

make PR in correct repo

f89aa40

mypy changes

28cee4a

mypy changes

b535c73

remove GH workflow

89ffde7

update logic for obtaining pinned revision

2e53614

Add case for --revision=latest flag

c76caaa

comment change

10d2f64

DanielRyanSmith force-pushed the pin-chromium branch from 6b87fa0 to 10d2f64 Compare July 20, 2022 19:02

DanielRyanSmith closed this Jul 25, 2022

DanielRyanSmith reopened this Jul 25, 2022

moz-wptsync-bot added the mozilla:gecko-blocked label Jul 25, 2022

DanielRyanSmith merged commit c787cac into web-platform-tests:master Aug 9, 2022

moz-wptsync-bot removed the mozilla:gecko-blocked label Aug 9, 2022

DanielRyanSmith deleted the pin-chromium branch August 9, 2022 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pin Chromium revision for binary installs #33855

Pin Chromium revision for binary installs #33855

DanielRyanSmith commented Apr 28, 2022 •

edited

Loading

foolip commented Apr 29, 2022

DanielRyanSmith commented May 2, 2022

foolip May 2, 2022

foolip May 2, 2022

DanielRyanSmith May 2, 2022

jgraham commented May 3, 2022

foolip commented May 3, 2022

foolip May 3, 2022

jgraham commented May 4, 2022

DanielRyanSmith commented May 4, 2022

DanielRyanSmith commented Jul 6, 2022

foolip left a comment

	title: "Update latest available Chromium revisions"
	title: "Update pinned Chromium revision"

Pin Chromium revision for binary installs #33855

Pin Chromium revision for binary installs #33855

Conversation

DanielRyanSmith commented Apr 28, 2022 • edited Loading

foolip commented Apr 29, 2022

DanielRyanSmith commented May 2, 2022

foolip May 2, 2022

Choose a reason for hiding this comment

foolip May 2, 2022

Choose a reason for hiding this comment

DanielRyanSmith May 2, 2022

Choose a reason for hiding this comment

jgraham commented May 3, 2022

foolip commented May 3, 2022

foolip May 3, 2022

Choose a reason for hiding this comment

jgraham commented May 4, 2022

DanielRyanSmith commented May 4, 2022

DanielRyanSmith commented Jul 6, 2022

foolip left a comment

Choose a reason for hiding this comment

DanielRyanSmith commented Apr 28, 2022 •

edited

Loading