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: #1815 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Richard Lau <[email protected]>
nodejs · Jul 11, 2019 · 66ad305 · 66ad305
1 parent 7e7fce3
commit 66ad305
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 188 deletions.
diff --git 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
@@ -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')))
   }
 }