From 7baf0d4e11249e4d114b1601f71f7358e472496c Mon Sep 17 00:00:00 2001 From: Simon Lecoq <22963968+lowlighter@users.noreply.github.com> Date: Fri, 13 Oct 2023 02:48:44 +0000 Subject: [PATCH 1/6] feat: support shared browser install for concurrent getBinary() calls --- deno.jsonc | 2 +- deno.lock | 2 + src/browser.ts | 13 +++ src/cache.ts | 91 ++++++++++++++++++- ..._get_binary_test.ts => get_binary_test.ts} | 0 tests/install_lock_file_test.ts | 29 ++++++ 6 files changed, 134 insertions(+), 3 deletions(-) rename tests/{_get_binary_test.ts => get_binary_test.ts} (100%) create mode 100644 tests/install_lock_file_test.ts diff --git a/deno.jsonc b/deno.jsonc index ed74976..b63bcb6 100644 --- a/deno.jsonc +++ b/deno.jsonc @@ -2,7 +2,7 @@ "tasks": { // The task to automatically generate `./src/celestial.ts` "bind": "deno run -A ./bindings/_tools/generate/mod.ts && deno fmt", - "test": "deno test -A --trace-ops", + "test": "deno test -A --trace-ops --parallel", "bench": "deno bench -A", "www": "cd docs && pyro dev" }, diff --git a/deno.lock b/deno.lock index 37572f9..e9a2add 100644 --- a/deno.lock +++ b/deno.lock @@ -90,6 +90,8 @@ "https://deno.land/std@0.201.0/testing/snapshot.ts": "fd91f03c258c316bc9faf815846d85e80e0c82622b28ee44b57ec66dd91d3408", "https://deno.land/std@0.203.0/assert/assert.ts": "9a97dad6d98c238938e7540736b826440ad8c1c1e54430ca4c4e623e585607ee", "https://deno.land/std@0.203.0/assert/assertion_error.ts": "4d0bde9b374dfbcbe8ac23f54f567b77024fb67dbb1906a852d67fe050d42f56", + "https://deno.land/std@0.203.0/async/deadline.ts": "58f72a3cc0fcb731b2cc055ba046f4b5be3349ff6bf98f2e793c3b969354aab2", + "https://deno.land/std@0.203.0/async/delay.ts": "a6142eb44cdd856b645086af2b811b1fcce08ec06bb7d50969e6a872ee9b8659", "https://deno.land/std@0.203.0/async/retry.ts": "296fb9c323e1325a69bee14ba947e7da7409a8dd9dd646d70cb51ea0d301f24e", "https://deno.land/std@0.203.0/fs/exists.ts": "cb59a853d84871d87acab0e7936a4dac11282957f8e195102c5a7acb42546bb8", "https://deno.land/x/progress@v1.3.9/deps.ts": "83050e627263931d853ba28b7c15c80bf4be912bea7e0d3d13da2bc0aaf7889d", diff --git a/src/browser.ts b/src/browser.ts index 4f007f0..617cad9 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -7,6 +7,7 @@ import { WEBSOCKET_ENDPOINT_REGEX, websocketReady } from "./util.ts"; async function runCommand( command: Deno.Command, + { windowsErr21RetryAttempts = 60 } = {}, ): Promise<{ process: Deno.ChildProcess; endpoint: string }> { const process = command.spawn(); let endpoint = null; @@ -45,6 +46,18 @@ async function runCommand( } if (error) { + const { code } = await process.status; + stack.push(`Process exited with code ${code}`); + // Handle recoverable error code 21 on Windows + // https://source.chromium.org/chromium/chromium/src/+/main:net/base/net_error_list.h;l=90-91 + if ( + (Deno.build.os === "windows") && (code === 21) && + windowsErr21RetryAttempts > 0 + ) { + return runCommand(command, { + windowsErr21RetryAttempts: windowsErr21RetryAttempts - 1, + }); + } console.error(stack.join("\n")); throw new Error("Your binary refused to boot"); } diff --git a/src/cache.ts b/src/cache.ts index 7cbfab1..5963899 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -5,7 +5,8 @@ import { dirname } from "https://deno.land/std@0.201.0/path/dirname.ts"; import { join } from "https://deno.land/std@0.201.0/path/join.ts"; import { ZipReader } from "https://deno.land/x/zipjs@v2.7.29/index.js"; import ProgressBar from "https://deno.land/x/progress@v1.3.9/mod.ts"; -import { exists } from "https://deno.land/std@0.203.0/fs/exists.ts"; +import { exists, existsSync } from "https://deno.land/std@0.203.0/fs/exists.ts"; +import { retry } from "https://deno.land/std@0.203.0/async/retry.ts"; export const SUPPORTED_VERSIONS = { chrome: "118.0.5943.0", @@ -18,6 +19,8 @@ const HOME_PATH = Deno.build.os === "windows" const BASE_PATH = resolve(HOME_PATH, ".astral"); const CONFIG_FILE = "cache.json"; +const LOCK_FILES = {} as { [cache: string]: { [product: string]: Lock } }; + interface KnownGoodVersions { timestamps: string; versions: { @@ -53,6 +56,7 @@ function getCachedConfig({ cache = BASE_PATH }): Record { export async function cleanCache({ cache = BASE_PATH } = {}) { try { if (await exists(cache)) { + delete LOCK_FILES[cache]; await Deno.remove(cache, { recursive: true }); } } catch (error) { @@ -120,19 +124,42 @@ async function decompressArchive(source: string, destination: string) { /** * Get path for the binary for this OS. Downloads a browser if none is cached. */ -export async function getBinary( +export function getBinary( browser: "chrome" | "firefox", { cache = BASE_PATH } = {}, +): Promise { + const product = `${browser}-${SUPPORTED_VERSIONS[browser]}`; + return _getBinary(browser, { cache }).finally(() => + LOCK_FILES[cache]?.[product]?.release() + ); +} + +export async function _getBinary( + browser: Parameters[0], + { cache = BASE_PATH }: NonNullable[1]>, ): Promise { // TODO(lino-levan): fix firefox downloading const VERSION = SUPPORTED_VERSIONS[browser]; + const product = `${browser}-${SUPPORTED_VERSIONS[browser]}`; const config = getCachedConfig({ cache }); + // If the config doesn't have the revision and there is a lock file, reload config after release + if (!config[VERSION] && LOCK_FILES[cache]?.[product]?.exists()) { + await LOCK_FILES[cache]?.[product]?.waitRelease(); + Object.assign(config, getCachedConfig({ cache })); + } + // If the config doesn't have the revision, download it and return that if (!config[VERSION]) { const quiet = await isQuietInstall(); ensureDirSync(cache); + const lock = new Lock({ cache }); + LOCK_FILES[cache] ??= {}; + LOCK_FILES[cache][product] = lock; + if (!lock.create()) { + return _getBinary(browser, { cache }); + } const versions = await knownGoodVersions(); const version = versions.versions.filter((val) => val.version === VERSION @@ -225,3 +252,63 @@ export async function getBinary( "Unsupported platform, provide a path to a chromium or firefox binary instead", ); } + +/** + * Create a lock file in cache + * Only the process with the same PID can release created lock file through this API + * TODO: Use Deno.flock/Deno.funlock when stabilized (https://deno.land/api@v1.37.1?s=Deno.flock&unstable) + */ +class Lock { + readonly path; + + constructor({ cache = BASE_PATH } = {}) { + this.path = resolve(cache, ".lock"); + } + + /** Returns true if lock file exists */ + exists() { + return existsSync(this.path); + } + + /** Create a lock file and returns true if it succeeds, false if it was already existing */ + create() { + try { + Deno.writeTextFileSync(this.path, `${Deno.pid}`, { createNew: true }); + return true; + } catch (error) { + if (!(error instanceof Deno.errors.AlreadyExists)) { + throw error; + } + return false; + } + } + + /** Release lock file */ + release() { + try { + if (Deno.readTextFileSync(this.path) === `${Deno.pid}`) { + Deno.removeSync(this.path); + } + } catch (error) { + if (!(error instanceof Deno.errors.NotFound)) { + throw error; + } + } + } + + /** Wait for lock release */ + async waitRelease({ timeout = 60000 } = {}) { + await retry(() => { + if (this.exists()) { + throw new Error( + `Timeout while waiting for lockfile release at: ${this.path}`, + ); + } + }, { + maxTimeout: timeout, + maxAttempts: Infinity, + multiplier: 1, + minTimeout: 100, + }); + } +} diff --git a/tests/_get_binary_test.ts b/tests/get_binary_test.ts similarity index 100% rename from tests/_get_binary_test.ts rename to tests/get_binary_test.ts diff --git a/tests/install_lock_file_test.ts b/tests/install_lock_file_test.ts new file mode 100644 index 0000000..e44fe76 --- /dev/null +++ b/tests/install_lock_file_test.ts @@ -0,0 +1,29 @@ +import { getBinary, launch } from "../mod.ts"; +import { deadline } from "https://deno.land/std@0.203.0/async/deadline.ts"; +// Tests should be performed in directory different from others tests as cache is cleaned during this one +//Deno.env.set("ASTRAL_QUIET_INSTALL", "true"); +const cache = await Deno.makeTempDir({ + prefix: "astral_test_install_lock_file", +}); + +Deno.test("Test concurrent getBinary calls", async () => { + // Spawn concurrent getBinary calls + const promises = []; + for (let i = 0; i < 20; i++) { + promises.push(getBinary("chrome", { cache })); + } + const path = await Promise.race(promises); + + // Ensure binary sent by first promise is executable + const browser = await launch({ path }); + + // Other promises should resolve at around the same time as they wait for lock file + await deadline(Promise.all(promises), 250); + + // Ensure binary is still working (no files overwritten) + await browser.newPage("https://example.com"); + await browser.close(); +}); + +// Cleaning +await Deno.remove(cache, { recursive: true }); From f35d54b4480a345f1133109cd3a398c9b9df2254 Mon Sep 17 00:00:00 2001 From: Simon Lecoq <22963968+lowlighter@users.noreply.github.com> Date: Fri, 13 Oct 2023 02:54:50 +0000 Subject: [PATCH 2/6] fix: perm in test issue --- src/cache.ts | 8 ++++---- tests/get_binary_test.ts | 2 +- tests/install_lock_file_test.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cache.ts b/src/cache.ts index 5963899..2cae9d1 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -126,17 +126,17 @@ async function decompressArchive(source: string, destination: string) { */ export function getBinary( browser: "chrome" | "firefox", - { cache = BASE_PATH } = {}, + { cache = BASE_PATH, timeout = 60000 } = {}, ): Promise { const product = `${browser}-${SUPPORTED_VERSIONS[browser]}`; - return _getBinary(browser, { cache }).finally(() => + return _getBinary(browser, { cache, timeout }).finally(() => LOCK_FILES[cache]?.[product]?.release() ); } export async function _getBinary( browser: Parameters[0], - { cache = BASE_PATH }: NonNullable[1]>, + { cache = BASE_PATH, timeout }: NonNullable[1]>, ): Promise { // TODO(lino-levan): fix firefox downloading const VERSION = SUPPORTED_VERSIONS[browser]; @@ -146,7 +146,7 @@ export async function _getBinary( // If the config doesn't have the revision and there is a lock file, reload config after release if (!config[VERSION] && LOCK_FILES[cache]?.[product]?.exists()) { - await LOCK_FILES[cache]?.[product]?.waitRelease(); + await LOCK_FILES[cache]?.[product]?.waitRelease({ timeout }); Object.assign(config, getCachedConfig({ cache })); } diff --git a/tests/get_binary_test.ts b/tests/get_binary_test.ts index 50340eb..1ccc82d 100644 --- a/tests/get_binary_test.ts +++ b/tests/get_binary_test.ts @@ -9,7 +9,7 @@ import { assertStringIncludes } from "https://deno.land/std@0.201.0/assert/asser Deno.env.set("ASTRAL_QUIET_INSTALL", "true"); const cache = await Deno.makeTempDir({ prefix: "astral_test_get_binary" }); const permissions = { - write: [cache], + write: [cache, `${Deno.env.get("HOME")}/.config/chromium/SingletonLock`], read: [cache], net: true, env: true, diff --git a/tests/install_lock_file_test.ts b/tests/install_lock_file_test.ts index e44fe76..9fffa9e 100644 --- a/tests/install_lock_file_test.ts +++ b/tests/install_lock_file_test.ts @@ -1,7 +1,7 @@ import { getBinary, launch } from "../mod.ts"; import { deadline } from "https://deno.land/std@0.203.0/async/deadline.ts"; // Tests should be performed in directory different from others tests as cache is cleaned during this one -//Deno.env.set("ASTRAL_QUIET_INSTALL", "true"); +Deno.env.set("ASTRAL_QUIET_INSTALL", "true"); const cache = await Deno.makeTempDir({ prefix: "astral_test_install_lock_file", }); From 98758093ee4d844bfd99743ca409da1229ae14d4 Mon Sep 17 00:00:00 2001 From: Simon Lecoq <22963968+lowlighter@users.noreply.github.com> Date: Fri, 13 Oct 2023 02:59:33 +0000 Subject: [PATCH 3/6] fix: permissions on mac os --- tests/get_binary_test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/get_binary_test.ts b/tests/get_binary_test.ts index 1ccc82d..c973f7d 100644 --- a/tests/get_binary_test.ts +++ b/tests/get_binary_test.ts @@ -9,7 +9,15 @@ import { assertStringIncludes } from "https://deno.land/std@0.201.0/assert/asser Deno.env.set("ASTRAL_QUIET_INSTALL", "true"); const cache = await Deno.makeTempDir({ prefix: "astral_test_get_binary" }); const permissions = { - write: [cache, `${Deno.env.get("HOME")}/.config/chromium/SingletonLock`], + write: [ + cache, + // Chromium lock on Linux + `${Deno.env.get("HOME")}/.config/chromium/SingletonLock`, + // Chromium lock on MacOS + `${ + Deno.env.get("HOME") + }/Library/Application Support/Chromium/SingletonLock`, + ], read: [cache], net: true, env: true, From 43945069ee85e4b3728cd8dbfcaf743dce32790c Mon Sep 17 00:00:00 2001 From: Simon Lecoq <22963968+lowlighter@users.noreply.github.com> Date: Fri, 13 Oct 2023 18:12:43 +0000 Subject: [PATCH 4/6] apply suggestions --- src/browser.ts | 11 +++-------- src/cache.ts | 2 +- tests/get_binary_test.ts | 5 +++-- tests/install_lock_file_test.ts | 8 +++++--- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/browser.ts b/src/browser.ts index 617cad9..87efee9 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -7,7 +7,7 @@ import { WEBSOCKET_ENDPOINT_REGEX, websocketReady } from "./util.ts"; async function runCommand( command: Deno.Command, - { windowsErr21RetryAttempts = 60 } = {}, + { retries = 60 } = {}, ): Promise<{ process: Deno.ChildProcess; endpoint: string }> { const process = command.spawn(); let endpoint = null; @@ -50,13 +50,8 @@ async function runCommand( stack.push(`Process exited with code ${code}`); // Handle recoverable error code 21 on Windows // https://source.chromium.org/chromium/chromium/src/+/main:net/base/net_error_list.h;l=90-91 - if ( - (Deno.build.os === "windows") && (code === 21) && - windowsErr21RetryAttempts > 0 - ) { - return runCommand(command, { - windowsErr21RetryAttempts: windowsErr21RetryAttempts - 1, - }); + if ((Deno.build.os === "windows") && (code === 21) && retries > 0) { + return runCommand(command, { retries: retries - 1 }); } console.error(stack.join("\n")); throw new Error("Your binary refused to boot"); diff --git a/src/cache.ts b/src/cache.ts index 2cae9d1..ec7fc16 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -134,7 +134,7 @@ export function getBinary( ); } -export async function _getBinary( +async function _getBinary( browser: Parameters[0], { cache = BASE_PATH, timeout }: NonNullable[1]>, ): Promise { diff --git a/tests/get_binary_test.ts b/tests/get_binary_test.ts index c973f7d..f22eb3a 100644 --- a/tests/get_binary_test.ts +++ b/tests/get_binary_test.ts @@ -69,5 +69,6 @@ Deno.test("Test download after failure", { permissions }, async () => { assert(await getBinary("chrome", { cache: testCache })); }); -// Cleaning -await Deno.remove(cache, { recursive: true }); +Deno.test("Clean cache after tests", async () => { + await cleanCache({ cache }); +}); diff --git a/tests/install_lock_file_test.ts b/tests/install_lock_file_test.ts index 9fffa9e..089745d 100644 --- a/tests/install_lock_file_test.ts +++ b/tests/install_lock_file_test.ts @@ -1,4 +1,4 @@ -import { getBinary, launch } from "../mod.ts"; +import { cleanCache, getBinary, launch } from "../mod.ts"; import { deadline } from "https://deno.land/std@0.203.0/async/deadline.ts"; // Tests should be performed in directory different from others tests as cache is cleaned during this one Deno.env.set("ASTRAL_QUIET_INSTALL", "true"); @@ -8,6 +8,7 @@ const cache = await Deno.makeTempDir({ Deno.test("Test concurrent getBinary calls", async () => { // Spawn concurrent getBinary calls + await cleanCache({ cache }); const promises = []; for (let i = 0; i < 20; i++) { promises.push(getBinary("chrome", { cache })); @@ -25,5 +26,6 @@ Deno.test("Test concurrent getBinary calls", async () => { await browser.close(); }); -// Cleaning -await Deno.remove(cache, { recursive: true }); +Deno.test("Clean cache after tests", async () => { + await cleanCache({ cache }); +}); From 3f621f0653794c5407a10b1c6d5d106d19855fb5 Mon Sep 17 00:00:00 2001 From: Simon Lecoq <22963968+lowlighter@users.noreply.github.com> Date: Sun, 15 Oct 2023 00:17:10 +0000 Subject: [PATCH 5/6] fix: merge getbinary intro try/finally --- src/cache.ts | 214 +++++++++++++++++++++++++-------------------------- 1 file changed, 105 insertions(+), 109 deletions(-) diff --git a/src/cache.ts b/src/cache.ts index ec7fc16..848757d 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -124,133 +124,129 @@ async function decompressArchive(source: string, destination: string) { /** * Get path for the binary for this OS. Downloads a browser if none is cached. */ -export function getBinary( +export async function getBinary( browser: "chrome" | "firefox", { cache = BASE_PATH, timeout = 60000 } = {}, -): Promise { - const product = `${browser}-${SUPPORTED_VERSIONS[browser]}`; - return _getBinary(browser, { cache, timeout }).finally(() => - LOCK_FILES[cache]?.[product]?.release() - ); -} - -async function _getBinary( - browser: Parameters[0], - { cache = BASE_PATH, timeout }: NonNullable[1]>, ): Promise { // TODO(lino-levan): fix firefox downloading const VERSION = SUPPORTED_VERSIONS[browser]; const product = `${browser}-${SUPPORTED_VERSIONS[browser]}`; + try { + const config = getCachedConfig({ cache }); - const config = getCachedConfig({ cache }); + // If the config doesn't have the revision and there is a lock file, reload config after release + if (!config[VERSION] && LOCK_FILES[cache]?.[product]?.exists()) { + await LOCK_FILES[cache]?.[product]?.waitRelease({ timeout }); + Object.assign(config, getCachedConfig({ cache })); + } - // If the config doesn't have the revision and there is a lock file, reload config after release - if (!config[VERSION] && LOCK_FILES[cache]?.[product]?.exists()) { - await LOCK_FILES[cache]?.[product]?.waitRelease({ timeout }); - Object.assign(config, getCachedConfig({ cache })); - } + // If the config doesn't have the revision, download it and return that + if (!config[VERSION]) { + const quiet = await isQuietInstall(); + ensureDirSync(cache); + const lock = new Lock({ cache }); + LOCK_FILES[cache] ??= {}; + LOCK_FILES[cache][product] = lock; + if (!lock.create()) { + return getBinary(browser, { cache, timeout }); + } + const versions = await knownGoodVersions(); + const version = versions.versions.filter((val) => + val.version === VERSION + )[0]; + const download = version.downloads.chrome.filter((val) => { + if (Deno.build.os === "darwin" && Deno.build.arch === "aarch64") { + return val.platform === "mac-arm64"; + } else if (Deno.build.os === "darwin" && Deno.build.arch === "x86_64") { + return val.platform === "mac-x64"; + } else if (Deno.build.os === "windows") { + return val.platform === "win64"; + } else if (Deno.build.os === "linux") { + return val.platform === "linux64"; + } + throw new Error( + "Unsupported platform, provide a path to a chromium or firefox binary instead", + ); + })[0]; - // If the config doesn't have the revision, download it and return that - if (!config[VERSION]) { - const quiet = await isQuietInstall(); - ensureDirSync(cache); - const lock = new Lock({ cache }); - LOCK_FILES[cache] ??= {}; - LOCK_FILES[cache][product] = lock; - if (!lock.create()) { - return _getBinary(browser, { cache }); - } - const versions = await knownGoodVersions(); - const version = versions.versions.filter((val) => - val.version === VERSION - )[0]; - const download = version.downloads.chrome.filter((val) => { - if (Deno.build.os === "darwin" && Deno.build.arch === "aarch64") { - return val.platform === "mac-arm64"; - } else if (Deno.build.os === "darwin" && Deno.build.arch === "x86_64") { - return val.platform === "mac-x64"; - } else if (Deno.build.os === "windows") { - return val.platform === "win64"; - } else if (Deno.build.os === "linux") { - return val.platform === "linux64"; + const req = await fetch(download.url); + if (!req.body) { + throw new Error( + "Download failed, please check your internet connection and try again", + ); + } + if (quiet) { + await Deno.writeFile(resolve(cache, `raw_${VERSION}.zip`), req.body); + } else { + const reader = req.body.getReader(); + const archive = await Deno.open(resolve(cache, `raw_${VERSION}.zip`), { + write: true, + truncate: true, + create: true, + }); + const bar = new ProgressBar({ + title: `Downloading ${browser} ${VERSION}`, + total: Number(req.headers.get("Content-Length") ?? 0), + clear: true, + display: ":title :bar :percent", + }); + let downloaded = 0; + do { + const { done, value } = await reader.read(); + if (done) { + break; + } + await Deno.write(archive.rid, value); + downloaded += value.length; + bar.render(downloaded); + } while (true); + Deno.close(archive.rid); + console.log(`Download complete (${browser} version ${VERSION})`); } - throw new Error( - "Unsupported platform, provide a path to a chromium or firefox binary instead", + await decompressArchive( + resolve(cache, `raw_${VERSION}.zip`), + resolve(cache, VERSION), ); - })[0]; - const req = await fetch(download.url); - if (!req.body) { - throw new Error( - "Download failed, please check your internet connection and try again", + config[VERSION] = resolve(cache, VERSION); + Deno.writeTextFileSync( + resolve(cache, CONFIG_FILE), + JSON.stringify(config), ); } - if (quiet) { - await Deno.writeFile(resolve(cache, `raw_${VERSION}.zip`), req.body); - } else { - const reader = req.body.getReader(); - const archive = await Deno.open(resolve(cache, `raw_${VERSION}.zip`), { - write: true, - truncate: true, - create: true, - }); - const bar = new ProgressBar({ - title: `Downloading ${browser} ${VERSION}`, - total: Number(req.headers.get("Content-Length") ?? 0), - clear: true, - display: ":title :bar :percent", - }); - let downloaded = 0; - do { - const { done, value } = await reader.read(); - if (done) { - break; - } - await Deno.write(archive.rid, value); - downloaded += value.length; - bar.render(downloaded); - } while (true); - Deno.close(archive.rid); - console.log(`Download complete (${browser} version ${VERSION})`); - } - await decompressArchive( - resolve(cache, `raw_${VERSION}.zip`), - resolve(cache, VERSION), - ); - config[VERSION] = resolve(cache, VERSION); - Deno.writeTextFileSync(resolve(cache, CONFIG_FILE), JSON.stringify(config)); - } - - // It now exists, return the path to the known good binary - const folder = config[VERSION]; + // It now exists, return the path to the known good binary + const folder = config[VERSION]; - if (Deno.build.os === "darwin" && Deno.build.arch === "aarch64") { - return resolve( - folder, - "chrome-mac-arm64", - "Google Chrome for Testing.app", - "Contents", - "MacOS", - "Google Chrome for Testing", - ); - } else if (Deno.build.os === "darwin" && Deno.build.arch === "x86_64") { - return resolve( - folder, - "chrome-mac-x64", - "Google Chrome for Testing.app", - "Contents", - "MacOS", - "Google Chrome for Testing", + if (Deno.build.os === "darwin" && Deno.build.arch === "aarch64") { + return resolve( + folder, + "chrome-mac-arm64", + "Google Chrome for Testing.app", + "Contents", + "MacOS", + "Google Chrome for Testing", + ); + } else if (Deno.build.os === "darwin" && Deno.build.arch === "x86_64") { + return resolve( + folder, + "chrome-mac-x64", + "Google Chrome for Testing.app", + "Contents", + "MacOS", + "Google Chrome for Testing", + ); + } else if (Deno.build.os === "windows") { + return resolve(folder, "chrome-win64", "chrome.exe"); + } else if (Deno.build.os === "linux") { + return resolve(folder, "chrome-linux64", "chrome"); + } + throw new Error( + "Unsupported platform, provide a path to a chromium or firefox binary instead", ); - } else if (Deno.build.os === "windows") { - return resolve(folder, "chrome-win64", "chrome.exe"); - } else if (Deno.build.os === "linux") { - return resolve(folder, "chrome-linux64", "chrome"); + } finally { + LOCK_FILES[cache]?.[product]?.release(); } - throw new Error( - "Unsupported platform, provide a path to a chromium or firefox binary instead", - ); } /** From 1472d17b56f4aeb95e40f24969650a59976467ae Mon Sep 17 00:00:00 2001 From: Simon Lecoq <22963968+lowlighter@users.noreply.github.com> Date: Sun, 15 Oct 2023 00:32:09 +0000 Subject: [PATCH 6/6] fix: finally behaviour (return early removes the lock) --- src/cache.ts | 126 ++++++++++++++++---------------- tests/install_lock_file_test.ts | 2 +- 2 files changed, 64 insertions(+), 64 deletions(-) diff --git a/src/cache.ts b/src/cache.ts index 848757d..99666bd 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -131,44 +131,44 @@ export async function getBinary( // TODO(lino-levan): fix firefox downloading const VERSION = SUPPORTED_VERSIONS[browser]; const product = `${browser}-${SUPPORTED_VERSIONS[browser]}`; - try { - const config = getCachedConfig({ cache }); + const config = getCachedConfig({ cache }); - // If the config doesn't have the revision and there is a lock file, reload config after release - if (!config[VERSION] && LOCK_FILES[cache]?.[product]?.exists()) { - await LOCK_FILES[cache]?.[product]?.waitRelease({ timeout }); - Object.assign(config, getCachedConfig({ cache })); - } + // If the config doesn't have the revision and there is a lock file, reload config after release + if (!config[VERSION] && LOCK_FILES[cache]?.[product]?.exists()) { + await LOCK_FILES[cache]?.[product]?.waitRelease({ timeout }); + Object.assign(config, getCachedConfig({ cache })); + } - // If the config doesn't have the revision, download it and return that - if (!config[VERSION]) { - const quiet = await isQuietInstall(); - ensureDirSync(cache); - const lock = new Lock({ cache }); - LOCK_FILES[cache] ??= {}; - LOCK_FILES[cache][product] = lock; - if (!lock.create()) { - return getBinary(browser, { cache, timeout }); + // If the config doesn't have the revision, download it and return that + if (!config[VERSION]) { + const quiet = await isQuietInstall(); + const versions = await knownGoodVersions(); + const version = versions.versions.filter((val) => + val.version === VERSION + )[0]; + const download = version.downloads.chrome.filter((val) => { + if (Deno.build.os === "darwin" && Deno.build.arch === "aarch64") { + return val.platform === "mac-arm64"; + } else if (Deno.build.os === "darwin" && Deno.build.arch === "x86_64") { + return val.platform === "mac-x64"; + } else if (Deno.build.os === "windows") { + return val.platform === "win64"; + } else if (Deno.build.os === "linux") { + return val.platform === "linux64"; } - const versions = await knownGoodVersions(); - const version = versions.versions.filter((val) => - val.version === VERSION - )[0]; - const download = version.downloads.chrome.filter((val) => { - if (Deno.build.os === "darwin" && Deno.build.arch === "aarch64") { - return val.platform === "mac-arm64"; - } else if (Deno.build.os === "darwin" && Deno.build.arch === "x86_64") { - return val.platform === "mac-x64"; - } else if (Deno.build.os === "windows") { - return val.platform === "win64"; - } else if (Deno.build.os === "linux") { - return val.platform === "linux64"; - } - throw new Error( - "Unsupported platform, provide a path to a chromium or firefox binary instead", - ); - })[0]; + throw new Error( + "Unsupported platform, provide a path to a chromium or firefox binary instead", + ); + })[0]; + ensureDirSync(cache); + const lock = new Lock({ cache }); + LOCK_FILES[cache] ??= {}; + LOCK_FILES[cache][product] = lock; + if (!lock.create()) { + return getBinary(browser, { cache, timeout }); + } + try { const req = await fetch(download.url); if (!req.body) { throw new Error( @@ -213,40 +213,40 @@ export async function getBinary( resolve(cache, CONFIG_FILE), JSON.stringify(config), ); + } finally { + LOCK_FILES[cache]?.[product]?.release(); } + } - // It now exists, return the path to the known good binary - const folder = config[VERSION]; + // It now exists, return the path to the known good binary + const folder = config[VERSION]; - if (Deno.build.os === "darwin" && Deno.build.arch === "aarch64") { - return resolve( - folder, - "chrome-mac-arm64", - "Google Chrome for Testing.app", - "Contents", - "MacOS", - "Google Chrome for Testing", - ); - } else if (Deno.build.os === "darwin" && Deno.build.arch === "x86_64") { - return resolve( - folder, - "chrome-mac-x64", - "Google Chrome for Testing.app", - "Contents", - "MacOS", - "Google Chrome for Testing", - ); - } else if (Deno.build.os === "windows") { - return resolve(folder, "chrome-win64", "chrome.exe"); - } else if (Deno.build.os === "linux") { - return resolve(folder, "chrome-linux64", "chrome"); - } - throw new Error( - "Unsupported platform, provide a path to a chromium or firefox binary instead", + if (Deno.build.os === "darwin" && Deno.build.arch === "aarch64") { + return resolve( + folder, + "chrome-mac-arm64", + "Google Chrome for Testing.app", + "Contents", + "MacOS", + "Google Chrome for Testing", + ); + } else if (Deno.build.os === "darwin" && Deno.build.arch === "x86_64") { + return resolve( + folder, + "chrome-mac-x64", + "Google Chrome for Testing.app", + "Contents", + "MacOS", + "Google Chrome for Testing", ); - } finally { - LOCK_FILES[cache]?.[product]?.release(); + } else if (Deno.build.os === "windows") { + return resolve(folder, "chrome-win64", "chrome.exe"); + } else if (Deno.build.os === "linux") { + return resolve(folder, "chrome-linux64", "chrome"); } + throw new Error( + "Unsupported platform, provide a path to a chromium or firefox binary instead", + ); } /** diff --git a/tests/install_lock_file_test.ts b/tests/install_lock_file_test.ts index 089745d..86cb9b4 100644 --- a/tests/install_lock_file_test.ts +++ b/tests/install_lock_file_test.ts @@ -1,7 +1,7 @@ import { cleanCache, getBinary, launch } from "../mod.ts"; import { deadline } from "https://deno.land/std@0.203.0/async/deadline.ts"; // Tests should be performed in directory different from others tests as cache is cleaned during this one -Deno.env.set("ASTRAL_QUIET_INSTALL", "true"); +//Deno.env.set("ASTRAL_QUIET_INSTALL", "true"); const cache = await Deno.makeTempDir({ prefix: "astral_test_install_lock_file", });