From 8f086a1cd3c4d7848cbd2f05c528b31c0c64a62e Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 31 Jul 2021 13:17:21 -0700 Subject: [PATCH] feat: workspace-root config, implicit workspace Related-to: https://github.com/npm/cli/pull/3596 --- lib/find-project-dir.js | 28 +++++ lib/index.js | 172 ++++++++++++++++++++++++------- test/find-project-dir.js | 28 +++++ test/index.js | 214 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 402 insertions(+), 40 deletions(-) create mode 100644 lib/find-project-dir.js create mode 100644 test/find-project-dir.js diff --git a/lib/find-project-dir.js b/lib/find-project-dir.js new file mode 100644 index 0000000..7a13812 --- /dev/null +++ b/lib/find-project-dir.js @@ -0,0 +1,28 @@ +const walkUp = require('walk-up-path') +const { relative, resolve, dirname, join } = require('path') +const { promisify } = require('util') +const fs = require('fs') +const stat = promisify(fs.stat) + +// 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. +module.exports = async (start, end = null) => { + 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')) + .then(st => st.isDirectory()) + .catch(() => false), + stat(resolve(p, 'package.json')) + .then(st => st.isFile()) + .catch(() => false), + ])).some(is => is) + if (hasAny) + return p + if (end && relative(p, end) === '') + break + } + + return start +} diff --git a/lib/index.js b/lib/index.js index f947896..25c3d11 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,5 +1,4 @@ // TODO: set the scope config from package.json or explicit cli config -const walkUp = require('walk-up-path') const ini = require('ini') const nopt = require('nopt') const mkdirp = require('mkdirp-infer-owner') @@ -9,7 +8,8 @@ const myUid = process.getuid && process.getuid() /* istanbul ignore next */ const myGid = process.getgid && process.getgid() -const { resolve, dirname, join } = require('path') +const findProjectDir = require('./find-project-dir.js') +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,13 @@ class Config { set (key, val, where = 'cli') { if (!this.loaded) throw new Error('call config.load() before setting values') + + if (key === 'workspace-root' && !['project', 'workspace'].includes(where)) + throw new Error(`cannot set workspace-root in ${where} config`) + + 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) @@ -251,14 +262,17 @@ 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') // 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() process.emit('timeEnd', 'config:load:project') + // then user config, which can affect globalconfig location process.emit('time', 'config:load:user') await this.loadUserConfig() @@ -451,6 +465,16 @@ class Config { throw new Error(m) } + if (where === 'workspace' && obj) { + const keys = Object.keys(obj).filter(k => k !== 'workspace-root') + if (obj['workspace-root'] && keys.length) { + const m = 'workspace-root set, ignoring other workspace-level configs' + this.log.warn('config', m, keys) + for (const k of keys) + delete obj[k] + } + } + conf.source = source this.sources.set(source, where) if (er) { @@ -482,13 +506,27 @@ class Config { return parseField(f, key, this, listElement) } + 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']) { + 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) - // 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 this[_readFile](file) + this[_loadObject](data, type, file, er) process.emit('timeEnd', 'config:load:file:' + file) } @@ -497,51 +535,85 @@ 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. + + // if we are currently in ~, then don't walk up past that backstop + const cliPrefix = this[_get]('prefix', 'cli') + const projectDir = cliPrefix || await 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 { - this.data.get('project').source = '(same as "user" config, ignored)' + if (this[_get]('userconfig') && relative(projectFile, this[_get]('userconfig')) === '') { + this.localPrefix = projectDir + this.data.get('project').source = '(project same as "user" config, ignored)' this.sources.set(this.data.get('project').source, 'project') - } - } - - async loadLocalPrefix () { - const cliPrefix = this[_get]('prefix', 'cli') - if (cliPrefix) { - this.localPrefix = cliPrefix + this.data.get('workspace').source = '(workspace same as "user" config, ignored)' + this.sources.set(this.data.get('workspace').source, 'workspace') return } - for (const p of walkUp(this.cwd)) { - // walk up until we have a nm dir or a pj file - const hasAny = (await Promise.all([ - stat(resolve(p, 'node_modules')) - .then(st => st.isDirectory()) - .catch(() => false), - stat(resolve(p, 'package.json')) - .then(st => st.isFile()) - .catch(() => false), - ])).some(is => is) - if (hasAny) { - this.localPrefix = p - return + // 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 } - this.localPrefix = this.cwd + // 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', [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 + } } loadUserConfig () { - return this[_loadFile](this[_get]('userconfig'), 'user') + const userconfig = this[_get]('userconfig') || resolve(this.home, '.npmrc') + return this[_loadFile](userconfig, 'user') } loadGlobalConfig () { @@ -567,7 +639,30 @@ class Config { try { this.setCredentialsByURI(reg, creds) } catch (_) {} } - const iniData = ini.stringify(conf.data).trim() + '\n' + const data = { ...conf.data } + const { source } = conf + + if (data['workspace-root']) { + const dir = dirname(source) + data['workspace-root'] = relative(dir, data['workspace-root']) + } + + // do not save an empty workspace-root field in the project config + if (data['workspace-root'] === '') { + this.log.warn('config', 'Ignoring empty \'workspace-root\' config') + delete data['workspace-root'] + } + + if (data['workspace-root']) { + const keys = Object.keys(data).filter(k => k !== 'workspace-root') + if (keys.length) { + const er = new Error('Cannot set other configs with workspace-root') + this.log.verbose('config', 'trying to set other keys with workspace-root', keys) + throw Object.assign(er, { found: keys }) + } + } + + 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 => {}) @@ -583,6 +678,7 @@ class Config { if (st && (st.uid !== myUid || st.gid !== myGid)) await chown(conf.source, st.uid, st.gid).catch(() => {}) } + const mode = where === 'user' ? 0o600 : 0o666 await chmod(conf.source, mode) } diff --git a/test/find-project-dir.js b/test/find-project-dir.js new file mode 100644 index 0000000..c9cb2d9 --- /dev/null +++ b/test/find-project-dir.js @@ -0,0 +1,28 @@ +const t = require('tap') +const findProjectDir = require('../lib/find-project-dir.js') +const { resolve } = require('path') + +t.test('walk up, but do not pass end', async t => { + const path = t.testdir({ + hasnm: { + node_modules: {}, + a: { b: { c: {}}}, + }, + haspj: { + 'package.json': JSON.stringify({}), + a: { b: { c: {}}}, + }, + }) + t.equal( + await findProjectDir(resolve(`${path}/hasnm/a/b/c`)), + resolve(path, 'hasnm') + ) + t.equal( + await findProjectDir(resolve(`${path}/haspj/a/b/c`)), + resolve(path, 'haspj') + ) + t.equal( + await findProjectDir(resolve(`${path}/haspj/a/b/c`), resolve(path, 'haspj/a')), + resolve(path, 'haspj/a/b/c') + ) +}) diff --git a/test/index.js b/test/index.js index 87d22e2..d5e1ec8 100644 --- a/test/index.js +++ b/test/index.js @@ -4,7 +4,7 @@ const t = require('tap') const fs = require('fs') const { readFile, readFileSync } = fs fs.readFile = (path, ...args) => { - if (path.match(/WEIRD-ERROR/)) { + if (path && path.match(/WEIRD-ERROR/)) { const cb = args.pop() cb(Object.assign(new Error('weird error'), { code: 'EWEIRD' })) } else @@ -362,7 +362,8 @@ loglevel = yolo [resolve(path, 'npm/npmrc'), 'builtin'], ['command line options', 'cli'], ['environment', 'env'], - ['(same as "user" config, ignored)', 'project'], + ['(workspace same as "user" config, ignored)', 'workspace'], + ['(project same as "user" config, ignored)', 'project'], [resolve(path, 'project/.npmrc'), 'user'], ])) @@ -930,3 +931,212 @@ t.test('nerfdart auths set at the top level into the registry', async t => { }) } }) + +t.test('workspace-root settings', async t => { + const registry = 'https://registry.npmjs.org/' + const fixtureDef = { + '.npmrc': 'userconfigfile = true', + project: { + 'package.json': JSON.stringify({ workspaces: ['foo', 'bar'] }), + '.npmrc': 'foo = bar', + foo: { + 'package.json': JSON.stringify({ name: 'foo', version: '1.2.3' }), + '.npmrc': 'workspace-root = ..', + x: { y: { z: {}}}, + }, + // no ws config in bar + bar: { + 'package.json': JSON.stringify({ name: 'bar', version: '1.2.3' }), + x: { y: { z: {}}}, + }, + }, + } + + const opts = path => ({ + shorthands: {}, + argv: [], + env: { + HOME: path, + }, + definitions: { + registry: { default: registry }, + 'workspace-root': { + default: undefined, + type: require('path'), + }, + workspace: { + default: [], + type: [Array, String], + }, + }, + npmPath: process.cwd(), + }) + + t.test('works in the root of the workspace', async t => { + const path = t.testdir(fixtureDef) + const c = new Config({ + ...opts(path), + cwd: resolve(path, 'project/foo'), + }) + await c.load() + t.strictSame(c.get('workspace'), ['foo']) + t.strictSame(c.get('workspace-root'), resolve(path, 'project')) + t.equal(c.get('userconfigfile'), true) + t.equal(c.localPrefix, resolve(path, 'project')) + t.equal(c.cwd, resolve(path, 'project/foo')) + }) + + t.test('works in workspace subpath', async t => { + const path = t.testdir(fixtureDef) + const c = new Config({ + ...opts(path), + cwd: resolve(path, 'project/foo/x/y/z'), + }) + await c.load() + t.strictSame(c.get('workspace'), ['foo']) + t.strictSame(c.get('workspace-root'), resolve(path, 'project')) + t.equal(c.get('userconfigfile'), true) + t.equal(c.localPrefix, resolve(path, 'project')) + t.equal(c.cwd, resolve(path, 'project/foo/x/y/z')) + }) + + t.test('does not work without ws config file', async t => { + const path = t.testdir(fixtureDef) + const c = new Config({ + ...opts(path), + cwd: resolve(path, 'project/bar'), + }) + await c.load() + t.strictSame(c.get('workspace'), []) + t.strictSame(c.get('workspace-root'), undefined) + t.equal(c.get('userconfigfile'), true) + t.equal(c.localPrefix, resolve(path, 'project/bar')) + t.equal(c.cwd, resolve(path, 'project/bar')) + }) + + t.test('set wsconfig explicitly', async t => { + const path = t.testdir(fixtureDef) + const c = new Config({ + ...opts(path), + cwd: resolve(path, 'project/bar'), + }) + await c.load() + t.strictSame(c.get('workspace'), []) + t.strictSame(c.get('workspace-root'), undefined) + t.equal(c.get('userconfigfile'), true) + t.equal(c.localPrefix, resolve(path, 'project/bar')) + t.equal(c.cwd, resolve(path, 'project/bar')) + c.set('workspace-root', resolve(path, 'project'), 'project') + await c.save('project') + const file = resolve(path, 'project/bar/.npmrc') + t.equal(fs.readFileSync(file, 'utf8'), `workspace-root=..\n`) + }) + + t.test('refuse to save empty wsconfig', async t => { + const path = t.testdir(fixtureDef) + const c = new Config({ + ...opts(path), + cwd: resolve(path, 'project/bar'), + }) + await c.load() + t.strictSame(c.get('workspace'), []) + t.strictSame(c.get('workspace-root'), undefined) + t.equal(c.get('userconfigfile'), true) + t.equal(c.localPrefix, resolve(path, 'project/bar')) + t.equal(c.cwd, resolve(path, 'project/bar')) + c.set('workspace-root', resolve(path, 'project/bar'), 'project') + await c.save('project') + t.throws(() => fs.statSync(resolve(path, 'project/bar/.npmrc')), { + code: 'ENOENT', + }) + }) + + t.test('saving wsroot config removes other project stuff', async t => { + const path = t.testdir(fixtureDef) + const c = new Config({ + ...opts(path), + cwd: resolve(path, 'project/bar'), + }) + await c.load() + t.strictSame(c.get('workspace'), []) + t.strictSame(c.get('workspace-root'), undefined) + t.equal(c.get('userconfigfile'), true) + t.equal(c.localPrefix, resolve(path, 'project/bar')) + t.equal(c.cwd, resolve(path, 'project/bar')) + c.set('workspace-root', resolve(path, 'project'), 'project') + c.set('other', 'thing', 'project') + t.throws(() => c.set('workspace-root', resolve(path, 'project'), 'global')) + t.throws(() => c.set('workspace-root', resolve(path, 'project'), 'user')) + await t.rejects(c.save('project'), { + message: 'Cannot set other configs with workspace-root', + }) + c.delete('other', 'project') + await c.save('project') + t.equal(fs.readFileSync(resolve(path, 'project/bar/.npmrc'), 'utf8'), + `workspace-root=..\n`) + }) + + t.test('set wsroot with explicit cli config', async t => { + const path = t.testdir(fixtureDef) + const c = new Config({ + ...opts(path), + argv: [process.execPath, __filename, `--workspace-root=${path}/project`], + cwd: resolve(path, 'project/bar'), + }) + await c.load() + t.strictSame(c.get('workspace'), ['bar']) + t.strictSame(c.get('workspace-root'), resolve(path, 'project')) + t.equal(c.get('userconfigfile'), true) + t.equal(c.localPrefix, resolve(path, 'project')) + t.equal(c.cwd, resolve(path, 'project/bar')) + }) + + t.test('set wsroot to current project with explicit cli config', async t => { + const path = t.testdir(fixtureDef) + const c = new Config({ + ...opts(path), + argv: [process.execPath, __filename, `--workspace-root=${path}/project/bar`], + cwd: resolve(path, 'project/bar'), + }) + await c.load() + t.strictSame(c.get('workspace'), []) + t.strictSame(c.get('workspace-root'), resolve(path, 'project/bar')) + t.equal(c.get('userconfigfile'), true) + t.equal(c.localPrefix, resolve(path, 'project/bar')) + t.equal(c.cwd, resolve(path, 'project/bar')) + }) + + t.test('warn if wsroot set along with other configs', async t => { + const path = t.testdir({ + ...fixtureDef, + project: { + ...fixtureDef.project, + foo: { + ...fixtureDef.project.foo, + '.npmrc': 'workspace-root = ..\nfoo-ws-conf = true\n', + }, + }, + }) + const logs = [] + const log = { + warn: (...msg) => logs.push(['warn', ...msg]), + verbose: (...msg) => logs.push(['verbose', ...msg]), + } + const c = new Config({ + ...opts(path), + log, + cwd: resolve(path, 'project/foo'), + }) + await c.load() + t.strictSame(logs, [ + [ + 'warn', + 'config', + 'workspace-root set, ignoring other workspace-level configs', + [ + 'foo-ws-conf', + ], + ], + ]) + }) +})