Skip to content

Commit 8b7ccfe

Browse files
committed
Fix infinite install loop.
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]>
1 parent 57279a2 commit 8b7ccfe

File tree

2 files changed

+52
-8
lines changed

2 files changed

+52
-8
lines changed

lib/install.js

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
1+
module.exports = exports = function (gyp, argv, callback) {
2+
return install(fs, gyp, argv, callback)
3+
}
14

2-
module.exports = exports = install
3-
4-
module.exports.test = { download: download, readCAFile: readCAFile }
5+
module.exports.test = {
6+
download: download,
7+
install: install,
8+
readCAFile: readCAFile,
9+
}
510

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

@@ -23,7 +28,7 @@ var fs = require('graceful-fs')
2328
, processRelease = require('./process-release')
2429
, win = process.platform == 'win32'
2530

26-
function install (gyp, argv, callback) {
31+
function install (fs, gyp, argv, callback) {
2732

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

@@ -82,7 +87,7 @@ function install (gyp, argv, callback) {
8287
log.verbose('install', 'version not already installed, continuing with install', release.version)
8388
go()
8489
} else if (err.code == 'EACCES') {
85-
eaccesFallback()
90+
eaccesFallback(err)
8691
} else {
8792
cb(err)
8893
}
@@ -127,7 +132,7 @@ function install (gyp, argv, callback) {
127132
mkdir(devDir, function (err, created) {
128133
if (err) {
129134
if (err.code == 'EACCES') {
130-
eaccesFallback()
135+
eaccesFallback(err)
131136
} else {
132137
cb(err)
133138
}
@@ -403,7 +408,9 @@ function install (gyp, argv, callback) {
403408
* the compilation will succeed...
404409
*/
405410

406-
function eaccesFallback () {
411+
function eaccesFallback (err) {
412+
var noretry = '--node_gyp_internal_noretry'
413+
if (-1 !== argv.indexOf(noretry)) return cb(err)
407414
var tmpdir = osenv.tmpdir()
408415
gyp.devDir = path.resolve(tmpdir, '.node-gyp')
409416
log.warn('EACCES', 'user "%s" does not have permission to access the dev dir "%s"', osenv.user(), devDir)
@@ -412,7 +419,7 @@ function install (gyp, argv, callback) {
412419
log.verbose('tmpdir == cwd', 'automatically will remove dev files after to save disk space')
413420
gyp.todo.push({ name: 'remove', args: argv })
414421
}
415-
gyp.commands.install(argv, cb)
422+
gyp.commands.install([noretry].concat(argv), cb)
416423
}
417424

418425
}

test/test-install.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict'
2+
3+
var test = require('tape')
4+
var install = require('../lib/install').test.install
5+
6+
test('EACCES retry once', function (t) {
7+
t.plan(3)
8+
9+
var fs = {}
10+
fs.stat = function (path, cb) {
11+
var err = new Error()
12+
err.code = 'EACCES'
13+
cb(err)
14+
t.ok(true);
15+
}
16+
17+
18+
var gyp = {}
19+
gyp.devDir = __dirname
20+
gyp.opts = {}
21+
gyp.opts.ensure = true
22+
gyp.commands = {}
23+
gyp.commands.install = function (argv, cb) {
24+
install(fs, gyp, argv, cb)
25+
}
26+
gyp.commands.remove = function (argv, cb) {
27+
cb()
28+
}
29+
30+
gyp.commands.install([], function (err) {
31+
t.ok(true)
32+
if (/"pre" versions of node cannot be installed/.test(err.message)) {
33+
t.ok(true)
34+
t.ok(true)
35+
}
36+
})
37+
})

0 commit comments

Comments
 (0)