diff --git a/.changeset/new-vans-guess.md b/.changeset/new-vans-guess.md new file mode 100644 index 00000000000..d34f97678b4 --- /dev/null +++ b/.changeset/new-vans-guess.md @@ -0,0 +1,7 @@ +--- +"@pnpm/core": patch +"@pnpm/filter-lockfile": patch +"@pnpm/headless": patch +--- + +fix: also include missing deeply linked workspace packages at headless installation diff --git a/fixtures/workspace-external-depends-deep/.npmrc b/fixtures/workspace-external-depends-deep/.npmrc new file mode 100644 index 00000000000..361359b61c2 --- /dev/null +++ b/fixtures/workspace-external-depends-deep/.npmrc @@ -0,0 +1,4 @@ +link-workspace-packages = deep +prefer-workspace-packages = true +shared-workspace-lockfile = true +save-workspace-protocol = rolling diff --git a/fixtures/workspace-external-depends-deep/package.json b/fixtures/workspace-external-depends-deep/package.json new file mode 100644 index 00000000000..6fe879fb0a8 --- /dev/null +++ b/fixtures/workspace-external-depends-deep/package.json @@ -0,0 +1,7 @@ +{ + "name": "root", + "version": "1.0.0", + "dependencies": { + "is-positive": "1.0.0" + } +} diff --git a/fixtures/workspace-external-depends-deep/packages/f/package.json b/fixtures/workspace-external-depends-deep/packages/f/package.json new file mode 100644 index 00000000000..4ce399ec780 --- /dev/null +++ b/fixtures/workspace-external-depends-deep/packages/f/package.json @@ -0,0 +1,9 @@ +{ + "name": "@kenrick95/internal-f", + "version": "1.0.0", + "dependencies": { + "is-positive": "1.0.0", + "is-negative": "1.0.0" + } +} + diff --git a/fixtures/workspace-external-depends-deep/packages/g/package.json b/fixtures/workspace-external-depends-deep/packages/g/package.json new file mode 100644 index 00000000000..073b6561cb7 --- /dev/null +++ b/fixtures/workspace-external-depends-deep/packages/g/package.json @@ -0,0 +1,8 @@ +{ + "name": "@kenrick95/internal-g", + "version": "1.0.0", + "dependencies": { + "@kenrick95/external-depend-on-internal-dep": "1.0.0", + "is-positive": "1.0.0" + } +} diff --git a/fixtures/workspace-external-depends-deep/pnpm-lock.yaml b/fixtures/workspace-external-depends-deep/pnpm-lock.yaml new file mode 100644 index 00000000000..75a9d186fd7 --- /dev/null +++ b/fixtures/workspace-external-depends-deep/pnpm-lock.yaml @@ -0,0 +1,43 @@ +lockfileVersion: 5.4 + +importers: + + .: + specifiers: + is-positive: 1.0.0 + dependencies: + is-positive: 1.0.0 + + packages/f: + specifiers: + is-negative: 1.0.0 + is-positive: 1.0.0 + dependencies: + is-negative: 1.0.0 + is-positive: 1.0.0 + + packages/g: + specifiers: + '@kenrick95/external-depend-on-internal-dep': 1.0.0 + is-positive: 1.0.0 + dependencies: + '@kenrick95/external-depend-on-internal-dep': 1.0.0 + is-positive: 1.0.0 + +packages: + + /@kenrick95/external-depend-on-internal-dep/1.0.0: + resolution: {integrity: sha512-r7XGZxVJ+Ixq1m5aljeyXtvBK94k9mXw0R9mxHhtmEznVCOiu/vAwO5csUvr8FKtIJrm3r+9PHnIgaDJt8YQOA==} + dependencies: + '@kenrick95/internal-f': link:packages/f + dev: false + + /is-negative/1.0.0: + resolution: {integrity: sha512-1aKMsFUc7vYQGzt//8zhkjRWPoYkajY/I5MJEvrc0pDoHXrW7n5ri8DYxhy3rR+Dk0QFl7GjHHsZU1sppQrWtw==} + engines: {node: '>=0.10.0'} + dev: false + + /is-positive/1.0.0: + resolution: {integrity: sha1-iACYVrZKLx632LsBeUGEJK4EUss=} + engines: {node: '>=0.10.0'} + dev: false diff --git a/fixtures/workspace-external-depends-deep/pnpm-workspace.yaml b/fixtures/workspace-external-depends-deep/pnpm-workspace.yaml new file mode 100644 index 00000000000..eccc335f93b --- /dev/null +++ b/fixtures/workspace-external-depends-deep/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +packages: + - 'packages/**' \ No newline at end of file diff --git a/packages/core/package.json b/packages/core/package.json index e5eb8a90baf..7375d2fd68c 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -45,6 +45,7 @@ "@pnpm/read-modules-dir": "workspace:*", "@pnpm/read-package-json": "workspace:*", "@pnpm/read-project-manifest": "workspace:*", + "@pnpm/read-projects-context": "workspace:*", "@pnpm/remove-bins": "workspace:*", "@pnpm/resolve-dependencies": "workspace:*", "@pnpm/resolver-base": "workspace:*", diff --git a/packages/core/tsconfig.json b/packages/core/tsconfig.json index 024996f28a4..9519c584a8d 100644 --- a/packages/core/tsconfig.json +++ b/packages/core/tsconfig.json @@ -120,6 +120,9 @@ { "path": "../read-project-manifest" }, + { + "path": "../read-projects-context" + }, { "path": "../remove-bins" }, diff --git a/packages/filter-lockfile/src/filterLockfileByImportersAndEngine.ts b/packages/filter-lockfile/src/filterLockfileByImportersAndEngine.ts index ea2b563aa09..5b00f31b933 100644 --- a/packages/filter-lockfile/src/filterLockfileByImportersAndEngine.ts +++ b/packages/filter-lockfile/src/filterLockfileByImportersAndEngine.ts @@ -14,6 +14,31 @@ import filterImporter from './filterImporter' const logger = pnpmLogger('lockfile') +function toImporterDepPaths ( + lockfile: Lockfile, + importerIds: string[], + opts: { + include: { [dependenciesField in DependenciesField]: boolean } + } +) { + const importerDeps = importerIds + .map(importerId => lockfile.importers[importerId]) + .map(importer => ({ + ...(opts.include.dependencies ? importer.dependencies : {}), + ...(opts.include.devDependencies ? importer.devDependencies : {}), + ...(opts.include.optionalDependencies + ? importer.optionalDependencies + : {}), + })) + .map(Object.entries) + + const importerDepsPaths = unnest(importerDeps) + .map(([pkgName, ref]) => dp.refToRelative(ref, pkgName)) + .filter(nodeId => nodeId !== null) as string[] + + return importerDepsPaths +} + export default function filterByImportersAndEngine ( lockfile: Lockfile, importerIds: string[], @@ -29,30 +54,26 @@ export default function filterByImportersAndEngine ( lockfileDir: string skipped: Set } -): Lockfile { - const importerDeps = importerIds - .map((importerId) => lockfile.importers[importerId]) - .map((importer) => ({ - ...(opts.include.dependencies ? importer.dependencies : {}), - ...(opts.include.devDependencies ? importer.devDependencies : {}), - ...(opts.include.optionalDependencies ? importer.optionalDependencies : {}), - })) - .map(Object.entries) - const directDepPaths = unnest(importerDeps) - .map(([pkgName, ref]) => dp.refToRelative(ref, pkgName)) - .filter((nodeId) => nodeId !== null) as string[] +): { lockfile: Lockfile, importerIds: string[] } { + const importerIdSet = new Set(importerIds) as Set - const packages = (lockfile.packages != null) - ? pickPkgsWithAllDeps(lockfile.packages, directDepPaths, { - currentEngine: opts.currentEngine, - engineStrict: opts.engineStrict, - failOnMissingDependencies: opts.failOnMissingDependencies, - include: opts.include, - includeIncompatiblePackages: opts.includeIncompatiblePackages === true, - lockfileDir: opts.lockfileDir, - skipped: opts.skipped, - }) - : {} + const directDepPaths = toImporterDepPaths(lockfile, importerIds, { + include: opts.include, + }) + + const packages = + lockfile.packages != null + ? pickPkgsWithAllDeps(lockfile, directDepPaths, importerIdSet, { + currentEngine: opts.currentEngine, + engineStrict: opts.engineStrict, + failOnMissingDependencies: opts.failOnMissingDependencies, + include: opts.include, + includeIncompatiblePackages: + opts.includeIncompatiblePackages === true, + lockfileDir: opts.lockfileDir, + skipped: opts.skipped, + }) + : {} const importers = importerIds.reduce((acc, importerId) => { acc[importerId] = filterImporter(lockfile.importers[importerId], opts.include) @@ -68,15 +89,19 @@ export default function filterByImportersAndEngine ( }, { ...lockfile.importers }) return { - ...lockfile, - importers, - packages, + lockfile: { + ...lockfile, + importers, + packages, + }, + importerIds: Array.from(importerIdSet), } } function pickPkgsWithAllDeps ( - pkgSnapshots: PackageSnapshots, + lockfile: Lockfile, depPaths: string[], + importerIdSet: Set, opts: { currentEngine: { nodeVersion: string @@ -91,14 +116,15 @@ function pickPkgsWithAllDeps ( } ) { const pickedPackages = {} as PackageSnapshots - pkgAllDeps({ pkgSnapshots, pickedPackages }, depPaths, true, opts) + pkgAllDeps({ lockfile, pickedPackages, importerIdSet }, depPaths, true, opts) return pickedPackages } function pkgAllDeps ( ctx: { - pkgSnapshots: PackageSnapshots + lockfile: Lockfile pickedPackages: PackageSnapshots + importerIdSet: Set }, depPaths: string[], parentIsInstallable: boolean, @@ -117,7 +143,7 @@ function pkgAllDeps ( ) { for (const depPath of depPaths) { if (ctx.pickedPackages[depPath]) continue - const pkgSnapshot = ctx.pkgSnapshots[depPath] + const pkgSnapshot = ctx.lockfile.packages![depPath] if (!pkgSnapshot && !depPath.startsWith('link:')) { if (opts.failOnMissingDependencies) { throw new LockfileMissingDependencyError(depPath) @@ -140,13 +166,15 @@ function pkgAllDeps ( libc: pkgSnapshot.libc, } // TODO: depPath is not the package ID. Should be fixed - installable = opts.includeIncompatiblePackages || packageIsInstallable(pkgSnapshot.id ?? depPath, pkg, { - engineStrict: opts.engineStrict, - lockfileDir: opts.lockfileDir, - nodeVersion: opts.currentEngine.nodeVersion, - optional: pkgSnapshot.optional === true, - pnpmVersion: opts.currentEngine.pnpmVersion, - }) !== false + installable = + opts.includeIncompatiblePackages || + packageIsInstallable(pkgSnapshot.id ?? depPath, pkg, { + engineStrict: opts.engineStrict, + lockfileDir: opts.lockfileDir, + nodeVersion: opts.currentEngine.nodeVersion, + optional: pkgSnapshot.optional === true, + pnpmVersion: opts.currentEngine.pnpmVersion, + }) !== false if (!installable) { if (!ctx.pickedPackages[depPath] && pkgSnapshot.optional === true) { opts.skipped.add(depPath) @@ -156,14 +184,39 @@ function pkgAllDeps ( } } ctx.pickedPackages[depPath] = pkgSnapshot - const nextRelDepPaths = Object.entries( - { - ...pkgSnapshot.dependencies, - ...(opts.include.optionalDependencies ? pkgSnapshot.optionalDependencies : {}), + const nextRelDepPaths = Object.entries({ + ...pkgSnapshot.dependencies, + ...(opts.include.optionalDependencies + ? pkgSnapshot.optionalDependencies + : {}), + }) + .map(([pkgName, ref]) => { + if (ref.startsWith('link:')) { + return ref + } + return dp.refToRelative(ref, pkgName) + }) + .filter(nodeId => nodeId !== null) as string[] + + // Also include missing deeply linked workspace project + const actualNextRelDepPaths = [] + const additionalImporterIds = [] + for (const nextDepPath of nextRelDepPaths) { + if (nextDepPath.startsWith('link:')) { + const ref = nextDepPath.slice(5) + additionalImporterIds.push(ref) + ctx.importerIdSet.add(ref) + } else { + actualNextRelDepPaths.push(nextDepPath) + } + } + + actualNextRelDepPaths.push( + ...toImporterDepPaths(ctx.lockfile, additionalImporterIds, { + include: opts.include, }) - .map(([pkgName, ref]) => dp.refToRelative(ref, pkgName)) - .filter((nodeId) => nodeId !== null) as string[] + ) - pkgAllDeps(ctx, nextRelDepPaths, installable, opts) + pkgAllDeps(ctx, actualNextRelDepPaths, installable, opts) } } diff --git a/packages/filter-lockfile/test/filterByImportersAndEngine.ts b/packages/filter-lockfile/test/filterByImportersAndEngine.ts index 5a10af816d9..8be9145254b 100644 --- a/packages/filter-lockfile/test/filterByImportersAndEngine.ts +++ b/packages/filter-lockfile/test/filterByImportersAndEngine.ts @@ -119,7 +119,7 @@ test('filterByImportersAndEngine(): skip packages that are not installable', () } ) - expect(filteredLockfile).toStrictEqual({ + expect(filteredLockfile.lockfile).toStrictEqual({ importers: { 'project-1': { dependencies: { @@ -298,7 +298,7 @@ test('filterByImportersAndEngine(): filter the packages that set os and cpu', () } ) - expect(filteredLockfile).toStrictEqual({ + expect(filteredLockfile.lockfile).toStrictEqual({ importers: { 'project-1': { dependencies: { @@ -466,7 +466,7 @@ test('filterByImportersAndEngine(): filter the packages that set libc', () => { } ) - expect(filteredLockfile).toStrictEqual({ + expect(filteredLockfile.lockfile).toStrictEqual({ importers: { 'project-1': { dependencies: { diff --git a/packages/headless/src/index.ts b/packages/headless/src/index.ts index d9f2a502502..a72d6343919 100644 --- a/packages/headless/src/index.ts +++ b/packages/headless/src/index.ts @@ -240,10 +240,10 @@ export async function headlessInstall (opts: HeadlessOptions) { registries: opts.registries, skipped, } - const importerIds = (opts.ignorePackageManifest === true || opts.nodeLinker === 'hoisted') + const initialImporterIds = (opts.ignorePackageManifest === true || opts.nodeLinker === 'hoisted') ? Object.keys(wantedLockfile.importers) : selectedProjects.map(({ id }) => id) - const filteredLockfile = filterLockfileByImportersAndEngine(wantedLockfile, importerIds, { + const { lockfile: filteredLockfile, importerIds } = filterLockfileByImportersAndEngine(wantedLockfile, initialImporterIds, { ...filterOpts, currentEngine: opts.currentEngine, engineStrict: opts.engineStrict, @@ -251,6 +251,22 @@ export async function headlessInstall (opts: HeadlessOptions) { includeIncompatiblePackages: opts.force, lockfileDir, }) + + // Update selectedProjects to add missing projects. importerIds will have the updated ids, found from deeply linked workspace projects + const missingIds = [] as string[] + const initialImporterIdSet = new Set(initialImporterIds) + for (const id of importerIds) { + if (!initialImporterIdSet.has(id)) { + missingIds.push(id) + } + } + for (const id of missingIds) { + const additionalProject = Object.values(opts.allProjects).find((project) => project.id === id) + if (additionalProject) { + selectedProjects.push(additionalProject) + } + } + const lockfileToDepGraphOpts = { ...opts, importerIds, diff --git a/packages/headless/test/index.ts b/packages/headless/test/index.ts index 0bfea74be98..461136eafb8 100644 --- a/packages/headless/test/index.ts +++ b/packages/headless/test/index.ts @@ -831,3 +831,28 @@ test('installing in a workspace with node-linker=hoisted', async () => { function readPkgVersion (dir: string): string { return loadJsonFile.sync<{ version: string }>(path.join(dir, 'package.json')).version } + +test('installing a package deeply installs all required dependencies', async () => { + const workspaceFixture = f.prepare('workspace-external-depends-deep') + const projects = [ + path.join(workspaceFixture), path.join(workspaceFixture, 'packages/f'), + path.join(workspaceFixture, 'packages/g'), + workspaceFixture, + ] + + await headlessInstall( + await testDefaults({ + lockfileDir: workspaceFixture, + projects, + selectedProjectDirs: [projects[1]], + }) + ) + + for (const projectDir of projects) { + if (projectDir === workspaceFixture) { + continue + } + const projectAssertion = assertProject(projectDir) + expect(projectAssertion.requireModule('is-positive')).toBeTruthy() + } +}) diff --git a/packages/headless/test/utils/testDefaults.ts b/packages/headless/test/utils/testDefaults.ts index 4ece65ceee4..0e08e8bac50 100644 --- a/packages/headless/test/utils/testDefaults.ts +++ b/packages/headless/test/utils/testDefaults.ts @@ -75,7 +75,7 @@ export default async function testDefaults ( version: '1.0.0', }, pendingBuilds, - selectedProjectDirs: projects.map((project) => project.rootDir), + selectedProjectDirs: opts.selectedProjectDirs ?? projects.map((project) => project.rootDir), allProjects: fromPairs( await Promise.all(projects.map(async (project) => [project.rootDir, { ...project, manifest: await safeReadPackageFromDir(project.rootDir) }])) ), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1e858d7c430..a9ed37447dc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -640,6 +640,9 @@ importers: '@pnpm/read-project-manifest': specifier: workspace:* version: link:../read-project-manifest + '@pnpm/read-projects-context': + specifier: workspace:* + version: link:../read-projects-context '@pnpm/remove-bins': specifier: workspace:* version: link:../remove-bins