Skip to content

Commit

Permalink
Fix infinite install loop.
Browse files Browse the repository at this point in the history
Retry the download+install dance only once after encountering an EACCES.

That only happens when both the devdir (usually: `$HOME/.node-gyp`) and
the current working directory aren't writable.  Users won't often hit
that except through `sudo npm install` because npm drops privileges
before executing node-gyp.

Fixes: #1383
PR-URL: #1384
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
  • Loading branch information
bnoordhuis committed Jun 8, 2018
1 parent 2580b91 commit 6f1286f
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 8 deletions.
23 changes: 15 additions & 8 deletions lib/install.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
module.exports = exports = function (gyp, argv, callback) {
return install(fs, gyp, argv, callback)
}

module.exports = exports = install

module.exports.test = { download: download, readCAFile: readCAFile }
module.exports.test = {
download: download,
install: install,
readCAFile: readCAFile,
}

exports.usage = 'Install node development files for the specified node version.'

Expand All @@ -25,7 +30,7 @@ var fs = require('graceful-fs')
, processRelease = require('./process-release')
, win = process.platform == 'win32'

function install (gyp, argv, callback) {
function install (fs, gyp, argv, callback) {

var release = processRelease(argv, gyp, process.version, process.release)

Expand Down Expand Up @@ -84,7 +89,7 @@ function install (gyp, argv, callback) {
log.verbose('install', 'version not already installed, continuing with install', release.version)
go()
} else if (err.code == 'EACCES') {
eaccesFallback()
eaccesFallback(err)
} else {
cb(err)
}
Expand Down Expand Up @@ -129,7 +134,7 @@ function install (gyp, argv, callback) {
mkdir(devDir, function (err, created) {
if (err) {
if (err.code == 'EACCES') {
eaccesFallback()
eaccesFallback(err)
} else {
cb(err)
}
Expand Down Expand Up @@ -409,7 +414,9 @@ function install (gyp, argv, callback) {
* the compilation will succeed...
*/

function eaccesFallback () {
function eaccesFallback (err) {
var noretry = '--node_gyp_internal_noretry'
if (-1 !== argv.indexOf(noretry)) return cb(err)
var tmpdir = osenv.tmpdir()
gyp.devDir = path.resolve(tmpdir, '.node-gyp')
log.warn('EACCES', 'user "%s" does not have permission to access the dev dir "%s"', osenv.user(), devDir)
Expand All @@ -418,7 +425,7 @@ function install (gyp, argv, callback) {
log.verbose('tmpdir == cwd', 'automatically will remove dev files after to save disk space')
gyp.todo.push({ name: 'remove', args: argv })
}
gyp.commands.install(argv, cb)
gyp.commands.install([noretry].concat(argv), cb)
}

}
Expand Down
37 changes: 37 additions & 0 deletions test/test-install.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict'

var test = require('tape')
var install = require('../lib/install').test.install

test('EACCES retry once', function (t) {
t.plan(3)

var fs = {}
fs.stat = function (path, cb) {
var err = new Error()
err.code = 'EACCES'
cb(err)
t.ok(true);
}


var gyp = {}
gyp.devDir = __dirname
gyp.opts = {}
gyp.opts.ensure = true
gyp.commands = {}
gyp.commands.install = function (argv, cb) {
install(fs, gyp, argv, cb)
}
gyp.commands.remove = function (argv, cb) {
cb()
}

gyp.commands.install([], function (err) {
t.ok(true)
if (/"pre" versions of node cannot be installed/.test(err.message)) {
t.ok(true)
t.ok(true)
}
})
})

0 comments on commit 6f1286f

Please sign in to comment.