Skip to content

Commit

Permalink
devops: encode build number together with Chromium revision (#3769)
Browse files Browse the repository at this point in the history
This is an alternative approach to #3698 that was setting up a custom
mapping between chromium revisions and our mirrored builds. For example, we were
taking chromium `792639` and re-packaging it to our CDN as Chromium 1000.

One big downside of this opaque mapping was inability to quickly
understand which Chromium is mirrored to CDN.

To solve this, this patch starts treating browser revision as a fractional number,
with and integer part being a chromium revision, and fractional
part being our build number. For example, we can generate builds `792639`, `792639.1`,
`792639.2` etc, all of which will pick Chromium `792639` and re-package it to our CDN.

In the Playwright code itself, there are a handful of places that treat
browser revision as integer, exclusively to compare revision with some particular
revision numbers. This code would still work as-is, but I changed these places
to use `parseFloat` instead of `parseInt` for correctness.
  • Loading branch information
aslushnikov authored Sep 4, 2020
1 parent dfc0006 commit a755d10
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 6 deletions.
11 changes: 7 additions & 4 deletions browser_patches/chromium/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ mkdir -p output
cd output

BUILD_NUMBER=$(head -1 ../BUILD_NUMBER)
# Support BUILD_NUMBER in the form of <CRREV>.<GENERATION>
# This will allow us to bump generation to produce new builds.
CRREV="${BUILD_NUMBER%.*}
CHROMIUM_URL=""
CHROMIUM_FOLDER_NAME=""
Expand All @@ -20,24 +23,24 @@ FFMPEG_URL=""
FFMPEG_BIN_PATH=""
if [[ $1 == "--win32" ]]; then
CHROMIUM_URL="https://storage.googleapis.com/chromium-browser-snapshots/Win/${BUILD_NUMBER}/chrome-win.zip"
CHROMIUM_URL="https://storage.googleapis.com/chromium-browser-snapshots/Win/${CRREV}/chrome-win.zip"
CHROMIUM_FOLDER_NAME="chrome-win"
CHROMIUM_FILES_TO_REMOVE+=("chrome-win/interactive_ui_tests.exe")
FFMPEG_URL="https://playwright2.blob.core.windows.net/builds/ffmpeg/${FFMPEG_VERSION}/ffmpeg-win32.zip"
FFMPEG_BIN_PATH="ffmpeg.exe"
elif [[ $1 == "--win64" ]]; then
CHROMIUM_URL="https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/${BUILD_NUMBER}/chrome-win.zip"
CHROMIUM_URL="https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/${CRREV}/chrome-win.zip"
CHROMIUM_FOLDER_NAME="chrome-win"
CHROMIUM_FILES_TO_REMOVE+=("chrome-win/interactive_ui_tests.exe")
FFMPEG_URL="https://playwright2.blob.core.windows.net/builds/ffmpeg/${FFMPEG_VERSION}/ffmpeg-win64.zip"
FFMPEG_BIN_PATH="ffmpeg.exe"
elif [[ $1 == "--mac" ]]; then
CHROMIUM_URL="https://storage.googleapis.com/chromium-browser-snapshots/Mac/${BUILD_NUMBER}/chrome-mac.zip"
CHROMIUM_URL="https://storage.googleapis.com/chromium-browser-snapshots/Mac/${CRREV}/chrome-mac.zip"
CHROMIUM_FOLDER_NAME="chrome-mac"
FFMPEG_URL="https://playwright2.blob.core.windows.net/builds/ffmpeg/${FFMPEG_VERSION}/ffmpeg-mac.zip"
FFMPEG_BIN_PATH="ffmpeg"
elif [[ $1 == "--linux" ]]; then
CHROMIUM_URL="https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/${BUILD_NUMBER}/chrome-linux.zip"
CHROMIUM_URL="https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/${CRREV}/chrome-linux.zip"
CHROMIUM_FOLDER_NAME="chrome-linux"
# Even though we could bundle ffmpeg on Linux (2.5MB zipped), we
# prefer rely on system-installed ffmpeg instead.
Expand Down
2 changes: 1 addition & 1 deletion src/install/browserFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ function getDownloadUrl(browserName: BrowserName, revision: number, platform: Br
}

function revisionURL(browser: BrowserDescriptor, platform = browserPaths.hostPlatform): string {
const revision = parseInt(browser.revision, 10);
const revision = parseFloat(browser.revision);
const serverHost = getDownloadHost(browser.name, revision);
const urlTemplate = getDownloadUrl(browser.name, revision, platform);
assert(urlTemplate, `ERROR: Playwright does not support ${browser.name} on ${platform}`);
Expand Down
2 changes: 1 addition & 1 deletion src/install/installer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ async function validateCache(packagePath: string, browsersPath: string, linksDir
const browsersToDownload = await readBrowsersToDownload(linkTarget);
for (const browser of browsersToDownload) {
const usedBrowserPath = browserPaths.browserDirectory(browsersPath, browser);
const browserRevision = parseInt(browser.revision, 10);
const browserRevision = parseFloat(browser.revision);
// Old browser installations don't have marker file.
const shouldHaveMarkerFile = (browser.name === 'chromium' && browserRevision >= 786218) ||
(browser.name === 'firefox' && browserRevision >= 1128) ||
Expand Down

0 comments on commit a755d10

Please sign in to comment.