From c10c226ceb303f6f9f5a5b6a1b6da1ff65f653d4 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 1 Apr 2024 21:55:47 +0200 Subject: [PATCH 1/2] Separate read and write operations on lastKnownGood.json Also skip overwriting lastKnownGood.json with same content. Signed-off-by: Jakob Ackermann --- sources/Engine.ts | 129 +++++++++++++++++---------------------- sources/corepackUtils.ts | 27 +++----- tests/_runCli.ts | 6 +- tests/main.test.ts | 92 +++++++++++++++++++++++++++- 4 files changed, 160 insertions(+), 94 deletions(-) diff --git a/sources/Engine.ts b/sources/Engine.ts index aeec58442..359a6a2d8 100644 --- a/sources/Engine.ts +++ b/sources/Engine.ts @@ -1,5 +1,4 @@ import {UsageError} from 'clipanion'; -import type {FileHandle} from 'fs/promises'; import fs from 'fs'; import path from 'path'; import process from 'process'; @@ -25,50 +24,58 @@ export type PackageManagerRequest = { binaryVersion: string | null; }; -export function getLastKnownGoodFile(flag = `r`) { - return fs.promises.open(path.join(folderUtils.getCorepackHomeFolder(), `lastKnownGood.json`), flag); -} -async function createLastKnownGoodFile() { - await fs.promises.mkdir(folderUtils.getCorepackHomeFolder(), {recursive: true}); - return getLastKnownGoodFile(`w`); +function getLastKnownGoodFilePath() { + return path.join(folderUtils.getCorepackHomeFolder(), `lastKnownGood.json`); } -export async function getJSONFileContent(fh: FileHandle) { - let lastKnownGood: unknown; +export async function getLastKnownGood(): Promise> { + let raw: string; try { - lastKnownGood = JSON.parse(await fh.readFile(`utf8`)); + raw = await fs.promises.readFile(getLastKnownGoodFilePath(), `utf8`); + } catch (err) { + if ((err as NodeError)?.code === `ENOENT`) return {}; + throw err; + } + + try { + const parsed = JSON.parse(raw); + if (!parsed) return {}; + if (typeof parsed !== `object`) return {}; + Object.entries(parsed).forEach(([key, value]) => { + if (typeof value !== `string`) { + // Ensure that all entries are strings. + delete parsed[key]; + } + }); + return parsed; } catch { // Ignore errors; too bad - return undefined; + return {}; } - - return lastKnownGood; } -async function overwriteJSONFileContent(fh: FileHandle, content: unknown) { - await fh.truncate(0); - await fh.write(`${JSON.stringify(content, null, 2)}\n`, 0); +async function createLastKnownGoodFile(lastKnownGood: Record) { + const content = `${JSON.stringify(lastKnownGood, null, 2)}\n`; + await fs.promises.mkdir(folderUtils.getCorepackHomeFolder(), {recursive: true}); + await fs.promises.writeFile(getLastKnownGoodFilePath(), content, `utf8`); } -export function getLastKnownGoodFromFileContent(lastKnownGood: unknown, packageManager: string) { - if (typeof lastKnownGood === `object` && lastKnownGood !== null && - Object.hasOwn(lastKnownGood, packageManager)) { - const override = (lastKnownGood as any)[packageManager]; - if (typeof override === `string`) { - return override; - } - } +export function getLastKnownGoodFromFileContent(lastKnownGood: Record, packageManager: string) { + if (Object.hasOwn(lastKnownGood, packageManager)) + return lastKnownGood[packageManager]; return undefined; } -export async function activatePackageManagerFromFileHandle(lastKnownGoodFile: FileHandle, lastKnownGood: unknown, locator: Locator) { - if (typeof lastKnownGood !== `object` || lastKnownGood === null) - lastKnownGood = {}; +export async function activatePackageManager(lastKnownGood: Record, locator: Locator) { + if (lastKnownGood[locator.name] === locator.reference) { + debugUtils.log(`${locator.name}@${locator.reference} is already Last Known Good version`); + return; + } - (lastKnownGood as Record)[locator.name] = locator.reference; + lastKnownGood[locator.name] = locator.reference; debugUtils.log(`Setting ${locator.name}@${locator.reference} as Last Known Good version`); - await overwriteJSONFileContent(lastKnownGoodFile, lastKnownGood); + await createLastKnownGoodFile(lastKnownGood); } export class Engine { @@ -150,54 +157,32 @@ export class Engine { if (typeof definition === `undefined`) throw new UsageError(`This package manager (${packageManager}) isn't supported by this corepack build`); - let lastKnownGoodFile = await getLastKnownGoodFile(`r+`).catch(err => { - if ((err as NodeError)?.code !== `ENOENT` && (err as NodeError)?.code !== `EROFS`) { - throw err; - } - }); - try { - const lastKnownGood = lastKnownGoodFile == null || await getJSONFileContent(lastKnownGoodFile!); - const lastKnownGoodForThisPackageManager = getLastKnownGoodFromFileContent(lastKnownGood, packageManager); - if (lastKnownGoodForThisPackageManager) - return lastKnownGoodForThisPackageManager; - - if (process.env.COREPACK_DEFAULT_TO_LATEST === `0`) - return definition.default; - - const reference = await corepackUtils.fetchLatestStableVersion(definition.fetchLatestFrom); - - try { - lastKnownGoodFile ??= await createLastKnownGoodFile(); - await activatePackageManagerFromFileHandle(lastKnownGoodFile, lastKnownGood, { - name: packageManager, - reference, - }); - } catch { - // If for some reason, we cannot update the last known good file, we - // ignore the error. - } + const lastKnownGood = await getLastKnownGood(); + const lastKnownGoodForThisPackageManager = getLastKnownGoodFromFileContent(lastKnownGood, packageManager); + if (lastKnownGoodForThisPackageManager) + return lastKnownGoodForThisPackageManager; - return reference; - } finally { - await lastKnownGoodFile?.close(); - } - } + if (process.env.COREPACK_DEFAULT_TO_LATEST === `0`) + return definition.default; - async activatePackageManager(locator: Locator) { - let emptyFile = false; - const lastKnownGoodFile = await getLastKnownGoodFile(`r+`).catch(err => { - if ((err as NodeError)?.code === `ENOENT`) { - emptyFile = true; - return getLastKnownGoodFile(`w`); - } + const reference = await corepackUtils.fetchLatestStableVersion(definition.fetchLatestFrom); - throw err; - }); try { - await activatePackageManagerFromFileHandle(lastKnownGoodFile, emptyFile || await getJSONFileContent(lastKnownGoodFile), locator); - } finally { - await lastKnownGoodFile.close(); + await activatePackageManager(lastKnownGood, { + name: packageManager, + reference, + }); + } catch { + // If for some reason, we cannot update the last known good file, we + // ignore the error. } + + return reference; + } + + async activatePackageManager(locator: Locator) { + const lastKnownGood = await getLastKnownGood(); + await activatePackageManager(lastKnownGood, locator); } async ensurePackageManager(locator: Locator) { diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index 8757bc5b6..7d5e1f518 100644 --- a/sources/corepackUtils.ts +++ b/sources/corepackUtils.ts @@ -1,6 +1,5 @@ import {createHash} from 'crypto'; import {once} from 'events'; -import {FileHandle} from 'fs/promises'; import fs from 'fs'; import type {Dir} from 'fs'; import Module from 'module'; @@ -325,26 +324,14 @@ export async function installVersion(installTarget: string, locator: Locator, {s } if (locatorIsASupportedPackageManager && process.env.COREPACK_DEFAULT_TO_LATEST !== `0`) { - let lastKnownGoodFile: FileHandle; - try { - lastKnownGoodFile = await engine.getLastKnownGoodFile(`r+`); - const lastKnownGood = await engine.getJSONFileContent(lastKnownGoodFile); - const defaultVersion = engine.getLastKnownGoodFromFileContent(lastKnownGood, locator.name); - if (defaultVersion) { - const currentDefault = semver.parse(defaultVersion)!; - const downloadedVersion = locatorReference as semver.SemVer; - if (currentDefault.major === downloadedVersion.major && semver.lt(currentDefault, downloadedVersion)) { - await engine.activatePackageManagerFromFileHandle(lastKnownGoodFile, lastKnownGood, locator); - } - } - } catch (err) { - // ENOENT would mean there are no lastKnownGoodFile, in which case we can ignore. - if ((err as nodeUtils.NodeError)?.code !== `ENOENT`) { - throw err; + const lastKnownGood = await engine.getLastKnownGood(); + const defaultVersion = engine.getLastKnownGoodFromFileContent(lastKnownGood, locator.name); + if (defaultVersion) { + const currentDefault = semver.parse(defaultVersion)!; + const downloadedVersion = locatorReference as semver.SemVer; + if (currentDefault.major === downloadedVersion.major && semver.lt(currentDefault, downloadedVersion)) { + await engine.activatePackageManager(lastKnownGood, locator); } - } finally { - // @ts-expect-error used before assigned - await lastKnownGoodFile?.close(); } } diff --git a/tests/_runCli.ts b/tests/_runCli.ts index e53adec4a..52cee362b 100644 --- a/tests/_runCli.ts +++ b/tests/_runCli.ts @@ -4,11 +4,15 @@ import * as path from 'path'; import {pathToFileURL} from 'url'; export async function runCli(cwd: PortablePath, argv: Array, withCustomRegistry?: boolean): Promise<{exitCode: number | null, stdout: string, stderr: string}> { + return spawnCmd(cwd, process.execPath, [`--no-warnings`, ...(withCustomRegistry ? [`--import`, pathToFileURL(path.join(__dirname, `_registryServer.mjs`)) as any as string] : [`-r`, require.resolve(`./recordRequests.js`)]), require.resolve(`../dist/corepack.js`), ...argv]); +} + +export async function spawnCmd(cwd: PortablePath, cmd: string, argv: Array): Promise<{exitCode: number | null, stdout: string, stderr: string}> { const out: Array = []; const err: Array = []; return new Promise((resolve, reject) => { - const child = spawn(process.execPath, [`--no-warnings`, ...(withCustomRegistry ? [`--import`, pathToFileURL(path.join(__dirname, `_registryServer.mjs`)) as any as string] : [`-r`, require.resolve(`./recordRequests.js`)]), require.resolve(`../dist/corepack.js`), ...argv], { + const child = spawn(cmd, argv, { cwd: npath.fromPortablePath(cwd), env: process.env, stdio: `pipe`, diff --git a/tests/main.test.ts b/tests/main.test.ts index 8df247e52..030717229 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -7,7 +7,7 @@ import config from '../config.json'; import * as folderUtils from '../sources/folderUtils'; import {SupportedPackageManagerSet} from '../sources/types'; -import {runCli} from './_runCli'; +import {runCli, spawnCmd} from './_runCli'; beforeEach(async () => { @@ -476,6 +476,96 @@ it(`should support disabling the network accesses from the environment`, async ( }); }); +describe(`read-only and offline environment`, () => { + it(`should support running in project scope`, async () => { + await xfs.mktempPromise(async cwd => { + // Reset to default + delete process.env.COREPACK_DEFAULT_TO_LATEST; + + // Prepare fake project + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { + packageManager: `yarn@2.2.2`, + }); + + // $ corepack install + await expect(runCli(cwd, [`install`])).resolves.toMatchObject({ + stdout: `Adding yarn@2.2.2 to the cache...\n`, + stderr: ``, + exitCode: 0, + }); + + // Let corepack discover the latest yarn version. + // BUG: This should not be necessary with a fully specified version in package.json plus populated corepack cache. + // Engine.executePackageManagerRequest needs to defer the fallback work. This requires a big refactoring. + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + }); + + // Make COREPACK_HOME ro + const home = npath.toPortablePath(folderUtils.getCorepackHomeFolder()); + await xfs.chmodPromise(ppath.join(home, `lastKnownGood.json`), 0o444); + await xfs.chmodPromise(home, 0o555); + + // Use fake proxies to simulate offline mode + process.env.HTTP_PROXY = `0.0.0.0`; + process.env.HTTPS_PROXY = `0.0.0.0`; + + // $ corepack yarn --version + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + stdout: `2.2.2\n`, + stderr: ``, + exitCode: 0, + }); + }); + }); + + it(`should support running globally`, async () => { + await xfs.mktempPromise(async installDir => { + // Reset to default + delete process.env.COREPACK_DEFAULT_TO_LATEST; + + // $ corepack enable + await expect(runCli(installDir, [`enable`, `--install-directory`, npath.fromPortablePath(installDir), `yarn`])).resolves.toMatchObject({ + stdout: ``, + stderr: ``, + exitCode: 0, + }); + + // Simulate the effect of `$ corepack enable` without the custom --install-directory option. + process.env.PATH = `${npath.toPortablePath(installDir)}:${process.env.PATH}`; + + // $ corepack install --global yarn@2.2.2 + await expect(runCli(installDir, [`install`, `--global`, `yarn@2.2.2`])).resolves.toMatchObject({ + stdout: `Installing yarn@2.2.2...\n`, + stderr: ``, + exitCode: 0, + }); + + // Make COREPACK_HOME ro + const home = npath.toPortablePath(folderUtils.getCorepackHomeFolder()); + await xfs.chmodPromise(ppath.join(home, `lastKnownGood.json`), 0o444); + await xfs.chmodPromise(home, 0o555); + + // Use fake proxies to simulate offline mode + process.env.HTTP_PROXY = `0.0.0.0`; + process.env.HTTPS_PROXY = `0.0.0.0`; + + // $ corepack yarn --version + await expect(runCli(installDir, [`yarn`, `--version`])).resolves.toMatchObject({ + stdout: `2.2.2\n`, + stderr: ``, + exitCode: 0, + }); + // $ yarn --version + await expect(spawnCmd(installDir, `yarn`, [`--version`])).resolves.toMatchObject({ + stdout: `2.2.2\n`, + stderr: ``, + exitCode: 0, + }); + }); + }); +}); + it(`should support hydrating package managers from cached archives`, async () => { await xfs.mktempPromise(async cwd => { await expect(runCli(cwd, [`pack`, `yarn@2.2.2`])).resolves.toMatchObject({ From 759dc4d7eac2c2738ad7746e54bf3fb08d85ac4f Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 13 Apr 2024 13:25:28 +0200 Subject: [PATCH 2/2] remove failing test on Windows --- tests/_runCli.ts | 6 +----- tests/main.test.ts | 14 +------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/tests/_runCli.ts b/tests/_runCli.ts index 52cee362b..e53adec4a 100644 --- a/tests/_runCli.ts +++ b/tests/_runCli.ts @@ -4,15 +4,11 @@ import * as path from 'path'; import {pathToFileURL} from 'url'; export async function runCli(cwd: PortablePath, argv: Array, withCustomRegistry?: boolean): Promise<{exitCode: number | null, stdout: string, stderr: string}> { - return spawnCmd(cwd, process.execPath, [`--no-warnings`, ...(withCustomRegistry ? [`--import`, pathToFileURL(path.join(__dirname, `_registryServer.mjs`)) as any as string] : [`-r`, require.resolve(`./recordRequests.js`)]), require.resolve(`../dist/corepack.js`), ...argv]); -} - -export async function spawnCmd(cwd: PortablePath, cmd: string, argv: Array): Promise<{exitCode: number | null, stdout: string, stderr: string}> { const out: Array = []; const err: Array = []; return new Promise((resolve, reject) => { - const child = spawn(cmd, argv, { + const child = spawn(process.execPath, [`--no-warnings`, ...(withCustomRegistry ? [`--import`, pathToFileURL(path.join(__dirname, `_registryServer.mjs`)) as any as string] : [`-r`, require.resolve(`./recordRequests.js`)]), require.resolve(`../dist/corepack.js`), ...argv], { cwd: npath.fromPortablePath(cwd), env: process.env, stdio: `pipe`, diff --git a/tests/main.test.ts b/tests/main.test.ts index 030717229..b0d194fc2 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -7,7 +7,7 @@ import config from '../config.json'; import * as folderUtils from '../sources/folderUtils'; import {SupportedPackageManagerSet} from '../sources/types'; -import {runCli, spawnCmd} from './_runCli'; +import {runCli} from './_runCli'; beforeEach(async () => { @@ -524,17 +524,12 @@ describe(`read-only and offline environment`, () => { // Reset to default delete process.env.COREPACK_DEFAULT_TO_LATEST; - // $ corepack enable await expect(runCli(installDir, [`enable`, `--install-directory`, npath.fromPortablePath(installDir), `yarn`])).resolves.toMatchObject({ stdout: ``, stderr: ``, exitCode: 0, }); - // Simulate the effect of `$ corepack enable` without the custom --install-directory option. - process.env.PATH = `${npath.toPortablePath(installDir)}:${process.env.PATH}`; - - // $ corepack install --global yarn@2.2.2 await expect(runCli(installDir, [`install`, `--global`, `yarn@2.2.2`])).resolves.toMatchObject({ stdout: `Installing yarn@2.2.2...\n`, stderr: ``, @@ -550,18 +545,11 @@ describe(`read-only and offline environment`, () => { process.env.HTTP_PROXY = `0.0.0.0`; process.env.HTTPS_PROXY = `0.0.0.0`; - // $ corepack yarn --version await expect(runCli(installDir, [`yarn`, `--version`])).resolves.toMatchObject({ stdout: `2.2.2\n`, stderr: ``, exitCode: 0, }); - // $ yarn --version - await expect(spawnCmd(installDir, `yarn`, [`--version`])).resolves.toMatchObject({ - stdout: `2.2.2\n`, - stderr: ``, - exitCode: 0, - }); }); }); });