diff --git a/.yarn/versions/0a83996d.yml b/.yarn/versions/0a83996d.yml new file mode 100644 index 000000000000..8cbe5429de82 --- /dev/null +++ b/.yarn/versions/0a83996d.yml @@ -0,0 +1,24 @@ +releases: + "@yarnpkg/cli": patch + "@yarnpkg/plugin-node-modules": patch + "@yarnpkg/pnpify": patch + +declined: + - "@yarnpkg/plugin-compat" + - "@yarnpkg/plugin-constraints" + - "@yarnpkg/plugin-dlx" + - "@yarnpkg/plugin-essentials" + - "@yarnpkg/plugin-init" + - "@yarnpkg/plugin-interactive-tools" + - "@yarnpkg/plugin-npm-cli" + - "@yarnpkg/plugin-pack" + - "@yarnpkg/plugin-patch" + - "@yarnpkg/plugin-pnp" + - "@yarnpkg/plugin-stage" + - "@yarnpkg/plugin-typescript" + - "@yarnpkg/plugin-version" + - "@yarnpkg/plugin-workspace-tools" + - vscode-zipfs + - "@yarnpkg/builder" + - "@yarnpkg/core" + - "@yarnpkg/doctor" diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts index 2f320b7f801a..37ff71b5ff04 100644 --- a/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts +++ b/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts @@ -878,6 +878,38 @@ describe(`Node_Modules`, () => { }) ); + test(`should still hoist direct dependencies from portal target to parent with nmHoistingLimits: dependencies`, + makeTemporaryEnv({}, + { + nodeLinker: `node-modules`, + nmHoistingLimits: `dependencies`, + }, + async ({path, run, source}) => { + await xfs.mktempPromise(async portalTarget => { + await xfs.writeJsonPromise(`${portalTarget}/package.json` as PortablePath, { + name: `portal`, + dependencies: { + [`one-fixed-dep`]: `1.0.0`, + }, + }); + await xfs.writeJsonPromise(`${path}/package.json` as PortablePath, { + dependencies: { + [`portal`]: `portal:${portalTarget}`, + }, + }); + + const {stdout} = await run(`install`); + + await expect(source(`require('one-fixed-dep')`)).resolves.toMatchObject({ + version: `1.0.0`, + }); + expect(stdout).toMatch(new RegExp(`--preserve-symlinks`)); + expect(await xfs.existsPromise(`${portalTarget}/node_modules` as PortablePath)).toBeFalsy(); + expect(await xfs.existsPromise(`${path}/node_modules/no-deps` as PortablePath)).toBeFalsy(); + }); + }) + ); + test(`should error out on external portal requiring a dependency that conflicts with parent package`, makeTemporaryEnv({}, { @@ -888,7 +920,10 @@ describe(`Node_Modules`, () => { await xfs.writeJsonPromise(`${portalTarget}/package.json` as PortablePath, { name: `portal`, dependencies: { - [`no-deps`]: `2.0.0`, + 'no-deps': `2.0.0`, + }, + peerDependencies: { + 'no-deps-bins': `1.0.0`, }, }); @@ -896,6 +931,7 @@ describe(`Node_Modules`, () => { dependencies: { portal: `portal:${portalTarget}`, 'no-deps': `1.0.0`, + 'no-deps-bins': `1.0.0`, }, }); @@ -911,6 +947,47 @@ describe(`Node_Modules`, () => { }) ); + test(`should error out if two same-parent portals have conflict between their direct dependencies`, + makeTemporaryEnv({}, + { + nodeLinker: `node-modules`, + }, + async ({path, run}) => { + await xfs.mktempPromise(async portalTarget1 => { + await xfs.mktempPromise(async portalTarget2 => { + await xfs.writeJsonPromise(`${portalTarget1}/package.json` as PortablePath, { + name: `portal1`, + dependencies: { + 'no-deps': `2.0.0`, + }, + }); + await xfs.writeJsonPromise(`${portalTarget2}/package.json` as PortablePath, { + name: `portal2`, + dependencies: { + 'no-deps': `1.0.0`, + }, + }); + + await xfs.writeJsonPromise(`${path}/package.json` as PortablePath, { + dependencies: { + portal1: `portal:${portalTarget1}`, + portal2: `portal:${portalTarget2}`, + }, + }); + + let stdout; + try { + await run(`install`); + } catch (e) { + stdout = e.stdout; + } + + expect(stdout).toMatch(new RegExp(`dependency no-deps@npm:1.0.0 conflicts with dependency no-deps@npm:2.0.0 from sibling portal portal1`)); + }); + }); + }) + ); + test( `should not warn when depending on workspaces with postinstall`, makeTemporaryEnv( diff --git a/packages/gatsby/content/advanced/error-codes.md b/packages/gatsby/content/advanced/error-codes.md index ed0352a65bd1..12a242ba1f1b 100644 --- a/packages/gatsby/content/advanced/error-codes.md +++ b/packages/gatsby/content/advanced/error-codes.md @@ -328,9 +328,11 @@ A packageExtension is detected by Yarn as being unused, which means that the sel A packageExtension is detected by Yarn as being unneeded, which means that the selected packages have the same behavior with and without the extension. -## YN0071 - `NM_CANT_INSTALL_PORTAL` +## YN0071 - `NM_CANT_INSTALL_EXTERNAL_SOFT_LINK` -A portal dependency cannot be installed, because incompatible version of a dependency exists in the parent package. This prevents portal representation for node_modules installs without a need to write files into portal's target directory, which is forbidden for security reasons. +An external soft link (portal) cannot be installed, because incompatible version of a dependency exists in the parent package. This prevents portal representation for node_modules installs without a need to write files into portal's target directory, which is forbidden for security reasons. + +**Workarounds** If the ranges for conflicting dependencies overlap between portal target and portal parent, the workaround is to use `yarn dedupe foo` (where `foo` is the conflicting dependency name) to upgrade the conflicting dependencies to the highest available versions, if `yarn dedupe` is used without arguments, all the dependencies across the project will be upgraded to the highest versions within their ranges in `package.json`. Another alternative is to use `link:` protocol instead of `portal:` and install dependencies inside the target directory explicitely. ## YN0072 - `NM_PRESERVE_SYMLINKS_REQUIRED` diff --git a/packages/plugin-node-modules/sources/NodeModulesLinker.ts b/packages/plugin-node-modules/sources/NodeModulesLinker.ts index 4247d7095814..d5921d03142e 100644 --- a/packages/plugin-node-modules/sources/NodeModulesLinker.ts +++ b/packages/plugin-node-modules/sources/NodeModulesLinker.ts @@ -274,7 +274,7 @@ class NodeModulesInstaller implements Installer { }, }; - const {tree, errors, preserveSymlinksRequired} = buildNodeModulesTree(pnpApi, {pnpifyFs: false, hoistingLimitsByCwd, project: this.opts.project}); + const {tree, errors, preserveSymlinksRequired} = buildNodeModulesTree(pnpApi, {pnpifyFs: false, validateExternalSoftLinks: true, hoistingLimitsByCwd, project: this.opts.project}); if (!tree) { for (const {messageName, text} of errors) this.opts.report.reportError(messageName, text); @@ -555,7 +555,7 @@ type LocationTree = Map; const parseLocation = (location: PortablePath, {skipPrefix}: {skipPrefix: PortablePath}): {locationRoot: PortablePath, segments: Array} => { const projectRelativePath = ppath.contains(skipPrefix, location); if (projectRelativePath === null) - throw new Error(`Assertion failed: Cannot process a path that isn't part of the requested prefix (${location} isn't within ${skipPrefix})`); + throw new Error(`Assertion failed: Writing attempt prevented to ${location} which is outside project root: ${skipPrefix}`); const allSegments = projectRelativePath .split(ppath.sep) @@ -1070,7 +1070,7 @@ async function persistBinSymlinks(previousBinSymlinks: BinSymlinkMap, binSymlink } else { await xfs.removePromise(symlinkPath); await symlinkPromise(target, symlinkPath); - if (ppath.contains(target, projectCwd) !== null) { + if (ppath.contains(projectCwd, await xfs.realpathPromise(target)) !== null) { await xfs.chmodPromise(target, 0o755); } } diff --git a/packages/yarnpkg-core/sources/MessageName.ts b/packages/yarnpkg-core/sources/MessageName.ts index e82b14993b75..42850af50783 100644 --- a/packages/yarnpkg-core/sources/MessageName.ts +++ b/packages/yarnpkg-core/sources/MessageName.ts @@ -73,7 +73,7 @@ export enum MessageName { UNUSED_PACKAGE_EXTENSION = 68, REDUNDANT_PACKAGE_EXTENSION = 69, AUTO_NM_SUCCESS = 70, - NM_CANT_INSTALL_PORTAL = 71, + NM_CANT_INSTALL_EXTERNAL_SOFT_LINK = 71, NM_PRESERVE_SYMLINKS_REQUIRED = 72, } diff --git a/packages/yarnpkg-pnpify/sources/buildNodeModulesTree.ts b/packages/yarnpkg-pnpify/sources/buildNodeModulesTree.ts index 8a31f2e57c16..1d2cf30feb94 100644 --- a/packages/yarnpkg-pnpify/sources/buildNodeModulesTree.ts +++ b/packages/yarnpkg-pnpify/sources/buildNodeModulesTree.ts @@ -1,4 +1,4 @@ -import {structUtils, Project, MessageName} from '@yarnpkg/core'; +import {structUtils, Project, MessageName, Locator} from '@yarnpkg/core'; import {toFilename, npath, ppath} from '@yarnpkg/fslib'; import {NativePath, PortablePath, Filename} from '@yarnpkg/fslib'; import {PnpApi, PhysicalPackageLocator, PackageInformation, DependencyTarget} from '@yarnpkg/pnp'; @@ -51,6 +51,7 @@ export type NodeModulesTreeErrors = Array<{messageName: MessageName, text: strin export interface NodeModulesTreeOptions { pnpifyFs?: boolean; + validateExternalSoftLinks?: boolean; hoistingLimitsByCwd?: Map; project?: Project; } @@ -138,6 +139,12 @@ export const buildLocatorMap = (nodeModulesTree: NodeModulesTree): NodeModulesLo return map; }; +const areRealLocatorsEqual = (a: Locator, b: Locator) => { + const realA = structUtils.isVirtualLocator(a) ? structUtils.devirtualizeLocator(a) : a; + const realB = structUtils.isVirtualLocator(b) ? structUtils.devirtualizeLocator(b) : b; + return structUtils.areLocatorsEqual(realA, realB); +}; + /** * Traverses PnP tree and produces input for the `RawHoister` * @@ -224,6 +231,14 @@ const buildPackageTree = (pnp: PnpApi, options: NodeModulesTreeOptions): { packa const nodes = new Map(); const getNodeKey = (name: string, locator: PhysicalPackageLocator) => `${stringifyLocator(locator)}:${name}`; + const isExternalSoftLink = (pkg: PackageInformation, locator: PhysicalPackageLocator) => { + if (pkg.linkType !== LinkType.SOFT || !options.project) + return false; + + const realSoftLinkPath = npath.toPortablePath(pnp.resolveVirtual && locator.reference && locator.reference.startsWith(`virtual:`) ? pnp.resolveVirtual(pkg.packageLocation)! : pkg.packageLocation); + return ppath.contains(options.project.cwd, realSoftLinkPath) === null; + }; + const addPackageToTree = (name: string, pkg: PackageInformation, locator: PhysicalPackageLocator, parent: HoisterTree, parentDependencies: Map, parentRelativeCwd: PortablePath, isHoistBorder: boolean) => { const nodeKey = getNodeKey(name, locator); let node = nodes.get(nodeKey); @@ -234,22 +249,6 @@ const buildPackageTree = (pnp: PnpApi, options: NodeModulesTreeOptions): { packa nodes.set(nodeKey, packageTree); } - if (pkg.linkType === LinkType.SOFT && options.project && ppath.contains(options.project.cwd, npath.toPortablePath(pkg.packageLocation)) === null) { - if (pkg.packageDependencies.size > 0) - preserveSymlinksRequired = true; - - for (const [name, referencish] of pkg.packageDependencies) { - const dependencyLocator = structUtils.parseLocator(Array.isArray(referencish) ? `${referencish[0]}@${referencish[1]}` : `${name}@${referencish}`); - const parentReferencish = parentDependencies.get(name); - if (parentReferencish) { - const parentDependencyLocator = structUtils.parseLocator(Array.isArray(parentReferencish) ? `${parentReferencish[0]}@${parentReferencish[1]}` : `${name}@${parentReferencish}`); - if (!structUtils.areLocatorsEqual(parentDependencyLocator, dependencyLocator)) { - errors.push({messageName: MessageName.NM_CANT_INSTALL_PORTAL, text: `Cannot link ${structUtils.prettyIdent(options.project.configuration, structUtils.parseIdent(locator.name))} into ${structUtils.prettyLocator(options.project.configuration, structUtils.parseLocator(`${parent.identName}@${parent.reference}`))} dependency ${structUtils.prettyLocator(options.project.configuration, dependencyLocator)} conflicts with parent dependency ${structUtils.prettyLocator(options.project.configuration, parentDependencyLocator)}`}); - } - } - } - } - if (!node) { node = { name, @@ -262,7 +261,7 @@ const buildPackageTree = (pnp: PnpApi, options: NodeModulesTreeOptions): { packa nodes.set(nodeKey, node); } - if (isHoistBorder) { + if (isHoistBorder && !isExternalSoftLink(pkg, locator)) { const parentLocatorKey = stringifyLocator({name: parent.identName, reference: parent.reference}); const dependencyBorders = hoistingLimits.get(parentLocatorKey) || new Set(); hoistingLimits.set(parentLocatorKey, dependencyBorders); @@ -298,6 +297,7 @@ const buildPackageTree = (pnp: PnpApi, options: NodeModulesTreeOptions): { packa parent.dependencies.add(node); if (!isSeen) { + const siblingPortalDependencyMap = new Map(); for (const [depName, referencish] of allDependencies) { if (referencish !== null) { const depLocator = pnp.getLocator(depName, referencish); @@ -306,9 +306,53 @@ const buildPackageTree = (pnp: PnpApi, options: NodeModulesTreeOptions): { packa const depPkg = pnp.getPackageInformation(pkgLocator); if (depPkg === null) throw new Error(`Assertion failed: Expected the package to have been registered`); + const isExternalSoftLinkDep = isExternalSoftLink(depPkg, depLocator); + + if (options.validateExternalSoftLinks && options.project && isExternalSoftLinkDep) { + if (depPkg.packageDependencies.size > 0) + preserveSymlinksRequired = true; + + for (const [name, referencish] of depPkg.packageDependencies) { + const portalDependencyLocator = structUtils.parseLocator(Array.isArray(referencish) ? `${referencish[0]}@${referencish[1]}` : `${name}@${referencish}`); + // Ignore self-references during portal hoistability check + if (stringifyLocator(portalDependencyLocator) !== stringifyLocator(depLocator)) { + const parentDependencyReferencish = allDependencies.get(name); + if (parentDependencyReferencish) { + const parentDependencyLocator = structUtils.parseLocator(Array.isArray(parentDependencyReferencish) ? `${parentDependencyReferencish[0]}@${parentDependencyReferencish[1]}` : `${name}@${parentDependencyReferencish}`); + if (!areRealLocatorsEqual(parentDependencyLocator, portalDependencyLocator)) { + errors.push({ + messageName: MessageName.NM_CANT_INSTALL_EXTERNAL_SOFT_LINK, + text: `Cannot link ${structUtils.prettyIdent(options.project.configuration, structUtils.parseIdent(depLocator.name))} ` + + `into ${structUtils.prettyLocator(options.project.configuration, structUtils.parseLocator(`${locator.name}@${locator.reference}`))} ` + + `dependency ${structUtils.prettyLocator(options.project.configuration, portalDependencyLocator)} ` + + `conflicts with parent dependency ${structUtils.prettyLocator(options.project.configuration, parentDependencyLocator)}`, + }); + } + } else { + const siblingPortalDependency = siblingPortalDependencyMap.get(name); + if (siblingPortalDependency) { + const siblingReferncish = siblingPortalDependency.target; + const siblingPortalDependencyLocator = structUtils.parseLocator(Array.isArray(siblingReferncish) ? `${siblingReferncish[0]}@${siblingReferncish[1]}` : `${name}@${siblingReferncish}`); + if (!areRealLocatorsEqual(siblingPortalDependencyLocator, portalDependencyLocator)) { + errors.push({ + messageName: MessageName.NM_CANT_INSTALL_EXTERNAL_SOFT_LINK, + text: `Cannot link ${structUtils.prettyIdent(options.project.configuration, structUtils.parseIdent(depLocator.name))} ` + + `into ${structUtils.prettyLocator(options.project.configuration, structUtils.parseLocator(`${locator.name}@${locator.reference}`))} ` + + `dependency ${structUtils.prettyLocator(options.project.configuration, portalDependencyLocator)} ` + + `conflicts with dependency ${structUtils.prettyLocator(options.project.configuration, siblingPortalDependencyLocator)} ` + + `from sibling portal ${structUtils.prettyIdent(options.project.configuration, structUtils.parseIdent(siblingPortalDependency.portal.name))}`, + }); + } + } else { + siblingPortalDependencyMap.set(name, {target: portalDependencyLocator.reference, portal: depLocator}); + } + } + } + } + } const parentHoistingLimits = options.hoistingLimitsByCwd?.get(parentRelativeCwd); - const relativeDepCwd = ppath.relative(topPkgPortableLocation, npath.toPortablePath(depPkg.packageLocation)) || PortablePath.dot; + const relativeDepCwd = isExternalSoftLinkDep ? parentRelativeCwd : ppath.relative(topPkgPortableLocation, npath.toPortablePath(depPkg.packageLocation)) || PortablePath.dot; const depHoistingLimits = options.hoistingLimitsByCwd?.get(relativeDepCwd); const isHoistBorder = parentHoistingLimits === NodeModulesHoistingLimits.DEPENDENCIES || depHoistingLimits === NodeModulesHoistingLimits.DEPENDENCIES diff --git a/packages/yarnpkg-pnpify/sources/hoist.ts b/packages/yarnpkg-pnpify/sources/hoist.ts index 0e19cbe3e3a9..1228058c18c6 100644 --- a/packages/yarnpkg-pnpify/sources/hoist.ts +++ b/packages/yarnpkg-pnpify/sources/hoist.ts @@ -196,8 +196,10 @@ const getUsedDependencies = (rootNodePath: Array): Map