fix: don't fail when multiple containers use the same store (#6853)

ref #6817

.changeset/six-spies-fly.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/package-requester": patch
+"@pnpm/store.cafs": patch
+"pnpm": patch
+---
+
+When several containers use the same store simultaneously, there's a chance that multiple containers may create a temporary file at the same time. In such scenarios, pnpm could fail to rename the temporary file in one of the containers. This issue has been addressed: pnpm will no longer fail if the temporary file is absent but the destination file exists.

pkg-manager/package-requester/package.json

@@ -55,7 +55,6 @@
     "path-temp": "^2.1.0",
     "promise-share": "^1.0.0",
     "ramda": "npm:@pnpm/ramda@0.28.1",
-    "rename-overwrite": "^4.0.3",
     "safe-promise-defer": "^1.0.1",
     "semver": "^7.5.4",
     "ssri": "10.0.4"

pkg-manager/package-requester/src/packageRequester.ts

@@ -8,6 +8,7 @@ import {
   getFilePathInCafs as _getFilePathInCafs,
   type PackageFileInfo,
   type PackageFilesIndex,
+  optimisticRenameOverwrite,
 } from '@pnpm/store.cafs'
 import { fetchingProgressLogger, progressLogger } from '@pnpm/core-loggers'
 import { pickFetcher } from '@pnpm/pick-fetcher'
@@ -51,7 +52,6 @@ import pDefer from 'p-defer'
 import { fastPathTemp as pathTemp } from 'path-temp'
 import pShare from 'promise-share'
 import pick from 'ramda/src/pick'
-import renameOverwrite from 'rename-overwrite'
 import semver from 'semver'
 import ssri from 'ssri'
 import { equalOrSemverEqual } from './equalOrSemverEqual'
@@ -658,7 +658,7 @@ async function writeJsonFile (filePath: string, data: unknown) {
   await fs.mkdir(targetDir, { recursive: true })
   const temp = pathTemp(filePath)
   await gfs.writeFile(temp, JSON.stringify(data))
-  await renameOverwrite(temp, filePath)
+  await optimisticRenameOverwrite(temp, filePath)
 }
 
 async function readBundledManifest (pkgJsonPath: string): Promise<BundledManifest> {

pnpm-lock.yaml

@@ -3642,9 +3642,6 @@ importers:
       ramda:
         specifier: npm:@pnpm/ramda@0.28.1
         version: /@pnpm/ramda@0.28.1
-      rename-overwrite:
-        specifier: ^4.0.3
-        version: 4.0.3
       safe-promise-defer:
         specifier: ^1.0.1
         version: 1.0.1
@@ -9109,7 +9106,7 @@ packages:
     resolution: {integrity: sha512-IO+MJPVhoqz+28h1qLAcBEH2+xHMK6MTyHJc7MTnnYb6wsoLR29POVGJ7LycmVXIqyy/4/2ShP5sUwTXuOwb/w==}
     dependencies:
       '@types/minimatch': 5.1.2
-      '@types/node': 20.4.2
+      '@types/node': 14.18.53
     dev: true
 
   /@types/graceful-fs@4.1.6:

store/cafs/src/index.ts

@@ -15,7 +15,7 @@ import {
   getFilePathByModeInCafs,
   modeIsExecutable,
 } from './getFilePathInCafs'
-import { writeBufferToCafs } from './writeBufferToCafs'
+import { optimisticRenameOverwrite, writeBufferToCafs } from './writeBufferToCafs'
 
 export type { IntegrityLike } from 'ssri'
 
@@ -27,6 +27,7 @@ export {
   getFilePathInCafs,
   type PackageFileInfo,
   type PackageFilesIndex,
+  optimisticRenameOverwrite,
 }
 
 export type CafsLocker = Map<string, number>

store/cafs/src/writeBufferToCafs.ts

@@ -1,4 +1,4 @@
-import { promises as fs, type Stats } from 'fs'
+import { existsSync, promises as fs, type Stats } from 'fs'
 import path from 'path'
 import renameOverwrite from 'rename-overwrite'
 import type ssri from 'ssri'
@@ -41,13 +41,32 @@ export async function writeBufferToCafs (
     // We log the creation time ourselves and save it in the package index file.
     // Having this information allows us to skip content checks for files that were not modified since "birth time".
     const birthtimeMs = Date.now()
-    await renameOverwrite(temp, fileDest)
+    await optimisticRenameOverwrite(temp, fileDest)
     return birthtimeMs
   })()
   locker.set(fileDest, p)
   return p
 }
 
+export async function optimisticRenameOverwrite (temp: string, fileDest: string) {
+  try {
+    await renameOverwrite(temp, fileDest)
+  } catch (err: any) { // eslint-disable-line
+    if (err.code !== 'ENOENT' || !existsSync(fileDest)) throw err
+    // The temporary file path is created by appending the process ID to the target file name.
+    // This is done to avoid lots of random crypto number generations.
+    //   PR with related performance optimization: https://github.com/pnpm/pnpm/pull/6817
+    //
+    // Probably the only scenario in which the temp directory will dissappear
+    // before being renamed is when two containers use the same mounted directory
+    // for their content-addressable store. In this case there's a chance that the process ID
+    // will be the same in both containers.
+    //
+    // As a workaround, if the temp file doesn't exist but the target file does,
+    // we just ignore the issue and assume that the target file is correct.
+  }
+}
+
 /**
  * The process ID is appended to the file name to create a temporary file.
  * If the process fails, on rerun the new temp file may get a filename the got left over.

store/cafs/test/optimisticRenameOverwrite.test.ts

@@ -0,0 +1,11 @@
+import fs from 'fs'
+import path from 'path'
+import tempy from 'tempy'
+import { optimisticRenameOverwrite } from '../src/writeBufferToCafs'
+
+test("optimisticRenameOverwrite() doesn't crash if target file exists", async () => {
+  const tempDir = tempy.directory()
+  const dest = path.join(tempDir, 'file')
+  fs.writeFileSync(dest, '', 'utf8')
+  await optimisticRenameOverwrite(`${dest}_tmp`, dest)
+})