From 29fc688451173059388fb98b42abc580a88893be Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 14 Dec 2022 03:36:01 +0200 Subject: [PATCH 01/10] feat: store locations of deps when node-linker is set to hoisted --- pkg-manager/headless/src/index.ts | 2 ++ pkg-manager/headless/src/lockfileToDepGraph.ts | 1 + .../headless/src/lockfileToHoistedDepGraph.ts | 15 +++++++++++---- pkg-manager/modules-yaml/src/index.ts | 1 + 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index 4ab7dd70a36..ceac09271fe 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -283,6 +283,7 @@ export async function headlessInstall (opts: HeadlessOptions) { directDependenciesByImporterId, graph, hierarchy, + locations, pkgLocationsByDepPath, prevGraph, symlinkedDirectDependenciesByImporterId, @@ -528,6 +529,7 @@ export async function headlessInstall (opts: HeadlessOptions) { included: opts.include, injectedDeps, layoutVersion: LAYOUT_VERSION, + locations, nodeLinker: opts.nodeLinker, packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`, pendingBuilds: opts.pendingBuilds, diff --git a/pkg-manager/headless/src/lockfileToDepGraph.ts b/pkg-manager/headless/src/lockfileToDepGraph.ts index 0680a9ff62a..62a9b820891 100644 --- a/pkg-manager/headless/src/lockfileToDepGraph.ts +++ b/pkg-manager/headless/src/lockfileToDepGraph.ts @@ -80,6 +80,7 @@ export interface LockfileToDepGraphResult { directDependenciesByImporterId: DirectDependenciesByImporterId graph: DependenciesGraph hierarchy?: DepHierarchy + locations?: Record symlinkedDirectDependenciesByImporterId?: DirectDependenciesByImporterId prevGraph?: DependenciesGraph pkgLocationsByDepPath?: Record diff --git a/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts b/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts index 7745af6001c..79308e213e0 100644 --- a/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts +++ b/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts @@ -74,8 +74,9 @@ async function _lockfileToHoistedDepGraph ( graph, pkgLocationsByDepPath: {}, } + const locations: Record = {} const hierarchy = { - [opts.lockfileDir]: await fetchDeps(fetchDepsOpts, modulesDir, tree.dependencies), + [opts.lockfileDir]: await fetchDeps(fetchDepsOpts, modulesDir, tree.dependencies, locations), } const directDependenciesByImporterId: DirectDependenciesByImporterId = { '.': directDepsMap(Object.keys(hierarchy[opts.lockfileDir]), graph), @@ -87,7 +88,7 @@ async function _lockfileToHoistedDepGraph ( const importerId = reference.replace('workspace:', '') const projectDir = path.join(opts.lockfileDir, importerId) const modulesDir = path.join(projectDir, 'node_modules') - const nextHierarchy = (await fetchDeps(fetchDepsOpts, modulesDir, rootDep.dependencies)) + const nextHierarchy = (await fetchDeps(fetchDepsOpts, modulesDir, rootDep.dependencies, locations)) hierarchy[projectDir] = nextHierarchy const importer = lockfile.importers[importerId] @@ -102,6 +103,7 @@ async function _lockfileToHoistedDepGraph ( hierarchy, pkgLocationsByDepPath: fetchDepsOpts.pkgLocationsByDepPath, symlinkedDirectDependenciesByImporterId, + locations, } } @@ -138,7 +140,8 @@ async function fetchDeps ( pkgLocationsByDepPath: Record } & LockfileToHoistedDepGraphOptions, modules: string, - deps: Set + deps: Set, + locations: Record ): Promise { const depHierarchy = {} await Promise.all(Array.from(deps).map(async (dep) => { @@ -215,7 +218,11 @@ async function fetchDeps ( opts.pkgLocationsByDepPath[depPath] = [] } opts.pkgLocationsByDepPath[depPath].push(dir) - depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies) + depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies, locations) + if (!locations[depPath]) { + locations[depPath] = [] + } + locations[depPath].push(path.relative(opts.lockfileDir, dir)) opts.graph[dir].children = getChildren(pkgSnapshot, opts.pkgLocationsByDepPath, opts) })) return depHierarchy diff --git a/pkg-manager/modules-yaml/src/index.ts b/pkg-manager/modules-yaml/src/index.ts index ffc62241c0a..9269bc5df5d 100644 --- a/pkg-manager/modules-yaml/src/index.ts +++ b/pkg-manager/modules-yaml/src/index.ts @@ -31,6 +31,7 @@ export interface Modules { storeDir: string virtualStoreDir: string injectedDeps?: Record + locations?: Record } export async function readModulesManifest (modulesDir: string): Promise { From 3e3261f202889d285f7bc584157720bfb3f9187d Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 14 Dec 2022 04:09:27 +0200 Subject: [PATCH 02/10] perf: don't fetch deps that are already in node_modules, when node-linker=hoisted is used --- pkg-manager/core/src/install/index.ts | 2 + pkg-manager/headless/src/index.ts | 1 + .../headless/src/linkHoistedModules.ts | 56 ++++++++++--------- .../headless/src/lockfileToHoistedDepGraph.ts | 43 ++++++++------ 4 files changed, 57 insertions(+), 45 deletions(-) diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index a44047f38c9..00c8392791e 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -344,6 +344,7 @@ export async function mutateModules ( nodeVersion: opts.nodeVersion, pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '', }, + currentDependenciesLocations: ctx.modulesFile?.locations, patchedDependencies: patchedDependenciesWithResolvedPath, selectedProjectDirs: projects.map((project) => project.rootDir), allProjects: ctx.projects, @@ -1177,6 +1178,7 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => { nodeVersion: opts.nodeVersion, pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '', }, + currentDependenciesLocations: ctx.modulesFile?.locations, selectedProjectDirs: projects.map((project) => project.rootDir), allProjects: ctx.projects, prunedAt: ctx.modulesFile?.prunedAt, diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index ceac09271fe..6a62c33c250 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -120,6 +120,7 @@ export interface HeadlessOptions { hoistedDependencies: HoistedDependencies hoistPattern?: string[] publicHoistPattern?: string[] + currentDependenciesLocations?: Record lockfileDir: string modulesDir?: string virtualStoreDir?: string diff --git a/pkg-manager/headless/src/linkHoistedModules.ts b/pkg-manager/headless/src/linkHoistedModules.ts index 5d0fb4bac42..884aa28ebe4 100644 --- a/pkg-manager/headless/src/linkHoistedModules.ts +++ b/pkg-manager/headless/src/linkHoistedModules.ts @@ -95,36 +95,38 @@ 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, + 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) }) ) diff --git a/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts b/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts index 79308e213e0..7e88873e8a0 100644 --- a/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts +++ b/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts @@ -32,6 +32,7 @@ export interface LockfileToHoistedDepGraphOptions { externalDependencies?: Set importerIds: string[] include: IncludedDependencies + currentDependenciesLocations?: Record lockfileDir: string nodeVersion: string pnpmVersion: string @@ -176,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 - 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.currentDependenciesLocations?.[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, @@ -222,7 +229,7 @@ async function fetchDeps ( if (!locations[depPath]) { locations[depPath] = [] } - locations[depPath].push(path.relative(opts.lockfileDir, dir)) + locations[depPath].push(depLocation) opts.graph[dir].children = getChildren(pkgSnapshot, opts.pkgLocationsByDepPath, opts) })) return depHierarchy From f8077be9163e85192641f10333bdd22e834a6224 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 14 Dec 2022 19:16:10 +0200 Subject: [PATCH 03/10] fix: hoisting --- fs/indexed-pkg-importer/src/importIndexedDir.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/indexed-pkg-importer/src/importIndexedDir.ts b/fs/indexed-pkg-importer/src/importIndexedDir.ts index 40c331f161d..79298575970 100644 --- a/fs/indexed-pkg-importer/src/importIndexedDir.ts +++ b/fs/indexed-pkg-importer/src/importIndexedDir.ts @@ -19,6 +19,11 @@ export async function importIndexedDir ( const stage = pathTemp(path.dirname(newDir)) try { await tryImportIndexedDir(importFile, stage, filenames) + try { + await fs.rename(path.join(newDir, 'node_modules'), path.join(stage, 'node_modules')) + } catch (err) { + // TODO: merge directories maybe + } await renameOverwrite(stage, newDir) } catch (err: any) { // eslint-disable-line try { From fc163b8a84d3206c95b71858301ec2b9978deac4 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 18 Dec 2022 02:26:40 +0200 Subject: [PATCH 04/10] fix: keep modules dir --- .../src/importIndexedDir.ts | 19 ++++++++++++------- fs/indexed-pkg-importer/src/index.ts | 6 +++--- pkg-manager/core/src/install/link.ts | 6 ++---- .../headless/src/linkHoistedModules.ts | 1 + store/cafs-types/src/index.ts | 1 + store/create-cafs-store/src/index.ts | 7 ++++++- store/store-controller-types/src/index.ts | 1 + 7 files changed, 26 insertions(+), 15 deletions(-) diff --git a/fs/indexed-pkg-importer/src/importIndexedDir.ts b/fs/indexed-pkg-importer/src/importIndexedDir.ts index 79298575970..2c72ac2bd41 100644 --- a/fs/indexed-pkg-importer/src/importIndexedDir.ts +++ b/fs/indexed-pkg-importer/src/importIndexedDir.ts @@ -14,15 +14,20 @@ export type ImportFile = (src: string, dest: string) => Promise export async function importIndexedDir ( importFile: ImportFile, newDir: string, - filenames: Record + filenames: Record, + opts: { + keepModulesDir?: boolean + } ) { const stage = pathTemp(path.dirname(newDir)) try { await tryImportIndexedDir(importFile, stage, filenames) - try { - await fs.rename(path.join(newDir, 'node_modules'), path.join(stage, 'node_modules')) - } catch (err) { - // TODO: merge directories maybe + if (opts.keepModulesDir) { + try { + await fs.rename(path.join(newDir, 'node_modules'), path.join(stage, 'node_modules')) + } catch (err) { + // TODO: merge directories maybe + } } await renameOverwrite(stage, newDir) } catch (err: any) { // eslint-disable-line @@ -42,7 +47,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') { @@ -52,7 +57,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 diff --git a/fs/indexed-pkg-importer/src/index.ts b/fs/indexed-pkg-importer/src/index.ts index d48d8609165..c84950762a3 100644 --- a/fs/indexed-pkg-importer/src/index.ts +++ b/fs/indexed-pkg-importer/src/index.ts @@ -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 @@ -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 @@ -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 diff --git a/pkg-manager/core/src/install/link.ts b/pkg-manager/core/src/install/link.ts index 63ccf8fdc8b..d655b6836c2 100644 --- a/pkg-manager/core/src/install/link.ts +++ b/pkg-manager/core/src/install/link.ts @@ -46,9 +46,7 @@ export async function linkPackages ( opts: { currentLockfile: Lockfile dedupeDirectDeps: boolean - dependenciesByProjectId: { - [id: string]: { [alias: string]: string } - } + dependenciesByProjectId: Record> force: boolean depsStateCache: DepsStateCache extraNodePaths: string[] @@ -61,7 +59,7 @@ export async function linkPackages ( linkedDependenciesByProjectId: Record lockfileDir: string makePartialCurrentLockfile: boolean - outdatedDependencies: { [pkgId: string]: string } + outdatedDependencies: Record pruneStore: boolean pruneVirtualStore: boolean registries: Registries diff --git a/pkg-manager/headless/src/linkHoistedModules.ts b/pkg-manager/headless/src/linkHoistedModules.ts index 884aa28ebe4..4e539801d28 100644 --- a/pkg-manager/headless/src/linkHoistedModules.ts +++ b/pkg-manager/headless/src/linkHoistedModules.ts @@ -114,6 +114,7 @@ async function linkAllPkgsInOrder ( 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, }) diff --git a/store/cafs-types/src/index.ts b/store/cafs-types/src/index.ts index c2b8c5cb960..1f7f718e8c0 100644 --- a/store/cafs-types/src/index.ts +++ b/store/cafs-types/src/index.ts @@ -30,6 +30,7 @@ export interface ImportPackageOpts { sideEffectsCacheKey?: string filesResponse: PackageFilesResponse force: boolean + keepModulesDir?: boolean } export type ImportPackageFunction = ( diff --git a/store/create-cafs-store/src/index.ts b/store/create-cafs-store/src/index.ts index 5fa7b45a2fe..fd77b6a147a 100644 --- a/store/create-cafs-store/src/index.ts +++ b/store/create-cafs-store/src/index.ts @@ -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 } } } diff --git a/store/store-controller-types/src/index.ts b/store/store-controller-types/src/index.ts index 7271f16ec7b..4ea73acc4a0 100644 --- a/store/store-controller-types/src/index.ts +++ b/store/store-controller-types/src/index.ts @@ -147,6 +147,7 @@ export interface ImportOptions { filesMap: FilesMap force: boolean fromStore: boolean + keepModulesDir?: boolean } export type ImportIndexedPackage = (to: string, opts: ImportOptions) => Promise From 8b7f19947d0f1be58af9006227d561f5e8fc603b Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 18 Dec 2022 02:37:23 +0200 Subject: [PATCH 05/10] fix: move or merge modules dir --- .../src/importIndexedDir.ts | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/fs/indexed-pkg-importer/src/importIndexedDir.ts b/fs/indexed-pkg-importer/src/importIndexedDir.ts index 2c72ac2bd41..09eae8873e7 100644 --- a/fs/indexed-pkg-importer/src/importIndexedDir.ts +++ b/fs/indexed-pkg-importer/src/importIndexedDir.ts @@ -1,4 +1,4 @@ -import { promises as fs } from 'fs' +import { promises as fs, existsSync } from 'fs' import path from 'path' import { globalWarn, logger } from '@pnpm/logger' import rimraf from '@zkochan/rimraf' @@ -23,11 +23,8 @@ export async function importIndexedDir ( try { await tryImportIndexedDir(importFile, stage, filenames) if (opts.keepModulesDir) { - try { - await fs.rename(path.join(newDir, 'node_modules'), path.join(stage, 'node_modules')) - } catch (err) { - // TODO: merge directories maybe - } + // 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 @@ -118,3 +115,21 @@ function getUniqueFileMap (fileMap: Record) { uniqueFileMap, } } + +async function moveOrMergeModulesDirs (src: string, dest: string) { + try { + await fs.rename(src, dest) + } catch (err) { + if (existsSync(dest)) { + await mergeModulesDirs(src, dest) + } + 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)))) +} From 6ed0d3423f0ed5a77d156678378715c99a87a73c Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 18 Dec 2022 03:10:07 +0200 Subject: [PATCH 06/10] fix: move or merge modules dir --- fs/indexed-pkg-importer/src/importIndexedDir.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/fs/indexed-pkg-importer/src/importIndexedDir.ts b/fs/indexed-pkg-importer/src/importIndexedDir.ts index 09eae8873e7..738ccfe37ca 100644 --- a/fs/indexed-pkg-importer/src/importIndexedDir.ts +++ b/fs/indexed-pkg-importer/src/importIndexedDir.ts @@ -1,4 +1,4 @@ -import { promises as fs, existsSync } from 'fs' +import { promises as fs } from 'fs' import path from 'path' import { globalWarn, logger } from '@pnpm/logger' import rimraf from '@zkochan/rimraf' @@ -119,11 +119,17 @@ function getUniqueFileMap (fileMap: Record) { async function moveOrMergeModulesDirs (src: string, dest: string) { try { await fs.rename(src, dest) - } catch (err) { - if (existsSync(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': await mergeModulesDirs(src, dest) + return + default: + throw err } - throw err } } From 385d7dc825de619d7bf4683881407b8c84125284 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 18 Dec 2022 03:38:36 +0200 Subject: [PATCH 07/10] test: merge node_modules dirs --- .../test/importIndexedDir.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 fs/indexed-pkg-importer/test/importIndexedDir.test.ts diff --git a/fs/indexed-pkg-importer/test/importIndexedDir.test.ts b/fs/indexed-pkg-importer/test/importIndexedDir.test.ts new file mode 100644 index 00000000000..c8c6ccda5a2 --- /dev/null +++ b/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']) +}) From e96fb10e1b408d4a2c930d99da9853cafb35b1b8 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 18 Dec 2022 15:12:35 +0200 Subject: [PATCH 08/10] test: fix on Windows --- fs/indexed-pkg-importer/src/importIndexedDir.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/indexed-pkg-importer/src/importIndexedDir.ts b/fs/indexed-pkg-importer/src/importIndexedDir.ts index 738ccfe37ca..41e04303128 100644 --- a/fs/indexed-pkg-importer/src/importIndexedDir.ts +++ b/fs/indexed-pkg-importer/src/importIndexedDir.ts @@ -125,6 +125,7 @@ async function moveOrMergeModulesDirs (src: string, dest: string) { // If src directory doesn't exist, there is nothing to do return case 'ENOTEMPTY': + case 'EPERM': // This error code is thrown on Windows await mergeModulesDirs(src, dest) return default: From d7001c6fd61620e9cb98bef7351293dd6464323c Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 18 Dec 2022 16:27:06 +0200 Subject: [PATCH 09/10] refactor: hoisting --- .../src/importIndexedDir.ts | 1 + pkg-manager/core/src/install/index.ts | 4 ++-- pkg-manager/headless/src/index.ts | 6 ++--- .../headless/src/lockfileToDepGraph.ts | 2 +- .../headless/src/lockfileToHoistedDepGraph.ts | 24 +++++++++---------- pkg-manager/modules-yaml/src/index.ts | 2 +- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/fs/indexed-pkg-importer/src/importIndexedDir.ts b/fs/indexed-pkg-importer/src/importIndexedDir.ts index 41e04303128..5b5994bfbef 100644 --- a/fs/indexed-pkg-importer/src/importIndexedDir.ts +++ b/fs/indexed-pkg-importer/src/importIndexedDir.ts @@ -126,6 +126,7 @@ async function moveOrMergeModulesDirs (src: string, dest: string) { 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: diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index 00c8392791e..a4ea065d367 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -344,7 +344,7 @@ export async function mutateModules ( nodeVersion: opts.nodeVersion, pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '', }, - currentDependenciesLocations: ctx.modulesFile?.locations, + currentHoistedLocations: ctx.modulesFile?.hoistedLocations, patchedDependencies: patchedDependenciesWithResolvedPath, selectedProjectDirs: projects.map((project) => project.rootDir), allProjects: ctx.projects, @@ -1178,7 +1178,7 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => { nodeVersion: opts.nodeVersion, pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '', }, - currentDependenciesLocations: ctx.modulesFile?.locations, + currentHoistedLocations: ctx.modulesFile?.hoistedLocations, selectedProjectDirs: projects.map((project) => project.rootDir), allProjects: ctx.projects, prunedAt: ctx.modulesFile?.prunedAt, diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index 6a62c33c250..88ebbb1645e 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -120,7 +120,7 @@ export interface HeadlessOptions { hoistedDependencies: HoistedDependencies hoistPattern?: string[] publicHoistPattern?: string[] - currentDependenciesLocations?: Record + currentHoistedLocations?: Record lockfileDir: string modulesDir?: string virtualStoreDir?: string @@ -284,7 +284,7 @@ export async function headlessInstall (opts: HeadlessOptions) { directDependenciesByImporterId, graph, hierarchy, - locations, + hoistedLocations, pkgLocationsByDepPath, prevGraph, symlinkedDirectDependenciesByImporterId, @@ -530,7 +530,7 @@ export async function headlessInstall (opts: HeadlessOptions) { included: opts.include, injectedDeps, layoutVersion: LAYOUT_VERSION, - locations, + hoistedLocations, nodeLinker: opts.nodeLinker, packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`, pendingBuilds: opts.pendingBuilds, diff --git a/pkg-manager/headless/src/lockfileToDepGraph.ts b/pkg-manager/headless/src/lockfileToDepGraph.ts index 62a9b820891..bbd68ac6900 100644 --- a/pkg-manager/headless/src/lockfileToDepGraph.ts +++ b/pkg-manager/headless/src/lockfileToDepGraph.ts @@ -80,7 +80,7 @@ export interface LockfileToDepGraphResult { directDependenciesByImporterId: DirectDependenciesByImporterId graph: DependenciesGraph hierarchy?: DepHierarchy - locations?: Record + hoistedLocations?: Record symlinkedDirectDependenciesByImporterId?: DirectDependenciesByImporterId prevGraph?: DependenciesGraph pkgLocationsByDepPath?: Record diff --git a/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts b/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts index 7e88873e8a0..ce8463a41d2 100644 --- a/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts +++ b/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts @@ -32,7 +32,7 @@ export interface LockfileToHoistedDepGraphOptions { externalDependencies?: Set importerIds: string[] include: IncludedDependencies - currentDependenciesLocations?: Record + currentHoistedLocations?: Record lockfileDir: string nodeVersion: string pnpmVersion: string @@ -74,10 +74,10 @@ async function _lockfileToHoistedDepGraph ( lockfile, graph, pkgLocationsByDepPath: {}, + hoistedLocations: {} as Record, } - const locations: Record = {} const hierarchy = { - [opts.lockfileDir]: await fetchDeps(fetchDepsOpts, modulesDir, tree.dependencies, locations), + [opts.lockfileDir]: await fetchDeps(fetchDepsOpts, modulesDir, tree.dependencies), } const directDependenciesByImporterId: DirectDependenciesByImporterId = { '.': directDepsMap(Object.keys(hierarchy[opts.lockfileDir]), graph), @@ -89,7 +89,7 @@ async function _lockfileToHoistedDepGraph ( const importerId = reference.replace('workspace:', '') const projectDir = path.join(opts.lockfileDir, importerId) const modulesDir = path.join(projectDir, 'node_modules') - const nextHierarchy = (await fetchDeps(fetchDepsOpts, modulesDir, rootDep.dependencies, locations)) + const nextHierarchy = (await fetchDeps(fetchDepsOpts, modulesDir, rootDep.dependencies)) hierarchy[projectDir] = nextHierarchy const importer = lockfile.importers[importerId] @@ -104,7 +104,7 @@ async function _lockfileToHoistedDepGraph ( hierarchy, pkgLocationsByDepPath: fetchDepsOpts.pkgLocationsByDepPath, symlinkedDirectDependenciesByImporterId, - locations, + hoistedLocations: fetchDepsOpts.hoistedLocations, } } @@ -139,10 +139,10 @@ async function fetchDeps ( graph: DependenciesGraph lockfile: Lockfile pkgLocationsByDepPath: Record + hoistedLocations: Record } & LockfileToHoistedDepGraphOptions, modules: string, - deps: Set, - locations: Record + deps: Set ): Promise { const depHierarchy = {} await Promise.all(Array.from(deps).map(async (dep) => { @@ -180,7 +180,7 @@ async function fetchDeps ( const depLocation = path.relative(opts.lockfileDir, dir) const resolution = pkgSnapshotToResolution(depPath, pkgSnapshot, opts.registries) let fetchResponse!: ReturnType - const skipFetch = opts.currentDependenciesLocations?.[depPath]?.includes(depLocation) + const skipFetch = opts.currentHoistedLocations?.[depPath]?.includes(depLocation) if (skipFetch) { fetchResponse = {} as any // eslint-disable-line @typescript-eslint/no-explicit-any } else { @@ -225,11 +225,11 @@ async function fetchDeps ( opts.pkgLocationsByDepPath[depPath] = [] } opts.pkgLocationsByDepPath[depPath].push(dir) - depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies, locations) - if (!locations[depPath]) { - locations[depPath] = [] + depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies) + if (!opts.hoistedLocations[depPath]) { + opts.hoistedLocations[depPath] = [] } - locations[depPath].push(depLocation) + opts.hoistedLocations[depPath].push(depLocation) opts.graph[dir].children = getChildren(pkgSnapshot, opts.pkgLocationsByDepPath, opts) })) return depHierarchy diff --git a/pkg-manager/modules-yaml/src/index.ts b/pkg-manager/modules-yaml/src/index.ts index 9269bc5df5d..0029cc669fd 100644 --- a/pkg-manager/modules-yaml/src/index.ts +++ b/pkg-manager/modules-yaml/src/index.ts @@ -31,7 +31,7 @@ export interface Modules { storeDir: string virtualStoreDir: string injectedDeps?: Record - locations?: Record + hoistedLocations?: Record } export async function readModulesManifest (modulesDir: string): Promise { From 863e213ba1831544aa9f69c5351f28ccba084fa7 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 18 Dec 2022 17:16:57 +0200 Subject: [PATCH 10/10] docs: add changesets --- .changeset/cold-pigs-burn.md | 8 ++++++++ .changeset/late-swans-impress.md | 7 +++++++ .changeset/wet-coins-greet.md | 5 +++++ 3 files changed, 20 insertions(+) create mode 100644 .changeset/cold-pigs-burn.md create mode 100644 .changeset/late-swans-impress.md create mode 100644 .changeset/wet-coins-greet.md diff --git a/.changeset/cold-pigs-burn.md b/.changeset/cold-pigs-burn.md new file mode 100644 index 00000000000..e2ffe199865 --- /dev/null +++ b/.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. diff --git a/.changeset/late-swans-impress.md b/.changeset/late-swans-impress.md new file mode 100644 index 00000000000..c8e565fb394 --- /dev/null +++ b/.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). diff --git a/.changeset/wet-coins-greet.md b/.changeset/wet-coins-greet.md new file mode 100644 index 00000000000..caaa22f37e9 --- /dev/null +++ b/.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.