chore(refactor): finish passing npm context

No more requiring npm as a singleton.  This will now allow us to move
forward with the other refactoring we need to always use the npm object
itself in tests, not a mocked one.

PR-URL: #3388
Credit: @wraithgar
Close: #3388
Reviewed-by: @ruyadorno

lib/cli.js

@@ -20,6 +20,7 @@ module.exports = (process) => {
 
   const npm = require('../lib/npm.js')
   const errorHandler = require('../lib/utils/error-handler.js')
+  errorHandler.setNpm(npm)
 
   // if npm is called as "npmg" or "npm_g", then
   // run in global mode.

lib/utils/error-handler.js

@@ -1,28 +1,27 @@
+let npm // set by the cli
 let cbCalled = false
 const log = require('npmlog')
-const npm = require('../npm.js')
 let itWorked = false
 const path = require('path')
+const writeFileAtomic = require('write-file-atomic')
+const mkdirp = require('mkdirp-infer-owner')
+const fs = require('graceful-fs')
 let wroteLogFile = false
 let exitCode = 0
 const errorMessage = require('./error-message.js')
 const replaceInfo = require('./replace-info.js')
 
-const cacheFile = require('./cache-file.js')
-
 let logFileName
 const getLogFile = () => {
+  // we call this multiple times, so we need to treat it as a singleton because
+  // the date is part of the name
   if (!logFileName)
     logFileName = path.resolve(npm.config.get('cache'), '_logs', (new Date()).toISOString().replace(/[.:]/g, '_') + '-debug.log')
 
   return logFileName
 }
 
-const timings = {
-  version: npm.version,
-  command: process.argv.slice(2),
-  logfile: null,
-}
+const timings = {}
 process.on('timing', (name, value) => {
   if (timings[name])
     timings[name] += value
@@ -35,9 +34,21 @@ process.on('exit', code => {
   log.disableProgress()
   if (npm.config && npm.config.loaded && npm.config.get('timing')) {
     try {
-      timings.logfile = getLogFile()
-      cacheFile.append('_timing.json', JSON.stringify(timings) + '\n')
-    } catch (_) {
+      const file = path.resolve(npm.config.get('cache'), '_timing.json')
+      const dir = path.dirname(npm.config.get('cache'))
+      mkdirp.sync(dir)
+
+      fs.appendFileSync(file, JSON.stringify({
+        command: process.argv.slice(2),
+        logfile: getLogFile(),
+        version: npm.version,
+        ...timings,
+      }) + '\n')
+
+      const st = fs.lstatSync(path.dirname(npm.config.get('cache')))
+      fs.chownSync(dir, st.uid, st.gid)
+      fs.chownSync(file, st.uid, st.gid)
+    } catch (ex) {
       // ignore
     }
   }
@@ -174,7 +185,7 @@ const errorHandler = (er) => {
       log.error(k, v)
   }
 
-  const msg = errorMessage(er)
+  const msg = errorMessage(er, npm)
   for (const errline of [...msg.summary, ...msg.detail])
     log.error(...errline)
 
@@ -214,7 +225,15 @@ const writeLogFile = () => {
           logOutput += line + os.EOL
         })
     })
-    cacheFile.write(getLogFile(), logOutput)
+
+    const file = getLogFile()
+    const dir = path.dirname(file)
+    mkdirp.sync(dir)
+    writeFileAtomic.sync(file, logOutput)
+
+    const st = fs.lstatSync(path.dirname(npm.config.get('cache')))
+    fs.chownSync(dir, st.uid, st.gid)
+    fs.chownSync(file, st.uid, st.gid)
 
     // truncate once it's been written.
     log.record.length = 0
@@ -226,3 +245,6 @@ const writeLogFile = () => {
 
 module.exports = errorHandler
 module.exports.exit = exit
+module.exports.setNpm = (n) => {
+  npm = n
+}

lib/utils/error-message.js

@@ -1,12 +1,11 @@
-const npm = require('../npm.js')
 const { format } = require('util')
 const { resolve } = require('path')
 const nameValidator = require('validate-npm-package-name')
 const npmlog = require('npmlog')
 const replaceInfo = require('./replace-info.js')
-const { report: explainEresolve } = require('./explain-eresolve.js')
+const { report } = require('./explain-eresolve.js')
 
-module.exports = (er) => {
+module.exports = (er, npm) => {
   const short = []
   const detail = []
 
@@ -19,7 +18,7 @@ module.exports = (er) => {
     case 'ERESOLVE':
       short.push(['ERESOLVE', er.message])
       detail.push(['', ''])
-      detail.push(['', explainEresolve(er)])
+      detail.push(['', report(er, npm.color, resolve(npm.cache, 'eresolve-report.txt'))])
       break
 
     case 'ENOLOCK': {

lib/utils/explain-eresolve.js

@@ -1,20 +1,14 @@
 // this is called when an ERESOLVE error is caught in the error-handler,
 // or when there's a log.warn('eresolve', msg, explanation), to turn it
 // into a human-intelligible explanation of what's wrong and how to fix.
-//
-// TODO: abstract out the explainNode methods into a separate util for
-// use by a future `npm explain <path || spec>` command.
-
-const npm = require('../npm.js')
 const { writeFileSync } = require('fs')
-const { resolve } = require('path')
 const { explainEdge, explainNode, printNode } = require('./explain-dep.js')
 
 // expl is an explanation object that comes from Arborist.  It looks like:
 // Depth is how far we want to want to descend into the object making a report.
 // The full report (ie, depth=Infinity) is always written to the cache folder
 // at ${cache}/eresolve-report.txt along with full json.
-const explainEresolve = (expl, color, depth) => {
+const explain = (expl, color, depth) => {
   const { edge, current, peerConflict, currentEdge } = expl
 
   const out = []
@@ -42,9 +36,7 @@ const explainEresolve = (expl, color, depth) => {
 }
 
 // generate a full verbose report and tell the user how to fix it
-const report = (expl, depth = 4) => {
-  const fullReport = resolve(npm.cache, 'eresolve-report.txt')
-
+const report = (expl, color, fullReport) => {
   const orNoStrict = expl.strictPeerDeps ? '--no-strict-peer-deps, ' : ''
   const fix = `Fix the upstream dependency conflict, or retry
 this command with ${orNoStrict}--force, or --legacy-peer-deps
@@ -54,7 +46,7 @@ to accept an incorrect (and potentially broken) dependency resolution.`
 
 ${new Date().toISOString()}
 
-${explainEresolve(expl, false, Infinity)}
+${explain(expl, false, Infinity)}
 
 ${fix}
 
@@ -63,13 +55,10 @@ Raw JSON explanation object:
 ${JSON.stringify(expl, null, 2)}
 `, 'utf8')
 
-  return explainEresolve(expl, npm.color, depth) +
+  return explain(expl, color, 4) +
     `\n\n${fix}\n\nSee ${fullReport} for a full report.`
 }
 
-// the terser explain method for the warning when using --force
-const explain = (expl, depth = 2) => explainEresolve(expl, npm.color, depth)
-
 module.exports = {
   explain,
   report,

lib/utils/setup-log.js

@@ -14,10 +14,22 @@ module.exports = (config) => {
 
   const { warn } = log
 
+  const stdoutTTY = process.stdout.isTTY
+  const stderrTTY = process.stderr.isTTY
+  const dumbTerm = process.env.TERM === 'dumb'
+  const stderrNotDumb = stderrTTY && !dumbTerm
+  const enableColorStderr = color === 'always' ? true
+    : color === false ? false
+    : stderrTTY
+
+  const enableColorStdout = color === 'always' ? true
+    : color === false ? false
+    : stdoutTTY
+
   log.warn = (heading, ...args) => {
     if (heading === 'ERESOLVE' && args[1] && typeof args[1] === 'object') {
       warn(heading, args[0])
-      return warn('', explain(args[1]))
+      return warn('', explain(args[1], enableColorStdout, 2))
     }
     return warn(heading, ...args)
   }
@@ -29,19 +41,6 @@ module.exports = (config) => {
 
   log.heading = config.get('heading') || 'npm'
 
-  const stdoutTTY = process.stdout.isTTY
-  const stderrTTY = process.stderr.isTTY
-  const dumbTerm = process.env.TERM === 'dumb'
-  const stderrNotDumb = stderrTTY && !dumbTerm
-
-  const enableColorStderr = color === 'always' ? true
-    : color === false ? false
-    : stderrTTY
-
-  const enableColorStdout = color === 'always' ? true
-    : color === false ? false
-    : stdoutTTY
-
   if (enableColorStderr)
     log.enableColor()
   else

tap-snapshots/test/lib/utils/error-handler.js.test.cjs

@@ -8,7 +8,7 @@
 exports[`test/lib/utils/error-handler.js TAP handles unknown error > should have expected log contents for unknown error 1`] = `
 0 verbose code 1
 1 error foo A complete log of this run can be found in:
-1 error foo     {CWD}/cachefolder/_logs/expecteddate-debug.log
+1 error foo     {CWD}/test/lib/utils/tap-testdir-error-handler/_logs/expecteddate-debug.log
 2 verbose stack Error: ERROR
 3 verbose cwd {CWD}
 4 verbose Foo 1.0.0