Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Accept Python 3 when NODE_GYP_PYTHON3 is defined #1815

Closed
wants to merge 5 commits into from
Closed

Accept Python 3 when NODE_GYP_PYTHON3 is defined #1815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c79708d
python: move Python detection to its own file
joaocgreis Jul 8, 2019
e2c75b4
python: accept Python 3 when NODE_GYP_PYTHON3=1
joaocgreis Jul 8, 2019
cc6c659
fixup: rename NODE_GYP_PYTHON3 to EXPERIMENTAL_NODE_GYP_PYTHON3
joaocgreis Jul 9, 2019
4617ecb
fixup: reject outdated versions 3.0-3.4
joaocgreis Jul 9, 2019
19f02f6
fixup: fix linting
joaocgreis Jul 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,61 @@ cache: pip
matrix:
include:
- name: "Python 2.7 on Linux"
env: NODE_GYP_FORCE_PYTHON=python2
Copy link
Contributor

@cclauss cclauss Jul 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Linux runs Travis will set the environment variable $TRAVIS_PYTHON_VERSION == 2.7 so perhaps that it would be better to leverage that rather than create a new environment variable.

Copy link
Contributor

@cclauss cclauss Jul 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I think about it more, I now understand that you are looking for a more general solution that is useful well beyond just Travis CI. What if we name the variable NODE_GYP_PYTHON2 and set it to True in legacy Python environments and set it to False or leave it undefined in Python 3 environments? In this way we would not need to carry this baggage into future years. With just 176 days left before Python 2 end of life we should be making execution on Python 3 the default behavior and making running on Python 2 be the special case.

python: 2.7
- name: "Python 2.7 on macOS"
os: osx
osx_image: xcode10.2
language: shell # 'language: python' is not yet supported on macOS
env: NODE_GYP_FORCE_PYTHON=python2
before_install: HOMEBREW_NO_AUTO_UPDATE=1 brew install npm
- name: "Node.js 6 & Python 2.7 on Windows"
os: windows
language: node_js
node_js: 6 # node
env: PATH=/c/Python27:/c/Python27/Scripts:$PATH
env: >-
PATH=/c/Python27:/c/Python27/Scripts:$PATH
NODE_GYP_FORCE_PYTHON=/c/Python27/python.exe
before_install: choco install python2
- name: "Node.js 12 & Python 2.7 on Windows"
os: windows
language: node_js
node_js: 12 # node
env: PATH=/c/Python27:/c/Python27/Scripts:$PATH
env: >-
PATH=/c/Python27:/c/Python27/Scripts:$PATH
NODE_GYP_FORCE_PYTHON=/c/Python27/python.exe
before_install: choco install python2
- name: "Node.js 6 & Python 3.7 on Linux"
python: 3.7
env: NODE_GYP_FORCE_PYTHON=python3 EXPERIMENTAL_NODE_GYP_PYTHON3=1
before_install: nvm install 6
- name: "Node.js 8 & Python 3.7 on Linux"
python: 3.7
env: NODE_GYP_FORCE_PYTHON=python3 EXPERIMENTAL_NODE_GYP_PYTHON3=1
before_install: nvm install 8
- name: "Node.js 10 & Python 3.7 on Linux"
python: 3.7
env: NODE_GYP_FORCE_PYTHON=python3 EXPERIMENTAL_NODE_GYP_PYTHON3=1
before_install: nvm install 10
- name: "Node.js 12 & Python 3.7 on Linux"
python: 3.7
env: NODE_GYP_FORCE_PYTHON=python3 EXPERIMENTAL_NODE_GYP_PYTHON3=1
before_install: nvm install 12
- name: "Node.js 12 & Python 3.7 on Windows"
os: windows
language: node_js
node_js: 12 # node
env: >-
PATH=/c/Python37:/c/Python37/Scripts:$PATH
NODE_GYP_FORCE_PYTHON=/c/Python37/python.exe
EXPERIMENTAL_NODE_GYP_PYTHON3=1
before_install: choco install python
allow_failures:
- env: NODE_GYP_FORCE_PYTHON=python3 EXPERIMENTAL_NODE_GYP_PYTHON3=1
- env: >-
PATH=/c/Python37:/c/Python37/Scripts:$PATH
NODE_GYP_FORCE_PYTHON=/c/Python37/python.exe
EXPERIMENTAL_NODE_GYP_PYTHON3=1
install:
#- pip install -r requirements.txt
- pip install flake8 # pytest # add another testing frameworks later
Expand All @@ -45,6 +70,7 @@ before_script:
- flake8 . --count --exit-zero --ignore=E111,E114,W503 --max-complexity=10 --max-line-length=127 --statistics
- npm install
script:
- node -e 'require("npmlog").level="verbose"; require("./lib/find-python")(null,()=>{})'
- npm test
#- pytest --capture=sys # add other tests here
notifications:
Expand Down
286 changes: 2 additions & 284 deletions lib/configure.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@ const fs = require('graceful-fs')
const path = require('path')
const log = require('npmlog')
const os = require('os')
const semver = require('semver')
const mkdirp = require('mkdirp')
const cp = require('child_process')
const extend = require('util')._extend // eslint-disable-line
const processRelease = require('./process-release')
const win = process.platform === 'win32'
const findNodeDirectory = require('./find-node-directory')
const msgFormat = require('util').format
const logWithPrefix = require('./util').logWithPrefix
var findPython = require('./find-python')
if (win) {
var findVisualStudio = require('./find-visualstudio')
}
Expand Down Expand Up @@ -375,287 +372,8 @@ function findAccessibleSync (logprefix, dir, candidates) {
return undefined
}

function PythonFinder (configPython, callback) {
this.callback = callback
this.configPython = configPython
this.errorLog = []
}

PythonFinder.prototype = {
log: logWithPrefix(log, 'find Python'),
argsExecutable: [ '-c', 'import sys; print(sys.executable);' ],
argsVersion: [ '-c', 'import sys; print("%s.%s.%s" % sys.version_info[:3]);' ],
semverRange: '>=2.6.0 <3.0.0',

// These can be overridden for testing:
execFile: cp.execFile,
env: process.env,
win: win,
pyLauncher: 'py.exe',
defaultLocation: path.join(process.env.SystemDrive || 'C:', 'Python27',
'python.exe'),

// Logs a message at verbose level, but also saves it to be displayed later
// at error level if an error occurs. This should help diagnose the problem.
addLog: function addLog (message) {
this.log.verbose(message)
this.errorLog.push(message)
},

// Find Python by trying a sequence of possibilities.
// Ignore errors, keep trying until Python is found.
findPython: function findPython () {
const SKIP = 0; const FAIL = 1
const toCheck = [
{
before: () => {
if (!this.configPython) {
this.addLog(
'Python is not set from command line or npm configuration')
return SKIP
}
this.addLog('checking Python explicitly set from command line or ' +
'npm configuration')
this.addLog('- "--python=" or "npm config get python" is ' +
`"${this.configPython}"`)
},
check: this.checkCommand,
arg: this.configPython
},
{
before: () => {
if (!this.env.PYTHON) {
this.addLog('Python is not set from environment variable PYTHON')
return SKIP
}
this.addLog(
'checking Python explicitly set from environment variable PYTHON')
this.addLog(`- process.env.PYTHON is "${this.env.PYTHON}"`)
},
check: this.checkCommand,
arg: this.env.PYTHON
},
{
before: () => { this.addLog('checking if "python2" can be used') },
check: this.checkCommand,
arg: 'python2'
},
{
before: () => { this.addLog('checking if "python" can be used') },
check: this.checkCommand,
arg: 'python'
},
{
before: () => {
if (!this.win) {
// Everything after this is Windows specific
return FAIL
}
this.addLog(
'checking if the py launcher can be used to find Python 2')
},
check: this.checkPyLauncher
},
{
before: () => {
this.addLog(
'checking if Python 2 is installed in the default location')
},
check: this.checkExecPath,
arg: this.defaultLocation
}
]

function runChecks (err) {
this.log.silly('runChecks: err = %j', (err && err.stack) || err)

const check = toCheck.shift()
if (!check) {
return this.fail()
}

const before = check.before.apply(this)
if (before === SKIP) {
return runChecks.apply(this)
}
if (before === FAIL) {
return this.fail()
}

const args = [ runChecks.bind(this) ]
if (check.arg) {
args.unshift(check.arg)
}
check.check.apply(this, args)
}

runChecks.apply(this)
},

// Check if command is a valid Python to use.
// Will exit the Python finder on success.
// If on Windows, run in a CMD shell to support BAT/CMD launchers.
checkCommand: function checkCommand (command, errorCallback) {
var exec = command
var args = this.argsExecutable
var shell = false
if (this.win) {
// Arguments have to be manually quoted
exec = `"${exec}"`
args = args.map(a => `"${a}"`)
shell = true
}

this.log.verbose(`- executing "${command}" to get executable path`)
this.run(exec, args, shell, function (err, execPath) {
// Possible outcomes:
// - Error: not in PATH, not executable or execution fails
// - Gibberish: the next command to check version will fail
// - Absolute path to executable
if (err) {
this.addLog(`- "${command}" is not in PATH or produced an error`)
return errorCallback(err)
}
this.addLog(`- executable path is "${execPath}"`)
this.checkExecPath(execPath, errorCallback)
}.bind(this))
},

// Check if the py launcher can find a valid Python to use.
// Will exit the Python finder on success.
// Distributions of Python on Windows by default install with the "py.exe"
// Python launcher which is more likely to exist than the Python executable
// being in the $PATH.
// Because the Python launcher supports all versions of Python, we have to
// explicitly request a Python 2 version. This is done by supplying "-2" as
// the first command line argument. Since "py.exe -2" would be an invalid
// executable for "execFile", we have to use the launcher to figure out
// where the actual "python.exe" executable is located.
checkPyLauncher: function checkPyLauncher (errorCallback) {
this.log.verbose(
`- executing "${this.pyLauncher}" to get Python 2 executable path`)
this.run(this.pyLauncher, [ '-2', ...this.argsExecutable ], false,
function (err, execPath) {
// Possible outcomes: same as checkCommand
if (err) {
this.addLog(
`- "${this.pyLauncher}" is not in PATH or produced an error`)
return errorCallback(err)
}
this.addLog(`- executable path is "${execPath}"`)
this.checkExecPath(execPath, errorCallback)
}.bind(this))
},

// Check if a Python executable is the correct version to use.
// Will exit the Python finder on success.
checkExecPath: function checkExecPath (execPath, errorCallback) {
this.log.verbose(`- executing "${execPath}" to get version`)
this.run(execPath, this.argsVersion, false, function (err, version) {
// Possible outcomes:
// - Error: executable can not be run (likely meaning the command wasn't
// a Python executable and the previous command produced gibberish)
// - Gibberish: somehow the last command produced an executable path,
// this will fail when verifying the version
// - Version of the Python executable
if (err) {
this.addLog(`- "${execPath}" could not be run`)
return errorCallback(err)
}
this.addLog(`- version is "${version}"`)

const range = semver.Range(this.semverRange)
var valid = false
try {
valid = range.test(version)
} catch (err) {
this.log.silly('range.test() threw:\n%s', err.stack)
this.addLog(`- "${execPath}" does not have a valid version`)
this.addLog('- is it a Python executable?')
return errorCallback(err)
}

if (!valid) {
this.addLog(`- version is ${version} - should be ${this.semverRange}`)
this.addLog('- THIS VERSION OF PYTHON IS NOT SUPPORTED')
return errorCallback(new Error(
`Found unsupported Python version ${version}`))
}
this.succeed(execPath, version)
}.bind(this))
},

// Run an executable or shell command, trimming the output.
run: function run (exec, args, shell, callback) {
var env = extend({}, this.env)
env.TERM = 'dumb'
const opts = { env: env, shell: shell }

this.log.silly('execFile: exec = %j', exec)
this.log.silly('execFile: args = %j', args)
this.log.silly('execFile: opts = %j', opts)
try {
this.execFile(exec, args, opts, execFileCallback.bind(this))
} catch (err) {
this.log.silly('execFile: threw:\n%s', err.stack)
return callback(err)
}

function execFileCallback (err, stdout, stderr) {
this.log.silly('execFile result: err = %j', (err && err.stack) || err)
this.log.silly('execFile result: stdout = %j', stdout)
this.log.silly('execFile result: stderr = %j', stderr)
if (err) {
return callback(err)
}
const execPath = stdout.trim()
callback(null, execPath)
}
},

succeed: function succeed (execPath, version) {
this.log.info(`using Python version ${version} found at "${execPath}"`)
process.nextTick(this.callback.bind(null, null, execPath))
},

fail: function fail () {
const errorLog = this.errorLog.join('\n')

const pathExample = this.win ? 'C:\\Path\\To\\python.exe'
: '/path/to/pythonexecutable'
// For Windows 80 col console, use up to the column before the one marked
// with X (total 79 chars including logger prefix, 58 chars usable here):
// X
const info = [
'**********************************************************',
'You need to install the latest version of Python 2.7.',
'Node-gyp should be able to find and use Python. If not,',
'you can try one of the following options:',
`- Use the switch --python="${pathExample}"`,
' (accepted by both node-gyp and npm)',
'- Set the environment variable PYTHON',
'- Set the npm configuration variable python:',
` npm config set python "${pathExample}"`,
'For more information consult the documentation at:',
'https://github.com/nodejs/node-gyp#installation',
'**********************************************************'
].join('\n')

this.log.error(`\n${errorLog}\n\n${info}\n`)
process.nextTick(this.callback.bind(null, new Error(
'Could not find any Python 2 installation to use')))
}
}

function findPython (configPython, callback) {
var finder = new PythonFinder(configPython, callback)
finder.findPython()
}

module.exports = configure
module.exports.test = {
PythonFinder: PythonFinder,
findAccessibleSync: findAccessibleSync,
findPython: findPython
findAccessibleSync: findAccessibleSync
}
module.exports.usage = 'Generates ' + (win ? 'MSVC project files' : 'a Makefile') + ' for the current module'
Loading