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

feat: store locations of deps when node-linker is set to hoisted #5795

Merged
merged 10 commits into from Dec 18, 2022
8 changes: 8 additions & 0 deletions .changeset/cold-pigs-burn.md
@@ -0,0 +1,8 @@
---
"@pnpm/fs.indexed-pkg-importer": minor
"@pnpm/cafs-types": minor
"@pnpm/create-cafs-store": minor
"@pnpm/store-controller-types": minor
---

A new option added to package importer for keeping modules directory: `keepModulesDir`. When this is set to true, if a package already exist at the target location and it has a node_modules directory, then that node_modules directory is moved to the newly imported dependency. This is only needed when node-linker=hoisted is used.
7 changes: 7 additions & 0 deletions .changeset/late-swans-impress.md
@@ -0,0 +1,7 @@
---
"pnpm": minor
"@pnpm/core": minor
"@pnpm/headless": minor
---

When the hoisted node linker is used, preserve `node_modules` directories when linking new dependencies. This improves performance, when installing in a project that already has a `node_modules` directory [#5795](https://github.com/pnpm/pnpm/pull/5795).
5 changes: 5 additions & 0 deletions .changeset/wet-coins-greet.md
@@ -0,0 +1,5 @@
---
"@pnpm/modules-yaml": minor
---

New field saved in the modules state file: `hoistedLocations`. This field maps the locations of dependencies, when `node-linker=hoisted` is used.
39 changes: 36 additions & 3 deletions fs/indexed-pkg-importer/src/importIndexedDir.ts
Expand Up @@ -14,11 +14,18 @@ export type ImportFile = (src: string, dest: string) => Promise<void>
export async function importIndexedDir (
importFile: ImportFile,
newDir: string,
filenames: Record<string, string>
filenames: Record<string, string>,
opts: {
keepModulesDir?: boolean
}
) {
const stage = pathTemp(path.dirname(newDir))
try {
await tryImportIndexedDir(importFile, stage, filenames)
if (opts.keepModulesDir) {
// Keeping node_modules is needed only when the hoisted node linker is used.
await moveOrMergeModulesDirs(path.join(newDir, 'node_modules'), path.join(stage, 'node_modules'))
}
await renameOverwrite(stage, newDir)
} catch (err: any) { // eslint-disable-line
try {
Expand All @@ -37,7 +44,7 @@ export async function importIndexedDir (
'which is an issue on case-insensitive filesystems. ' +
`The conflicting file names are: ${JSON.stringify(conflictingFileNames)}`
)
await importIndexedDir(importFile, newDir, uniqueFileMap)
await importIndexedDir(importFile, newDir, uniqueFileMap, opts)
return
}
if (err['code'] === 'ENOENT') {
Expand All @@ -47,7 +54,7 @@ export async function importIndexedDir (
The package linked to "${path.relative(process.cwd(), newDir)}" had \
files with invalid names: ${invalidFilenames.join(', ')}. \
They were renamed.`)
await importIndexedDir(importFile, newDir, sanitizedFilenames)
await importIndexedDir(importFile, newDir, sanitizedFilenames, opts)
return
}
throw err
Expand Down Expand Up @@ -108,3 +115,29 @@ function getUniqueFileMap (fileMap: Record<string, string>) {
uniqueFileMap,
}
}

async function moveOrMergeModulesDirs (src: string, dest: string) {
try {
await fs.rename(src, dest)
} catch (err: any) { // eslint-disable-line
switch (err.code) {
case 'ENOENT':
// If src directory doesn't exist, there is nothing to do
return
case 'ENOTEMPTY':
case 'EPERM': // This error code is thrown on Windows
// The newly added dependency might have node_modules if it has bundled dependencies.
await mergeModulesDirs(src, dest)
return
default:
throw err
}
}
}

async function mergeModulesDirs (src: string, dest: string) {
const srcFiles = await fs.readdir(src)
const destFiles = new Set(await fs.readdir(dest))
const filesToMove = srcFiles.filter((file) => !destFiles.has(file))
await Promise.all(filesToMove.map((file) => fs.rename(path.join(src, file), path.join(dest, file))))
}
6 changes: 3 additions & 3 deletions fs/indexed-pkg-importer/src/index.ts
Expand Up @@ -110,7 +110,7 @@ async function clonePkg (
const pkgJsonPath = path.join(to, 'package.json')

if (!opts.fromStore || opts.force || !await exists(pkgJsonPath)) {
await importIndexedDir(cloneFile, to, opts.filesMap)
await importIndexedDir(cloneFile, to, opts.filesMap, opts)
return 'clone'
}
return undefined
Expand All @@ -130,7 +130,7 @@ async function hardlinkPkg (
opts.force ||
!await pkgLinkedToStore(opts.filesMap, to)
) {
await importIndexedDir(importFile, to, opts.filesMap)
await importIndexedDir(importFile, to, opts.filesMap, opts)
return 'hardlink'
}
return undefined
Expand Down Expand Up @@ -188,7 +188,7 @@ export async function copyPkg (
const pkgJsonPath = path.join(to, 'package.json')

if (!opts.fromStore || opts.force || !await exists(pkgJsonPath)) {
await importIndexedDir(fs.copyFile, to, opts.filesMap)
await importIndexedDir(fs.copyFile, to, opts.filesMap, opts)
return 'copy'
}
return undefined
Expand Down
21 changes: 21 additions & 0 deletions fs/indexed-pkg-importer/test/importIndexedDir.test.ts
@@ -0,0 +1,21 @@
import { tempDir } from '@pnpm/prepare'
import { promises as fs, mkdirSync, writeFileSync } from 'fs'
import path from 'path'
import { importIndexedDir } from '../src/importIndexedDir'

test('importIndexedDir() keepModulesDir merges node_modules', async () => {
const tmp = tempDir()
mkdirSync(path.join(tmp, 'src/node_modules/a'), { recursive: true })
writeFileSync(path.join(tmp, 'src/node_modules/a/index.js'), 'module.exports = 1')

mkdirSync(path.join(tmp, 'dest/node_modules/b'), { recursive: true })
writeFileSync(path.join(tmp, 'dest/node_modules/b/index.js'), 'module.exports = 1')

const newDir = path.join(tmp, 'dest')
const filenames = {
'node_modules/a/index.js': path.join(tmp, 'src/node_modules/a/index.js'),
}
await importIndexedDir(fs.link, newDir, filenames, { keepModulesDir: true })

expect(await fs.readdir(path.join(newDir, 'node_modules'))).toEqual(['a', 'b'])
})
2 changes: 2 additions & 0 deletions pkg-manager/core/src/install/index.ts
Expand Up @@ -344,6 +344,7 @@ export async function mutateModules (
nodeVersion: opts.nodeVersion,
pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '',
},
currentHoistedLocations: ctx.modulesFile?.hoistedLocations,
patchedDependencies: patchedDependenciesWithResolvedPath,
selectedProjectDirs: projects.map((project) => project.rootDir),
allProjects: ctx.projects,
Expand Down Expand Up @@ -1177,6 +1178,7 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => {
nodeVersion: opts.nodeVersion,
pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '',
},
currentHoistedLocations: ctx.modulesFile?.hoistedLocations,
selectedProjectDirs: projects.map((project) => project.rootDir),
allProjects: ctx.projects,
prunedAt: ctx.modulesFile?.prunedAt,
Expand Down
6 changes: 2 additions & 4 deletions pkg-manager/core/src/install/link.ts
Expand Up @@ -46,9 +46,7 @@ export async function linkPackages (
opts: {
currentLockfile: Lockfile
dedupeDirectDeps: boolean
dependenciesByProjectId: {
[id: string]: { [alias: string]: string }
}
dependenciesByProjectId: Record<string, Record<string, string>>
force: boolean
depsStateCache: DepsStateCache
extraNodePaths: string[]
Expand All @@ -61,7 +59,7 @@ export async function linkPackages (
linkedDependenciesByProjectId: Record<string, LinkedDependency[]>
lockfileDir: string
makePartialCurrentLockfile: boolean
outdatedDependencies: { [pkgId: string]: string }
outdatedDependencies: Record<string, string>
pruneStore: boolean
pruneVirtualStore: boolean
registries: Registries
Expand Down
3 changes: 3 additions & 0 deletions pkg-manager/headless/src/index.ts
Expand Up @@ -120,6 +120,7 @@ export interface HeadlessOptions {
hoistedDependencies: HoistedDependencies
hoistPattern?: string[]
publicHoistPattern?: string[]
currentHoistedLocations?: Record<string, string[]>
lockfileDir: string
modulesDir?: string
virtualStoreDir?: string
Expand Down Expand Up @@ -283,6 +284,7 @@ export async function headlessInstall (opts: HeadlessOptions) {
directDependenciesByImporterId,
graph,
hierarchy,
hoistedLocations,
pkgLocationsByDepPath,
prevGraph,
symlinkedDirectDependenciesByImporterId,
Expand Down Expand Up @@ -528,6 +530,7 @@ export async function headlessInstall (opts: HeadlessOptions) {
included: opts.include,
injectedDeps,
layoutVersion: LAYOUT_VERSION,
hoistedLocations,
nodeLinker: opts.nodeLinker,
packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`,
pendingBuilds: opts.pendingBuilds,
Expand Down
57 changes: 30 additions & 27 deletions pkg-manager/headless/src/linkHoistedModules.ts
Expand Up @@ -95,36 +95,39 @@ async function linkAllPkgsInOrder (
await Promise.all(
Object.entries(hierarchy).map(async ([dir, deps]) => {
const depNode = graph[dir]
let filesResponse!: PackageFilesResponse
try {
filesResponse = await depNode.fetchingFiles()
} catch (err: any) { // eslint-disable-line
if (depNode.optional) return
throw err
}
if (depNode.fetchingFiles) {
let filesResponse!: PackageFilesResponse
try {
filesResponse = await depNode.fetchingFiles()
} catch (err: any) { // eslint-disable-line
if (depNode.optional) return
throw err
}

let sideEffectsCacheKey: string | undefined
if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) {
sideEffectsCacheKey = _calcDepState(dir, {
isBuilt: !opts.ignoreScripts && depNode.requiresBuild,
patchFileHash: depNode.patchFile?.hash,
})
}
const { importMethod, isBuilt } = await storeController.importPackage(depNode.dir, {
filesResponse,
force: opts.force || depNode.depPath !== prevGraph[dir]?.depPath,
requiresBuild: depNode.requiresBuild || depNode.patchFile != null,
sideEffectsCacheKey,
})
if (importMethod) {
progressLogger.debug({
method: importMethod,
requester: opts.lockfileDir,
status: 'imported',
to: depNode.dir,
let sideEffectsCacheKey: string | undefined
if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) {
sideEffectsCacheKey = _calcDepState(dir, {
isBuilt: !opts.ignoreScripts && depNode.requiresBuild,
patchFileHash: depNode.patchFile?.hash,
})
}
const { importMethod, isBuilt } = await storeController.importPackage(depNode.dir, {
filesResponse,
force: opts.force || depNode.depPath !== prevGraph[dir]?.depPath,
keepModulesDir: true,
requiresBuild: depNode.requiresBuild || depNode.patchFile != null,
sideEffectsCacheKey,
})
if (importMethod) {
progressLogger.debug({
method: importMethod,
requester: opts.lockfileDir,
status: 'imported',
to: depNode.dir,
})
}
depNode.isBuilt = isBuilt
}
depNode.isBuilt = isBuilt
return linkAllPkgsInOrder(storeController, graph, prevGraph, deps, dir, opts)
})
)
Expand Down
1 change: 1 addition & 0 deletions pkg-manager/headless/src/lockfileToDepGraph.ts
Expand Up @@ -80,6 +80,7 @@ export interface LockfileToDepGraphResult {
directDependenciesByImporterId: DirectDependenciesByImporterId
graph: DependenciesGraph
hierarchy?: DepHierarchy
hoistedLocations?: Record<string, string[]>
symlinkedDirectDependenciesByImporterId?: DirectDependenciesByImporterId
prevGraph?: DependenciesGraph
pkgLocationsByDepPath?: Record<string, string[]>
Expand Down
48 changes: 31 additions & 17 deletions pkg-manager/headless/src/lockfileToHoistedDepGraph.ts
Expand Up @@ -32,6 +32,7 @@ export interface LockfileToHoistedDepGraphOptions {
externalDependencies?: Set<string>
importerIds: string[]
include: IncludedDependencies
currentHoistedLocations?: Record<string, string[]>
lockfileDir: string
nodeVersion: string
pnpmVersion: string
Expand Down Expand Up @@ -73,6 +74,7 @@ async function _lockfileToHoistedDepGraph (
lockfile,
graph,
pkgLocationsByDepPath: {},
hoistedLocations: {} as Record<string, string[]>,
}
const hierarchy = {
[opts.lockfileDir]: await fetchDeps(fetchDepsOpts, modulesDir, tree.dependencies),
Expand Down Expand Up @@ -102,6 +104,7 @@ async function _lockfileToHoistedDepGraph (
hierarchy,
pkgLocationsByDepPath: fetchDepsOpts.pkgLocationsByDepPath,
symlinkedDirectDependenciesByImporterId,
hoistedLocations: fetchDepsOpts.hoistedLocations,
}
}

Expand Down Expand Up @@ -136,6 +139,7 @@ async function fetchDeps (
graph: DependenciesGraph
lockfile: Lockfile
pkgLocationsByDepPath: Record<string, string[]>
hoistedLocations: Record<string, string[]>
} & LockfileToHoistedDepGraphOptions,
modules: string,
deps: Set<HoisterResult>
Expand Down Expand Up @@ -173,25 +177,31 @@ async function fetchDeps (
return
}
const dir = path.join(modules, dep.name)
const depLocation = path.relative(opts.lockfileDir, dir)
const resolution = pkgSnapshotToResolution(depPath, pkgSnapshot, opts.registries)
let fetchResponse!: ReturnType<FetchPackageToStoreFunction>
try {
fetchResponse = opts.storeController.fetchPackage({
force: false,
lockfileDir: opts.lockfileDir,
pkg: {
id: packageId,
resolution,
},
expectedPkg: {
name: pkgName,
version: pkgVersion,
},
})
if (fetchResponse instanceof Promise) fetchResponse = await fetchResponse
} catch (err: any) { // eslint-disable-line
if (pkgSnapshot.optional) return
throw err
const skipFetch = opts.currentHoistedLocations?.[depPath]?.includes(depLocation)
if (skipFetch) {
fetchResponse = {} as any // eslint-disable-line @typescript-eslint/no-explicit-any
} else {
try {
fetchResponse = opts.storeController.fetchPackage({
force: false,
lockfileDir: opts.lockfileDir,
pkg: {
id: packageId,
resolution,
},
expectedPkg: {
name: pkgName,
version: pkgVersion,
},
})
if (fetchResponse instanceof Promise) fetchResponse = await fetchResponse
} catch (err: any) { // eslint-disable-line
if (pkgSnapshot.optional) return
throw err
}
}
opts.graph[dir] = {
alias: dep.name,
Expand All @@ -216,6 +226,10 @@ async function fetchDeps (
}
opts.pkgLocationsByDepPath[depPath].push(dir)
depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies)
if (!opts.hoistedLocations[depPath]) {
opts.hoistedLocations[depPath] = []
}
opts.hoistedLocations[depPath].push(depLocation)
opts.graph[dir].children = getChildren(pkgSnapshot, opts.pkgLocationsByDepPath, opts)
}))
return depHierarchy
Expand Down
1 change: 1 addition & 0 deletions pkg-manager/modules-yaml/src/index.ts
Expand Up @@ -31,6 +31,7 @@ export interface Modules {
storeDir: string
virtualStoreDir: string
injectedDeps?: Record<string, string[]>
hoistedLocations?: Record<string, string[]>
}

export async function readModulesManifest (modulesDir: string): Promise<Modules | null> {
Expand Down
1 change: 1 addition & 0 deletions store/cafs-types/src/index.ts
Expand Up @@ -30,6 +30,7 @@ export interface ImportPackageOpts {
sideEffectsCacheKey?: string
filesResponse: PackageFilesResponse
force: boolean
keepModulesDir?: boolean
}

export type ImportPackageFunction = (
Expand Down
7 changes: 6 additions & 1 deletion store/create-cafs-store/src/index.ts
Expand Up @@ -33,7 +33,12 @@ function createPackageImporter (
? 'clone-or-copy'
: (opts.filesResponse.packageImportMethod ?? packageImportMethod)
const impPkg = cachedImporterCreator(pkgImportMethod)
const importMethod = await impPkg(to, { filesMap, fromStore: opts.filesResponse.fromStore, force: opts.force })
const importMethod = await impPkg(to, {
filesMap,
fromStore: opts.filesResponse.fromStore,
force: opts.force,
keepModulesDir: Boolean(opts.keepModulesDir),
})
return { importMethod, isBuilt }
}
}
Expand Down