diff --git a/lib/commands/version.js b/lib/commands/version.js index 67b17cfbf3967..334655a9755dc 100644 --- a/lib/commands/version.js +++ b/lib/commands/version.js @@ -90,6 +90,28 @@ class Version extends BaseCommand { flatOptions, localPrefix, } = this.npm + + // if the workspace was only selected implicitly (the command was run from + // inside a workspace directory without --workspace/--workspaces flags), + // bump just that workspace without reading every other workspace's + // package.json or updating the root project, so that parallel invocations + // across workspaces cannot race each other + // https://github.com/npm/cli/issues/9412 + const explicitWorkspaces = config.get('workspaces') || + (config.get('workspace', 'cli') || []).length + if (!explicitWorkspaces) { + const pkgJson = require('@npmcli/package-json') + const [path] = config.get('workspace', 'default') + const { content: { name } } = await pkgJson.normalize(path) + output.standard(name) + const version = await libnpmversion(args[0], { + ...flatOptions, + 'git-tag-version': false, + path, + }) + return output.standard(`${prefix}${version}`) + } + await this.setWorkspaces() const updatedWorkspaces = [] for (const [name, path] of this.workspaces) { diff --git a/test/lib/commands/version.js b/test/lib/commands/version.js index 35987ed83cb7a..399450313ddf0 100644 --- a/test/lib/commands/version.js +++ b/test/lib/commands/version.js @@ -361,5 +361,94 @@ t.test('empty versions', async t => { 'should not have a lockfile since have not reified' ) }) + + t.test('implicit workspace from cwd does not read sibling workspaces', async t => { + const libnpmversionCalls = [] + const { version, outputs, prefix } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'workspaces-test', + version: '1.0.0', + workspaces: ['workspace-a', 'workspace-b'], + }), + 'workspace-a': { + 'package.json': JSON.stringify({ + name: 'workspace-a', + version: '1.0.0', + }), + }, + 'workspace-b': { + 'package.json': JSON.stringify({ + name: 'workspace-b', + version: '1.0.0', + }), + }, + }, + chdir: (dirs) => resolve(dirs.prefix, 'workspace-a'), + mocks: { + libnpmversion: (arg, opts) => { + libnpmversionCalls.push(opts) + return '1.0.1' + }, + }, + }) + + await version.exec(['patch']) + + t.same( + outputs, + ['workspace-a', 'v1.0.1'], + 'outputs only the implicitly selected workspace' + ) + t.equal(libnpmversionCalls.length, 1, 'bumped a single workspace') + t.equal( + libnpmversionCalls[0].path, + resolve(prefix, 'workspace-a'), + 'bumped the workspace the command was run from' + ) + t.equal( + libnpmversionCalls[0]['git-tag-version'], + false, + 'does not git tag from a workspace' + ) + t.throws( + () => statSync(resolve(prefix, 'package-lock.json')), + 'should not have a lockfile since have not reified' + ) + }) + + t.test('implicit workspace works while a sibling package.json is invalid', async t => { + const { version, outputs } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'workspaces-test', + version: '1.0.0', + workspaces: ['workspace-a', 'workspace-b'], + }), + 'workspace-a': { + 'package.json': JSON.stringify({ + name: 'workspace-a', + version: '1.0.0', + }), + }, + 'workspace-b': { + // simulates another process writing this file concurrently + 'package.json': '', + }, + }, + chdir: (dirs) => resolve(dirs.prefix, 'workspace-a'), + mocks: { + libnpmversion: () => '1.0.1', + }, + }) + + await version.exec(['patch']) + + t.same( + outputs, + ['v1.0.1'], + 'falls back to standalone behavior and still bumps the version' + ) + }) }) }) diff --git a/workspaces/config/lib/index.js b/workspaces/config/lib/index.js index 4121c2a7a3840..4d7d65cf94f35 100644 --- a/workspaces/config/lib/index.js +++ b/workspaces/config/lib/index.js @@ -829,7 +829,18 @@ class Config { } const mapWorkspaces = require('@npmcli/map-workspaces') - const workspaces = await mapWorkspaces({ cwd: p, pkg }) + let workspaces + try { + workspaces = await mapWorkspaces({ cwd: p, pkg }) + } catch (err) { + // reading the workspace map can fail transiently when another + // process is writing a sibling workspace's package.json at the + // same moment (https://github.com/npm/cli/issues/9412), so treat + // the current prefix as a standalone project instead of failing + // before any command gets a chance to run + log.warn('config', `failed to read workspaces at ${p}: ${err.message}`) + continue + } for (const w of workspaces.values()) { if (w === this.localPrefix) { // see if there's a .npmrc file in the workspace, if so log a warning diff --git a/workspaces/config/test/index.js b/workspaces/config/test/index.js index 60941c7760985..2bb86a8b5d49c 100644 --- a/workspaces/config/test/index.js +++ b/workspaces/config/test/index.js @@ -1439,6 +1439,50 @@ t.test('workspaces', async (t) => { t.teardown(() => process.off('log', logHandler)) t.afterEach(() => logs.length = 0) + t.test('corrupt sibling workspace package.json does not break detection', async (t) => { + const brokenPath = resolve(t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['./workspaces/*'], + }), + workspaces: { + one: { + 'package.json': JSON.stringify({ + name: 'one', + version: '1.0.0', + }), + }, + broken: { + // simulates another process writing this file concurrently + 'package.json': '', + }, + }, + })) + const cwd = process.cwd() + t.teardown(() => process.chdir(cwd)) + process.chdir(`${brokenPath}/workspaces/one`) + + const config = new Config({ + npmPath: cwd, + env: {}, + argv: [process.execPath, __filename], + cwd: join(`${brokenPath}/workspaces/one`), + shorthands, + definitions, + nerfDarts, + }) + + await config.load() + t.equal(config.localPrefix, join(brokenPath, 'workspaces', 'one'), + 'localPrefix is the workspace itself') + t.same(config.get('workspace'), [], 'did not set a workspace') + const warnings = logs.filter(l => l[0] === 'warn') + t.match(warnings[0], + ['warn', 'config', /^failed to read workspaces/], + 'warned about the unreadable workspace map') + }) + t.test('finds own parent', async (t) => { const cwd = process.cwd() t.teardown(() => process.chdir(cwd))