From 2b5e447d89f8bf2a0c2e409e7e1fa802e5e32bb9 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 27 Jul 2021 18:24:39 -0700 Subject: [PATCH 1/2] X --- lib/utils/config/definitions.js | 45 +++++++++++++++++++ .../@npmcli/arborist/lib/arborist/reify.js | 13 ++++++ node_modules/@npmcli/config/lib/index.js | 28 +++++++++--- 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js index 36b8a84a61c47..b1a6cff284ae1 100644 --- a/lib/utils/config/definitions.js +++ b/lib/utils/config/definitions.js @@ -1668,6 +1668,23 @@ define('save-prod', { }, }) +define('save-workspace-root', { + default: true, + type: Boolean, + envExport: false, + description: ` + When installing a workspace project, npm will place a \`.npmrc\` + file in each workspace folder by default specifying the workspace + root, so that a command from within that folder will behave the + same as if it was run from the root project with a \`--workspace\` + option. + + Set \`save-workspace-root\` to false to disable this behavior. + `, + flatten, +}) + + define('scope', { default: '', defaultDescription: ` @@ -2139,6 +2156,34 @@ define('workspaces', { `, }) +define('workspace-root', { + default: null, + type: path, + hint: '', + envExport: false, + description: ` + When operating within a workspace folder as the current working + directory, this config value may be set to tell npm to use the + specified folder as the project root. + + If set in a config file at \`./.npmrc\` (ie, a project-level config + file), no other options may be set in that file, as the workspace + root's project config will supercede it. + + When installing a workspace project, npm will place a \`.npmrc\` + file in each workspace folder by default specifying the workspace + root, so that a command from within that folder will behave the + same as if it was run from the root project with a \`--workspace\` + option. + + Note: when set as a relative path within a config file, this option + is resolved relative to the directory holding the config file. This + differs from most path-type configs, which are always resolved relative + to the current working directory. + `, + flatten, +}) + define('yes', { default: null, type: [null, Boolean], diff --git a/node_modules/@npmcli/arborist/lib/arborist/reify.js b/node_modules/@npmcli/arborist/lib/arborist/reify.js index 1cfa6034eadb8..93582d202696a 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/reify.js +++ b/node_modules/@npmcli/arborist/lib/arborist/reify.js @@ -12,6 +12,7 @@ const {depth: dfwalk} = require('treeverse') const fs = require('fs') const {promisify} = require('util') const symlink = promisify(fs.symlink) +const writeFile = promisify(fs.writeFile) const mkdirp = require('mkdirp-infer-owner') const justMkdirp = require('mkdirp') const moveFile = require('@npmcli/move-file') @@ -84,6 +85,8 @@ const _omitOptional = Symbol('omitOptional') const _omitPeer = Symbol('omitPeer') const _global = Symbol.for('global') +const _saveWorkspaceRoot = Symbol('saveWorkspaceRoot') +const _writeWorkspaceRootConfig = Symbol('writeWorkspaceRootConfig') const _pruneBundledMetadeps = Symbol('pruneBundledMetadeps') @@ -101,12 +104,14 @@ module.exports = cls => class Reifier extends cls { packageLockOnly = false, dryRun = false, formatPackageLock = true, + saveWorkspaceRoot = true, } = options this[_dryRun] = !!dryRun this[_packageLockOnly] = !!packageLockOnly this[_savePrefix] = savePrefix this[_formatPackageLock] = !!formatPackageLock + this[_saveWorkspaceRoot] = !!saveWorkspaceRoot this.diff = null this[_retiredPaths] = {} @@ -525,6 +530,8 @@ module.exports = cls => class Reifier extends cls { } await this[_checkBins](node) await this[_extractOrLink](node) + if (node.isWorkspace && this[_saveWorkspaceRoot]) + await this[_writeWorkspaceRootConfig](node) await this[_warnDeprecated](node) }) @@ -572,6 +579,12 @@ module.exports = cls => class Reifier extends cls { }) } + async [_writeWorkspaceRootConfig] (node) { + const rootConfig = relative(node.realpath, node.root.realpath) + const file = resolve(node.realpath, '.npmrc') + await writeFile(file, `workspace-root = ${rootConfig}\n`) + } + async [_symlink] (node) { const dir = dirname(node.path) const target = node.realpath diff --git a/node_modules/@npmcli/config/lib/index.js b/node_modules/@npmcli/config/lib/index.js index f947896f0ba34..da5512bbbdf3c 100644 --- a/node_modules/@npmcli/config/lib/index.js +++ b/node_modules/@npmcli/config/lib/index.js @@ -251,6 +251,7 @@ class Config { process.emit('time', 'config:load:cli') this.loadCLI() process.emit('timeEnd', 'config:load:cli') + process.emit('time', 'config:load:env') this.loadEnv() process.emit('timeEnd', 'config:load:env') @@ -259,6 +260,7 @@ class Config { process.emit('time', 'config:load:project') await this.loadProjectConfig() process.emit('timeEnd', 'config:load:project') + // then user config, which can affect globalconfig location process.emit('time', 'config:load:user') await this.loadUserConfig() @@ -484,11 +486,18 @@ class Config { async [_loadFile] (file, type) { process.emit('time', 'config:load:file:' + file) + console.error('whats readFile?', typeof readFile) // only catch the error from readFile, not from the loadObject call - await readFile(file, 'utf8').then( - data => this[_loadObject](ini.parse(data), type, file), - er => this[_loadObject](null, type, file, er) - ) + const [er, data] = await readFile(file, 'utf8') + .then(data => [null, ini.parse(data)], er => [er, null]) + + // workspace-root is relative to the file when set in a file, not cwd + // otherwise we get non-portable options set in saved ws project configs + // we set it back to a relative path when saving. + if (data && data['workspace-root']) + data['workspace-root'] = resolve(dirname(file), data['workspace-root']) + + this[_loadObject](data, type, file, er) process.emit('timeEnd', 'config:load:file:' + file) } @@ -521,6 +530,10 @@ class Config { return } + // if we have a workspace-root set at this point, then that's + // our local prefix, and walkup sets the --workspace config. + const wsRoot = this[_get]('workpace-root') + for (const p of walkUp(this.cwd)) { // walk up until we have a nm dir or a pj file const hasAny = (await Promise.all([ @@ -567,7 +580,12 @@ class Config { try { this.setCredentialsByURI(reg, creds) } catch (_) {} } - const iniData = ini.stringify(conf.data).trim() + '\n' + const data = conf.data['workspace-root'] ? { + ...conf.data, + 'workspace-root': relative(dirname(conf.source), conf.data['workspace-root']), + } : conf.data + + const iniData = ini.stringify(data).trim() + '\n' if (!iniData.trim()) { // ignore the unlink error (eg, if file doesn't exist) await unlink(conf.source).catch(er => {}) From 556a5bf3bec9f02a1425b02ed5af5cc2fa39904d Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 29 Jul 2021 16:57:09 -0700 Subject: [PATCH 2/2] x --- docs/content/using-npm/config.md | 39 +++++ node_modules/@npmcli/config/lib/index.js | 139 ++++++++++++++---- .../lib/utils/config/definitions.js.test.cjs | 45 ++++++ .../lib/utils/config/describe-all.js.test.cjs | 39 +++++ .../test/lib/utils/exit-handler.js.test.cjs | 20 +-- 5 files changed, 245 insertions(+), 37 deletions(-) diff --git a/docs/content/using-npm/config.md b/docs/content/using-npm/config.md index c4d1afed35cc8..fd2d2a8123fa0 100644 --- a/docs/content/using-npm/config.md +++ b/docs/content/using-npm/config.md @@ -1080,6 +1080,20 @@ you want to move it to be a non-optional production dependency. This is the default behavior if `--save` is true, and neither `--save-dev` or `--save-optional` are true. +#### `save-workspace-root` + +* Default: true +* Type: Boolean + +When installing a workspace project, npm will place a `.npmrc` file in each +workspace folder by default specifying the workspace root, so that a command +from within that folder will behave the same as if it was run from the root +project with a `--workspace` option. + +Set `save-workspace-root` to false to disable this behavior. + +This value is not exported to the environment for child processes. + #### `scope` * Default: the scope of the current project, if any, or "" @@ -1377,6 +1391,31 @@ brand new workspace within the project. This value is not exported to the environment for child processes. +#### `workspace-root` + +* Default: null +* Type: Path + +When operating within a workspace folder as the current working directory, +this config value may be set to tell npm to use the specified folder as the +project root. + +If set in a config file at `./.npmrc` (ie, a project-level config file), no +other options may be set in that file, as the workspace root's project +config will supercede it. + +When installing a workspace project, npm will place a `.npmrc` file in each +workspace folder by default specifying the workspace root, so that a command +from within that folder will behave the same as if it was run from the root +project with a `--workspace` option. + +Note: when set as a relative path within a config file, this option is +resolved relative to the directory holding the config file. This differs +from most path-type configs, which are always resolved relative to the +current working directory. + +This value is not exported to the environment for child processes. + #### `workspaces` * Default: false diff --git a/node_modules/@npmcli/config/lib/index.js b/node_modules/@npmcli/config/lib/index.js index da5512bbbdf3c..e13e86ea97225 100644 --- a/node_modules/@npmcli/config/lib/index.js +++ b/node_modules/@npmcli/config/lib/index.js @@ -9,7 +9,7 @@ const myUid = process.getuid && process.getuid() /* istanbul ignore next */ const myGid = process.getgid && process.getgid() -const { resolve, dirname, join } = require('path') +const { relative, resolve, dirname, join } = require('path') const { homedir } = require('os') const { promisify } = require('util') const fs = require('fs') @@ -53,6 +53,8 @@ const confFileTypes = new Set([ 'global', 'user', 'project', + // the place where we store 'workspace-root' and the implicit workspace + 'workspace', ]) const confTypes = new Set([ @@ -65,9 +67,11 @@ const confTypes = new Set([ const _loaded = Symbol('loaded') const _get = Symbol('get') +const _set = Symbol('set') const _find = Symbol('find') const _loadObject = Symbol('loadObject') const _loadFile = Symbol('loadFile') +const _readFile = Symbol('readFile') const _checkDeprecated = Symbol('checkDeprecated') const _flatten = Symbol('flatten') const _flatOptions = Symbol('flatOptions') @@ -195,6 +199,9 @@ class Config { set (key, val, where = 'cli') { if (!this.loaded) throw new Error('call config.load() before setting values') + return this[_set](key, val, where) + } + [_set] (key, val, where) { if (!confTypes.has(where)) throw new Error('invalid config location param: ' + where) this[_checkDeprecated](key) @@ -257,8 +264,14 @@ class Config { process.emit('timeEnd', 'config:load:env') // next project config, which can affect userconfig location + // if we have a workspace config, we end up loading it there, too. process.emit('time', 'config:load:project') await this.loadProjectConfig() + // console.error({ + // 'workspace-root': this[_get]('workspace-root'), + // 'workspace': this[_get]('workspace'), + // localPrefix: this.localPrefix, + // }) process.emit('timeEnd', 'config:load:project') // then user config, which can affect globalconfig location @@ -484,19 +497,26 @@ class Config { return parseField(f, key, this, listElement) } - async [_loadFile] (file, type) { - process.emit('time', 'config:load:file:' + file) - console.error('whats readFile?', typeof readFile) - // only catch the error from readFile, not from the loadObject call + async [_readFile] (file) { + process.emit('time', 'config:load:readfile:' + file) + const [er, data] = await readFile(file, 'utf8') .then(data => [null, ini.parse(data)], er => [er, null]) // workspace-root is relative to the file when set in a file, not cwd // otherwise we get non-portable options set in saved ws project configs // we set it back to a relative path when saving. - if (data && data['workspace-root']) + if (data && data['workspace-root']) { data['workspace-root'] = resolve(dirname(file), data['workspace-root']) + } + process.emit('timeEnd', 'config:load:readfile:' + file) + return [er, data] + } + + async [_loadFile] (file, type) { + process.emit('time', 'config:load:file:' + file) + const [er, data] = await this[_readFile](file) this[_loadObject](data, type, file, er) process.emit('timeEnd', 'config:load:file:' + file) } @@ -506,35 +526,86 @@ class Config { } async loadProjectConfig () { - // the localPrefix can be set by the CLI config, but otherwise is - // found by walking up the folder tree - await this.loadLocalPrefix() - const projectFile = resolve(this.localPrefix, '.npmrc') + // if --prefix is in cli, then that is our localPrefix, full stop + // in that case, we do not walk up the folder tree, do not define + // an implicit workspace, etc. We're done. + // + // walk up from cwd to the nearest nm/pj folder. this is the projectDir + // + // if we already have a workspace-root defined, then the workspace-root + // is the only place a "project" config can be. look there for it, + // set the workspace-root as our localPrefix. If the projectDir is the + // same as our localPrefix, then we're done, and there is no implicit + // workspace. Otherwise, the implicit workspace is the projectDir we + // walked up to. Done. + // + // check the projectDir for a .npmrc. If none found, then we have no + // project config, and no implicit workspace. + // + // If the projectDir/.npmrc sets workspace-root, then load it as the + // workspace config. resolve the workspace-root, and load the project + // config from that location. + // + // Any time that we set a workspace-root, we should *also* set the + // localPrefix. + + const projectDir = await this.findProjectDir(this.cwd) + const projectFile = resolve(projectDir, '.npmrc') + // if we're in the ~ directory, and there happens to be a node_modules // folder (which is not TOO uncommon, it turns out), then we can end // up loading the "project" config where the "userconfig" will be, // which causes some calamaties. So, we only load project config if // it doesn't match what the userconfig will be. - if (projectFile !== this[_get]('userconfig')) - return this[_loadFile](projectFile, 'project') - else { + if (projectFile === this[_get]('userconfig')) { + this.localPrefix = projectDir this.data.get('project').source = '(same as "user" config, ignored)' this.sources.set(this.data.get('project').source, 'project') + this.data.get('workspace').source = '(same as "user" config, ignored)' + this.sources.set(this.data.get('workspace').source, 'workspace') + return } - } - async loadLocalPrefix () { - const cliPrefix = this[_get]('prefix', 'cli') - if (cliPrefix) { - this.localPrefix = cliPrefix + // if we already set a workspace root before, then we know that whatever + // that was set to is our localPrefix, and the current project (if it's + // different) is an implicit workspace. + const wsRootBefore = this[_get]('workspace-root') + if (wsRootBefore) { + this.localPrefix = wsRootBefore + await this[_loadFile](`${wsRootBefore}/.npmrc`, 'project') + if (projectDir !== wsRootBefore) { + await this[_loadFile](projectFile, 'workspace') + this[_set]('workspace', [relative(this.localPrefix, projectDir)], 'workspace') + } return } - // if we have a workspace-root set at this point, then that's - // our local prefix, and walkup sets the --workspace config. - const wsRoot = this[_get]('workpace-root') + // ok, we have to check to see if a workspace-root is set in this file + // if it is, then we need to actually load the REAL project configs + // from the effective workspace-root. + const [er, data] = await this[_readFile](projectFile) + const wsRoot = !er && data && data['workspace-root'] + if (wsRoot) { + this.localPrefix = wsRoot + await this[_loadFile](resolve(wsRoot, '.npmrc'), 'project') + this[_loadObject](data, 'workspace', projectFile, er) + this[_set]('workspace-root', wsRoot, 'workspace') + this[_set]('workspace', [relative(this.localPrefix, projectDir)], 'workspace') + return + } else { + this.localPrefix = projectDir + this[_loadObject](data, 'project', projectFile, er) + this.data.get('workspace').source = '(same as "project" config, ignored)' + this.sources.set(this.data.get('workspace').source, 'workspace') + return + } + } - for (const p of walkUp(this.cwd)) { + // starting from the start dir, walk up until we hit the first + // folder with a node_modules or package.json. if none are found, + // then return the start dir itself. + async findProjectDir (start, end = resolve('/')) { + for (const p of walkUp(start)) { // walk up until we have a nm dir or a pj file const hasAny = (await Promise.all([ stat(resolve(p, 'node_modules')) @@ -544,13 +615,22 @@ class Config { .then(st => st.isFile()) .catch(() => false), ])).some(is => is) - if (hasAny) { - this.localPrefix = p - return - } + if (hasAny) + return p + if (p === end) + break } - this.localPrefix = this.cwd + return start + } + + async loadLocalPrefix () { + const cliPrefix = this[_get]('prefix', 'cli') + if (cliPrefix) { + this.localPrefix = cliPrefix + return + } + this.localPrefix = await this.findProjectDir(this.cwd) } loadUserConfig () { @@ -585,6 +665,11 @@ class Config { 'workspace-root': relative(dirname(conf.source), conf.data['workspace-root']), } : conf.data + // do not save an empty workspace-root field in the project config + if (data['workspace-root'] === '') { + delete data['workspace-root'] + } + const iniData = ini.stringify(data).trim() + '\n' if (!iniData.trim()) { // ignore the unlink error (eg, if file doesn't exist) diff --git a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs index 01b137b8af54a..e7fb304d9f5de 100644 --- a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs +++ b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs @@ -119,6 +119,7 @@ Array [ "save-peer", "save-prefix", "save-prod", + "save-workspace-root", "scope", "script-shell", "searchexclude", @@ -149,6 +150,7 @@ Array [ "which", "workspace", "workspaces", + "workspace-root", "yes", ] ` @@ -1449,6 +1451,22 @@ This is the default behavior if \`--save\` is true, and neither \`--save-dev\` or \`--save-optional\` are true. ` +exports[`test/lib/utils/config/definitions.js TAP > config description for save-workspace-root 1`] = ` +#### \`save-workspace-root\` + +* Default: true +* Type: Boolean + +When installing a workspace project, npm will place a \`.npmrc\` file in each +workspace folder by default specifying the workspace root, so that a command +from within that folder will behave the same as if it was run from the root +project with a \`--workspace\` option. + +Set \`save-workspace-root\` to false to disable this behavior. + +This value is not exported to the environment for child processes. +` + exports[`test/lib/utils/config/definitions.js TAP > config description for scope 1`] = ` #### \`scope\` @@ -1843,6 +1861,33 @@ brand new workspace within the project. This value is not exported to the environment for child processes. ` +exports[`test/lib/utils/config/definitions.js TAP > config description for workspace-root 1`] = ` +#### \`workspace-root\` + +* Default: null +* Type: Path + +When operating within a workspace folder as the current working directory, +this config value may be set to tell npm to use the specified folder as the +project root. + +If set in a config file at \`./.npmrc\` (ie, a project-level config file), no +other options may be set in that file, as the workspace root's project +config will supercede it. + +When installing a workspace project, npm will place a \`.npmrc\` file in each +workspace folder by default specifying the workspace root, so that a command +from within that folder will behave the same as if it was run from the root +project with a \`--workspace\` option. + +Note: when set as a relative path within a config file, this option is +resolved relative to the directory holding the config file. This differs +from most path-type configs, which are always resolved relative to the +current working directory. + +This value is not exported to the environment for child processes. +` + exports[`test/lib/utils/config/definitions.js TAP > config description for workspaces 1`] = ` #### \`workspaces\` diff --git a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs index 8487b45174cc3..ac1150e05ce51 100644 --- a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs +++ b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs @@ -958,6 +958,20 @@ you want to move it to be a non-optional production dependency. This is the default behavior if \`--save\` is true, and neither \`--save-dev\` or \`--save-optional\` are true. +#### \`save-workspace-root\` + +* Default: true +* Type: Boolean + +When installing a workspace project, npm will place a \`.npmrc\` file in each +workspace folder by default specifying the workspace root, so that a command +from within that folder will behave the same as if it was run from the root +project with a \`--workspace\` option. + +Set \`save-workspace-root\` to false to disable this behavior. + +This value is not exported to the environment for child processes. + #### \`scope\` * Default: the scope of the current project, if any, or "" @@ -1255,6 +1269,31 @@ brand new workspace within the project. This value is not exported to the environment for child processes. +#### \`workspace-root\` + +* Default: null +* Type: Path + +When operating within a workspace folder as the current working directory, +this config value may be set to tell npm to use the specified folder as the +project root. + +If set in a config file at \`./.npmrc\` (ie, a project-level config file), no +other options may be set in that file, as the workspace root's project +config will supercede it. + +When installing a workspace project, npm will place a \`.npmrc\` file in each +workspace folder by default specifying the workspace root, so that a command +from within that folder will behave the same as if it was run from the root +project with a \`--workspace\` option. + +Note: when set as a relative path within a config file, this option is +resolved relative to the directory holding the config file. This differs +from most path-type configs, which are always resolved relative to the +current working directory. + +This value is not exported to the environment for child processes. + #### \`workspaces\` * Default: false diff --git a/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs b/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs index eb383c104a674..a547b63b2fbc2 100644 --- a/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs +++ b/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs @@ -6,15 +6,15 @@ */ 'use strict' exports[`test/lib/utils/exit-handler.js TAP handles unknown error > should have expected log contents for unknown error 1`] = ` -24 verbose stack Error: ERROR -25 verbose cwd {CWD} -26 verbose Foo 1.0.0 -27 verbose argv "/node" "{CWD}/test/lib/utils/exit-handler.js" -28 verbose node v1.0.0 -29 verbose npm v1.0.0 -30 error code ERROR -31 error ERR ERROR -32 error ERR ERROR -33 verbose exit 1 +26 verbose stack Error: ERROR +27 verbose cwd {CWD} +28 verbose Foo 1.0.0 +29 verbose argv "/node" "{CWD}/test/lib/utils/exit-handler.js" +30 verbose node v1.0.0 +31 verbose npm v1.0.0 +32 error code ERROR +33 error ERR ERROR +34 error ERR ERROR +35 verbose exit 1 `