perf: do not build the same dependency multiple times when node-linker is hoisted (#5814)

This commit is contained in:
Zoltan Kochan
2022-12-21 12:29:59 +02:00
committed by GitHub
parent 7030cc26e8
commit 3360c9f4b0
7 changed files with 173 additions and 21 deletions

View File

@@ -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.

View File

@@ -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"

View File

@@ -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()
}
}
}

View File

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

View File

@@ -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')
})

View File

@@ -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,

45
pnpm-lock.yaml generated
View File

@@ -857,6 +857,9 @@ importers:
'@pnpm/error':
specifier: workspace:*
version: link:../../packages/error
'@pnpm/fs.hard-link-dir':
specifier: workspace:*
version: link:../../fs/hard-link-dir
'@pnpm/graph-sequencer':
specifier: 1.0.0
version: 1.0.0
@@ -878,6 +881,9 @@ importers:
'@pnpm/types':
specifier: workspace:*
version: link:../../packages/types
p-defer:
specifier: ^3.0.0
version: 3.0.0
patch-package:
specifier: ^6.5.0
version: 6.5.0
@@ -6767,7 +6773,7 @@ packages:
engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0}
dependencies:
'@jest/types': 29.3.1
'@types/node': 18.11.9
'@types/node': 18.11.17
chalk: 4.1.2
jest-message-util: 29.3.1
jest-util: 29.3.1
@@ -6788,14 +6794,14 @@ packages:
'@jest/test-result': 29.3.1
'@jest/transform': 29.3.1_@babel+types@7.20.5
'@jest/types': 29.3.1
'@types/node': 18.11.9
'@types/node': 18.11.17
ansi-escapes: 4.3.2
chalk: 4.1.2
ci-info: 3.6.1
exit: 0.1.2
graceful-fs: 4.2.10
jest-changed-files: 29.2.0
jest-config: 29.3.1_x4zmdo4imr2n2hhykfyzszwk6y
jest-config: 29.3.1_uvwivoxixlgocwjbikx7evbpf4
jest-haste-map: 29.3.1
jest-message-util: 29.3.1
jest-regex-util: 29.2.0
@@ -6883,7 +6889,7 @@ packages:
'@jest/transform': 29.3.1_@babel+types@7.20.5
'@jest/types': 29.3.1
'@jridgewell/trace-mapping': 0.3.17
'@types/node': 18.11.9
'@types/node': 18.11.17
chalk: 4.1.2
collect-v8-coverage: 1.0.1
exit: 0.1.2
@@ -6973,7 +6979,7 @@ packages:
'@jest/schemas': 29.0.0
'@types/istanbul-lib-coverage': 2.0.4
'@types/istanbul-reports': 3.0.1
'@types/node': 18.11.9
'@types/node': 18.11.17
'@types/yargs': 17.0.13
chalk: 4.1.2
dev: true
@@ -6985,7 +6991,7 @@ packages:
'@jest/schemas': 29.0.0
'@types/istanbul-lib-coverage': 2.0.4
'@types/istanbul-reports': 3.0.1
'@types/node': 18.11.9
'@types/node': 18.11.17
'@types/yargs': 17.0.13
chalk: 4.1.2
dev: true
@@ -8469,7 +8475,7 @@ packages:
dependencies:
'@types/http-cache-semantics': 4.0.1
'@types/keyv': 3.1.4
'@types/node': 18.11.9
'@types/node': 18.11.17
'@types/responselike': 1.0.0
/@types/concat-stream/2.0.0:
@@ -8497,7 +8503,7 @@ packages:
resolution: {integrity: sha512-l6NQsDDyQUVeoTynNpC9uRvCUint/gSUXQA2euwmTuWGvPY5LSDUu6tkCtJB2SvGQlJQzLaKqcGZP4//7EDveA==}
dependencies:
'@types/minimatch': 5.1.2
'@types/node': 18.11.9
'@types/node': 18.11.17
dev: true
/@types/graceful-fs/4.1.5:
@@ -8567,7 +8573,7 @@ packages:
/@types/keyv/3.1.4:
resolution: {integrity: sha512-BQ5aZNSCpj7D6K2ksrRCTmKRLEpnPvWDiLPfoGyhZ++8YtiK9d/3DBKPJgry359X/P1PfruyYwvnvwFjuEiEIg==}
dependencies:
'@types/node': 18.11.9
'@types/node': 18.11.17
/@types/lodash/4.14.181:
resolution: {integrity: sha512-n3tyKthHJbkiWhDZs3DkhkCzt2MexYHXlX0td5iMplyfwketaOeKboEVBqzceH7juqvEg3q5oUoBFxSLu7zFag==}
@@ -8611,7 +8617,6 @@ packages:
/@types/node/18.11.17:
resolution: {integrity: sha512-HJSUJmni4BeDHhfzn6nF0sVmd1SMezP7/4F0Lq+aXzmp2xm9O7WXrUtHW/CHlYVtZUbByEvWidHqRtcJXGF2Ng==}
dev: true
/@types/node/18.11.9:
resolution: {integrity: sha512-CRpX21/kGdzjOpFsZSkcrXMGIBWMGNIHXXBVFSH+ggkftxg+XYP20TESbh+zFvFj3EQOl5byk0HTRn1IL6hbqg==}
@@ -8648,7 +8653,7 @@ packages:
/@types/responselike/1.0.0:
resolution: {integrity: sha512-85Y2BjiufFzaMIlvJDvTTB8Fxl2xfLo4HgmHzVBz08w4wDePCTjYw66PdrolO0kzli3yam/YCgRufyo1DdQVTA==}
dependencies:
'@types/node': 18.11.9
'@types/node': 18.11.17
/@types/retry/0.12.2:
resolution: {integrity: sha512-XISRgDJ2Tc5q4TRqvgJtzsRkFYNJzZrhTdtMoGVBttwzzQJkPnS3WWTFc7kuDRoPtPakl+T+OfdEUjYJj7Jbow==}
@@ -9068,7 +9073,7 @@ packages:
resolution: {integrity: sha512-xGj0G1xvOIRrQkB2aT/oBWscDqq20NAG4qSbM9gT7px58nyVAZvhBOSMyEEkVyTeHtJtPIMdADKbXawdZqnrVQ==}
engines: {node: '>=14.15.0'}
dependencies:
'@types/node': 18.11.9
'@types/node': 18.11.17
'@yarnpkg/fslib': 3.0.0-rc.25
/@yarnpkg/shell/3.2.0-rc.8_typanion@3.12.1:
@@ -12841,7 +12846,7 @@ packages:
- supports-color
dev: true
/jest-config/29.3.1_x4zmdo4imr2n2hhykfyzszwk6y:
/jest-config/29.3.1_uvwivoxixlgocwjbikx7evbpf4:
resolution: {integrity: sha512-y0tFHdj2WnTEhxmGUK1T7fgLen7YK4RtfvpLFBXfQkh2eMJAQq24Vx9472lvn5wg0MAO6B+iPfJfzdR9hJYalg==}
engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0}
peerDependencies:
@@ -12856,7 +12861,7 @@ packages:
'@babel/core': 7.20.5
'@jest/test-sequencer': 29.3.1
'@jest/types': 29.3.1
'@types/node': 18.11.9
'@types/node': 18.11.17
babel-jest: 29.3.1_5zl3o24ezxc3cc3y3khgvpammi
chalk: 4.1.2
ci-info: 3.6.1
@@ -12933,7 +12938,7 @@ packages:
dependencies:
'@jest/types': 29.3.1
'@types/graceful-fs': 4.1.5
'@types/node': 18.11.9
'@types/node': 18.11.17
anymatch: 3.1.2
fb-watchman: 2.0.2
graceful-fs: 4.2.10
@@ -13039,7 +13044,7 @@ packages:
'@jest/test-result': 29.3.1
'@jest/transform': 29.3.1_@babel+types@7.20.5
'@jest/types': 29.3.1
'@types/node': 18.11.9
'@types/node': 18.11.17
chalk: 4.1.2
emittery: 0.13.1
graceful-fs: 4.2.10
@@ -13071,7 +13076,7 @@ packages:
'@jest/test-result': 29.3.1
'@jest/transform': 29.3.1_@babel+types@7.20.5
'@jest/types': 29.3.1
'@types/node': 18.11.9
'@types/node': 18.11.17
chalk: 4.1.2
cjs-module-lexer: 1.2.2
collect-v8-coverage: 1.0.1
@@ -13128,7 +13133,7 @@ packages:
engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0}
dependencies:
'@jest/types': 29.1.2
'@types/node': 18.11.9
'@types/node': 18.11.17
chalk: 4.1.2
ci-info: 3.6.1
graceful-fs: 4.2.10
@@ -13140,7 +13145,7 @@ packages:
engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0}
dependencies:
'@jest/types': 29.3.1
'@types/node': 18.11.9
'@types/node': 18.11.17
chalk: 4.1.2
ci-info: 3.6.1
graceful-fs: 4.2.10
@@ -13165,7 +13170,7 @@ packages:
dependencies:
'@jest/test-result': 29.3.1
'@jest/types': 29.3.1
'@types/node': 18.11.9
'@types/node': 18.11.17
ansi-escapes: 4.3.2
chalk: 4.1.2
emittery: 0.13.1