perf: do not build the same dependency multiple times when node-linke…

…r is hoisted (#5814)

.changeset/sour-cougars-deliver.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/build-modules": minor
+"pnpm": minor
+---
+
+When the hoister node-linker is used, pnpm should not build the same package multiple times during installation. If a package is present at multipe locations because hoisting could not hoist them to a single directory, then the package should only built in one of the locations and copied to the rest.

exec/build-modules/package.json

@@ -37,12 +37,14 @@
     "@pnpm/calc-dep-state": "workspace:*",
     "@pnpm/core-loggers": "workspace:*",
     "@pnpm/error": "workspace:*",
+    "@pnpm/fs.hard-link-dir": "workspace:*",
     "@pnpm/graph-sequencer": "1.0.0",
     "@pnpm/lifecycle": "workspace:*",
     "@pnpm/link-bins": "workspace:*",
     "@pnpm/read-package-json": "workspace:*",
     "@pnpm/store-controller-types": "workspace:*",
     "@pnpm/types": "workspace:*",
+    "p-defer": "^3.0.0",
     "patch-package": "^6.5.0",
     "ramda": "npm:@pnpm/ramda@0.28.1",
     "run-groups": "^3.0.1"

exec/build-modules/src/index.ts

@@ -5,9 +5,11 @@ import { PnpmError } from '@pnpm/error'
 import { runPostinstallHooks } from '@pnpm/lifecycle'
 import { linkBins, linkBinsOfPackages } from '@pnpm/link-bins'
 import { logger } from '@pnpm/logger'
+import { hardLinkDir } from '@pnpm/fs.hard-link-dir'
 import { readPackageJsonFromDir, safeReadPackageJsonFromDir } from '@pnpm/read-package-json'
 import { StoreController } from '@pnpm/store-controller-types'
 import { DependencyManifest } from '@pnpm/types'
+import pDefer, { DeferredPromise } from 'p-defer'
 import { applyPatch } from 'patch-package/dist/applyPatches'
 import pickBy from 'ramda/src/pickBy'
 import runGroups from 'run-groups'
@@ -38,12 +40,16 @@ export async function buildModules (
     sideEffectsCacheWrite: boolean
     storeController: StoreController
     rootModulesDir: string
+    hoistedLocations?: Record<string, string[]>
   }
 ) {
   const warn = (message: string) => logger.warn({ message, prefix: opts.lockfileDir })
   // postinstall hooks
 
   const buildDepOpts = { ...opts, warn }
+  if (opts.hoistedLocations) {
+    buildDepOpts['builtHoistedDeps'] = {}
+  }
   const chunks = buildSequence(depGraph, rootDepPaths)
   const groups = chunks.map((chunk) => {
     chunk = chunk.filter((depPath) => {
@@ -81,10 +87,19 @@ async function buildDependency (
     sideEffectsCacheWrite: boolean
     storeController: StoreController
     unsafePerm: boolean
+    hoistedLocations?: Record<string, string[]>
+    builtHoistedDeps?: Record<string, DeferredPromise<void>>
     warn: (message: string) => void
   }
 ) {
   const depNode = depGraph[depPath]
+  if (opts.builtHoistedDeps) {
+    if (opts.builtHoistedDeps[depNode.depPath]) {
+      await opts.builtHoistedDeps[depNode.depPath].promise
+      return
+    }
+    opts.builtHoistedDeps[depNode.depPath] = pDefer()
+  }
   try {
     await linkBinsOfDependencies(depNode, depGraph, opts)
     const isPatched = depNode.patchFile?.path != null
@@ -147,6 +162,18 @@ async function buildDependency (
       return
     }
     throw err
+  } finally {
+    const hoistedLocationsOfDep = opts.hoistedLocations?.[depNode.depPath]
+    if (hoistedLocationsOfDep) {
+      // There is no need to build the same package in every location.
+      // We just copy the built package to every location where it is present.
+      const currentHoistedLocation = path.relative(opts.lockfileDir, depNode.dir)
+      const nonBuiltHoistedDeps = hoistedLocationsOfDep?.filter((hoistedLocation) => hoistedLocation !== currentHoistedLocation)
+      await hardLinkDir(depNode.dir, nonBuiltHoistedDeps)
+    }
+    if (opts.builtHoistedDeps) {
+      opts.builtHoistedDeps[depNode.depPath].resolve()
+    }
   }
 }
 

exec/build-modules/tsconfig.json

@@ -9,6 +9,9 @@
     "../../__typings__/**/*.d.ts"
   ],
   "references": [
+    {
+      "path": "../../fs/hard-link-dir"
+    },
     {
       "path": "../../packages/calc-dep-state"
     },

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

@@ -1,11 +1,14 @@
 import * as path from 'path'
 import { promises as fs } from 'fs'
+import { assertProject } from '@pnpm/assert-project'
 import { LifecycleLog } from '@pnpm/core-loggers'
-import { prepareEmpty } from '@pnpm/prepare'
+import { prepareEmpty, preparePackages } from '@pnpm/prepare'
 import {
   addDependenciesToPackage,
   install,
   mutateModulesInSingleProject,
+  MutatedProject,
+  mutateModules,
 } from '@pnpm/core'
 import rimraf from '@zkochan/rimraf'
 import isWindows from 'is-windows'
@@ -608,3 +611,108 @@ test('ignore-dep-scripts', async () => {
     expect(await exists('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')).toBeFalsy()
   }
 })
+
+test('run pre/postinstall scripts in a workspace that uses node-linker=hoisted', async () => {
+  const projects = preparePackages([
+    {
+      location: 'project-1',
+      package: { name: 'project-1' },
+    },
+    {
+      location: 'project-2',
+      package: { name: 'project-2' },
+    },
+    {
+      location: 'project-3',
+      package: { name: 'project-3' },
+    },
+    {
+      location: 'project-4',
+      package: { name: 'project-4' },
+    },
+  ])
+  const importers: MutatedProject[] = [
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-2'),
+    },
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-3'),
+    },
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-4'),
+    },
+  ]
+  const allProjects = [
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-1',
+        version: '1.0.0',
+
+        dependencies: {
+          '@pnpm.e2e/pre-and-postinstall-scripts-example': '1',
+        },
+      },
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-2',
+        version: '1.0.0',
+
+        dependencies: {
+          '@pnpm.e2e/pre-and-postinstall-scripts-example': '1',
+        },
+      },
+      rootDir: path.resolve('project-2'),
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-3',
+        version: '1.0.0',
+
+        dependencies: {
+          '@pnpm.e2e/pre-and-postinstall-scripts-example': '2',
+        },
+      },
+      rootDir: path.resolve('project-3'),
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-4',
+        version: '1.0.0',
+
+        dependencies: {
+          '@pnpm.e2e/pre-and-postinstall-scripts-example': '2',
+        },
+      },
+      rootDir: path.resolve('project-4'),
+    },
+  ]
+  await mutateModules(importers, await testDefaults({
+    allProjects,
+    fastUnpack: false,
+    nodeLinker: 'hoisted',
+  }))
+  const rootProject = assertProject(process.cwd())
+  await rootProject.has('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')
+  await rootProject.has('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')
+  await projects['project-1'].hasNot('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')
+  await projects['project-1'].hasNot('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')
+  await projects['project-2'].hasNot('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')
+  await projects['project-2'].hasNot('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')
+  await projects['project-3'].has('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')
+  await projects['project-3'].has('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')
+  await projects['project-4'].has('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')
+  await projects['project-4'].has('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')
+})

pkg-manager/headless/src/index.ts

@@ -467,6 +467,7 @@ export async function headlessInstall (opts: HeadlessOptions) {
       extraEnv,
       depsStateCache,
       ignoreScripts: opts.ignoreScripts || opts.ignoreDepScripts,
+      hoistedLocations,
       lockfileDir,
       optional: opts.include.optionalDependencies,
       preferSymlinkedExecutables: opts.preferSymlinkedExecutables,