Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .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.
2 changes: 2 additions & 0 deletions exec/build-modules/package.json
Expand Up @@ -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"
Expand Down
27 changes: 27 additions & 0 deletions exec/build-modules/src/index.ts
Expand Up @@ -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'
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions exec/build-modules/tsconfig.json
Expand Up @@ -9,6 +9,9 @@
"../../__typings__/**/*.d.ts"
],
"references": [
{
"path": "../../fs/hard-link-dir"
},
{
"path": "../../packages/calc-dep-state"
},
Expand Down
110 changes: 109 additions & 1 deletion 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'
Expand Down Expand Up @@ -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')
})
1 change: 1 addition & 0 deletions pkg-manager/headless/src/index.ts
Expand Up @@ -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,
Expand Down