Skip to content

Commit c6a8734

Browse files
wraithgarwraithgar
committed
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
1 parent e5abf2a commit c6a8734

14 files changed

Lines changed: 137 additions & 294 deletions

‎lib/cli.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ module.exports = (process) => {
2020

2121
const npm = require('../lib/npm.js')
2222
const errorHandler = require('../lib/utils/error-handler.js')
23+
errorHandler.setNpm(npm)
2324

2425
// if npm is called as "npmg" or "npm_g", then
2526
// run in global mode.

‎lib/utils/cache-file.js‎

Lines changed: 0 additions & 66 deletions
This file was deleted.

‎lib/utils/error-handler.js‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,27 @@
1+
let npm // set by the cli
12
let cbCalled = false
23
const log = require('npmlog')
3-
const npm = require('../npm.js')
44
let itWorked = false
55
const path = require('path')
6+
const writeFileAtomic = require('write-file-atomic')
7+
const mkdirp = require('mkdirp-infer-owner')
8+
const fs = require('graceful-fs')
69
let wroteLogFile = false
710
let exitCode = 0
811
const errorMessage = require('./error-message.js')
912
const replaceInfo = require('./replace-info.js')
1013

11-
const cacheFile = require('./cache-file.js')
12-
1314
let logFileName
1415
const getLogFile = () => {
16+
// we call this multiple times, so we need to treat it as a singleton because
17+
// the date is part of the name
1518
if (!logFileName)
1619
logFileName = path.resolve(npm.config.get('cache'), '_logs', (new Date()).toISOString().replace(/[.:]/g, '_') + '-debug.log')
1720

1821
return logFileName
1922
}
2023

21-
const timings = {
22-
version: npm.version,
23-
command: process.argv.slice(2),
24-
logfile: null,
25-
}
24+
const timings = {}
2625
process.on('timing', (name, value) => {
2726
if (timings[name])
2827
timings[name] += value
@@ -35,9 +34,21 @@ process.on('exit', code => {
3534
log.disableProgress()
3635
if (npm.config && npm.config.loaded && npm.config.get('timing')) {
3736
try {
38-
timings.logfile = getLogFile()
39-
cacheFile.append('_timing.json', JSON.stringify(timings) + '\n')
40-
} catch (_) {
37+
const file = path.resolve(npm.config.get('cache'), '_timing.json')
38+
const dir = path.dirname(npm.config.get('cache'))
39+
mkdirp.sync(dir)
40+
41+
fs.appendFileSync(file, JSON.stringify({
42+
command: process.argv.slice(2),
43+
logfile: getLogFile(),
44+
version: npm.version,
45+
...timings,
46+
}) + '\n')
47+
48+
const st = fs.lstatSync(path.dirname(npm.config.get('cache')))
49+
fs.chownSync(dir, st.uid, st.gid)
50+
fs.chownSync(file, st.uid, st.gid)
51+
} catch (ex) {
4152
// ignore
4253
}
4354
}
@@ -174,7 +185,7 @@ const errorHandler = (er) => {
174185
log.error(k, v)
175186
}
176187

177-
const msg = errorMessage(er)
188+
const msg = errorMessage(er, npm)
178189
for (const errline of [...msg.summary, ...msg.detail])
179190
log.error(...errline)
180191

@@ -214,7 +225,15 @@ const writeLogFile = () => {
214225
logOutput += line + os.EOL
215226
})
216227
})
217-
cacheFile.write(getLogFile(), logOutput)
228+
229+
const file = getLogFile()
230+
const dir = path.dirname(file)
231+
mkdirp.sync(dir)
232+
writeFileAtomic.sync(file, logOutput)
233+
234+
const st = fs.lstatSync(path.dirname(npm.config.get('cache')))
235+
fs.chownSync(dir, st.uid, st.gid)
236+
fs.chownSync(file, st.uid, st.gid)
218237

219238
// truncate once it's been written.
220239
log.record.length = 0
@@ -226,3 +245,6 @@ const writeLogFile = () => {
226245

227246
module.exports = errorHandler
228247
module.exports.exit = exit
248+
module.exports.setNpm = (n) => {
249+
npm = n
250+
}

‎lib/utils/error-message.js‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
const npm = require('../npm.js')
21
const { format } = require('util')
32
const { resolve } = require('path')
43
const nameValidator = require('validate-npm-package-name')
54
const npmlog = require('npmlog')
65
const replaceInfo = require('./replace-info.js')
7-
const { report: explainEresolve } = require('./explain-eresolve.js')
6+
const { report } = require('./explain-eresolve.js')
87

9-
module.exports = (er) => {
8+
module.exports = (er, npm) => {
109
const short = []
1110
const detail = []
1211

@@ -19,7 +18,7 @@ module.exports = (er) => {
1918
case 'ERESOLVE':
2019
short.push(['ERESOLVE', er.message])
2120
detail.push(['', ''])
22-
detail.push(['', explainEresolve(er)])
21+
detail.push(['', report(er, npm.color, resolve(npm.cache, 'eresolve-report.txt'))])
2322
break
2423

2524
case 'ENOLOCK': {

‎lib/utils/explain-eresolve.js‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,14 @@
11
// this is called when an ERESOLVE error is caught in the error-handler,
22
// or when there's a log.warn('eresolve', msg, explanation), to turn it
33
// into a human-intelligible explanation of what's wrong and how to fix.
4-
//
5-
// TODO: abstract out the explainNode methods into a separate util for
6-
// use by a future `npm explain <path || spec>` command.
7-
8-
const npm = require('../npm.js')
94
const { writeFileSync } = require('fs')
10-
const { resolve } = require('path')
115
const { explainEdge, explainNode, printNode } = require('./explain-dep.js')
126

137
// expl is an explanation object that comes from Arborist. It looks like:
148
// Depth is how far we want to want to descend into the object making a report.
159
// The full report (ie, depth=Infinity) is always written to the cache folder
1610
// at ${cache}/eresolve-report.txt along with full json.
17-
const explainEresolve = (expl, color, depth) => {
11+
const explain = (expl, color, depth) => {
1812
const { edge, current, peerConflict, currentEdge } = expl
1913

2014
const out = []
@@ -42,9 +36,7 @@ const explainEresolve = (expl, color, depth) => {
4236
}
4337

4438
// generate a full verbose report and tell the user how to fix it
45-
const report = (expl, depth = 4) => {
46-
const fullReport = resolve(npm.cache, 'eresolve-report.txt')
47-
39+
const report = (expl, color, fullReport) => {
4840
const orNoStrict = expl.strictPeerDeps ? '--no-strict-peer-deps, ' : ''
4941
const fix = `Fix the upstream dependency conflict, or retry
5042
this command with ${orNoStrict}--force, or --legacy-peer-deps
@@ -54,7 +46,7 @@ to accept an incorrect (and potentially broken) dependency resolution.`
5446
5547
${new Date().toISOString()}
5648
57-
${explainEresolve(expl, false, Infinity)}
49+
${explain(expl, false, Infinity)}
5850
5951
${fix}
6052
@@ -63,13 +55,10 @@ Raw JSON explanation object:
6355
${JSON.stringify(expl, null, 2)}
6456
`, 'utf8')
6557

66-
return explainEresolve(expl, npm.color, depth) +
58+
return explain(expl, color, 4) +
6759
`\n\n${fix}\n\nSee ${fullReport} for a full report.`
6860
}
6961

70-
// the terser explain method for the warning when using --force
71-
const explain = (expl, depth = 2) => explainEresolve(expl, npm.color, depth)
72-
7362
module.exports = {
7463
explain,
7564
report,

‎lib/utils/setup-log.js‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,22 @@ module.exports = (config) => {
1414

1515
const { warn } = log
1616

17+
const stdoutTTY = process.stdout.isTTY
18+
const stderrTTY = process.stderr.isTTY
19+
const dumbTerm = process.env.TERM === 'dumb'
20+
const stderrNotDumb = stderrTTY && !dumbTerm
21+
const enableColorStderr = color === 'always' ? true
22+
: color === false ? false
23+
: stderrTTY
24+
25+
const enableColorStdout = color === 'always' ? true
26+
: color === false ? false
27+
: stdoutTTY
28+
1729
log.warn = (heading, ...args) => {
1830
if (heading === 'ERESOLVE' && args[1] && typeof args[1] === 'object') {
1931
warn(heading, args[0])
20-
return warn('', explain(args[1]))
32+
return warn('', explain(args[1], enableColorStdout, 2))
2133
}
2234
return warn(heading, ...args)
2335
}
@@ -29,19 +41,6 @@ module.exports = (config) => {
2941

3042
log.heading = config.get('heading') || 'npm'
3143

32-
const stdoutTTY = process.stdout.isTTY
33-
const stderrTTY = process.stderr.isTTY
34-
const dumbTerm = process.env.TERM === 'dumb'
35-
const stderrNotDumb = stderrTTY && !dumbTerm
36-
37-
const enableColorStderr = color === 'always' ? true
38-
: color === false ? false
39-
: stderrTTY
40-
41-
const enableColorStdout = color === 'always' ? true
42-
: color === false ? false
43-
: stdoutTTY
44-
4544
if (enableColorStderr)
4645
log.enableColor()
4746
else

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
exports[`test/lib/utils/error-handler.js TAP handles unknown error > should have expected log contents for unknown error 1`] = `
99
0 verbose code 1
1010
1 error foo A complete log of this run can be found in:
11-
1 error foo {CWD}/cachefolder/_logs/expecteddate-debug.log
11+
1 error foo {CWD}/test/lib/utils/tap-testdir-error-handler/_logs/expecteddate-debug.log
1212
2 verbose stack Error: ERROR
1313
3 verbose cwd {CWD}
1414
4 verbose Foo 1.0.0

0 commit comments

Comments
 (0)