Preserve builtin conf when installing npm globally

isaacs · ruyadorno · commit bc9afb195f5a · 2020-11-17T14:38:17.000-05:00
When a file named 'npmrc' is in the root of the npm module that is currently running, it adds config values that override the defaults (but not the global or user configs). This is a way for a system package installer to tell the npm that it installs to put its globals somewhere other than the default. In order to keep these configs around when users self-update npm with `npm i -g npm`, these config values must be "sticky", and ride along into the newly globally installed npm. This commit restores this behavior, fixing self-updating npm for Windows users, and any other systems that may make use of this functionality. Fixes: #2002 PR-URL: #2184 Credit: @isaacs Close: #2184 Reviewed-by: @ruyadorno
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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>/]<pkg>...] [--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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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')
+})
