Skip to content

Commit 946b4ef

Browse files
authored
fix(installer): create tmp directory inside browserPath (#2498)
`fs.rename` doesn't work across partitions, so we have to have tmp folder next to our final destination. Fixes #2494
1 parent bb4e959 commit 946b4ef

File tree

3 files changed

+39
-11
lines changed

3 files changed

+39
-11
lines changed

src/install/browserFetcher.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
import * as extract from 'extract-zip';
1919
import * as fs from 'fs';
2020
import * as ProxyAgent from 'https-proxy-agent';
21-
import * as os from 'os';
22-
import * as path from 'path';
2321
import * as ProgressBar from 'progress';
2422
import { getProxyForUrl } from 'proxy-from-env';
2523
import * as URL from 'url';
@@ -30,7 +28,6 @@ import { BrowserName, BrowserPlatform, BrowserDescriptor } from './browserPaths'
3028

3129
const unlinkAsync = util.promisify(fs.unlink.bind(fs));
3230
const chmodAsync = util.promisify(fs.chmod.bind(fs));
33-
const mkdtempAsync = util.promisify(fs.mkdtemp.bind(fs));
3431
const renameAsync = util.promisify(fs.rename.bind(fs));
3532
const existsAsync = (path: string): Promise<boolean> => new Promise(resolve => fs.stat(path, err => resolve(!err)));
3633

@@ -109,10 +106,10 @@ export async function downloadBrowserWithProgressBar(browserPath: string, browse
109106
}
110107

111108
const url = revisionURL(browser);
112-
const zipPath = path.join(os.tmpdir(), `playwright-download-${browser.name}-${browserPaths.hostPlatform}-${browser.revision}.zip`);
109+
const zipPath = browserPaths.browserZipFile(browserPath, browser);
113110
try {
114111
await downloadFile(url, zipPath, progress);
115-
const extractPath = await mkdtempAsync(path.join(os.tmpdir(), `playwright-extract-${browser.name}-${browserPaths.hostPlatform}-${browser.revision}-`));
112+
const extractPath = browserPaths.browserExtractDirectory(browserPath, browser);
116113
await extract(zipPath, { dir: extractPath});
117114
await chmodAsync(browserPaths.executablePath(extractPath, browser)!, 0o755);
118115
await renameAsync(extractPath, browserPath);

src/install/browserPaths.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,29 @@ export function browserDirectory(browsersPath: string, browser: BrowserDescripto
104104
return path.join(browsersPath, `${browser.name}-${browser.revision}`);
105105
}
106106

107-
export function isBrowserDirectory(browserPath: string): boolean {
108-
const baseName = path.basename(browserPath);
107+
export function isBrowserDirectory(aPath: string): boolean {
108+
const baseName = path.basename(aPath);
109109
return baseName.startsWith('chromium-') || baseName.startsWith('firefox-') || baseName.startsWith('webkit-');
110110
}
111+
112+
const BROWSER_EXTRACT_DIRECTORY_PREFIX = 'playwright-extract-';
113+
114+
export function isBrowserExtractDirectory(aPath: string): boolean {
115+
const baseName = path.basename(aPath);
116+
return baseName.startsWith(BROWSER_EXTRACT_DIRECTORY_PREFIX);
117+
}
118+
119+
export function browserExtractDirectory(browserPath: string, browser: BrowserDescriptor): string {
120+
return (path.join(path.dirname(browserPath), `${BROWSER_EXTRACT_DIRECTORY_PREFIX}${browser.name}-${hostPlatform}-${browser.revision}`));
121+
}
122+
123+
const BROWSER_ZIP_FILE_PREFIX = 'playwright-download-';
124+
125+
export function isBrowserZipFile(aPath: string): boolean {
126+
const baseName = path.basename(aPath);
127+
return baseName.startsWith(BROWSER_ZIP_FILE_PREFIX) && baseName.endsWith('.zip');
128+
}
129+
130+
export function browserZipFile(browserPath: string, browser: BrowserDescriptor): string {
131+
return path.join(path.dirname(browserPath), `${BROWSER_ZIP_FILE_PREFIX}${browser.name}-${hostPlatform}-${browser.revision}.zip`);
132+
}

src/install/installer.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const fsReaddirAsync = util.promisify(fs.readdir.bind(fs));
2828
const fsReadFileAsync = util.promisify(fs.readFile.bind(fs));
2929
const fsUnlinkAsync = util.promisify(fs.unlink.bind(fs));
3030
const fsWriteFileAsync = util.promisify(fs.writeFile.bind(fs));
31-
const removeFolderAsync = util.promisify(removeFolder);
31+
const rmAsync = util.promisify(removeFolder);
3232

3333
export async function installBrowsersWithProgressBar(packagePath: string) {
3434
const browsersPath = browserPaths.browsersPath(packagePath);
@@ -60,18 +60,27 @@ async function validateCache(packagePath: string, browsersPath: string, linksDir
6060
}
6161
}
6262

63-
// 2. Delete all unused browsers.
63+
// 2. Delete all stale browser extract directories and .zip files.
64+
// NOTE: this must not run concurrently with other installations.
65+
let staleFiles = (await fsReaddirAsync(browsersPath)).map(file => path.join(browsersPath, file));
66+
staleFiles = staleFiles.filter(file => browserPaths.isBrowserZipFile(file) || browserPaths.isBrowserExtractDirectory(file));
67+
for (const staleFile of staleFiles) {
68+
logPolitely('Removing leftover from interrupted installation ' + staleFile);
69+
await rmAsync(staleFile).catch(e => {});
70+
}
71+
72+
// 3. Delete all unused browsers.
6473
let downloadedBrowsers = (await fsReaddirAsync(browsersPath)).map(file => path.join(browsersPath, file));
6574
downloadedBrowsers = downloadedBrowsers.filter(file => browserPaths.isBrowserDirectory(file));
6675
const directories = new Set<string>(downloadedBrowsers);
6776
for (const browser of allBrowsers)
6877
directories.delete(browserPaths.browserDirectory(browsersPath, browser));
6978
for (const directory of directories) {
7079
logPolitely('Removing unused browser at ' + directory);
71-
await removeFolderAsync(directory).catch(e => {});
80+
await rmAsync(directory).catch(e => {});
7281
}
7382

74-
// 3. Install missing browsers for this package.
83+
// 4. Install missing browsers for this package.
7584
const myBrowsers = JSON.parse((await fsReadFileAsync(path.join(packagePath, 'browsers.json'))).toString())['browsers'] as browserPaths.BrowserDescriptor[];
7685
for (const browser of myBrowsers) {
7786
const browserPath = browserPaths.browserDirectory(browsersPath, browser);

0 commit comments

Comments
 (0)