diff --git a/__tests__/commands/install/bin-links.js b/__tests__/commands/install/bin-links.js index aa1d094d64..b8140b7b6b 100644 --- a/__tests__/commands/install/bin-links.js +++ b/__tests__/commands/install/bin-links.js @@ -165,3 +165,37 @@ test('Only top level (after hoisting) bin links should be linked', (): Promise { + // address https://github.com/yarnpkg/yarn/issues/5487 + test('nohoist bin should be linked to its own local module', (): Promise => { + return runInstall({binLinks: true}, 'install-bin-links-nohoist', async config => { + // make sure all links are created at the right locations and executed correctly + const stdout1 = await execCommand(config.cwd, ['node_modules', '.bin', 'exec-a'], []); + const stdout2 = await execCommand(config.cwd, ['node_modules', '.bin', 'exec-f'], []); + const stdout3 = await execCommand(config.cwd, ['packages', 'a-dep', 'node_modules', '.bin', 'found-me'], []); + const stdout4 = await execCommand(config.cwd, ['packages', 'f-dep', 'node_modules', '.bin', 'found-me'], []); + expect(stdout1[0]).toEqual('exec-a'); + expect(stdout2[0]).toEqual('exec-f'); + expect(stdout3[0]).toEqual('found-me'); + expect(stdout4[0]).toEqual('found-me'); + + // make sure the shared links: found-me are pointing to the local module + const localLink = '../found-me/bin.js'; + expect(await linkAt(config, 'packages', 'a-dep', 'node_modules', '.bin', 'found-me')).toEqual(localLink); + expect(await linkAt(config, 'packages', 'f-dep', 'node_modules', '.bin', 'found-me')).toEqual(localLink); + }); + }); + test('nohoist bin should not be linked at top level, unless it is a top-level package', (): Promise => { + return runInstall({binLinks: true}, 'install-bin-links-nohoist', async config => { + expect(await fs.exists(path.join(config.cwd, 'node_modules', '.bin', 'exec-a'))).toEqual(true); + expect(await fs.exists(path.join(config.cwd, 'node_modules', '.bin', 'exec-f'))).toEqual(true); + expect(await fs.exists(path.join(config.cwd, 'node_modules', '.bin', 'found-me'))).toEqual(false); + + // the top-level packages should never be marked nohoist, even if they match nohoist patterns. + // therefore, expect those still linked at the root node_modules. + expect(await fs.exists(path.join(config.cwd, 'node_modules', '.bin', 'top-module'))).toEqual(true); + expect(await linkAt(config, 'node_modules', '.bin', 'top-module')).toEqual('../top-module/bin.js'); + }); + }); +}); diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/found-me/bin.js b/__tests__/fixtures/install/install-bin-links-nohoist/found-me/bin.js new file mode 100755 index 0000000000..45a81d32fe --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/found-me/bin.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('found-me'); diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/found-me/package.json b/__tests__/fixtures/install/install-bin-links-nohoist/found-me/package.json new file mode 100644 index 0000000000..698b431aa9 --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/found-me/package.json @@ -0,0 +1,7 @@ +{ + "name": "found-me", + "version": "0.0.1", + "bin": { + "found-me": "./bin.js" + } +} diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/package.json b/__tests__/fixtures/install/install-bin-links-nohoist/package.json new file mode 100644 index 0000000000..d3adda7905 --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/package.json @@ -0,0 +1,11 @@ +{ + "name": "install-bin-links-nohoist", + "private": true, + "workspaces": { + "packages": ["packages/*"], + "nohoist": ["**/found-me", "**/found-me/**", "a-dep", "top-module"] + }, + "dependencies": { + "top-module": "file:top-module" + } +} diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/packages/a-dep/bin.js b/__tests__/fixtures/install/install-bin-links-nohoist/packages/a-dep/bin.js new file mode 100755 index 0000000000..e2f223f40d --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/packages/a-dep/bin.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('exec-a'); diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/packages/a-dep/package.json b/__tests__/fixtures/install/install-bin-links-nohoist/packages/a-dep/package.json new file mode 100644 index 0000000000..e696c54132 --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/packages/a-dep/package.json @@ -0,0 +1,10 @@ +{ + "name": "a-dep", + "version": "0.0.1", + "bin": { + "exec-a": "./bin.js" + }, + "dependencies": { + "found-me": "file:../../found-me" + } +} diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/packages/f-dep/bin.js b/__tests__/fixtures/install/install-bin-links-nohoist/packages/f-dep/bin.js new file mode 100755 index 0000000000..e93c21358a --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/packages/f-dep/bin.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('exec-f'); diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/packages/f-dep/package.json b/__tests__/fixtures/install/install-bin-links-nohoist/packages/f-dep/package.json new file mode 100644 index 0000000000..dc4a322894 --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/packages/f-dep/package.json @@ -0,0 +1,10 @@ +{ + "name": "f-dep", + "version": "0.0.1", + "bin": { + "exec-f": "./bin.js" + }, + "dependencies": { + "found-me": "file:../../found-me" + } +} diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/top-module/bin.js b/__tests__/fixtures/install/install-bin-links-nohoist/top-module/bin.js new file mode 100755 index 0000000000..45a81d32fe --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/top-module/bin.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('found-me'); diff --git a/__tests__/fixtures/install/install-bin-links-nohoist/top-module/package.json b/__tests__/fixtures/install/install-bin-links-nohoist/top-module/package.json new file mode 100644 index 0000000000..87b177f2c2 --- /dev/null +++ b/__tests__/fixtures/install/install-bin-links-nohoist/top-module/package.json @@ -0,0 +1,7 @@ +{ + "name": "top-module", + "version": "0.0.1", + "bin": { + "top-module": "./bin.js" + } +} diff --git a/src/package-linker.js b/src/package-linker.js index 83a6ca8a78..578248f339 100644 --- a/src/package-linker.js +++ b/src/package-linker.js @@ -4,7 +4,7 @@ import type {Manifest} from './types.js'; import type PackageResolver from './package-resolver.js'; import type {Reporter} from './reporters/index.js'; import type Config from './config.js'; -import type {HoistManifestTuples} from './package-hoister.js'; +import type {HoistManifestTuples, HoistManifestTuple} from './package-hoister.js'; import type {CopyQueueItem} from './util/fs.js'; import type {InstallArtifacts} from './package-install-scripts.js'; import PackageHoister from './package-hoister.js'; @@ -56,6 +56,7 @@ export default class PackageLinker { resolver: PackageResolver; config: Config; topLevelBinLinking: boolean; + _treeHash: ?Map; setArtifacts(artifacts: InstallArtifacts) { this.artifacts = artifacts; @@ -65,7 +66,12 @@ export default class PackageLinker { this.topLevelBinLinking = topLevelBinLinking; } - async linkSelfDependencies(pkg: Manifest, pkgLoc: string, targetBinLoc: string): Promise { + async linkSelfDependencies( + pkg: Manifest, + pkgLoc: string, + targetBinLoc: string, + override: boolean = false, + ): Promise { targetBinLoc = path.join(targetBinLoc, '.bin'); await fs.mkdirp(targetBinLoc); targetBinLoc = await fs.realpath(targetBinLoc); @@ -74,8 +80,10 @@ export default class PackageLinker { const dest = path.join(targetBinLoc, scriptName); const src = path.join(pkgLoc, scriptCmd); if (!await fs.exists(src)) { - // TODO maybe throw an error - continue; + if (!override) { + // TODO maybe throw an error + continue; + } } await linkBin(src, dest); } @@ -408,10 +416,15 @@ export default class PackageLinker { // create links in transient dependencies await promise.queue( flatTree, - async ([dest, {pkg}]) => { + async ([dest, {pkg, isNohoist, parts}]) => { if (pkg._reference && pkg._reference.location) { const binLoc = path.join(dest, this.config.getFolder(pkg)); await this.linkBinDependencies(pkg, binLoc); + if (isNohoist) { + // if nohoist, we need to override the binLink to point to the local destination + const parentBinLoc = this.getParentBinLoc(parts, flatTree); + await this.linkSelfDependencies(pkg, dest, parentBinLoc, true); + } tickBin(); } }, @@ -437,13 +450,37 @@ export default class PackageLinker { } } + _buildTreeHash(flatTree: HoistManifestTuples): Map { + const hash: Map = new Map(); + for (const [dest, hoistManifest] of flatTree) { + const key: string = hoistManifest.parts.join('#'); + hash.set(key, [dest, hoistManifest]); + } + this._treeHash = hash; + return hash; + } + + getParentBinLoc(parts: Array, flatTree: HoistManifestTuples): string { + const hash = this._treeHash || this._buildTreeHash(flatTree); + const parent = parts.slice(0, -1).join('#'); + const tuple = hash.get(parent); + if (!tuple) { + throw new Error(`failed to get parent '${parent}' binLoc`); + } + const [dest, hoistManifest] = tuple; + const parentBinLoc = path.join(dest, this.config.getFolder(hoistManifest.pkg)); + + return parentBinLoc; + } + determineTopLevelBinLinkOrder(flatTree: HoistManifestTuples): HoistManifestTuples { const linksToCreate = new Map(); for (const [dest, hoistManifest] of flatTree) { - const {pkg, isDirectRequire} = hoistManifest; + const {pkg, isDirectRequire, isNohoist} = hoistManifest; const {name} = pkg; - if (isDirectRequire || (this.topLevelBinLinking && !linksToCreate.has(name))) { + // a nohoist package should not be linked at topLevel bin + if (!isNohoist && (isDirectRequire || (this.topLevelBinLinking && !linksToCreate.has(name)))) { linksToCreate.set(name, [dest, hoistManifest]); } }