-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
python: more informative error #1269
Conversation
this.callback(new Error(errmsg)) | ||
const err = new Error( | ||
'\n******************************************************************\n' + | ||
`node-gyp can't use "${this.python}",\n` + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to change the wording from "can't find" to "can't use"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes because this error is printed both when python can't be found, and on windows when it's a bad version.
In #1268 the error outputed was:
stack Error: Can't find Python executable "C:\Program Files\Python35\python.EXE", you can set the PYTHON env variable.
which makes less sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to distinguish between "can't be found" and bad version? In the example output in the PR description (i.e. the not found case), I think the message is now worse.
node-gyp can't use "python",
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's quite convoluted code...
I'll give it another look in the morning but I can't promise I can find an elegant solution. This code comes after several rounds of geusses so it difficult to know what was found but deemed unsuitable or if nothing was found at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't notice this PR before doing my own fix. Solves the above issue and detects if there is a wrong version + a possible infinite loop fix. See #1325
+1, I have similar issue, and the error info is as following: C:\Program Files (x86)\Microsoft Visual Studio 11.0>npm config set python "D:/Python27/python.exe"
C:\Program Files (x86)\Microsoft Visual Studio 11.0>node-gyp rebuild
gyp info it worked if it ends with ok
gyp info using node-gyp@3.6.2
gyp info using node@8.0.0 | win32 | x64
gyp ERR! configure error
gyp ERR! stack Error: Can't find Python executable "python", you can set the PYTHON env variable.
gyp ERR! stack at PythonFinder.failNoPython (C:\Users\leon\AppData\Roaming\npm\node_modules\node-gyp\lib\configure.js:483:19)
gyp ERR! stack at PythonFinder.<anonymous> (C:\Users\leon\AppData\Roaming\npm\node_modules\node-gyp\lib\configure.js:508:16)
gyp ERR! stack at C:\Users\leon\AppData\Roaming\npm\node_modules\node-gyp\node_modules\graceful-fs\polyfills.js:284:29
gyp ERR! stack at FSReqWrap.oncomplete (fs.js:152:21)
gyp ERR! System Windows_NT 10.0.15063
gyp ERR! command "D:\\Program Files\\nodejs\\node.exe" "C:\\Users\\leon\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild"
gyp ERR! cwd C:\Program Files (x86)\Microsoft Visual Studio 11.0
gyp ERR! node -v v8.0.0
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok |
This should be definitely merged, the error message is vastly misleading. I just spent one hour figuring that I had the wrong python version installed. |
Same |
@bnoordhuis can someone please take a look at these python related PR's? |
@@ -461,7 +461,7 @@ PythonFinder.prototype = { | |||
this.log.silly('stripping "rc" identifier from version') | |||
version = version.replace(/rc(.*)$/ig, '') | |||
} | |||
var range = semver.Range('>=2.5.0 <3.0.0') | |||
var range = semver.Range('>=2.6.0 <3.0.0') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be 2.7.0 instead of 2.6.0 at this point? Python 2.6 has been EOL for 5 years.
'\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' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be 2.7.0 instead of 2.6.0 at this point? Python 2.6 has been EOL for 5 years.
`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' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be 2.7.0 instead of 2.6.0 at this point? Python 2.6 has been EOL for 5 years.
@refack I'm going to have to defer to you on the status of this and its suitability for merging, can you review and see if it's ready or not? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Included this commit as part of #1582, so the two PRs should land simultaneously. LGTM in case you want to go ahead with this independently.
PR-URL: #1269 Refs: #1582 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: João Reis <[email protected]>
Landed in 6b7c8e6 |
PR-URL: #1269 Refs: #1582 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: João Reis <[email protected]>
Checklist
npm install && npm test
passesDescription of change
more informative error when failing to find a suitable python: