Preserve builtin conf when installing npm globally

.eslintrc.json

@@ -133,7 +133,7 @@
     "no-shadow-restricted-names": "error",
     "no-sparse-arrays": "error",
     "no-tabs": "error",
-    "no-template-curly-in-string": "error",
+    "no-template-curly-in-string": "off",
     "no-this-before-super": "error",
     "no-throw-literal": "off",
     "no-trailing-spaces": "error",

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)

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 })

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 })

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 })

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 })

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 })

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(

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 })

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

package.json

@@ -205,9 +205,11 @@
     "posttest": "npm run lint",
     "eslint": "eslint",
     "lint": "npm run eslint -- \"lib/**/*.js\"",
+    "linttest": "npm run eslint -- test/lib test/bin --fix",
     "lintfix": "npm run lint -- --fix",
     "prelint": "rimraf test/npm_cache*",
-    "resetdeps": "bash scripts/resetdeps.sh"
+    "resetdeps": "bash scripts/resetdeps.sh",
+    "prepublishOnly": "npm run lint && npm run linttest"
   },
   "//": [
     "XXX temporarily only run unit tests while v7 beta is in progress",

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
+
+`

test/bin/npm-cli.js

@@ -5,6 +5,6 @@ t.test('loading the bin calls the implementation', t => {
     '../../lib/cli.js': proc => {
       t.equal(proc, process, 'called implementation with process object')
       t.end()
-    }
+    },
   })
 })

test/bin/npx-cli.js

@@ -64,7 +64,7 @@ t.test('use a bunch of deprecated switches and options', t => {
     '--shell-auto-fallback',
     '--ignore-existing',
     '-q',
-    'foobar'
+    'foobar',
   ]
 
   const expect = [
@@ -78,18 +78,18 @@ t.test('use a bunch of deprecated switches and options', t => {
     '--loglevel',
     'warn',
     '--',
-    'foobar'
+    'foobar',
   ]
   requireInject(npx, { [cli]: () => {} })
   t.strictSame(process.argv, expect)
   t.strictSame(logs, [
-    [ 'npx: the --npm argument has been removed.' ],
-    [ 'npx: the --node-arg argument has been removed.' ],
-    [ 'npx: the --n argument has been removed.' ],
-    [ 'npx: the --always-spawn argument has been removed.' ],
-    [ 'npx: the --shell-auto-fallback argument has been removed.' ],
-    [ 'npx: the --ignore-existing argument has been removed.' ],
-    [ 'See `npm help exec` for more information' ]
+    ['npx: the --npm argument has been removed.'],
+    ['npx: the --node-arg argument has been removed.'],
+    ['npx: the --n argument has been removed.'],
+    ['npx: the --always-spawn argument has been removed.'],
+    ['npx: the --shell-auto-fallback argument has been removed.'],
+    ['npx: the --ignore-existing argument has been removed.'],
+    ['See `npm help exec` for more information'],
   ])
   t.end()
 })