feat: separate read and write operations on lastKnownGood.json (#446)

das7pad · aduh95 · web-flow · commit c449adc81822 · 2024-04-13T11:38:56.000Z
Also skip overwriting `lastKnownGood.json` with same content.

Signed-off-by: Jakob Ackermann &lt;das7pad@outlook.com&gt;
Co-authored-by: Antoine du Hamel &lt;duhamelantoine1995@gmail.com&gt;
diff --git 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<Record<string, string>> {
+  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<string, string>) {
+  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<string, string>, 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<string, string>, locator: Locator) {
+  if (lastKnownGood[locator.name] === locator.reference) {
+    debugUtils.log(`${locator.name}@${locator.reference} is already Last Known Good version`);
+    return;
+  }
 
-  (lastKnownGood as Record<string, string>)[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
@@ -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/main.test.ts b/tests/main.test.ts
@@ -476,6 +476,84 @@ 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;
+
+      await expect(runCli(installDir, [`enable`, `--install-directory`, npath.fromPortablePath(installDir), `yarn`])).resolves.toMatchObject({
+        stdout: ``,
+        stderr: ``,
+        exitCode: 0,
+      });
+
+      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`;
+
+      await expect(runCli(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({
