fix: remove unsafe remove of install folder (#372)

* fix: remove unsafe remove of install folder

* test: update to use folderUtils

* perf: skip `existsSync` check

sources/Engine.ts

@@ -18,7 +18,7 @@ import {SupportedPackageManagers, SupportedPackageManagerSet} from './types';
 export type PreparedPackageManagerInfo = Awaited<ReturnType<Engine[`ensurePackageManager`]>>;
 
 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) {

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)) {
+  try {
+    const corepackFile = path.join(installFolder, `.corepack`);
     const corepackContent = await fs.promises.readFile(corepackFile, `utf8`);
     const corepackData = JSON.parse(corepackContent);
 
@@ -123,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);
@@ -172,14 +174,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);

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});
 

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: `[email protected]`,
     });
 
-    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: `[email protected]`,
     });
 
-    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: `[email protected]`,
     });
 
-    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: `[email protected]`,
     });
 
-    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`), {});