From 66ad30577594a37b08939948db1d4f1018d85335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Reis?= Date: Mon, 8 Jul 2019 22:09:45 +0100 Subject: [PATCH] python: accept Python 3 conditionally This allows us to start testing Python 3 without breaking node-gyp for users. This also adds support for NODE_GYP_FORCE_PYTHON, which will be the only Python binary acceptable when defined. PR-URL: https://github.com/nodejs/node-gyp/pull/1815 Reviewed-By: Christian Clauss Reviewed-By: Richard Lau --- .travis.yml | 30 ++++++++- lib/find-python.js | 141 +++++++++++++++++++++++---------------- test/test-find-python.js | 131 +----------------------------------- 3 files changed, 114 insertions(+), 188 deletions(-) diff --git a/.travis.yml b/.travis.yml index 54a13a9382..a6dd2dcf28 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,36 +4,61 @@ cache: pip matrix: include: - name: "Python 2.7 on Linux" + env: NODE_GYP_FORCE_PYTHON=python2 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 @@ -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: diff --git a/lib/find-python.js b/lib/find-python.js index ac16d477b1..1a4390dbd2 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -18,15 +18,18 @@ 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', + semverRange: process.env.EXPERIMENTAL_NODE_GYP_PYTHON3 ? '2.7.x || >=3.5.0' + : '>=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'), + winDefaultLocations: [ + path.join(process.env.SystemDrive || 'C:', 'Python27', 'python.exe'), + path.join(process.env.SystemDrive || 'C:', 'Python37', '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. @@ -39,65 +42,87 @@ PythonFinder.prototype = { // Ignore errors, keep trying until Python is found. findPython: function findPython () { const SKIP = 0; const FAIL = 1 - const toCheck = [ - { - before: () => { - if (!this.configPython) { + var toCheck = getChecks.apply(this) + + function getChecks () { + if (this.env.NODE_GYP_FORCE_PYTHON) { + return [ { + before: () => { 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}"`) + 'checking Python explicitly set from NODE_GYP_FORCE_PYTHON') + this.addLog('- process.env.NODE_GYP_FORCE_PYTHON is ' + + `"${this.env.NODE_GYP_FORCE_PYTHON}"`) + }, + check: this.checkCommand, + arg: this.env.NODE_GYP_FORCE_PYTHON + } ] + } + + var checks = [ + { + 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 }, - 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') + { + 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 }, - check: this.checkPyLauncher - }, - { - before: () => { - this.addLog( - 'checking if Python 2 is installed in the default location') + { + before: () => { this.addLog('checking if "python" can be used') }, + check: this.checkCommand, + arg: 'python' }, - check: this.checkExecPath, - arg: this.defaultLocation + { + before: () => { this.addLog('checking if "python2" can be used') }, + check: this.checkCommand, + arg: 'python2' + } + ] + + if (this.win) { + checks.push({ + before: () => { + this.addLog( + 'checking if the py launcher can be used to find Python 2') + }, + check: this.checkPyLauncher + }) + for (var i = 0; i < this.winDefaultLocations.length; ++i) { + const location = this.winDefaultLocations[i] + checks.push({ + before: () => { + this.addLog('checking if Python is ' + + `${location}`) + }, + check: this.checkExecPath, + arg: location + }) + } } - ] + + return checks + } function runChecks (err) { this.log.silly('runChecks: err = %j', (err && err.stack) || err) @@ -276,7 +301,7 @@ PythonFinder.prototype = { 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'))) + 'Could not find any Python installation to use'))) } } diff --git a/test/test-find-python.js b/test/test-find-python.js index cf8800f49a..c52a579666 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -5,6 +5,9 @@ const findPython = require('../lib/find-python') const execFile = require('child_process').execFile const PythonFinder = findPython.test.PythonFinder +delete process.env.PYTHON +delete process.env.NODE_GYP_FORCE_PYTHON + require('npmlog').level = 'warn' test('find python', function (t) { @@ -94,27 +97,6 @@ test('find python - python too old', function (t) { } }) -test('find python - python too new', function (t) { - t.plan(2) - - var f = new TestPythonFinder(null, done) - f.execFile = function (program, args, opts, cb) { - if (/sys\.executable/.test(args[args.length - 1])) { - cb(null, '/path/python') - } else if (/sys\.version_info/.test(args[args.length - 1])) { - cb(null, '3.0.0') - } else { - t.fail() - } - } - f.findPython() - - function done (err) { - t.ok(/Could not find any Python/.test(err)) - t.ok(/not supported/i.test(f.errorLog)) - } -}) - test('find python - no python', function (t) { t.plan(2) @@ -136,31 +118,6 @@ test('find python - no python', function (t) { } }) -test('find python - no python2', function (t) { - t.plan(2) - - var f = new TestPythonFinder(null, done) - f.execFile = function (program, args, opts, cb) { - if (/sys\.executable/.test(args[args.length - 1])) { - if (program === 'python2') { - cb(new Error('not found')) - } else { - cb(null, '/path/python') - } - } else if (/sys\.version_info/.test(args[args.length - 1])) { - cb(null, '2.7.14') - } else { - t.fail() - } - } - f.findPython() - - function done (err, python) { - t.strictEqual(err, null) - t.strictEqual(python, '/path/python') - } -}) - test('find python - no python2, no python, unix', function (t) { t.plan(2) @@ -215,88 +172,6 @@ test('find python - no python, use python launcher', function (t) { } }) -test('find python - python 3, use python launcher', function (t) { - t.plan(4) - - var f = new TestPythonFinder(null, done) - f.win = true - - f.execFile = function (program, args, opts, cb) { - if (program === 'py.exe') { - f.execFile = function (program, args, opts, cb) { - poison(f, 'execFile') - if (/sys\.version_info/.test(args[args.length - 1])) { - cb(null, '2.7.14') - } else { - t.fail() - } - } - t.notEqual(args.indexOf('-2'), -1) - t.notEqual(args.indexOf('-c'), -1) - return cb(null, 'Z:\\snake.exe') - } - if (/sys\.executable/.test(args[args.length - 1])) { - cb(null, '/path/python') - } else if (/sys\.version_info/.test(args[args.length - 1])) { - cb(null, '3.0.0') - } else { - t.fail() - } - } - f.findPython() - - function done (err, python) { - t.strictEqual(err, null) - t.strictEqual(python, 'Z:\\snake.exe') - } -}) - -test('find python - python 3, use python launcher, python 2 too old', - function (t) { - t.plan(6) - - var f = new TestPythonFinder(null, done) - f.win = true - - f.execFile = function (program, args, opts, cb) { - if (program === 'py.exe') { - f.execFile = function (program, args, opts, cb) { - if (/sys\.version_info/.test(args[args.length - 1])) { - f.execFile = function (program, args, opts, cb) { - if (/sys\.version_info/.test(args[args.length - 1])) { - poison(f, 'execFile') - t.strictEqual(program, f.defaultLocation) - cb(new Error('not found')) - } else { - t.fail() - } - } - t.strictEqual(program, 'Z:\\snake.exe') - cb(null, '2.3.4') - } else { - t.fail() - } - } - t.notEqual(args.indexOf('-2'), -1) - t.notEqual(args.indexOf('-c'), -1) - return cb(null, 'Z:\\snake.exe') - } - if (/sys\.executable/.test(args[args.length - 1])) { - cb(null, '/path/python') - } else if (/sys\.version_info/.test(args[args.length - 1])) { - cb(null, '3.0.0') - } else { - t.fail() - } - } - f.findPython() - - function done (err) { - t.ok(/Could not find any Python/.test(err)) - t.ok(/not supported/i.test(f.errorLog)) - } - }) - test('find python - no python, no python launcher, good guess', function (t) { t.plan(4)