From a8f8736ad08ffe0120495e6fb1222fc20417e7de Mon Sep 17 00:00:00 2001 From: merceyz Date: Sat, 3 Feb 2024 23:05:20 +0100 Subject: [PATCH 1/3] fix: remove unsafe remove of install folder --- sources/Engine.ts | 2 +- sources/corepackUtils.ts | 14 ++------------ sources/folderUtils.ts | 14 +++++++++++++- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/sources/Engine.ts b/sources/Engine.ts index 63c220b96..20b2fc687 100644 --- a/sources/Engine.ts +++ b/sources/Engine.ts @@ -18,7 +18,7 @@ import {SupportedPackageManagers, SupportedPackageManagerSet} from './types'; export type PreparedPackageManagerInfo = Awaited>; export function getLastKnownGoodFile(flag = `r`) { - return fs.promises.open(path.join(folderUtils.getInstallFolder(), `lastKnownGood.json`), flag); + return fs.promises.open(path.join(folderUtils.getCorepackHomeFolder(), `lastKnownGood.json`), flag); } export async function getJSONFileContent(fh: FileHandle) { diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index be4b6bd3a..e3bba8d29 100644 --- a/sources/corepackUtils.ts +++ b/sources/corepackUtils.ts @@ -109,11 +109,9 @@ export async function installVersion(installTarget: string, locator: Locator, {s const {version, build} = locatorReference; const installFolder = path.join(installTarget, locator.name, version); - const corepackFile = path.join(installFolder, `.corepack`); - // Older versions of Corepack didn't generate the `.corepack` file; in - // that case we just download the package manager anew. - if (fs.existsSync(corepackFile)) { + if (fs.existsSync(installFolder)) { + const corepackFile = path.join(installFolder, `.corepack`); const corepackContent = await fs.promises.readFile(corepackFile, `utf8`); const corepackData = JSON.parse(corepackContent); @@ -172,14 +170,6 @@ export async function installVersion(installTarget: string, locator: Locator, {s hash: serializedHash, })); - // The target folder may exist if a previous version of Corepack installed - // it but didn't create the `.corepack` file. In this case we need to - // remove it first. - await fs.promises.rm(installFolder, { - recursive: true, - force: true, - }); - await fs.promises.mkdir(path.dirname(installFolder), {recursive: true}); try { await fs.promises.rename(tmpFolder, installFolder); diff --git a/sources/folderUtils.ts b/sources/folderUtils.ts index 6fc0991c8..7ce475aa9 100644 --- a/sources/folderUtils.ts +++ b/sources/folderUtils.ts @@ -6,7 +6,12 @@ import process from 'process'; import type {NodeError} from './nodeUtils'; -export function getInstallFolder() { +/** + * If the install folder structure changes then increment this number. + */ +const INSTALL_FOLDER_VERSION = 1; + +export function getCorepackHomeFolder() { return ( process.env.COREPACK_HOME ?? join( @@ -18,6 +23,13 @@ export function getInstallFolder() { ); } +export function getInstallFolder() { + return join( + getCorepackHomeFolder(), + `v${INSTALL_FOLDER_VERSION}`, + ); +} + export function getTemporaryFolder(target: string = tmpdir()) { mkdirSync(target, {recursive: true}); From c8b579199b1109282dd73a49dce6b1d832ee596d Mon Sep 17 00:00:00 2001 From: merceyz Date: Sat, 10 Feb 2024 02:17:08 +0100 Subject: [PATCH 2/3] test: update to use folderUtils --- tests/main.test.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/main.test.ts b/tests/main.test.ts index e306acc89..c54fb1f5d 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -3,15 +3,13 @@ import {Filename, ppath, xfs, npath, PortablePath} from '@yarnpkg/fslib'; import process from 'node:process'; import config from '../config.json'; +import * as folderUtils from '../sources/folderUtils'; import {runCli} from './_runCli'; -let corepackHome!: PortablePath; beforeEach(async () => { - corepackHome = await xfs.mktempPromise(); - - process.env.COREPACK_HOME = npath.fromPortablePath(corepackHome); + process.env.COREPACK_HOME = npath.fromPortablePath(await xfs.mktempPromise()); process.env.COREPACK_DEFAULT_TO_LATEST = `0`; }); @@ -101,7 +99,7 @@ for (const [name, version] of testedPackageManagers) { } it(`should update the Known Good Release only when the major matches`, async () => { - await xfs.writeJsonPromise(ppath.join(corepackHome, `lastKnownGood.json`), { + await xfs.writeJsonPromise(ppath.join(npath.toPortablePath(folderUtils.getCorepackHomeFolder()), `lastKnownGood.json`), { yarn: `1.0.0`, }); @@ -645,7 +643,7 @@ it(`should not override the package manager exit code`, async () => { packageManager: `yarn@2.2.2`, }); - const yarnFolder = ppath.join(corepackHome, `yarn/2.2.2`); + const yarnFolder = ppath.join(npath.toPortablePath(folderUtils.getInstallFolder()), `yarn/2.2.2`); await xfs.mkdirPromise(yarnFolder, {recursive: true}); await xfs.writeJsonPromise(ppath.join(yarnFolder, `.corepack`), {}); @@ -670,7 +668,7 @@ it(`should not preserve the process.exitCode when a package manager throws`, asy packageManager: `yarn@2.2.2`, }); - const yarnFolder = ppath.join(corepackHome, `yarn/2.2.2`); + const yarnFolder = ppath.join(npath.toPortablePath(folderUtils.getInstallFolder()), `yarn/2.2.2`); await xfs.mkdirPromise(yarnFolder, {recursive: true}); await xfs.writeJsonPromise(ppath.join(yarnFolder, `.corepack`), {}); @@ -693,7 +691,7 @@ it(`should not set the exit code after successfully launching the package manage packageManager: `yarn@2.2.2`, }); - const yarnFolder = ppath.join(corepackHome, `yarn/2.2.2`); + const yarnFolder = ppath.join(npath.toPortablePath(folderUtils.getInstallFolder()), `yarn/2.2.2`); await xfs.mkdirPromise(yarnFolder, {recursive: true}); await xfs.writeJsonPromise(ppath.join(yarnFolder, `.corepack`), {}); @@ -719,7 +717,7 @@ it(`should support package managers in ESM format`, async () => { packageManager: `yarn@2.2.2`, }); - const yarnFolder = ppath.join(corepackHome, `yarn/2.2.2`); + const yarnFolder = ppath.join(npath.toPortablePath(folderUtils.getInstallFolder()), `yarn/2.2.2`); await xfs.mkdirPromise(yarnFolder, {recursive: true}); await xfs.writeJsonPromise(ppath.join(yarnFolder, `.corepack`), {}); From 48fa7a013c6d779a6d4f95353480146b829238d5 Mon Sep 17 00:00:00 2001 From: merceyz Date: Sun, 11 Feb 2024 15:20:03 +0100 Subject: [PATCH 3/3] perf: skip `existsSync` check --- sources/corepackUtils.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index e3bba8d29..f0be61f10 100644 --- a/sources/corepackUtils.ts +++ b/sources/corepackUtils.ts @@ -110,7 +110,7 @@ export async function installVersion(installTarget: string, locator: Locator, {s const installFolder = path.join(installTarget, locator.name, version); - if (fs.existsSync(installFolder)) { + try { const corepackFile = path.join(installFolder, `.corepack`); const corepackContent = await fs.promises.readFile(corepackFile, `utf8`); const corepackData = JSON.parse(corepackContent); @@ -121,6 +121,10 @@ export async function installVersion(installTarget: string, locator: Locator, {s hash: corepackData.hash as string, location: installFolder, }; + } catch (err) { + if ((err as nodeUtils.NodeError).code !== `ENOENT`) { + throw err; + } } const defaultNpmRegistryURL = spec.url.replace(`{}`, version);