fix nohoist binLink

__tests__/commands/install/bin-links.js

@@ -165,3 +165,37 @@ test('Only top level (after hoisting) bin links should be linked', (): Promise<v
     expect(stdout[0]).toEqual('uglify-js 3.0.14');
   });
 });
+
+describe('with nohoist', () => {
+  // address https://github.com/yarnpkg/yarn/issues/5487
+  test('nohoist bin should be linked to its own local module', (): Promise<void> => {
+    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<void> => {
+    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');
+    });
+  });
+});

__tests__/fixtures/install/install-bin-links-nohoist/found-me/bin.js

@@ -0,0 +1,2 @@
+#!/usr/bin/env node
+console.log('found-me');

__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"
+  }
+}

__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"
+  }
+}

__tests__/fixtures/install/install-bin-links-nohoist/packages/a-dep/bin.js

@@ -0,0 +1,2 @@
+#!/usr/bin/env node
+console.log('exec-a');

__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"
+  }
+}

__tests__/fixtures/install/install-bin-links-nohoist/packages/f-dep/bin.js

@@ -0,0 +1,2 @@
+#!/usr/bin/env node
+console.log('exec-f');

__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"
+  }
+}

__tests__/fixtures/install/install-bin-links-nohoist/top-module/bin.js

@@ -0,0 +1,2 @@
+#!/usr/bin/env node
+console.log('found-me');

__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"
+  }
+}

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<string, HoistManifestTuple>;
 
   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<void> {
+  async linkSelfDependencies(
+    pkg: Manifest,
+    pkgLoc: string,
+    targetBinLoc: string,
+    override: boolean = false,
+  ): Promise<void> {
     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<string, HoistManifestTuple> {
+    const hash: Map<string, HoistManifestTuple> = 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<string>, 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]);
       }
     }