diff --git a/tap-snapshots/workspaces/arborist/test/arborist/rebuild.js.test.cjs b/tap-snapshots/workspaces/arborist/test/arborist/rebuild.js.test.cjs new file mode 100644 index 0000000000000..225a21f1656d1 --- /dev/null +++ b/tap-snapshots/workspaces/arborist/test/arborist/rebuild.js.test.cjs @@ -0,0 +1,87 @@ +/* IMPORTANT + * This snapshot file is auto-generated, but designed for humans. + * It should be checked into source control and tracked carefully. + * Re-generate by setting TAP_SNAPSHOT=1 and running tests. + * Make sure to inspect the output below. Do not ignore changes! + */ +'use strict' +exports[`workspaces/arborist/test/arborist/rebuild.js TAP verify dep flags in script environments > saved script results 1`] = ` +Array [ + Object { + "cmd": "node ../../env.js", + "code": 0, + "event": "postinstall", + "pkg": Object { + "_id": "devdep@1.0.0", + "dependencies": Object { + "devopt": "", + }, + "name": "devdep", + "optionalDependencies": Object { + "opt-and-dev": "", + }, + "scripts": Object { + "postinstall": "node ../../env.js", + }, + "version": "1.0.0", + }, + "signal": null, + "stderr": "stderr", + "stdout": "npm_package_dev", + }, + Object { + "cmd": "node ../../env.js", + "code": 0, + "event": "postinstall", + "pkg": Object { + "_id": "devopt@1.0.0", + "name": "devopt", + "scripts": Object { + "postinstall": "node ../../env.js", + }, + "version": "1.0.0", + }, + "signal": null, + "stderr": "stderr", + "stdout": "npm_package_dev_optional", + }, + Object { + "cmd": "node ../../env.js", + "code": 0, + "event": "postinstall", + "pkg": Object { + "_id": "opt-and-dev@1.0.0", + "name": "opt-and-dev", + "scripts": Object { + "postinstall": "node ../../env.js", + }, + "version": "1.0.0", + }, + "signal": null, + "stderr": "stderr", + "stdout": String( + npm_package_dev + npm_package_optional + ), + }, + Object { + "cmd": "node ../../env.js", + "code": 0, + "event": "postinstall", + "pkg": Object { + "_id": "optdep@1.0.0", + "dependencies": Object { + "devopt": "", + }, + "name": "optdep", + "scripts": Object { + "postinstall": "node ../../env.js", + }, + "version": "1.0.0", + }, + "signal": null, + "stderr": "stderr", + "stdout": "npm_package_optional", + }, +] +` diff --git a/workspaces/arborist/lib/arborist/rebuild.js b/workspaces/arborist/lib/arborist/rebuild.js index 53c6f63e77d94..abbbbf8976bec 100644 --- a/workspaces/arborist/lib/arborist/rebuild.js +++ b/workspaces/arborist/lib/arborist/rebuild.js @@ -3,6 +3,9 @@ const PackageJson = require('@npmcli/package-json') const binLinks = require('bin-links') +const isWindows = require('bin-links/lib/is-windows.js') +/* istanbul ignore next */ +const linkBin = isWindows ? require('bin-links/lib/shim-bin.js') : require('bin-links/lib/link-bin.js') const localeCompare = require('@isaacs/string-locale-compare')('en') const promiseAllRejectLate = require('promise-all-reject-late') const runScript = require('@npmcli/run-script') @@ -11,7 +14,7 @@ const { depth: dfwalk } = require('treeverse') const { isNodeGypPackage, defaultGypInstallScript } = require('@npmcli/node-gyp') const { promiseRetry } = require('@gar/promise-retry') const { log, time } = require('proc-log') -const { resolve, delimiter } = require('node:path') +const { resolve, delimiter, dirname, relative, sep } = require('node:path') const { isScriptAllowed } = require('../script-allowed.js') const boolEnv = b => b ? '1' : '' @@ -73,9 +76,68 @@ module.exports = cls => class Builder extends cls { await this.#build(linkNodes, { type: 'links' }) } + if (this.options.binLinks) { + await this.#reconcileWorkspaceBins(linkNodes) + } + timeEnd() } + async #reconcileWorkspaceBins (rebuiltNodes) { + const queue = [] + + binLinks.resetSeen() + + for (const node of rebuiltNodes) { + if (!node.isWorkspace || !node.isLink) { + continue + } + + const wsTarget = node.target + + for (const edge of wsTarget.edgesOut.values()) { + if (edge.type === 'workspace' || !edge.to) { + continue + } + + const depNode = edge.to + + // Skip deps physically nested inside the workspace — + // #linkAllBins already created correct shims for these. + if (depNode.path.startsWith(wsTarget.path + sep)) { + continue + } + + const pkg = depNode.package + /* istanbul ignore next - PackageJson.normalize converts strings to objects, this is defensive */ + const bins = typeof pkg.bin === 'string' + ? { [pkg.name]: pkg.bin } + : pkg.bin + + if (!bins || typeof bins !== 'object' || Array.isArray(bins)) { + continue + } + + for (const [binName, binScript] of Object.entries(bins)) { + queue.push(async () => { + const to = resolve(wsTarget.path, 'node_modules', '.bin', binName) + const absFrom = resolve(depNode.path, binScript) + const from = relative(dirname(to), absFrom) + await linkBin({ path: depNode.path, from, to, absFrom, force: true }) + }) + } + } + } + + if (queue.length) { + const timeEnd = time.start('build:reconcileWorkspaceBins') + await promiseCallLimit(queue, { + limit: this.options.foregroundScripts ? 1 : undefined, + }) + timeEnd() + } + } + // if we don't have a set of nodes, then just rebuild // the actual tree on disk. async #loadDefaultNodes () { diff --git a/workspaces/arborist/test/arborist/rebuild.js b/workspaces/arborist/test/arborist/rebuild.js index 6c3062be89a00..31ca006eacdd6 100644 --- a/workspaces/arborist/test/arborist/rebuild.js +++ b/workspaces/arborist/test/arborist/rebuild.js @@ -904,6 +904,96 @@ t.test('only rebuild for workspace', async t => { t.throws(() => fs.readFileSync(bdepTxt, 'utf8'), { code: 'ENOENT' }, 'bdep not rebuilt') }) +t.test('workspace bin hoisting collision', async t => { + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'my-workspaces-powered-project', + workspaces: ['packages/*'], + }), + node_modules: { + 'dep-1': { + 'package.json': JSON.stringify({ + name: 'dep-1', + version: '1.0.0', + bin: './bin.js', + }), + 'bin.js': '#!/usr/bin/env node\\nconsole.log("dep-1")', + }, + 'dep-2': { + 'package.json': JSON.stringify({ + name: 'dep-2', + version: '1.0.0', + bin: { 'my-bin': './bin.js' }, + }), + 'bin.js': '#!/usr/bin/env node\\nconsole.log("dep-2")', + }, + 'dep-3': { + 'package.json': JSON.stringify({ + name: 'dep-3', + version: '1.0.0', + bin: { 'multi-bin-1': './bin1.js', 'multi-bin-2': './bin2.js' }, + }), + 'bin1.js': '#!/usr/bin/env node\\n', + 'bin2.js': '#!/usr/bin/env node\\n', + }, + 'dep-5': { + 'package.json': JSON.stringify({ + name: 'dep-5', + version: '1.0.0', + bin: ['invalid'], + }), + }, + workspace_a: t.fixture('symlink', '../packages/workspace_a'), + workspace_b: t.fixture('symlink', '../packages/workspace_b'), + workspace_c: t.fixture('symlink', '../packages/workspace_c'), + }, + packages: { + workspace_a: { + 'package.json': JSON.stringify({ + name: 'workspace_a', + version: '1.0.0', + dependencies: { 'dep-1': '1.0.0', 'dep-5': '1.0.0' }, + }), + }, + workspace_b: { + 'package.json': JSON.stringify({ + name: 'workspace_b', + version: '1.0.0', + dependencies: { 'dep-2': '1.0.0', 'dep-3': '1.0.0' }, + }), + }, + workspace_c: { + 'package.json': JSON.stringify({ + name: 'workspace_c', + version: '1.0.0', + dependencies: { 'dep-4': '1.0.0' }, + }), + node_modules: { + 'dep-4': { + 'package.json': JSON.stringify({ + name: 'dep-4', + version: '1.0.0', + bin: './bin.js', + }), + 'bin.js': '#!/usr/bin/env node\\n', + }, + }, + }, + }, + }) + + const arb = newArb({ path, foregroundScripts: true }) + await arb.rebuild() + + const checkFile = p => fs.statSync(resolve(path, p)).isFile() + + t.ok(checkFile('packages/workspace_a/node_modules/.bin/dep-1'), 'workspace_a local bin created') + t.ok(checkFile('packages/workspace_b/node_modules/.bin/my-bin'), 'workspace_b local bin created') + t.ok(checkFile('packages/workspace_b/node_modules/.bin/multi-bin-1'), 'workspace_b multi-bin 1 created') + t.ok(checkFile('packages/workspace_b/node_modules/.bin/multi-bin-2'), 'workspace_b multi-bin 2 created') + t.ok(checkFile('packages/workspace_c/node_modules/.bin/dep-4'), 'workspace_c nested bin created via linkAllBins') +}) + t.test('no workspaces', async t => { const path = t.testdir({ 'package.json': JSON.stringify({