fix: make shorter temp file names in the store (#6845)

close #6842

.changeset/green-donuts-pull.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/store.cafs": patch
+"pnpm": patch
+---
+
+The length of the temporary file names in the content-addressable store reduced in order to prevent `ENAMETOOLONG` errors from happening [#6842](https://github.com/pnpm/pnpm/issues/6842).

store/cafs/package.json

@@ -24,7 +24,6 @@
     "get-stream": "^6.0.1",
     "gunzip-maybe": "1.4.2",
     "p-limit": "^3.1.0",
-    "path-temp": "^2.1.0",
     "rename-overwrite": "^4.0.3",
     "safe-promise-defer": "^1.0.1",
     "ssri": "10.0.4",

store/cafs/src/index.ts

@@ -1,16 +1,11 @@
-import { promises as fs, type Stats } from 'fs'
-import path from 'path'
 import type { FileWriteResult, PackageFileInfo } from '@pnpm/cafs-types'
 import getStream from 'get-stream'
-import { fastPathTemp as pathTemp } from 'path-temp'
-import renameOverwrite from 'rename-overwrite'
 import ssri from 'ssri'
 import { addFilesFromDir } from './addFilesFromDir'
 import { addFilesFromTarball } from './addFilesFromTarball'
 import {
   checkPkgFilesIntegrity,
   type PackageFilesIndex,
-  verifyFileIntegrity,
 } from './checkPkgFilesIntegrity'
 import { readManifestFromStore } from './readManifestFromStore'
 import {
@@ -20,7 +15,7 @@ import {
   getFilePathByModeInCafs,
   modeIsExecutable,
 } from './getFilePathInCafs'
-import { writeFile } from './writeFile'
+import { writeBufferToCafs } from './writeBufferToCafs'
 
 export type { IntegrityLike } from 'ssri'
 
@@ -83,59 +78,3 @@ async function addBufferToCafs (
   )
   return { checkedAt, integrity }
 }
-
-async function writeBufferToCafs (
-  locker: Map<string, Promise<number>>,
-  cafsDir: string,
-  buffer: Buffer,
-  fileDest: string,
-  mode: number | undefined,
-  integrity: ssri.IntegrityLike
-): Promise<number> {
-  fileDest = path.join(cafsDir, fileDest)
-  if (locker.has(fileDest)) {
-    return locker.get(fileDest)!
-  }
-  const p = (async () => {
-    // This part is a bit redundant.
-    // When a file is already used by another package,
-    // we probably have validated its content already.
-    // However, there is no way to find which package index file references
-    // the given file. So we should revalidate the content of the file again.
-    if (await existsSame(fileDest, integrity)) {
-      return Date.now()
-    }
-
-    // This might be too cautious.
-    // The write is atomic, so in case pnpm crashes, no broken file
-    // will be added to the store.
-    // It might be a redundant step though, as we verify the contents of the
-    // files before linking
-    //
-    // If we don't allow --no-verify-store-integrity then we probably can write
-    // to the final file directly.
-    const temp = pathTemp(fileDest)
-    await writeFile(temp, buffer, mode)
-    // Unfortunately, "birth time" (time of file creation) is available not on all filesystems.
-    // 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)
-    return birthtimeMs
-  })()
-  locker.set(fileDest, p)
-  return p
-}
-
-async function existsSame (filename: string, integrity: ssri.IntegrityLike) {
-  let existingFile: Stats | undefined
-  try {
-    existingFile = await fs.stat(filename)
-  } catch (err) {
-    return false
-  }
-  return verifyFileIntegrity(filename, {
-    size: existingFile.size,
-    integrity,
-  })
-}

store/cafs/src/writeBufferToCafs.ts

@@ -0,0 +1,82 @@
+import { promises as fs, type Stats } from 'fs'
+import path from 'path'
+import renameOverwrite from 'rename-overwrite'
+import type ssri from 'ssri'
+import { verifyFileIntegrity } from './checkPkgFilesIntegrity'
+import { writeFile } from './writeFile'
+
+export async function writeBufferToCafs (
+  locker: Map<string, Promise<number>>,
+  cafsDir: string,
+  buffer: Buffer,
+  fileDest: string,
+  mode: number | undefined,
+  integrity: ssri.IntegrityLike
+): Promise<number> {
+  fileDest = path.join(cafsDir, fileDest)
+  if (locker.has(fileDest)) {
+    return locker.get(fileDest)!
+  }
+  const p = (async () => {
+    // This part is a bit redundant.
+    // When a file is already used by another package,
+    // we probably have validated its content already.
+    // However, there is no way to find which package index file references
+    // the given file. So we should revalidate the content of the file again.
+    if (await existsSame(fileDest, integrity)) {
+      return Date.now()
+    }
+
+    // This might be too cautious.
+    // The write is atomic, so in case pnpm crashes, no broken file
+    // will be added to the store.
+    // It might be a redundant step though, as we verify the contents of the
+    // files before linking
+    //
+    // If we don't allow --no-verify-store-integrity then we probably can write
+    // to the final file directly.
+    const temp = pathTemp(fileDest)
+    await writeFile(temp, buffer, mode)
+    // Unfortunately, "birth time" (time of file creation) is available not on all filesystems.
+    // 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)
+    return birthtimeMs
+  })()
+  locker.set(fileDest, p)
+  return p
+}
+
+/**
+ * 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.
+ * That is fine, the file will be overriden.
+ */
+export function pathTemp (file: string): string {
+  const basename = removeSuffix(path.basename(file))
+  return path.join(path.dirname(file), `${basename}${process.pid}`)
+}
+
+function removeSuffix (filePath: string): string {
+  const dashPosition = filePath.indexOf('-')
+  if (dashPosition === -1) return filePath
+  const withoutSuffix = filePath.substring(0, dashPosition)
+  if (filePath.substring(dashPosition) === '-exec') {
+    return `${withoutSuffix}x`
+  }
+  return withoutSuffix
+}
+
+async function existsSame (filename: string, integrity: ssri.IntegrityLike) {
+  let existingFile: Stats | undefined
+  try {
+    existingFile = await fs.stat(filename)
+  } catch (err) {
+    return false
+  }
+  return verifyFileIntegrity(filename, {
+    size: existingFile.size,
+    integrity,
+  })
+}

store/cafs/test/writeBufferToCafs.test.ts

@@ -0,0 +1,17 @@
+import fs from 'fs'
+import path from 'path'
+import ssri from 'ssri'
+import tempy from 'tempy'
+import { pathTemp, writeBufferToCafs } from '../src/writeBufferToCafs'
+
+describe('writeBufferToCafs', () => {
+  it('should not fail if a file already exists at the temp file location', async () => {
+    const cafsDir = tempy.directory()
+    const fileDest = 'abc'
+    const buffer = Buffer.from('abc')
+    const fullFileDest = path.join(cafsDir, fileDest)
+    fs.writeFileSync(pathTemp(fullFileDest), 'ccc', 'utf8')
+    await writeBufferToCafs(new Map(), cafsDir, buffer, fileDest, 420, ssri.fromData(buffer))
+    expect(fs.readFileSync(fullFileDest, 'utf8')).toBe('abc')
+  })
+})