Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

streamlines #6304

Merged
merged 2 commits into from
Mar 30, 2023
Merged

streamlines #6304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68bf5f3
fix: do less work loading ./lib/npm.js
wraithgar Mar 29, 2023
fbb6e94
fix: don't break up log message across lines
wraithgar Mar 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 34 additions & 79 deletions lib/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,28 @@
// This is separate to indicate that it should contain code we expect to work in
// all conceivably runnable versions of node. This is a best effort to catch
// syntax errors to give users a good error message if they are using a node
// version that doesn't allow syntax we are using such as private properties, etc
const createEnginesValidation = () => {
const node = process.version.replace(/-.*$/, '')
/* eslint-disable max-len */
// Code in this file should work in all conceivably runnable versions of node.
// A best effort is made to catch syntax errors to give users a good error message if they are using a node version that doesn't allow syntax we are using in other files such as private properties, etc

// Separated out for easier unit testing
module.exports = async process => {
// set it here so that regardless of what happens later, we don't
// leak any private CLI configs to other programs
process.title = 'npm'

// if npm is called as "npmg" or "npm_g", then run in global mode.
if (process.argv[1][process.argv[1].length - 1] === 'g') {
process.argv.splice(1, 1, 'npm', '-g')
}

const nodeVersion = process.version.replace(/-.*$/, '')
const pkg = require('../package.json')
const engines = pkg.engines.node
const npm = `v${pkg.version}`

const cols = Math.min(Math.max(20, process.stdout.columns) || 80, 80)
const wrap = (lines) => lines
.join(' ')
.split(/[ \n]+/)
.reduce((left, right) => {
const last = left.split('\n').pop()
const join = last.length && last.length + right.length > cols ? '\n' : ' '
return left + join + right
})
.trim()

const unsupportedMessage = wrap([
`npm ${npm} does not support Node.js ${node}.`,
`You should probably upgrade to a newer version of node as we can't make any`,
`promises that npm will work with this version.`,
`This version of npm supports the following node versions: \`${engines}\`.`,
'You can find the latest version at https://nodejs.org/.',
])

const brokenMessage = wrap([
`ERROR: npm ${npm} is known not to run on Node.js ${node}.`,
`You'll need to upgrade to a newer Node.js version in order to use this version of npm.`,
`This version of npm supports the following node versions: \`${engines}\`.`,
'You can find the latest version at https://nodejs.org/.',
])

// coverage ignored because this is only hit in very unsupported node versions
// and it's a best effort attempt to show something nice in those cases
const npmVersion = `v${pkg.version}`

const unsupportedMessage = `npm ${npmVersion} does not support Node.js ${nodeVersion}. This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.`

const brokenMessage = `ERROR: npm ${npmVersion} is known not to run on Node.js ${nodeVersion}. This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.`

// Coverage ignored because this is only hit in very unsupported node versions and it's a best effort attempt to show something nice in those cases
/* istanbul ignore next */
const syntaxErrorHandler = (err) => {
if (err instanceof SyntaxError) {
Expand All @@ -51,74 +38,42 @@ const createEnginesValidation = () => {
process.on('uncaughtException', syntaxErrorHandler)
process.on('unhandledRejection', syntaxErrorHandler)

return {
node,
engines,
unsupportedMessage,
off: () => {
process.off('uncaughtException', syntaxErrorHandler)
process.off('unhandledRejection', syntaxErrorHandler)
},
}
}

// Separated out for easier unit testing
module.exports = async process => {
// set it here so that regardless of what happens later, we don't
// leak any private CLI configs to other programs
process.title = 'npm'

// if npm is called as "npmg" or "npm_g", then run in global mode.
if (process.argv[1][process.argv[1].length - 1] === 'g') {
process.argv.splice(1, 1, 'npm', '-g')
}

// Nothing should happen before this line if we can't guarantee it will
// not have syntax errors in some version of node
const validateEngines = createEnginesValidation()

const satisfies = require('semver/functions/satisfies')
const exitHandler = require('./utils/exit-handler.js')
const Npm = require('./npm.js')
const npm = new Npm()
exitHandler.setNpm(npm)

// only log node and npm paths in argv initially since argv can contain
// sensitive info. a cleaned version will be logged later
// only log node and npm paths in argv initially since argv can contain sensitive info. a cleaned version will be logged later
const log = require('./utils/log-shim.js')
log.verbose('cli', process.argv.slice(0, 2).join(' '))
log.info('using', 'npm@%s', npm.version)
log.info('using', 'node@%s', process.version)

// At this point we've required a few files and can be pretty sure
// we dont contain invalid syntax for this version of node. It's
// possible a lazy require would, but that's unlikely enough that
// it's not worth catching anymore and we attach the more important
// exit handlers.
validateEngines.off()
// At this point we've required a few files and can be pretty sure we dont contain invalid syntax for this version of node. It's possible a lazy require would, but that's unlikely enough that it's not worth catching anymore and we attach the more important exit handlers.
process.off('uncaughtException', syntaxErrorHandler)
process.off('unhandledRejection', syntaxErrorHandler)
process.on('uncaughtException', exitHandler)
process.on('unhandledRejection', exitHandler)

// It is now safe to log a warning if they are using a version of node
// that is not going to fail on syntax errors but is still unsupported
// and untested and might not work reliably. This is safe to use the logger
// now which we want since this will show up in the error log too.
if (!satisfies(validateEngines.node, validateEngines.engines)) {
log.warn('cli', validateEngines.unsupportedMessage)
// It is now safe to log a warning if they are using a version of node that is not going to fail on syntax errors but is still unsupported and untested and might not work reliably. This is safe to use the logger now which we want since this will show up in the error log too.
if (!satisfies(nodeVersion, engines)) {
log.warn('cli', unsupportedMessage)
}

let cmd
// now actually fire up npm and run the command.
// this is how to use npm programmatically:
// Now actually fire up npm and run the command.
// This is how to use npm programmatically:
try {
await npm.load()

// npm -v
if (npm.config.get('version', 'cli')) {
npm.output(npm.version)
return exitHandler()
}

// npm --versions=cli
// npm --versions
if (npm.config.get('versions', 'cli')) {
npm.argv = ['version']
npm.config.set('usage', false, 'cli')
Expand Down
6 changes: 2 additions & 4 deletions lib/utils/exit-handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ const log = require('./log-shim.js')
const errorMessage = require('./error-message.js')
const replaceInfo = require('./replace-info.js')

const indent = (val) => Array.isArray(val) ? val.map(v => indent(v)) : ` ${val}`

let npm = null // set by the cli
let exitHandlerCalled = false
let showLogFileError = false
Expand Down Expand Up @@ -73,15 +71,15 @@ process.on('exit', code => {
const message = []

if (timingFile) {
message.push('Timing info written to:', indent(timingFile))
message.push(`Timing info written to: ${timingFile}`)
} else if (timing) {
message.push(
`The timing file was not written due to an error writing to the directory: ${timingDir}`
)
}

if (logFiles.length) {
message.push('A complete log of this run can be found in:', ...indent(logFiles))
message.push(`A complete log of this run can be found in: ${logFiles}`)
} else if (logsMax <= 0) {
// user specified no log file
message.push(`Log files were not written due to the config logs-max=${logsMax}`)
Expand Down
3 changes: 1 addition & 2 deletions smoke-tests/tap-snapshots/test/index.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ npm ERR! aliases: clean-install, ic, install-clean, isntall-clean
npm ERR!
npm ERR! Run "npm help ci" for more info

npm ERR! A complete log of this run can be found in:

npm ERR! A complete log of this run can be found in: {NPM}/{TESTDIR}/cache/_logs/{LOG}
`

exports[`test/index.js TAP basic npm diff > should have expected diff output 1`] = `
Expand Down
2 changes: 1 addition & 1 deletion smoke-tests/test/fixtures/setup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ module.exports = async (t, { testdir = {}, debug } = {}) => {
.replace(/\r\n/g, '\n')
.replace(/ \(in a browser\)/g, '')
.replace(/^npm@.* /gm, 'npm ')
.replace(/^.*debug-[0-9]+.log$/gm, '')
.replace(/[0-9TZ_-]*debug-[0-9]+.log$/gm, '{LOG}')
.replace(/in \d+[ms]+$/gm, 'in {TIME}')
}
const log = (...a) => debugLog(cleanOutput(a.join(' ')))
Expand Down
6 changes: 2 additions & 4 deletions tap-snapshots/test/lib/utils/exit-handler.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ XX error ERR DETAIL Unknown error
XX verbose exit 1
XX timing npm Completed in {TIME}ms
XX verbose code 1
XX error A complete log of this run can be found in:
XX error {CWD}/cache/_logs/{DATE}-debug-0.log
XX error A complete log of this run can be found in: {CWD}/cache/_logs/{DATE}-debug-0.log
`

exports[`test/lib/utils/exit-handler.js TAP handles unknown error with logs and debug file > logs 1`] = `
Expand Down Expand Up @@ -63,6 +62,5 @@ error ERR DETAIL Unknown error
verbose exit 1
timing npm Completed in {TIME}ms
verbose code 1
error A complete log of this run can be found in:
{CWD}/cache/_logs/{DATE}-debug-0.log
error A complete log of this run can be found in: {CWD}/cache/_logs/{DATE}-debug-0.log
`