Skip to content

Commit

Permalink
python: more informative error
Browse files Browse the repository at this point in the history
PR-URL: #1269
Refs: #1582
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: João Reis <reis@janeasystems.com>
  • Loading branch information
refack authored and rvagg committed Apr 24, 2019
1 parent d3b2122 commit 49ab79d
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 23 deletions.
1 change: 1 addition & 0 deletions .eslintrc.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
env:
node: true
es6: true
rules:
no-unused-vars: error
2 changes: 1 addition & 1 deletion .jshintrc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"asi": true,
"laxcomma": true,
"es5": true,
"esversion": 6,
"node": true,
"strict": false
}
8 changes: 6 additions & 2 deletions bin/node-gyp.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,12 @@ function run () {
prog.commands[command.name](command.args, function (err) {
if (err) {
log.error(command.name + ' error')
log.error('stack', err.stack)
errorMessage()
if (err.noPython) {
log.error(err.message)
} else {
log.error('stack', err.stack)
errorMessage()
}
log.error('not ok')
return process.exit(1)
}
Expand Down
35 changes: 24 additions & 11 deletions lib/configure.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ PythonFinder.prototype = {
'`%s -c "' + args[1] + '"` returned: %j',
this.python, stdout)
var version = stdout.trim()
var range = semver.Range('>=2.5.0 <3.0.0')
var range = semver.Range('>=2.6.0 <3.0.0')
var valid = false
try {
valid = range.test(version)
Expand All @@ -467,19 +467,32 @@ PythonFinder.prototype = {
},

failNoPython: function failNoPython () {
var errmsg =
'Can\'t find Python executable "' + this.python +
'", you can set the PYTHON env variable.'
this.callback(new Error(errmsg))
const err = new Error(
'\n******************************************************************\n' +
`node-gyp can't use "${this.python}",\n` +
'It is recommended that you install python 2.7, set the PYTHON env,\n' +
'or use the --python switch to point to a Python >= v2.6.0 & < 3.0.0.\n' +
'For more information consult the documentation at:\n' +
'https://github.com/nodejs/node-gyp#installation\n' +
'***********************************************************************'
);
err.noPython = true;
this.callback(err)
},

failPythonVersion: function failPythonVersion (badVersion) {
var errmsg =
'Python executable "' + this.python +
'" is v' + badVersion + ', which is not supported by gyp.\n' +
'You can pass the --python switch to point to ' +
'Python >= v2.5.0 & < 3.0.0.'
this.callback(new Error(errmsg))
const err = new Error(
'\n******************************************************************\n' +
`Python executable "${this.python}" is v${badVersion}\n` +
'this version is not supported by GYP and hence by node-gyp.\n' +
'It is recommended that you install python 2.7, set the PYTHON env,\n' +
'or use the --python switch to point to a Python >= v2.6.0 & < 3.0.0.\n' +
'For more information consult the documentation at:\n' +
'https://github.com/nodejs/node-gyp#installation\n' +
'***********************************************************************'
);
err.noPython = true;
this.callback(err)
},

// Called on Windows when "python" isn't available in the current $PATH.
Expand Down
21 changes: 12 additions & 9 deletions test/test-find-python.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ test('find python - python', function (t) {
})

test('find python - python too old', function (t) {
t.plan(4)
t.plan(5)

var f = new TestPythonFinder('python', done)
f.which = function(program, cb) {
Expand All @@ -89,12 +89,13 @@ test('find python - python too old', function (t) {
f.checkPython()

function done(err) {
t.ok(/is not supported by gyp/.test(err))
t.ok(/this version is not supported by GYP/.test(err))
t.ok(/For more information consult the documentation/.test(err))
}
})

test('find python - python too new', function (t) {
t.plan(4)
t.plan(5)

var f = new TestPythonFinder('python', done)
f.which = function(program, cb) {
Expand All @@ -109,7 +110,8 @@ test('find python - python too new', function (t) {
f.checkPython()

function done(err) {
t.ok(/is not supported by gyp/.test(err))
t.ok(/this version is not supported by GYP/.test(err))
t.ok(/For more information consult the documentation/.test(err))
}
})

Expand All @@ -124,7 +126,7 @@ test('find python - no python', function (t) {
f.checkPython()

function done(err) {
t.ok(/Can't find Python executable/.test(err))
t.ok(/For more information consult the documentation/.test(err))
}
})

Expand Down Expand Up @@ -171,7 +173,7 @@ test('find python - no python2, no python, unix', function (t) {
f.checkPython()

function done(err) {
t.ok(/Can't find Python executable/.test(err))
t.ok(/For more information consult the documentation/.test(err))
}
})

Expand Down Expand Up @@ -242,7 +244,7 @@ test('find python - python 3, use python launcher', function (t) {

test('find python - python 3, use python launcher, python 2 too old',
function (t) {
t.plan(9)
t.plan(10)

var f = new TestPythonFinder('python', done)
f.checkedPythonLauncher = false
Expand Down Expand Up @@ -272,7 +274,8 @@ test('find python - python 3, use python launcher, python 2 too old',
f.checkPython()

function done(err) {
t.ok(/is not supported by gyp/.test(err))
t.ok(/this version is not supported by GYP/.test(err))
t.ok(/For more information consult the documentation/.test(err))
}
})

Expand Down Expand Up @@ -334,6 +337,6 @@ test('find python - no python, no python launcher, bad guess', function (t) {
f.checkPython()

function done(err) {
t.ok(/Can't find Python executable/.test(err))
t.ok(/For more information consult the documentation/.test(err))
}
})

0 comments on commit 49ab79d

Please sign in to comment.