fix: don't copy optional dependencies from the store (#7255)

close #7046

.changeset/mean-panthers-teach.md

@@ -0,0 +1,8 @@
+---
+"@pnpm/create-cafs-store": patch
+"@pnpm/headless": patch
+"@pnpm/core": patch
+"pnpm": patch
+---
+
+Optional dependencies that do not have to be built will be reflinked (or hardlinked) to the store instead of copied [#7046](https://github.com/pnpm/pnpm/issues/7046).

.changeset/tiny-ears-boil.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/exec.files-include-install-scripts": major
+---
+
+Initial release.

cspell.json

@@ -76,6 +76,7 @@
     "gwhitney",
     "haptics",
     "hardlink",
+    "hardlinked",
     "hardlinking",
     "hardlinks",
     "hashbang",
@@ -191,6 +192,7 @@
     "refclone",
     "reflattened",
     "reflink",
+    "reflinked",
     "reflinks",
     "rehoist",
     "reka",

exec/files-include-install-scripts/README.md

@@ -0,0 +1,15 @@
+# @pnpm/exec.files-include-install-scripts
+
+> Checks if a package has files indicating that it needs to be built during installation
+
+[![npm version](https://img.shields.io/npm/v/@pnpm/exec.files-include-install-scripts.svg)](https://www.npmjs.com/package/@pnpm/exec.files-include-install-scripts)
+
+## Installation
+
+```sh
+pnpm add @pnpm/exec.files-include-install-scripts
+```
+
+## License
+
+MIT

exec/files-include-install-scripts/package.json

@@ -0,0 +1,43 @@
+{
+  "name": "@pnpm/exec.files-include-install-scripts",
+  "version": "0.0.0",
+  "description": "Checks if a package has files indicating that it needs to be built during installation",
+  "main": "lib/index.js",
+  "types": "lib/index.d.ts",
+  "files": [
+    "lib",
+    "!*.map"
+  ],
+  "engines": {
+    "node": ">=16.14"
+  },
+  "scripts": {
+    "lint": "eslint \"src/**/*.ts\"",
+    "test": "pnpm run compile",
+    "prepublishOnly": "pnpm run compile",
+    "fix": "tslint -c tslint.json src/**/*.ts test/**/*.ts --fix",
+    "compile": "tsc --build && pnpm run lint --fix"
+  },
+  "repository": "https://github.com/pnpm/pnpm/blob/main/exec/files-include-install-scripts",
+  "keywords": [
+    "pnpm8",
+    "pnpm"
+  ],
+  "license": "MIT",
+  "bugs": {
+    "url": "https://github.com/pnpm/pnpm/issues"
+  },
+  "homepage": "https://github.com/pnpm/pnpm/blob/main/exec/files-include-install-scripts#readme",
+  "peerDependencies": {
+  },
+  "dependencies": {
+  },
+  "devDependencies": {
+    "@pnpm/exec.files-include-install-scripts": "workspace:*"
+  },
+  "funding": "https://opencollective.com/pnpm",
+  "exports": {
+    ".": "./lib/index.js"
+  }
+}
+

exec/files-include-install-scripts/src/index.ts

@@ -0,0 +1,4 @@
+export function filesIncludeInstallScripts (filesIndex: Record<string, unknown>): boolean {
+  return filesIndex['binding.gyp'] != null ||
+    Object.keys(filesIndex).some((filename) => !(filename.match(/^[.]hooks[\\/]/) == null)) // TODO: optimize this
+}

exec/files-include-install-scripts/tsconfig.json

@@ -0,0 +1,13 @@
+{
+  "extends": "@pnpm/tsconfig",
+  "compilerOptions": {
+    "outDir": "lib",
+    "rootDir": "src"
+  },
+  "include": [
+    "src/**/*.ts",
+    "../../__typings__/**/*.d.ts"
+  ],
+  "references": [],
+  "composite": true
+}

exec/files-include-install-scripts/tsconfig.lint.json

@@ -0,0 +1,8 @@
+{
+  "extends": "./tsconfig.json",
+  "include": [
+    "src/**/*.ts",
+    "test/**/*.ts",
+    "../../__typings__/**/*.d.ts"
+  ]
+}

pkg-manager/core/src/install/link.ts

@@ -450,7 +450,7 @@ async function linkAllPkgs (
         filesResponse: files,
         force: opts.force,
         sideEffectsCacheKey,
-        requiresBuild: depNode.requiresBuild || depNode.patchFile != null,
+        requiresBuild: depNode.patchFile != null || (depNode.optional ? (depNode.requiresBuild ? undefined : false) : depNode.requiresBuild),
       })
       if (importMethod) {
         progressLogger.debug({

pkg-manager/core/test/install/optionalDependencies.ts

@@ -648,3 +648,40 @@ describe('supported architectures', () => {
     expect(fs.readdirSync('node_modules/.pnpm').length).toBe(5)
   })
 })
+
+test('optional dependency is hardlinked to the store if it does not require a build', async () => {
+  prepareEmpty()
+  const manifest = {
+    dependencies: {
+      '@pnpm.e2e/pkg-with-good-optional': '*',
+    },
+  }
+
+  const reporter = jest.fn()
+  await install(manifest, await testDefaults({ reporter }, {}, {}, { packageImportMethod: 'hardlink' }))
+
+  expect(reporter).toHaveBeenCalledWith(
+    expect.objectContaining({
+      level: 'debug',
+      name: 'pnpm:progress',
+      method: 'hardlink',
+      status: 'imported',
+      to: path.resolve('node_modules/.pnpm/is-positive@1.0.0/node_modules/is-positive'),
+    })
+  )
+
+  await rimraf('node_modules')
+
+  reporter.mockClear()
+  await install(manifest, await testDefaults({ frozenLockfile: true, reporter }, {}, {}, { packageImportMethod: 'hardlink' }))
+
+  expect(reporter).toHaveBeenCalledWith(
+    expect.objectContaining({
+      level: 'debug',
+      name: 'pnpm:progress',
+      method: 'hardlink',
+      status: 'imported',
+      to: path.resolve('node_modules/.pnpm/is-positive@1.0.0/node_modules/is-positive'),
+    })
+  )
+})

pkg-manager/headless/src/index.ts

@@ -809,7 +809,7 @@ async function linkAllPkgs (
         filesResponse,
         force: opts.force,
         disableRelinkLocalDirDeps: opts.disableRelinkLocalDirDeps,
-        requiresBuild: depNode.requiresBuild || depNode.patchFile != null,
+        requiresBuild: depNode.patchFile != null || (depNode.optional ? (depNode.requiresBuild ? undefined : false) : depNode.requiresBuild),
         sideEffectsCacheKey,
       })
       if (importMethod) {

pkg-manager/headless/src/linkHoistedModules.ts

@@ -127,7 +127,7 @@ async function linkAllPkgsInOrder (
             force: true,
             disableRelinkLocalDirDeps: opts.disableRelinkLocalDirDeps,
             keepModulesDir: true,
-            requiresBuild: depNode.requiresBuild || depNode.patchFile != null,
+            requiresBuild: depNode.patchFile != null || (depNode.optional ? (depNode.requiresBuild ? undefined : false) : depNode.requiresBuild),
             sideEffectsCacheKey,
           })
           if (importMethod) {

pkg-manager/resolve-dependencies/package.json

@@ -33,6 +33,7 @@
     "@pnpm/core-loggers": "workspace:*",
     "@pnpm/dependency-path": "workspace:*",
     "@pnpm/error": "workspace:*",
+    "@pnpm/exec.files-include-install-scripts": "workspace:*",
     "@pnpm/lockfile-types": "workspace:*",
     "@pnpm/lockfile-utils": "workspace:*",
     "@pnpm/manifest-utils": "workspace:*",

pkg-manager/resolve-dependencies/src/index.ts

@@ -1,5 +1,6 @@
 import path from 'path'
 import { PnpmError } from '@pnpm/error'
+import { filesIncludeInstallScripts } from '@pnpm/exec.files-include-install-scripts'
 import {
   packageManifestLogger,
 } from '@pnpm/core-loggers'
@@ -341,8 +342,7 @@ async function finishLockfileUpdates (
             Boolean(pkgJson.scripts.install) ||
             Boolean(pkgJson.scripts.postinstall)
           ) ||
-          files.filesIndex['binding.gyp'] ||
-            Object.keys(files.filesIndex).some((filename) => !(filename.match(/^[.]hooks[\\/]/) == null)) // TODO: optimize this
+          filesIncludeInstallScripts(files.filesIndex)
         )
       }
       if (typeof depNode.requiresBuild === 'function') {

pkg-manager/resolve-dependencies/tsconfig.json

@@ -12,6 +12,9 @@
     {
       "path": "../../config/pick-registry-for-package"
     },
+    {
+      "path": "../../exec/files-include-install-scripts"
+    },
     {
       "path": "../../fetching/pick-fetcher"
     },

pnpm-lock.yaml

@@ -1113,6 +1113,12 @@ importers:
         specifier: 0.28.20
         version: 0.28.20
 
+  exec/files-include-install-scripts:
+    devDependencies:
+      '@pnpm/exec.files-include-install-scripts':
+        specifier: workspace:*
+        version: 'link:'
+
   exec/lifecycle:
     dependencies:
       '@pnpm/core-loggers':
@@ -4068,6 +4074,9 @@ importers:
       '@pnpm/error':
         specifier: workspace:*
         version: link:../../packages/error
+      '@pnpm/exec.files-include-install-scripts':
+        specifier: workspace:*
+        version: link:../../exec/files-include-install-scripts
       '@pnpm/lockfile-types':
         specifier: workspace:*
         version: link:../../lockfile/lockfile-types
@@ -5602,6 +5611,9 @@ importers:
 
   store/create-cafs-store:
     dependencies:
+      '@pnpm/exec.files-include-install-scripts':
+        specifier: workspace:*
+        version: link:../../exec/files-include-install-scripts
       '@pnpm/fetcher-base':
         specifier: workspace:*
         version: link:../../fetching/fetcher-base

store/create-cafs-store/package.json

@@ -15,6 +15,7 @@
     "@pnpm/logger": "^5.0.0"
   },
   "dependencies": {
+    "@pnpm/exec.files-include-install-scripts": "workspace:*",
     "@pnpm/store.cafs": "workspace:*",
     "@pnpm/fetcher-base": "workspace:*",
     "@pnpm/fs.indexed-pkg-importer": "workspace:*",

store/create-cafs-store/src/index.ts

@@ -1,5 +1,6 @@
-import { promises as fs } from 'fs'
+import { promises as fs, readFileSync } from 'fs'
 import path from 'path'
+import { filesIncludeInstallScripts } from '@pnpm/exec.files-include-install-scripts'
 import {
   type CafsLocker,
   createCafs,
@@ -34,7 +35,8 @@ export function createPackageImporterAsync (
   const gfm = getFlatMap.bind(null, opts.cafsDir)
   return async (to, opts) => {
     const { filesMap, isBuilt } = gfm(opts.filesResponse, opts.sideEffectsCacheKey)
-    const pkgImportMethod = (opts.requiresBuild && !isBuilt)
+    const willBeBuilt = !isBuilt && (opts.requiresBuild ?? pkgRequiresBuild(filesMap))
+    const pkgImportMethod = willBeBuilt
       ? 'clone-or-copy'
       : (opts.filesResponse.packageImportMethod ?? packageImportMethod)
     const impPkg = cachedImporterCreator(pkgImportMethod)
@@ -63,7 +65,8 @@ function createPackageImporter (
   const gfm = getFlatMap.bind(null, opts.cafsDir)
   return (to, opts) => {
     const { filesMap, isBuilt } = gfm(opts.filesResponse, opts.sideEffectsCacheKey)
-    const pkgImportMethod = (opts.requiresBuild && !isBuilt)
+    const willBeBuilt = !isBuilt && (opts.requiresBuild ?? pkgRequiresBuild(filesMap))
+    const pkgImportMethod = willBeBuilt
       ? 'clone-or-copy'
       : (opts.filesResponse.packageImportMethod ?? packageImportMethod)
     const impPkg = cachedImporterCreator(pkgImportMethod)
@@ -128,3 +131,16 @@ export function createCafsStore (
     },
   }
 }
+
+function pkgRequiresBuild (filesMap: Record<string, string>) {
+  return filesIncludeInstallScripts(filesMap) ||
+    filesMap['package.json'] && pkgJsonHasInstallScripts(filesMap['package.json'])
+}
+
+function pkgJsonHasInstallScripts (file: string): boolean {
+  const pkgJson = JSON.parse(readFileSync(file, 'utf8'))
+  if (!pkgJson.scripts) return false
+  return Boolean(pkgJson.scripts.preinstall) ||
+    Boolean(pkgJson.scripts.install) ||
+    Boolean(pkgJson.scripts.postinstall)
+}

store/create-cafs-store/tsconfig.json

@@ -12,6 +12,9 @@
     {
       "path": "../../__utils__/prepare"
     },
+    {
+      "path": "../../exec/files-include-install-scripts"
+    },
     {
       "path": "../../fetching/fetcher-base"
     },