From 0b27cbe86d568cd66c9523345d125dcc74359edc Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Wed, 15 Feb 2023 20:41:22 +0100 Subject: [PATCH] refactor: unify working with the cache folder structure (#9681) --- packages/browsers/src/CLI.ts | 28 +++++++--- packages/browsers/src/CacheStructure.ts | 52 +++++++++++++++++++ packages/browsers/src/browsers.ts | 16 ++++++ packages/browsers/src/browsers/browsers.ts | 4 +- packages/browsers/src/browsers/chrome.ts | 18 ++----- packages/browsers/src/browsers/firefox.ts | 18 ++----- packages/browsers/src/detectPlatform.ts | 2 +- packages/browsers/src/fetch.ts | 33 +++++++----- packages/browsers/src/launcher.ts | 18 ++++--- .../browsers/test/src/chrome-data.spec.ts | 28 ++++------ packages/browsers/test/src/fetch.spec.ts | 15 +++--- .../browsers/test/src/firefox-data.spec.ts | 34 ++++-------- packages/browsers/test/src/launcher.spec.ts | 8 +-- 13 files changed, 168 insertions(+), 106 deletions(-) create mode 100644 packages/browsers/src/CacheStructure.ts diff --git a/packages/browsers/src/CLI.ts b/packages/browsers/src/CLI.ts index 683ff4faf94..47c1eb656e5 100644 --- a/packages/browsers/src/CLI.ts +++ b/packages/browsers/src/CLI.ts @@ -1,9 +1,24 @@ +/** + * Copyright 2023 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + import yargs from 'yargs'; import ProgressBar from 'progress'; import {hideBin} from 'yargs/helpers'; import {Browser, BrowserPlatform} from './browsers/types.js'; import {fetch} from './fetch.js'; -import path from 'path'; type Arguments = { browser: { @@ -44,11 +59,8 @@ export class CLI { browser: args.browser.name, revision: args.browser.revision, platform: args.platform, - outputDir: path.join( - args.path ?? this.#cachePath, - args.browser.name - ), - progressCallback: this.#makeProgressBar( + cacheDir: args.path ?? this.#cachePath, + downloadProgressCallback: this.#makeProgressCallback( args.browser.name, args.browser.revision ), @@ -82,8 +94,8 @@ export class CLI { return `${Math.round(mb * 10) / 10} Mb`; } - #makeProgressBar(browser: Browser, revision: string) { - let progressBar: ProgressBar | null = null; + #makeProgressCallback(browser: Browser, revision: string) { + let progressBar: ProgressBar; let lastDownloadedBytes = 0; return (downloadedBytes: number, totalBytes: number) => { if (!progressBar) { diff --git a/packages/browsers/src/CacheStructure.ts b/packages/browsers/src/CacheStructure.ts new file mode 100644 index 00000000000..6f115113af9 --- /dev/null +++ b/packages/browsers/src/CacheStructure.ts @@ -0,0 +1,52 @@ +/** + * Copyright 2023 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {Browser, BrowserPlatform} from './browsers/types.js'; +import path from 'path'; + +/** + * The cache used by Puppeteer relies on the following structure: + * + * - rootDir + * -- | browserRoot(browser1) + * ---- - | installationDir() + * ------ the browser-platform-revision + * ------ specific structure. + * -- | browserRoot(browser2) + * ---- - | installationDir() + * ------ the browser-platform-revision + * ------ specific structure. + * @internal + */ +export class CacheStructure { + #rootDir: string; + + constructor(rootDir: string) { + this.#rootDir = rootDir; + } + + browserRoot(browser: Browser): string { + return path.join(this.#rootDir, browser); + } + + installationDir( + browser: Browser, + platform: BrowserPlatform, + revision: string + ): string { + return path.join(this.browserRoot(browser), `${platform}-${revision}`); + } +} diff --git a/packages/browsers/src/browsers.ts b/packages/browsers/src/browsers.ts index 21c69ba883d..56e2261763c 100644 --- a/packages/browsers/src/browsers.ts +++ b/packages/browsers/src/browsers.ts @@ -1,5 +1,21 @@ #!/usr/bin/env node +/** + * Copyright 2023 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + import {CLI} from './CLI.js'; new CLI().run(process.argv); diff --git a/packages/browsers/src/browsers/browsers.ts b/packages/browsers/src/browsers/browsers.ts index daad7f80fcc..266bbaf161d 100644 --- a/packages/browsers/src/browsers/browsers.ts +++ b/packages/browsers/src/browsers/browsers.ts @@ -24,8 +24,8 @@ export const downloadUrls = { }; export const executablePathByBrowser = { - [Browser.CHROME]: chrome.executablePath, - [Browser.FIREFOX]: firefox.executablePath, + [Browser.CHROME]: chrome.relativeExecutablePath, + [Browser.FIREFOX]: firefox.relativeExecutablePath, }; export {Browser, BrowserPlatform}; diff --git a/packages/browsers/src/browsers/chrome.ts b/packages/browsers/src/browsers/chrome.ts index a8ea4af390c..6c8e8089754 100644 --- a/packages/browsers/src/browsers/chrome.ts +++ b/packages/browsers/src/browsers/chrome.ts @@ -57,26 +57,18 @@ export function resolveDownloadUrl( )}.zip`; } -export function executablePath( +export function relativeExecutablePath( platform: BrowserPlatform, - revision: string, - basePath = '' + _revision: string ): string { - const browserPath = path.join(basePath, `${platform}-${revision}`); switch (platform) { case BrowserPlatform.MAC: case BrowserPlatform.MAC_ARM: - return path.join( - browserPath, - 'Chromium.app', - 'Contents', - 'MacOS', - 'Chromium' - ); + return path.join('Chromium.app', 'Contents', 'MacOS', 'Chromium'); case BrowserPlatform.LINUX: - return path.join(browserPath, 'chrome'); + return 'chrome'; case BrowserPlatform.WIN32: case BrowserPlatform.WIN64: - return path.join(browserPath, 'chrome.exe'); + return 'chrome.exe'; } } diff --git a/packages/browsers/src/browsers/firefox.ts b/packages/browsers/src/browsers/firefox.ts index cb3c0419121..6f40e071398 100644 --- a/packages/browsers/src/browsers/firefox.ts +++ b/packages/browsers/src/browsers/firefox.ts @@ -38,26 +38,18 @@ export function resolveDownloadUrl( return `${baseUrl}/${archive(platform, revision)}`; } -export function executablePath( +export function relativeExecutablePath( platform: BrowserPlatform, - revision: string, - basePath = '' + _revision: string ): string { - const browserPath = path.join(basePath, `${platform}-${revision}`); switch (platform) { case BrowserPlatform.MAC_ARM: case BrowserPlatform.MAC: - return path.join( - browserPath, - 'Firefox Nightly.app', - 'Contents', - 'MacOS', - 'firefox' - ); + return path.join('Firefox Nightly.app', 'Contents', 'MacOS', 'firefox'); case BrowserPlatform.LINUX: - return path.join(browserPath, 'firefox', 'firefox'); + return path.join('firefox', 'firefox'); case BrowserPlatform.WIN32: case BrowserPlatform.WIN64: - return path.join(browserPath, 'firefox', 'firefox.exe'); + return path.join('firefox', 'firefox.exe'); } } diff --git a/packages/browsers/src/detectPlatform.ts b/packages/browsers/src/detectPlatform.ts index 23b4efa709b..fe01a7a708a 100644 --- a/packages/browsers/src/detectPlatform.ts +++ b/packages/browsers/src/detectPlatform.ts @@ -17,7 +17,7 @@ import os from 'os'; import {BrowserPlatform} from './browsers/browsers.js'; -export function detectPlatform(): BrowserPlatform | undefined { +export function detectBrowserPlatform(): BrowserPlatform | undefined { const platform = os.platform(); switch (platform) { case 'darwin': diff --git a/packages/browsers/src/fetch.ts b/packages/browsers/src/fetch.ts index e0f136822ab..099e05ed128 100644 --- a/packages/browsers/src/fetch.ts +++ b/packages/browsers/src/fetch.ts @@ -24,7 +24,8 @@ import {Browser, BrowserPlatform, downloadUrls} from './browsers/browsers.js'; import {downloadFile, headHttpRequest} from './httpUtil.js'; import assert from 'assert'; import {unpackArchive} from './fileUtil.js'; -import {detectPlatform} from './detectPlatform.js'; +import {detectBrowserPlatform} from './detectPlatform.js'; +import {CacheStructure} from './CacheStructure.js'; const debugFetch = debug('puppeteer:browsers:fetcher'); @@ -35,7 +36,7 @@ export interface Options { /** * Determines the path to download browsers to. */ - outputDir: string; + cacheDir: string; /** * Determines which platform the browser will be suited for. * @@ -54,7 +55,10 @@ export interface Options { /** * Provides information about the progress of the download. */ - progressCallback?: (downloadedBytes: number, totalBytes: number) => void; + downloadProgressCallback?: ( + downloadedBytes: number, + totalBytes: number + ) => void; } export type InstalledBrowser = { @@ -65,7 +69,7 @@ export type InstalledBrowser = { }; export async function fetch(options: Options): Promise { - options.platform ??= detectPlatform(); + options.platform ??= detectBrowserPlatform(); if (!options.platform) { throw new Error( `Cannot download a binary for the provided platform: ${os.platform()} (${os.arch()})` @@ -78,10 +82,16 @@ export async function fetch(options: Options): Promise { ); const fileName = url.toString().split('/').pop(); assert(fileName, `A malformed download URL was found: ${url}.`); - const archivePath = path.join(options.outputDir, fileName); - const outputPath = path.resolve( - options.outputDir, - `${options.platform}-${options.revision}` + const structure = new CacheStructure(options.cacheDir); + const browserRoot = structure.browserRoot(options.browser); + const archivePath = path.join(browserRoot, fileName); + if (!existsSync(browserRoot)) { + await mkdir(browserRoot, {recursive: true}); + } + const outputPath = structure.installationDir( + options.browser, + options.platform, + options.revision ); if (existsSync(outputPath)) { return { @@ -91,12 +101,9 @@ export async function fetch(options: Options): Promise { revision: options.revision, }; } - if (!existsSync(options.outputDir)) { - await mkdir(options.outputDir, {recursive: true}); - } try { debugFetch(`Downloading binary from ${url}`); - await downloadFile(url, archivePath, options.progressCallback); + await downloadFile(url, archivePath, options.downloadProgressCallback); debugFetch(`Installing ${archivePath} to ${outputPath}`); await unpackArchive(archivePath, outputPath); } finally { @@ -113,7 +120,7 @@ export async function fetch(options: Options): Promise { } export async function canFetch(options: Options): Promise { - options.platform ??= detectPlatform(); + options.platform ??= detectBrowserPlatform(); if (!options.platform) { throw new Error( `Cannot download a binary for the provided platform: ${os.platform()} (${os.arch()})` diff --git a/packages/browsers/src/launcher.ts b/packages/browsers/src/launcher.ts index dbdff12585c..a67f3d94e05 100644 --- a/packages/browsers/src/launcher.ts +++ b/packages/browsers/src/launcher.ts @@ -19,8 +19,10 @@ import { BrowserPlatform, executablePathByBrowser, } from './browsers/browsers.js'; -import {detectPlatform} from './detectPlatform.js'; +import {detectBrowserPlatform} from './detectPlatform.js'; import os from 'os'; +import path from 'path'; +import {CacheStructure} from './CacheStructure.js'; /** * @public @@ -29,7 +31,7 @@ export interface Options { /** * Root path to the storage directory. */ - path: string; + cacheDir: string; /** * Determines which platform the browser will be suited for. * @@ -48,15 +50,19 @@ export interface Options { } export function computeExecutablePath(options: Options): string { - options.platform ??= detectPlatform(); + options.platform ??= detectBrowserPlatform(); if (!options.platform) { throw new Error( `Cannot download a binary for the provided platform: ${os.platform()} (${os.arch()})` ); } - return executablePathByBrowser[options.browser]( + const installationDir = new CacheStructure(options.cacheDir).installationDir( + options.browser, options.platform, - options.revision, - options.path + options.revision + ); + return path.join( + installationDir, + executablePathByBrowser[options.browser](options.platform, options.revision) ); } diff --git a/packages/browsers/test/src/chrome-data.spec.ts b/packages/browsers/test/src/chrome-data.spec.ts index e86272129d8..507d1a76ebc 100644 --- a/packages/browsers/test/src/chrome-data.spec.ts +++ b/packages/browsers/test/src/chrome-data.spec.ts @@ -16,7 +16,7 @@ import { resolveDownloadUrl, - executablePath, + relativeExecutablePath, } from '../../lib/cjs/browsers/chrome.js'; import {BrowserPlatform} from '../../lib/cjs/browsers/browsers.js'; import assert from 'assert'; @@ -48,30 +48,24 @@ describe('Chrome', () => { it('should resolve executable paths', () => { assert.strictEqual( - executablePath(BrowserPlatform.LINUX, '12372323'), - path.join('linux-12372323', 'chrome') + relativeExecutablePath(BrowserPlatform.LINUX, '12372323'), + path.join('chrome') ); assert.strictEqual( - executablePath(BrowserPlatform.MAC, '12372323'), - path.join('mac-12372323', 'Chromium.app', 'Contents', 'MacOS', 'Chromium') + relativeExecutablePath(BrowserPlatform.MAC, '12372323'), + path.join('Chromium.app', 'Contents', 'MacOS', 'Chromium') ); assert.strictEqual( - executablePath(BrowserPlatform.MAC_ARM, '12372323'), - path.join( - 'mac_arm-12372323', - 'Chromium.app', - 'Contents', - 'MacOS', - 'Chromium' - ) + relativeExecutablePath(BrowserPlatform.MAC_ARM, '12372323'), + path.join('Chromium.app', 'Contents', 'MacOS', 'Chromium') ); assert.strictEqual( - executablePath(BrowserPlatform.WIN32, '12372323'), - path.join('win32-12372323', 'chrome.exe') + relativeExecutablePath(BrowserPlatform.WIN32, '12372323'), + path.join('chrome.exe') ); assert.strictEqual( - executablePath(BrowserPlatform.WIN64, '12372323'), - path.join('win64-12372323', 'chrome.exe') + relativeExecutablePath(BrowserPlatform.WIN64, '12372323'), + path.join('chrome.exe') ); }); }); diff --git a/packages/browsers/test/src/fetch.spec.ts b/packages/browsers/test/src/fetch.spec.ts index 4788f4fb7ba..155036c55a4 100644 --- a/packages/browsers/test/src/fetch.spec.ts +++ b/packages/browsers/test/src/fetch.spec.ts @@ -42,7 +42,7 @@ describe('fetch', () => { it('should check if a revision can be downloaded', async () => { assert.ok( await canFetch({ - outputDir: tmpDir, + cacheDir: tmpDir, browser: Browser.CHROME, platform: BrowserPlatform.LINUX, revision: testChromeRevision, @@ -53,7 +53,7 @@ describe('fetch', () => { it('should report if a revision is not downloadable', async () => { assert.strictEqual( await canFetch({ - outputDir: tmpDir, + cacheDir: tmpDir, browser: Browser.CHROME, platform: BrowserPlatform.LINUX, revision: 'unknown', @@ -66,11 +66,12 @@ describe('fetch', () => { this.timeout(60000); const expectedOutputPath = path.join( tmpDir, + 'chrome', `${BrowserPlatform.LINUX}-${testChromeRevision}` ); assert.strictEqual(fs.existsSync(expectedOutputPath), false); let browser = await fetch({ - outputDir: tmpDir, + cacheDir: tmpDir, browser: Browser.CHROME, platform: BrowserPlatform.LINUX, revision: testChromeRevision, @@ -79,7 +80,7 @@ describe('fetch', () => { assert.ok(fs.existsSync(expectedOutputPath)); // Second iteration should be no-op. browser = await fetch({ - outputDir: tmpDir, + cacheDir: tmpDir, browser: Browser.CHROME, platform: BrowserPlatform.LINUX, revision: testChromeRevision, @@ -92,11 +93,12 @@ describe('fetch', () => { this.timeout(60000); const expectedOutputPath = path.join( tmpDir, + 'firefox', `${BrowserPlatform.LINUX}-${testFirefoxRevision}` ); assert.strictEqual(fs.existsSync(expectedOutputPath), false); const browser = await fetch({ - outputDir: tmpDir, + cacheDir: tmpDir, browser: Browser.FIREFOX, platform: BrowserPlatform.LINUX, revision: testFirefoxRevision, @@ -113,11 +115,12 @@ describe('fetch', () => { this.timeout(120000); const expectedOutputPath = path.join( tmpDir, + 'firefox', `${BrowserPlatform.MAC}-${testFirefoxRevision}` ); assert.strictEqual(fs.existsSync(expectedOutputPath), false); const browser = await fetch({ - outputDir: tmpDir, + cacheDir: tmpDir, browser: Browser.FIREFOX, platform: BrowserPlatform.MAC, revision: testFirefoxRevision, diff --git a/packages/browsers/test/src/firefox-data.spec.ts b/packages/browsers/test/src/firefox-data.spec.ts index d4043bca2ba..296f529bb9d 100644 --- a/packages/browsers/test/src/firefox-data.spec.ts +++ b/packages/browsers/test/src/firefox-data.spec.ts @@ -15,7 +15,7 @@ */ import { - executablePath, + relativeExecutablePath, resolveDownloadUrl, } from '../../lib/cjs/browsers/firefox.js'; import {BrowserPlatform} from '../../lib/cjs/browsers/browsers.js'; @@ -48,36 +48,24 @@ describe('Firefox', () => { it('should resolve executable paths', () => { assert.strictEqual( - executablePath(BrowserPlatform.LINUX, '111.0a1'), - path.join('linux-111.0a1', 'firefox', 'firefox') + relativeExecutablePath(BrowserPlatform.LINUX, '111.0a1'), + path.join('firefox', 'firefox') ); assert.strictEqual( - executablePath(BrowserPlatform.MAC, '111.0a1'), - path.join( - 'mac-111.0a1', - 'Firefox Nightly.app', - 'Contents', - 'MacOS', - 'firefox' - ) + relativeExecutablePath(BrowserPlatform.MAC, '111.0a1'), + path.join('Firefox Nightly.app', 'Contents', 'MacOS', 'firefox') ); assert.strictEqual( - executablePath(BrowserPlatform.MAC_ARM, '111.0a1'), - path.join( - 'mac_arm-111.0a1', - 'Firefox Nightly.app', - 'Contents', - 'MacOS', - 'firefox' - ) + relativeExecutablePath(BrowserPlatform.MAC_ARM, '111.0a1'), + path.join('Firefox Nightly.app', 'Contents', 'MacOS', 'firefox') ); assert.strictEqual( - executablePath(BrowserPlatform.WIN32, '111.0a1'), - path.join('win32-111.0a1', 'firefox', 'firefox.exe') + relativeExecutablePath(BrowserPlatform.WIN32, '111.0a1'), + path.join('firefox', 'firefox.exe') ); assert.strictEqual( - executablePath(BrowserPlatform.WIN64, '111.0a1'), - path.join('win64-111.0a1', 'firefox', 'firefox.exe') + relativeExecutablePath(BrowserPlatform.WIN64, '111.0a1'), + path.join('firefox', 'firefox.exe') ); }); }); diff --git a/packages/browsers/test/src/launcher.spec.ts b/packages/browsers/test/src/launcher.spec.ts index 2ec6c37a516..141df31d78d 100644 --- a/packages/browsers/test/src/launcher.spec.ts +++ b/packages/browsers/test/src/launcher.spec.ts @@ -27,9 +27,9 @@ describe('launcher', () => { browser: Browser.CHROME, platform: BrowserPlatform.LINUX, revision: '123', - path: 'cache', + cacheDir: 'cache', }), - path.join('cache', 'linux-123', 'chrome') + path.join('cache', 'chrome', 'linux-123', 'chrome') ); }); it('should compute executable path for Firefox', () => { @@ -38,9 +38,9 @@ describe('launcher', () => { browser: Browser.FIREFOX, platform: BrowserPlatform.LINUX, revision: '123', - path: 'cache', + cacheDir: 'cache', }), - path.join('cache', 'linux-123', 'firefox', 'firefox') + path.join('cache', 'firefox', 'linux-123', 'firefox', 'firefox') ); }); });