Skip to content

Commit

Permalink
feat(install): very strict global npm engines
Browse files Browse the repository at this point in the history
This will do an engines check when installing npm globally and fail if
the new npm is known not to work in the current node version.

It will not work for older npm versions because they don't have an
engines field (it wasn't added till npm@6.14.0). It will at least
prevent npm@7 from being installed in node@8.

PR-URL: #3731
Credit: @wraithgar
Close: #3731
Reviewed-by: @nlf
  • Loading branch information
wraithgar committed Sep 9, 2021
1 parent 1ad0938 commit 6c12500
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 2 deletions.
22 changes: 20 additions & 2 deletions lib/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const log = require('npmlog')
const { resolve, join } = require('path')
const Arborist = require('@npmcli/arborist')
const runScript = require('@npmcli/run-script')
const pacote = require('pacote')
const checks = require('npm-install-checks')

const ArboristWorkspaceCmd = require('./workspaces/arborist-cmd.js')
class Install extends ArboristWorkspaceCmd {
Expand Down Expand Up @@ -126,6 +128,23 @@ class Install extends ArboristWorkspaceCmd {
const ignoreScripts = this.npm.config.get('ignore-scripts')
const isGlobalInstall = this.npm.config.get('global')
const where = isGlobalInstall ? globalTop : this.npm.prefix
const forced = this.npm.config.get('force')
const isDev = this.npm.config.get('dev')
const scriptShell = this.npm.config.get('script-shell') || undefined

// be very strict about engines when trying to update npm itself
const npmInstall = args.find(arg => arg.startsWith('npm@') || arg === 'npm')
if (isGlobalInstall && npmInstall) {
const npmManifest = await pacote.manifest(npmInstall)
try {
checks.checkEngine(npmManifest, npmManifest.version, process.version)
} catch (e) {
if (forced)
this.npm.log.warn('install', `Forcing global npm install with incompatible version ${npmManifest.version} into node ${process.version}`)
else
throw e
}
}

// don't try to install the prefix into itself
args = args.filter(a => resolve(a) !== this.npm.prefix)
Expand All @@ -135,7 +154,7 @@ class Install extends ArboristWorkspaceCmd {
args = ['.']

// TODO: Add warnings for other deprecated flags? or remove this one?
if (this.npm.config.get('dev'))
if (isDev)
log.warn('install', 'Usage of the `--dev` option is deprecated. Use `--include=dev` instead.')

const opts = {
Expand All @@ -150,7 +169,6 @@ class Install extends ArboristWorkspaceCmd {
await arb.reify(opts)

if (!args.length && !isGlobalInstall && !ignoreScripts) {
const scriptShell = this.npm.config.get('script-shell') || undefined
const scripts = [
'preinstall',
'install',
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
"node-gyp": "^7.1.2",
"nopt": "^5.0.0",
"npm-audit-report": "^2.1.5",
"npm-install-checks": "^4.0.0",
"npm-package-arg": "^8.1.5",
"npm-pick-manifest": "^6.1.1",
"npm-profile": "^5.0.3",
Expand Down
140 changes: 140 additions & 0 deletions test/lib/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,146 @@ t.test('should install globally using Arborist', (t) => {
})
})

t.test('npm i -g npm engines check success', (t) => {
const Install = t.mock('../../lib/install.js', {
'../../lib/utils/reify-finish.js': async () => {},
'@npmcli/arborist': function () {
this.reify = () => {}
},
pacote: {
manifest: () => {
return {
version: '100.100.100',
engines: {
node: '>1',
},
}
},
},
})
const npm = mockNpm({
globalDir: 'path/to/node_modules/',
config: {
global: true,
},
})
const install = new Install(npm)
install.exec(['npm'], er => {
if (er)
throw er
t.end()
})
})

t.test('npm i -g npm engines check failure', (t) => {
const Install = t.mock('../../lib/install.js', {
pacote: {
manifest: () => {
return {
_id: 'npm@1.2.3',
version: '100.100.100',
engines: {
node: '>1000',
},
}
},
},
})
const npm = mockNpm({
globalDir: 'path/to/node_modules/',
config: {
global: true,
},
})
const install = new Install(npm)
install.exec(['npm'], er => {
t.match(er, {
message: 'Unsupported engine',
pkgid: 'npm@1.2.3',
current: {
node: process.version,
npm: '100.100.100',
},
required: {
node: '>1000',
},
code: 'EBADENGINE',
})
t.end()
})
})

t.test('npm i -g npm engines check failure forced override', (t) => {
const Install = t.mock('../../lib/install.js', {
'../../lib/utils/reify-finish.js': async () => {},
'@npmcli/arborist': function () {
this.reify = () => {}
},
pacote: {
manifest: () => {
return {
_id: 'npm@1.2.3',
version: '100.100.100',
engines: {
node: '>1000',
},
}
},
},
})
const npm = mockNpm({
globalDir: 'path/to/node_modules/',
config: {
force: true,
global: true,
},
})
const install = new Install(npm)
install.exec(['npm'], er => {
if (er)
throw er
t.end()
})
})

t.test('npm i -g npm@version engines check failure', (t) => {
const Install = t.mock('../../lib/install.js', {
pacote: {
manifest: () => {
return {
_id: 'npm@1.2.3',
version: '100.100.100',
engines: {
node: '>1000',
},
}
},
},
})
const npm = mockNpm({
globalDir: 'path/to/node_modules/',
config: {
global: true,
},
})
const install = new Install(npm)
install.exec(['npm@100'], er => {
t.match(er, {
message: 'Unsupported engine',
pkgid: 'npm@1.2.3',
current: {
node: process.version,
npm: '100.100.100',
},
required: {
node: '>1000',
},
code: 'EBADENGINE',
})
t.end()
})
})

t.test('completion to folder', async t => {
const Install = t.mock('../../lib/install.js', {
'../../lib/utils/reify-finish.js': async () => {},
Expand Down

0 comments on commit 6c12500

Please sign in to comment.