diff --git a/lib/audit.js b/lib/audit.js index e77beab1eff61..cb8ab5b3a43f5 100644 --- a/lib/audit.js +++ b/lib/audit.js @@ -2,7 +2,7 @@ const Arborist = require('@npmcli/arborist') const auditReport = require('npm-audit-report') const npm = require('./npm.js') const output = require('./utils/output.js') -const reifyOutput = require('./utils/reify-output.js') +const reifyFinish = require('./utils/reify-finish.js') const auditError = require('./utils/audit-error.js') const audit = async args => { @@ -14,7 +14,7 @@ const audit = async args => { const fix = args[0] === 'fix' await arb.audit({ fix }) if (fix) - reifyOutput(arb) + await reifyFinish(arb) else { // will throw if there's an error, because this is an audit command auditError(arb.auditReport) diff --git a/lib/ci.js b/lib/ci.js index a1632e7e98064..1255fbc2646fd 100644 --- a/lib/ci.js +++ b/lib/ci.js @@ -1,7 +1,7 @@ const util = require('util') const Arborist = require('@npmcli/arborist') const rimraf = util.promisify(require('rimraf')) -const reifyOutput = require('./utils/reify-output.js') +const reifyFinish = require('./utils/reify-finish.js') const log = require('npmlog') const npm = require('./npm.js') @@ -35,7 +35,7 @@ const ci = async () => { ]) // npm ci should never modify the lockfile or package.json await arb.reify({ ...npm.flatOptions, save: false }) - reifyOutput(arb) + await reifyFinish(arb) } module.exports = Object.assign(cmd, { completion, usage }) diff --git a/lib/dedupe.js b/lib/dedupe.js index a08c9f3f8f334..fe8243e21e43d 100644 --- a/lib/dedupe.js +++ b/lib/dedupe.js @@ -2,7 +2,7 @@ const npm = require('./npm.js') const Arborist = require('@npmcli/arborist') const usageUtil = require('./utils/usage.js') -const reifyOutput = require('./utils/reify-output.js') +const reifyFinish = require('./utils/reify-finish.js') const usage = usageUtil('dedupe', 'npm dedupe') const completion = require('./utils/completion/none.js') @@ -18,7 +18,7 @@ const dedupe = async (args) => { dryRun, }) await arb.dedupe(npm.flatOptions) - reifyOutput(arb) + await reifyFinish(arb) } module.exports = Object.assign(cmd, { usage, completion }) diff --git a/lib/install.js b/lib/install.js index 5f04fcd4f9d6b..f621c85c23e1e 100644 --- a/lib/install.js +++ b/lib/install.js @@ -6,13 +6,15 @@ const util = require('util') const readdir = util.promisify(fs.readdir) const npm = require('./npm.js') const usageUtil = require('./utils/usage.js') -const reifyOutput = require('./utils/reify-output.js') +const reifyFinish = require('./utils/reify-finish.js') const log = require('npmlog') const { resolve, join } = require('path') const Arborist = require('@npmcli/arborist') const runScript = require('@npmcli/run-script') -const install = async (args, cb) => { +const cmd = async (args, cb) => install(args).then(() => cb()).catch(cb) + +const install = async args => { // the /path/to/node_modules/.. const globalTop = resolve(npm.globalDir, '..') const { ignoreScripts, global: isGlobalInstall } = npm.flatOptions @@ -34,38 +36,33 @@ const install = async (args, cb) => { path: where, }) - try { - await arb.reify({ - ...npm.flatOptions, - add: args, - }) - if (!args.length && !isGlobalInstall && !ignoreScripts) { - const { scriptShell } = npm.flatOptions - const scripts = [ - 'preinstall', - 'install', - 'postinstall', - 'prepublish', // XXX should we remove this finally?? - 'preprepare', - 'prepare', - 'postprepare', - ] - for (const event of scripts) { - await runScript({ - path: where, - args: [], - scriptShell, - stdio: 'inherit', - stdioString: true, - event, - }) - } + await arb.reify({ + ...npm.flatOptions, + add: args, + }) + if (!args.length && !isGlobalInstall && !ignoreScripts) { + const { scriptShell } = npm.flatOptions + const scripts = [ + 'preinstall', + 'install', + 'postinstall', + 'prepublish', // XXX should we remove this finally?? + 'preprepare', + 'prepare', + 'postprepare', + ] + for (const event of scripts) { + await runScript({ + path: where, + args: [], + scriptShell, + stdio: 'inherit', + stdioString: true, + event, + }) } - reifyOutput(arb) - cb() - } catch (er) { - cb(er) } + await reifyFinish(arb) } const usage = usageUtil( @@ -144,4 +141,4 @@ const completion = async (opts, cb) => { cb() } -module.exports = Object.assign(install, { usage, completion }) +module.exports = Object.assign(cmd, { usage, completion }) diff --git a/lib/link.js b/lib/link.js index d7303fd086cdd..bee44d43a7ff6 100644 --- a/lib/link.js +++ b/lib/link.js @@ -10,7 +10,7 @@ const semver = require('semver') const npm = require('./npm.js') const usageUtil = require('./utils/usage.js') -const reifyOutput = require('./utils/reify-output.js') +const reifyFinish = require('./utils/reify-finish.js') const completion = (opts, cb) => { const dir = npm.globalDir @@ -122,7 +122,7 @@ const linkInstall = async args => { add: names.map(l => `file:${resolve(globalTop, 'node_modules', l)}`), }) - reifyOutput(localArb) + await reifyFinish(localArb) } const linkPkg = async () => { @@ -133,7 +133,7 @@ const linkPkg = async () => { global: true, }) await arb.reify({ add: [`file:${npm.prefix}`] }) - reifyOutput(arb) + await reifyFinish(arb) } module.exports = Object.assign(cmd, { completion, usage }) diff --git a/lib/prune.js b/lib/prune.js index aa2ed378088e3..ea6ed4108aba2 100644 --- a/lib/prune.js +++ b/lib/prune.js @@ -3,7 +3,7 @@ const npm = require('./npm.js') const Arborist = require('@npmcli/arborist') const usageUtil = require('./utils/usage.js') -const reifyOutput = require('./utils/reify-output.js') +const reifyFinish = require('./utils/reify-finish.js') const usage = usageUtil('prune', 'npm prune [[<@scope>/]...] [--production]' @@ -19,7 +19,7 @@ const prune = async () => { path: where, }) await arb.prune(npm.flatOptions) - reifyOutput(arb) + await reifyFinish(arb) } module.exports = Object.assign(cmd, { usage, completion }) diff --git a/lib/uninstall.js b/lib/uninstall.js index ec997ae6457ab..dbaa992f500e0 100644 --- a/lib/uninstall.js +++ b/lib/uninstall.js @@ -5,7 +5,7 @@ const npm = require('./npm.js') const rpj = require('read-package-json-fast') const { resolve } = require('path') const usageUtil = require('./utils/usage.js') -const reifyOutput = require('./utils/reify-output.js') +const reifyFinish = require('./utils/reify-finish.js') const cmd = (args, cb) => rm(args).then(() => cb()).catch(cb) @@ -32,7 +32,7 @@ const rm = async args => { ...npm.flatOptions, rm: args, }) - reifyOutput(arb) + await reifyFinish(arb) } const usage = usageUtil( diff --git a/lib/update.js b/lib/update.js index 791e67e407643..0a786e30f312e 100644 --- a/lib/update.js +++ b/lib/update.js @@ -4,7 +4,7 @@ const Arborist = require('@npmcli/arborist') const log = require('npmlog') const npm = require('./npm.js') const usageUtil = require('./utils/usage.js') -const reifyOutput = require('./utils/reify-output.js') +const reifyFinish = require('./utils/reify-finish.js') const completion = require('./utils/completion/installed-deep.js') const usage = usageUtil( @@ -32,7 +32,7 @@ const update = async args => { }) await arb.reify({ update }) - reifyOutput(arb) + await reifyFinish(arb) } module.exports = Object.assign(cmd, { usage, completion }) diff --git a/lib/utils/reify-finish.js b/lib/utils/reify-finish.js new file mode 100644 index 0000000000000..76dba06cb570c --- /dev/null +++ b/lib/utils/reify-finish.js @@ -0,0 +1,31 @@ +const reifyOutput = require('./reify-output.js') +const npm = require('../npm.js') +const ini = require('ini') +const {writeFile} = require('fs').promises +const {resolve} = require('path') + +const reifyFinish = async arb => { + await saveBuiltinConfig(arb) + reifyOutput(arb) +} + +const saveBuiltinConfig = async arb => { + const { options: { global }, actualTree } = arb + if (!global) + return + + // if we are using a builtin config, and just installed npm as + // a top-level global package, we have to preserve that config. + const npmNode = actualTree.inventory.get('node_modules/npm') + if (!npmNode) + return + + const builtinConf = npm.config.data.get('builtin') + if (builtinConf.loadError) + return + + const content = ini.stringify(builtinConf.raw).trim() + '\n' + await writeFile(resolve(npmNode.path, 'npmrc'), content) +} + +module.exports = reifyFinish diff --git a/tap-snapshots/test-lib-utils-reify-finish.js-TAP.test.js b/tap-snapshots/test-lib-utils-reify-finish.js-TAP.test.js new file mode 100644 index 0000000000000..a82905a399679 --- /dev/null +++ b/tap-snapshots/test-lib-utils-reify-finish.js-TAP.test.js @@ -0,0 +1,15 @@ +/* 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[`test/lib/utils/reify-finish.js TAP should write if everything above passes > written config 1`] = ` +hasBuiltinConfig=true +x=y + +[nested] +foo=bar + +` diff --git a/test/lib/audit.js b/test/lib/audit.js index 4918cb2fc2711..723675c3233d7 100644 --- a/test/lib/audit.js +++ b/test/lib/audit.js @@ -5,7 +5,7 @@ const audit = require('../../lib/audit.js') t.test('should audit using Arborist', t => { let ARB_ARGS = null let AUDIT_CALLED = false - let REIFY_OUTPUT_CALLED = false + let REIFY_FINISH_CALLED = false let AUDIT_REPORT_CALLED = false let OUTPUT_CALLED = false let ARB_OBJ = null @@ -32,11 +32,11 @@ t.test('should audit using Arborist', t => { this.auditReport = {} } }, - '../../lib/utils/reify-output.js': arb => { + '../../lib/utils/reify-finish.js': arb => { if (arb !== ARB_OBJ) { throw new Error('got wrong object passed to reify-output') } - REIFY_OUTPUT_CALLED = true + REIFY_FINISH_CALLED = true }, '../../lib/utils/output.js': () => { OUTPUT_CALLED = true @@ -55,7 +55,7 @@ t.test('should audit using Arborist', t => { t.test('audit fix', t => { audit(['fix'], () => { - t.equal(REIFY_OUTPUT_CALLED, true, 'called reify output') + t.equal(REIFY_FINISH_CALLED, true, 'called reify output') t.end() }) }) diff --git a/test/lib/utils/reify-finish.js b/test/lib/utils/reify-finish.js new file mode 100644 index 0000000000000..c19956f0b2557 --- /dev/null +++ b/test/lib/utils/reify-finish.js @@ -0,0 +1,78 @@ +const t = require('tap') +const requireInject = require('require-inject') + +const npm = { + config: { + data: { + get: () => builtinConfMock, + }, + }, +} + +const builtinConfMock = { + loadError: new Error('no builtin config'), + raw: { hasBuiltinConfig: true, x: 'y', nested: { foo: 'bar' }}, +} + +const reifyOutput = () => {} + +let expectWrite = false +const realFs = require('fs') +const fs = { + ...realFs, + promises: { + ...realFs.promises, + writeFile: async (path, data) => { + if (!expectWrite) + throw new Error('did not expect to write builtin config file') + return realFs.promises.writeFile(path, data) + }, + }, +} + +const reifyFinish = requireInject('../../../lib/utils/reify-finish.js', { + fs, + '../../../lib/npm.js': npm, + '../../../lib/utils/reify-output.js': reifyOutput, +}) + +t.test('should not write if not global', async t => { + expectWrite = false + await reifyFinish({ + options: { global: false }, + actualTree: {}, + }) +}) + +t.test('should not write if no global npm module', async t => { + expectWrite = false + await reifyFinish({ + options: { global: true }, + actualTree: { + inventory: new Map(), + }, + }) +}) + +t.test('should not write if builtin conf had load error', async t => { + expectWrite = false + await reifyFinish({ + options: { global: true }, + actualTree: { + inventory: new Map([['node_modules/npm', {}]]), + }, + }) +}) + +t.test('should write if everything above passes', async t => { + expectWrite = true + delete builtinConfMock.loadError + const path = t.testdir() + await reifyFinish({ + options: { global: true }, + actualTree: { + inventory: new Map([['node_modules/npm', {path}]]), + }, + }) + t.matchSnapshot(fs.readFileSync(`${path}/npmrc`, 'utf8'), 'written config') +})