From 42d1bf2d11122ed18dfb2a380f9644231b9c2ecd Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Fri, 6 Nov 2020 13:19:30 +0300 Subject: [PATCH 01/25] lib, gyp: add support for non-eanglish letters in pathes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows 10 i discovered an issue that if there are non-english letters or symbols in path for python find-python.js script can't find it. This bug is couse by encoding issue which I have (i hope) fixed. At least this bug fix works for me. Have changed: modified: gyp/pylib/gyp/easy_xml.py python 2.7 handle xml_string in wrong way becouse of encoding (line 128) modified: gyp/pylib/gyp/input.py if "encoding:utf8" on line 240 doesn't marked cause to error new file: lib/find-python-script.py i have created this file for convience. Script which can handle non-eanglish letters and symbols cann't °fit one lne becouse it is necessary to specify instarctions for several versions of python modified: lib/find-python.js to make js understand python (line 249) --- gyp/pylib/gyp/easy_xml.py | 6 +++++- lib/find-python-script.py | 11 +++++++++++ lib/find-python.js | 5 +++-- 3 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 lib/find-python-script.py diff --git a/gyp/pylib/gyp/easy_xml.py b/gyp/pylib/gyp/easy_xml.py index e475b5530c..399d872b4f 100644 --- a/gyp/pylib/gyp/easy_xml.py +++ b/gyp/pylib/gyp/easy_xml.py @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import sys import re import os import locale @@ -121,7 +122,10 @@ def WriteXmlIfChanged(content, path, encoding="utf-8", pretty=False, win32=False default_encoding = locale.getdefaultlocale()[1] if default_encoding and default_encoding.upper() != encoding.upper(): - xml_string = xml_string.encode(encoding) + if sys.platform == "win32" and sys.version_info < (3, 7): + xml_string = xml_string.decode("cp1251").encode(encoding) + else: + xml_string = xml_string.encode(encoding) # Get the old content try: diff --git a/lib/find-python-script.py b/lib/find-python-script.py new file mode 100644 index 0000000000..170ae9950d --- /dev/null +++ b/lib/find-python-script.py @@ -0,0 +1,11 @@ +import sys, codecs; + +if (sys.stdout.encoding != "utf-8" and sys.platform == "win32"): + if sys.version_info > (3, 7): + sys.stdout.reconfigure(encoding='utf-8') + print(sys.executable) + else: + sys.stdout = codecs.getwriter("utf8")(sys.stdout) + print(sys.executable.decode("cp1251")) +else: + print(sys.executable) \ No newline at end of file diff --git a/lib/find-python.js b/lib/find-python.js index e6464d12c6..0be7658fa6 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -1,5 +1,6 @@ 'use strict' +const path = require('path') const log = require('npmlog') const semver = require('semver') const cp = require('child_process') @@ -47,7 +48,7 @@ function PythonFinder (configPython, callback) { PythonFinder.prototype = { log: logWithPrefix(log, 'find Python'), - argsExecutable: ['-c', 'import sys; print(sys.executable);'], + argsExecutable: [path.resolve(__dirname, 'find-python-script.py')], argsVersion: ['-c', 'import sys; print("%s.%s.%s" % sys.version_info[:3]);'], semverRange: '>=3.6.0', @@ -274,7 +275,7 @@ PythonFinder.prototype = { run: function run (exec, args, shell, callback) { var env = extend({}, this.env) env.TERM = 'dumb' - const opts = { env: env, shell: shell } + const opts = { env: env, shell: shell, encoding: 'utf8' } this.log.silly('execFile: exec = %j', exec) this.log.silly('execFile: args = %j', args) From 0f499bda062f2e99d743ff2b07a051b10a2e6d68 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Sun, 22 Nov 2020 21:00:44 +0300 Subject: [PATCH 02/25] test: add unit test for find-python-script.py created file test-find-python-script.py --- test/test-find-python-script.js | 83 +++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 test/test-find-python-script.js diff --git a/test/test-find-python-script.js b/test/test-find-python-script.js new file mode 100644 index 0000000000..b1afd05b14 --- /dev/null +++ b/test/test-find-python-script.js @@ -0,0 +1,83 @@ +// @ts-check +'use strict' +/** @typedef {import("tap")} Tap */ + +const test = require('tap').test +const execFile = require('child_process').execFile +const path = require('path') + +require('npmlog').level = 'warn' + +//* can use name as short descriptions + +/** + * @typedef Check + * @property {string} path - path to execurtable or command + * @property {string} name - very little description + */ + +/** + * @type {Check[]} + */ +const checks = [ + { path: process.env.PYTHON, name: 'env var PYTHON' }, + { path: process.env.python2, name: 'env var python2' }, + { path: 'python3', name: 'env var python3' } +] +const args = [path.resolve('./lib/find-python-script.py')] +const options = { + windowsHide: true +} + +/** + Getting output from find-python-script.py, + compare it to path provided to terminal. + If equale - test pass + + runs for all checks + + @private + @argument {Error} err - exec error + @argument {string} stdout - stdout buffer of child process + @argument {string} stderr + @this {{t, exec: Check}} + */ +function check (err, stdout, stderr) { + const { t, exec } = this + if (!err && !stderr) { + t.strictEqual( + stdout.trim(), + exec.path, + `${exec.name}: check path ${exec.path} equals ${stdout.trim()}` + ) + } else { + // @ts-ignore + if (err.code === 9009) { + t.skip(`skipped: ${exec.name} file not found`) + } else { + t.fail(`error: ${err}\n\nstderr: ${stderr}`) + } + } +} + +test('find-python-script', (t) => { + t.plan(checks.length) + + // context for check functions + const ctx = { + t: t, + exec: {} + } + + for (const exec of checks) { + // checking if env var exist + if (!(exec.path === undefined || exec.path === null)) { + ctx.exec = exec + // passing ctx as coppied object to make properties immutable from here + const boundedCheck = check.bind(Object.assign({}, ctx)) + execFile(exec.path, args, options, boundedCheck) + } else { + t.skip(`skipped: ${exec.name} doesn't exist or unavailable`) + } + } +}) From f18491ea99287265c88d855da7b5de6173b45281 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Thu, 24 Dec 2020 12:37:01 +0300 Subject: [PATCH 03/25] lib: reinvent find-python.js script now this script using promises and es6 classes !!!API HASN'T CHANGE add documentation make better logging (colorized output) make more convenient error handling (one function that catch all errors) --- lib/find-python-script.py | 10 +- lib/find-python.js | 345 --------------------- lib/new-find-python.js | 635 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 638 insertions(+), 352 deletions(-) delete mode 100644 lib/find-python.js create mode 100644 lib/new-find-python.js diff --git a/lib/find-python-script.py b/lib/find-python-script.py index 170ae9950d..5cf2979d25 100644 --- a/lib/find-python-script.py +++ b/lib/find-python-script.py @@ -1,11 +1,7 @@ -import sys, codecs; +import sys; if (sys.stdout.encoding != "utf-8" and sys.platform == "win32"): - if sys.version_info > (3, 7): - sys.stdout.reconfigure(encoding='utf-8') - print(sys.executable) - else: - sys.stdout = codecs.getwriter("utf8")(sys.stdout) - print(sys.executable.decode("cp1251")) + sys.stdout.reconfigure(encoding='utf-8') + print(sys.executable) else: print(sys.executable) \ No newline at end of file diff --git a/lib/find-python.js b/lib/find-python.js deleted file mode 100644 index 0be7658fa6..0000000000 --- a/lib/find-python.js +++ /dev/null @@ -1,345 +0,0 @@ -'use strict' - -const path = require('path') -const log = require('npmlog') -const semver = require('semver') -const cp = require('child_process') -const extend = require('util')._extend // eslint-disable-line -const win = process.platform === 'win32' -const logWithPrefix = require('./util').logWithPrefix - -const systemDrive = process.env.SystemDrive || 'C:' -const username = process.env.USERNAME || process.env.USER || getOsUserInfo() -const localAppData = process.env.LOCALAPPDATA || `${systemDrive}\\${username}\\AppData\\Local` -const foundLocalAppData = process.env.LOCALAPPDATA || username -const programFiles = process.env.ProgramW6432 || process.env.ProgramFiles || `${systemDrive}\\Program Files` -const programFilesX86 = process.env['ProgramFiles(x86)'] || `${programFiles} (x86)` - -const winDefaultLocationsArray = [] -for (const majorMinor of ['39', '38', '37', '36']) { - if (foundLocalAppData) { - winDefaultLocationsArray.push( - `${localAppData}\\Programs\\Python\\Python${majorMinor}\\python.exe`, - `${programFiles}\\Python${majorMinor}\\python.exe`, - `${localAppData}\\Programs\\Python\\Python${majorMinor}-32\\python.exe`, - `${programFiles}\\Python${majorMinor}-32\\python.exe`, - `${programFilesX86}\\Python${majorMinor}-32\\python.exe` - ) - } else { - winDefaultLocationsArray.push( - `${programFiles}\\Python${majorMinor}\\python.exe`, - `${programFiles}\\Python${majorMinor}-32\\python.exe`, - `${programFilesX86}\\Python${majorMinor}-32\\python.exe` - ) - } -} - -function getOsUserInfo () { - try { - return require('os').userInfo().username - } catch {} -} - -function PythonFinder (configPython, callback) { - this.callback = callback - this.configPython = configPython - this.errorLog = [] -} - -PythonFinder.prototype = { - log: logWithPrefix(log, 'find Python'), - argsExecutable: [path.resolve(__dirname, 'find-python-script.py')], - argsVersion: ['-c', 'import sys; print("%s.%s.%s" % sys.version_info[:3]);'], - semverRange: '>=3.6.0', - - // These can be overridden for testing: - execFile: cp.execFile, - env: process.env, - win: win, - pyLauncher: 'py.exe', - winDefaultLocations: winDefaultLocationsArray, - - // 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 - var toCheck = getChecks.apply(this) - - function getChecks () { - if (this.env.NODE_GYP_FORCE_PYTHON) { - return [{ - before: () => { - this.addLog( - '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 - }, - { - 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 "python3" can be used') }, - check: this.checkCommand, - arg: 'python3' - }, - { - before: () => { this.addLog('checking if "python" can be used') }, - check: this.checkCommand, - arg: 'python' - } - ] - - if (this.win) { - 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 - }) - } - checks.push({ - before: () => { - this.addLog( - 'checking if the py launcher can be used to find Python 3') - }, - check: this.checkPyLauncher - }) - } - - return checks - } - - 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 Python 2 and Python 3, we should - // explicitly request a Python 3 version. This is done by supplying "-3" as - // the first command line argument. Since "py.exe -3" 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 3 executable path`) - this.run(this.pyLauncher, ['-3', ...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 = new 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, encoding: 'utf8' } - - 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.', - '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 installation to use'))) - } -} - -function findPython (configPython, callback) { - var finder = new PythonFinder(configPython, callback) - finder.findPython() -} - -module.exports = findPython -module.exports.test = { - PythonFinder: PythonFinder, - findPython: findPython -} diff --git a/lib/new-find-python.js b/lib/new-find-python.js new file mode 100644 index 0000000000..61889325d9 --- /dev/null +++ b/lib/new-find-python.js @@ -0,0 +1,635 @@ +'use strict' +// @ts-check + +const path = require('path') +const log = require('npmlog') +const semver = require('semver') +const cp = require('child_process') +const extend = require('util')._extend // eslint-disable-line +const win = process.platform === 'win32' +const logWithPrefix = require('./util').logWithPrefix + +const systemDrive = process.env.SystemDrive || 'C:' +const username = process.env.USERNAME || process.env.USER || getOsUserInfo() +const localAppData = process.env.LOCALAPPDATA || `${systemDrive}\\${username}\\AppData\\Local` +const foundLocalAppData = process.env.LOCALAPPDATA || username +const programFiles = process.env.ProgramW6432 || process.env.ProgramFiles || `${systemDrive}\\Program Files` +const programFilesX86 = process.env['ProgramFiles(x86)'] || `${programFiles} (x86)` + +const winDefaultLocationsArray = [] +for (const majorMinor of ['39', '38', '37', '36']) { + if (foundLocalAppData) { + winDefaultLocationsArray.push( + `${localAppData}\\Programs\\Python\\Python${majorMinor}\\python.exe`, + `${programFiles}\\Python${majorMinor}\\python.exe`, + `${localAppData}\\Programs\\Python\\Python${majorMinor}-32\\python.exe`, + `${programFiles}\\Python${majorMinor}-32\\python.exe`, + `${programFilesX86}\\Python${majorMinor}-32\\python.exe` + ) + } else { + winDefaultLocationsArray.push( + `${programFiles}\\Python${majorMinor}\\python.exe`, + `${programFiles}\\Python${majorMinor}-32\\python.exe`, + `${programFilesX86}\\Python${majorMinor}-32\\python.exe` + ) + } +} + +function getOsUserInfo () { + try { + return require('os').userInfo().username + } catch {} +} + +//! after editing file dont forget run "npm test" and +//! change tests for this file if needed + +// ? may be some addition info in silly and verbose levels +// ? add safety to colorizeOutput function. E.g. when terminal doesn't +// ? support colorizing, just disable it (return given string) +// i hope i made not bad error handling but may be some improvements would be nice +// TODO: better error handler on linux/macOS + +const RED = '\x1b[31m' +const RESET = '\x1b[0m' +const GREEN = '\x1b[32m' + +/** + * Paint (not print, just colorize) string with selected color + * + * @param color color to set: RED or GREEN + * @param {string} string string to colorize + */ +function colorizeOutput (color, string) { + return color + string + RESET +} + +//! on windows debug running with locale cmd (e. g. chcp 866) encoding +// to avoid that uncoment next lines +// locale encdoings couse issues. See run func for more info +// this lines only for testing +// win ? cp.execSync("chcp 65001") : null +// log.level = "silly"; + +/** + * @class + */ +class PythonFinder { + /** + * + * @param {string} configPython force setted from terminal or npm config python path + * @param {(err:Error, found:string) => void} callback succsed/error callback from where result + * is available + */ + constructor (configPython, callback) { + this.callback = callback + this.configPython = configPython + this.errorLog = [] + + this.catchErrors = this.catchErrors.bind(this) + this.checkExecPath = this.checkExecPath.bind(this) + this.succeed = this.succeed.bind(this) + + this.SKIP = 0 + this.FAIL = 1 + + this.log = logWithPrefix(log, 'find Python') + + this.argsExecutable = [path.resolve(__dirname, 'find-python-script.py')] + this.argsVersion = [ + '-c', + 'import sys; print("%s.%s.%s" % sys.version_info[:3]);' + // for testing + // 'print("2.1.1")' + ] + this.semverRange = '>=3.6.0' + // These can be overridden for testing: + this.execFile = cp.execFile + this.env = process.env + this.win = win + this.pyLauncher = 'py.exe' + this.winDefaultLocations = winDefaultLocationsArray + } + + /** + * 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. + * + * ?message is array or one string + * + * @private + */ + 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. + * + * @public + */ + findPython () { + this.toCheck = this.getChecks() + + this.runChecks(this.toCheck) + } + + /** + * Getting list of checks which should be cheked + * + * @private + * @returns {check[]} + */ + getChecks () { + if (this.env.NODE_GYP_FORCE_PYTHON) { + /** + * @type {check[]} + */ + return [ + { + before: () => { + this.addLog( + 'checking Python explicitly set from NODE_GYP_FORCE_PYTHON' + ) + this.addLog( + '- process.env.NODE_GYP_FORCE_PYTHON is ' + + `"${this.env.NODE_GYP_FORCE_PYTHON}"` + ) + }, + checkFunc: this.checkCommand, + arg: this.env.NODE_GYP_FORCE_PYTHON + } + ] + } + + /** + * @type {check[]} + */ + const checks = [ + { + before: (name) => { + if (!this.configPython) { + this.addLog( + `${colorizeOutput( + GREEN, + 'Python is not set from command line or npm configuration' + )}` + ) + this.addLog('') + return this.SKIP + } + this.addLog( + 'checking Python explicitly set from command line or ' + + 'npm configuration' + ) + this.addLog( + '- "--python=" or "npm config get python" is ' + + `"${colorizeOutput(GREEN, this.configPython)}"` + ) + }, + checkFunc: this.checkCommand, + arg: this.configPython + }, + { + before: (name) => { + if (!this.env.PYTHON) { + this.addLog( + `Python is not set from environment variable ${colorizeOutput( + GREEN, + 'PYTHON' + )}` + ) + return this.SKIP + } + this.addLog( + 'checking Python explicitly set from environment ' + + 'variable PYTHON' + ) + this.addLog( + `${colorizeOutput( + GREEN, + 'process.env.PYTHON' + )} is "${colorizeOutput(GREEN, this.env.PYTHON)}"` + ) + }, + checkFunc: this.checkCommand, + arg: this.env.PYTHON, + // name used as very short description + name: 'process.env.PYTHON' + }, + { + checkFunc: this.checkCommand, + name: 'python3', + arg: 'python3' + }, + { + checkFunc: this.checkCommand, + name: 'python', + arg: 'python' + }, + ] + + if (this.win) { + for (let i = 0; i < this.winDefaultLocations.length; ++i) { + const location = this.winDefaultLocations[i] + checks.push({ + before: () => { + this.addLog( + `checking if Python is ${colorizeOutput(GREEN, location)}` + ) + }, + checkFunc: this.checkExecPath, + arg: location + }) + } + checks.push({ + before: () => { + this.addLog( + `checking if the ${colorizeOutput( + GREEN, + 'py launcher' + )} can be used to find Python` + ) + }, + checkFunc: this.checkPyLauncher, + name: 'py Launcher' + }) + } + + return checks + } + + /** + * Type for possible place where python is + * + * @typedef check + * @type {object} + * @property {(name: string) => number|void} [before] + * @property {function} checkFunc + * @property {*} [arg] + * @property {string} [name] + */ + + /** + * + * + * @private + * @argument {check[]} checks + */ + async runChecks (checks) { + // using this flag becouse Fail is happen when ALL checks fail + let fail = true + + for (const check of checks) { + if (check.before) { + const beforeResult = check.before.apply(this) + + // if pretask fail - skip + if (beforeResult === this.SKIP || beforeResult === this.FAIL) { + // ?optional + // TODO: write to result arr which tests is SKIPPED + continue + } + } + + try { + if (!check.before) { + this.addLog( + `checking if ${colorizeOutput( + GREEN, + check.name || check.arg + )} can be used` + ) + } + + this.log.verbose( + `executing "${colorizeOutput( + GREEN, + check.name || check.arg + )}" to get Python executable path` + ) + + const result = await check.checkFunc.apply(this, [check ? check.arg : null]) + fail = false + this.succeed(result.path, result.version) + + break + } catch (err) { + this.catchErrors(err, check) + } + } + + if (fail) { + this.fail() + } + } + + /** + * 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. + * + * @private + * @argument {string} command command which will be executed in shell + * @returns {Promise} + */ + checkCommand (command) { + let exec = command + let args = this.argsExecutable + let shell = false + if (this.win) { + // Arguments have to be manually quoted to avoid bugs with spaces in pathes + exec = `"${exec}"` + args = args.map((a) => `"${a}"`) + shell = true + } + + return new Promise((resolve, reject) => { + this.run(exec, args, shell).then(this.checkExecPath).then(resolve).catch(reject) + }) + } + + /** + * 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. + * + * @private + * @returns {Promise} + */ + checkPyLauncher () { + return new Promise((resolve, reject) => { + this.run(this.pyLauncher, this.argsExecutable, false) + .then(this.checkExecPath) + .then(resolve) + .catch(reject) + }) + } + + /** + * + * Check if a getted path is correct and + * Python executable hase the correct version to use. + * + * @private + * @argument {string} execPath path to check + * @returns {Promise} + */ + checkExecPath (execPath) { + // Returning new Promise instead of forwarding existing + // to pass both path and version + return new Promise((resolve, reject) => { + this.log.verbose(`- executing "${execPath}" to get version`) + this.run(execPath, this.argsVersion, false) + .then((ver) => { + // ? may be better code for version check + // ? may be move log messgaes to catchError func + const range = new semver.Range(this.semverRange) + let valid = false + + try { + valid = range.test(ver) + // throw new Error("test error") + } catch (err) { + this.log.silly(`range.test() threw:\n${err.stack}`) + this.addLog( + `"${colorizeOutput(RED, execPath)}" does not have a valid version` + ) + this.addLog('is it a Python executable?') + + reject(err) + } + + if (!valid) { + this.addLog( + `version is ${colorizeOutput( + RED, + ver + )} - should be ${colorizeOutput(RED, this.semverRange)}` + ) + this.addLog( + colorizeOutput(RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') + ) + // object with error passed for conveniences + //(becouse we can also pass stderr or some additional staff) + // eslint-disable-next-line prefer-promise-reject-errors + reject({ err: new Error(`Found unsupported Python version ${ver}`) }) + } + + resolve({ path: execPath, version: ver }) + }) + .catch(reject) + }) + } + + /** + * Run an executable or shell command, trimming the output. + * + * @private + * @argument {string} exec command or path without arguments to execute + * @argument {string[]} args command args + * @argument {boolean} shell need be documented + * @returns {Promise} + */ + run (exec, args, shell) { + return new Promise( + /** + * @this {PythonFinder} + * @argument {function} resolve + * @argument {function} reject + */ + function (resolve, reject) { + const env = extend({}, this.env) + env.TERM = 'dumb' + const opts = { env: env, shell: shell } + + this.log.verbose( + `${colorizeOutput(GREEN, 'execFile')}: exec = %j`, + exec + ) + this.log.verbose( + `${colorizeOutput(GREEN, 'execFile')}: args = %j`, + args + ) + this.log.silly('execFile: opts = ', JSON.stringify(opts, null, 2)) + + //* assume that user use utf8 compatible termnal + + //* prosible outcomes with error messages (err.message, error.stack, stderr) + //! on windows: + // issue of encoding (garbage in terminal ) when 866 or any other locale code + // page is setted + // possible solutions: + // use "cmd" command with flag "/U" and "/C" (for more informatiom help cmd) + // which "Causes the output of + // internal commands to a pipe or file to be Unicode" (utf16le) + //* note: find-python-script.py send output in utf8 then may become necessary + //* to reencoded string with Buffer.from(stderr).toString() or something + //* similar (if needed) + // for this solution all args should be passed as SINGLE string in quotes + // becouse cmd has such signature: CMD [/A | /U] [/Q] [/D] [/E:ON | /E:OFF] + // [/F:ON | /F:OFF] [/V:ON | /V:OFF] [[/S] [/C | /K] string] + //* all pathes/commands and each argument must be in quotes becouse if they have + //* spaces they will broke everything + this.execFile(exec, args, opts, execFileCallback.bind(this)) + + /** + * + * @param {Error} err + * @param {string} stdout + * @param {string} stderr + */ + function execFileCallback (err, stdout, stderr) { + this.log.silly( + `${colorizeOutput(RED, 'execFile result')}: err =`, + (err && err.stack) || err + ) + + // executed script shouldn't pass anythong to stderr if successful + if (err || stderr) { + reject({ err: err || null, stderr: stderr || null }) + } else { + // trim function removing endings which couse bugs when comparing strings + const stdoutTrimed = stdout.trim() + resolve(stdoutTrimed) + } + } + }.bind(this) + ) + } + + /** + * Main error handling function in module + * Promises should throw errors up to this function + * Also used for logging + * + * @private + * TODO: figure out err type + * @param {{err: Error, stderr: string}} error + * @param {check} check + */ + catchErrors (error, check) { + const { err, stderr } = error + + this.addLog(colorizeOutput(RED, `FAIL: ${check.name || check.arg}`)) + + // array of error codes (type of errors) that we handling + const catchedErrorsCods = ['ENOENT', 9009] + + // dont know type of terminal errors + // @ts-ignore + if (catchedErrorsCods.includes(err ? err.code : undefined)) { + // @ts-ignore + switch (err ? err.code : undefined) { + case 'ENOENT': + this.addLog( + `${colorizeOutput( + RED, + 'ERROR:' + // @ts-ignore + )} No such file or directory: ${colorizeOutput(RED, err.path)}` + ) + break + + case 9009: + this.addLog( + `${colorizeOutput( + RED, + 'ERROR:' + )} Command failed: file not found or not in PATH` + ) + break + } + } else { + this.addLog(`${colorizeOutput(RED, 'ERROR:')} ${err ? err.message : ''}`) + this.log.silly(err ? err.stack : '') + + if (stderr) { + this.addLog(`${colorizeOutput(RED, 'STDERR:')} ${stderr ? stderr.trim() : ''}`) + } + } + this.addLog('--------------------------------------------') + } + + /** + * Function which is called if python path founded + * + * @private + * @param {string} execPath founded path + * @param {string} version python version + */ + succeed (execPath, version) { + this.log.info( + `using Python version ${colorizeOutput( + GREEN, + version + )} found at "${colorizeOutput(GREEN, execPath)}"` + ) + process.nextTick(this.callback.bind(null, null, execPath)) + } + + /** + * @private + */ + fail () { + const errorLog = this.errorLog.map((str) => str.trim()).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 = [ + '**********************************************************', + 'If you have non-displayed characters set "UTF-8" encoding.', + 'You need to install the latest version of Python.', + '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, + // if changing error message dont forget also change it test file too + new Error('Could not find any Python installation to use') + ) + ) + } +} + +/** + * + * @param {string} configPython force setted from terminal or npm config python path + * @param {(err:Error, found:string)=> void} callback succsed/error callback from where result + * is available + */ +function findPython (configPython, callback) { + const finder = new PythonFinder(configPython, callback) + finder.findPython() +} + +// function for tests +/* findPython(null, (err, found) => { + console.log('found:', '\x1b[31m', found) + console.log('\x1b[0m') +}) + */ +module.exports = findPython +module.exports.test = { + PythonFinder: PythonFinder, + findPython: findPython +} From debeac8f79ff02763f126c8a2da6c1f4147a06f5 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Thu, 24 Dec 2020 12:38:40 +0300 Subject: [PATCH 04/25] test: add test for lib/new-find-python.js also remove old test for find-python --- test/test-find-python.js | 226 ---------------------------------- test/test-new-find-python.js | 228 +++++++++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 226 deletions(-) delete mode 100644 test/test-find-python.js create mode 100644 test/test-new-find-python.js diff --git a/test/test-find-python.js b/test/test-find-python.js deleted file mode 100644 index 67d0b2664f..0000000000 --- a/test/test-find-python.js +++ /dev/null @@ -1,226 +0,0 @@ -'use strict' - -delete process.env.PYTHON - -const test = require('tap').test -const findPython = require('../lib/find-python') -const execFile = require('child_process').execFile -const PythonFinder = findPython.test.PythonFinder - -require('npmlog').level = 'warn' - -test('find python', function (t) { - t.plan(4) - - findPython.test.findPython(null, function (err, found) { - t.strictEqual(err, null) - var proc = execFile(found, ['-V'], function (err, stdout, stderr) { - t.strictEqual(err, null) - t.ok(/Python 3/.test(stdout)) - t.strictEqual(stderr, '') - }) - proc.stdout.setEncoding('utf-8') - proc.stderr.setEncoding('utf-8') - }) -}) - -function poison (object, property) { - function fail () { - console.error(Error(`Property ${property} should not have been accessed.`)) - process.abort() - } - var descriptor = { - configurable: false, - enumerable: false, - get: fail, - set: fail - } - Object.defineProperty(object, property, descriptor) -} - -function TestPythonFinder () { - PythonFinder.apply(this, arguments) -} -TestPythonFinder.prototype = Object.create(PythonFinder.prototype) -// Silence npmlog - remove for debugging -TestPythonFinder.prototype.log = { - silly: () => {}, - verbose: () => {}, - info: () => {}, - warn: () => {}, - error: () => {} -} -delete TestPythonFinder.prototype.env.NODE_GYP_FORCE_PYTHON - -test('find python - python', function (t) { - t.plan(6) - - var f = new TestPythonFinder('python', done) - f.execFile = function (program, args, opts, cb) { - f.execFile = function (program, args, opts, cb) { - poison(f, 'execFile') - t.strictEqual(program, '/path/python') - t.ok(/sys\.version_info/.test(args[1])) - cb(null, '3.9.1') - } - t.strictEqual(program, - process.platform === 'win32' ? '"python"' : 'python') - t.ok(/sys\.executable/.test(args[1])) - cb(null, '/path/python') - } - f.findPython() - - function done (err, python) { - t.strictEqual(err, null) - t.strictEqual(python, '/path/python') - } -}) - -test('find python - python too old', 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, '2.3.4') - } 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) - - var f = new TestPythonFinder(null, done) - f.execFile = function (program, args, opts, cb) { - if (/sys\.executable/.test(args[args.length - 1])) { - cb(new Error('not found')) - } else if (/sys\.version_info/.test(args[args.length - 1])) { - cb(new Error('not a Python executable')) - } else { - t.fail() - } - } - f.findPython() - - function done (err) { - t.ok(/Could not find any Python/.test(err)) - t.ok(/not in PATH/.test(f.errorLog)) - } -}) - -test('find python - no python2, no python, unix', function (t) { - t.plan(2) - - var f = new TestPythonFinder(null, done) - f.checkPyLauncher = t.fail - f.win = false - - f.execFile = function (program, args, opts, cb) { - if (/sys\.executable/.test(args[args.length - 1])) { - cb(new Error('not found')) - } else { - t.fail() - } - } - f.findPython() - - function done (err) { - t.ok(/Could not find any Python/.test(err)) - t.ok(/not in PATH/.test(f.errorLog)) - } -}) - -test('find python - no python, 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') { - t.notEqual(args.indexOf('-3'), -1) - t.notEqual(args.indexOf('-c'), -1) - return cb(null, 'Z:\\snake.exe') - } - if (/sys\.executable/.test(args[args.length - 1])) { - cb(new Error('not found')) - } else if (f.winDefaultLocations.includes(program)) { - cb(new Error('not found')) - } else if (/sys\.version_info/.test(args[args.length - 1])) { - if (program === 'Z:\\snake.exe') { - cb(null, '3.9.0') - } else { - t.fail() - } - } else { - t.fail() - } - } - f.findPython() - - function done (err, python) { - t.strictEqual(err, null) - t.strictEqual(python, 'Z:\\snake.exe') - } -}) - -test('find python - no python, no python launcher, good guess', function (t) { - t.plan(2) - - var f = new TestPythonFinder(null, done) - f.win = true - const expectedProgram = f.winDefaultLocations[0] - - f.execFile = function (program, args, opts, cb) { - if (program === 'py.exe') { - return cb(new Error('not found')) - } - if (/sys\.executable/.test(args[args.length - 1])) { - cb(new Error('not found')) - } else if (program === expectedProgram && - /sys\.version_info/.test(args[args.length - 1])) { - cb(null, '3.7.3') - } else { - t.fail() - } - } - f.findPython() - - function done (err, python) { - t.strictEqual(err, null) - t.ok(python === expectedProgram) - } -}) - -test('find python - no python, no python launcher, bad guess', function (t) { - t.plan(2) - - var f = new TestPythonFinder(null, done) - f.win = true - - f.execFile = function (program, args, opts, cb) { - if (/sys\.executable/.test(args[args.length - 1])) { - cb(new Error('not found')) - } else if (/sys\.version_info/.test(args[args.length - 1])) { - cb(new Error('not a Python executable')) - } else { - t.fail() - } - } - f.findPython() - - function done (err) { - t.ok(/Could not find any Python/.test(err)) - t.ok(/not in PATH/.test(f.errorLog)) - } -}) diff --git a/test/test-new-find-python.js b/test/test-new-find-python.js new file mode 100644 index 0000000000..d9c0a50737 --- /dev/null +++ b/test/test-new-find-python.js @@ -0,0 +1,228 @@ +'use strict' + +const test = require('tap').test +const findPython = require('../lib/find-python') +const execFile = require('child_process').execFile +const PythonFinder = findPython.test.PythonFinder + +const npmlog = require('npmlog') +npmlog.level = 'silent' + +// what chould final error message displayed in terminal contain +const finalErrorMessage = 'Could not find any Python' + +//! dont forget manually call pythonFinderInstance.findPython() + +// String emmulating path coomand or anything else with spaces +// and UTF-8 charcters. +// Is returned by execFile +//! USE FOR ALL STRINGS +const testString = 'python one love♥' +const testVersions = { + outdated: '2.0.0', + normal: '3.7.0', + testError: new Error('test error') +} + +/** + * @typedef OptionsObj + * @property {boolean} shouldProduceError + * @property {boolean} checkingPyLauncher + * @property {boolean} isPythonOutdated + * @property {boolean} checkingWinDefaultPathes + * + */ + +/** + * + * @param {OptionsObj} optionsObj + */ +function TestExecFile (optionsObj) { + /** + * + * @this {PythonFinder} + */ + return function testExecFile (exec, args, options, callback) { + if (!(optionsObj ? optionsObj.shouldProduceError : false)) { + if (args === this.argsVersion) { + if (optionsObj ? optionsObj.checkingWinDefaultPathes : false) { + if (this.winDefaultLocations.includes(exec)) { + callback(null, testVersions.normal) + } else { + callback(new Error('not found')) + } + } else if (optionsObj ? optionsObj.isPythonOutdated : false) { + callback(null, testVersions.outdated, null) + } else { + callback(null, testVersions.normal, null) + } + } else if (args === this.win ? `"${this.argsExecutable}"` : this.argsExecutable) { + if (optionsObj ? optionsObj.checkingPyLauncher : false) { + if (exec === 'py.exe' || exec === (this.win ? '"python"' : 'python')) { + callback(null, testString, null) + } else { + callback(new Error('not found')) + } + } else if (optionsObj ? optionsObj.checkingWinDefaultPathes : false) { + callback(new Error('not found')) + } else { + callback(null, testString, null) + } + } else { + throw new Error( + `invalid arguments are provided! provided args +are: ${args};\n\nValid are: \n${this.argsExecutable}\n${this.argsVersion}` + ) + } + } else { + const testError = new Error( + `test error ${testString}; optionsObj: ${optionsObj}` + ) + callback(testError) + } + } +} + +/** + * + * @param {boolean} isPythonOutdated if true will return outadet version of python + * @param {OptionsObj} optionsObj + */ + +test('new-find-python', { buffered: true }, (t) => { + t.test('whole module tests', (t) => { + t.test('python found', (t) => { + const pythonFinderInstance = new PythonFinder(null, (err, path) => { + if (err) { + t.fail( + `musn't produce any errors if execFile doesn't produced error. ${err}` + ) + } else { + t.strictEqual(path, testString) + t.end() + } + }) + pythonFinderInstance.execFile = TestExecFile() + + pythonFinderInstance.findPython() + }) + + t.test('outdated version of python found', (t) => { + const pythonFinderInstance = new PythonFinder(null, (err, path) => { + if (!err) { + t.fail("mustn't return path of outdated") + } else { + t.end() + } + }) + + pythonFinderInstance.execFile = TestExecFile({ isPythonOutdated: true }) + + pythonFinderInstance.findPython() + }) + + t.test('no python on computer', (t) => { + const pythonFinderInstance = new PythonFinder(null, (err, path) => { + t.ok(err.message.includes(finalErrorMessage)) + t.end() + }) + + pythonFinderInstance.execFile = TestExecFile({ + shouldProduceError: true + }) + + pythonFinderInstance.findPython() + }) + + t.test('no python2, no python, unix', (t) => { + const pythonFinderInstance = new PythonFinder(null, (err, path) => { + t.false(path) + + t.true(err) + t.ok(err.message.includes(finalErrorMessage)) + t.end() + }) + + pythonFinderInstance.win = false + pythonFinderInstance.checkPyLauncher = t.fail + + pythonFinderInstance.execFile = TestExecFile({ + shouldProduceError: true + }) + + pythonFinderInstance.findPython() + }) + + t.test('no python, use python launcher', (t) => { + const pythonFinderInstance = new PythonFinder(null, (err, path) => { + t.strictEqual(err, null) + + t.strictEqual(path, testString) + + t.end() + }) + + pythonFinderInstance.win = true + + pythonFinderInstance.execFile = TestExecFile({ + checkingPyLauncher: true + }) + + pythonFinderInstance.findPython() + }) + + t.test('no python, no python launcher, checking win default locations', (t) => { + const pythonFinderInstance = new PythonFinder(null, (err, path) => { + t.strictEqual(err, null) + t.true(pythonFinderInstance.winDefaultLocations.includes(path)) + t.end() + }) + + pythonFinderInstance.win = true + + pythonFinderInstance.execFile = TestExecFile({ checkingWinDefaultPathes: true }) + pythonFinderInstance.findPython() + }) + + t.test('python is setted from config', (t) => { + const pythonFinderInstance = new PythonFinder(testString, (err, path) => { + t.strictEqual(err, null) + + t.strictEqual(path, testString) + + t.end() + }) + + pythonFinderInstance.win = true + + pythonFinderInstance.execFile = TestExecFile() + pythonFinderInstance.findPython() + }) + + t.end() + }) + + t.test('real testing (trying to find real python exec)', (t) => { + const pythonFinderInstance = new PythonFinder(null, (err, path) => { + t.strictEqual(err, null) + + execFile(path, ['-V'], (err, stdout, stderr) => { + t.false(err) + + if (stderr.includes('Python 2')) { + t.strictEqual(stdout, '') + t.ok(stderr.includes('Python 2')) + } else { + t.ok(stderr.includes('Python 3')) + t.strictEqual(stderr, '') + } + + t.end() + }) + }) + + pythonFinderInstance.findPython() + }) + + t.end() +}) From 43c35714521d3846ee08487b8f3d6516cd46c240 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Thu, 24 Dec 2020 13:10:11 +0300 Subject: [PATCH 05/25] test, lib: rename new-find-python.js and test-new-find-python.js new-find-python.js -> find-python.js test-new-find-python.js -> test-find-python.js --- lib/{new-find-python.js => find-python.js} | 4 ++-- test/{test-new-find-python.js => test-find-python.js} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename lib/{new-find-python.js => find-python.js} (96%) rename test/{test-new-find-python.js => test-find-python.js} (100%) diff --git a/lib/new-find-python.js b/lib/find-python.js similarity index 96% rename from lib/new-find-python.js rename to lib/find-python.js index 61889325d9..260c973a8f 100644 --- a/lib/new-find-python.js +++ b/lib/find-python.js @@ -414,8 +414,8 @@ class PythonFinder { this.addLog( colorizeOutput(RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') ) - // object with error passed for conveniences - //(becouse we can also pass stderr or some additional staff) + // object with error passed for conveniences + // (becouse we can also pass stderr or some additional staff) // eslint-disable-next-line prefer-promise-reject-errors reject({ err: new Error(`Found unsupported Python version ${ver}`) }) } diff --git a/test/test-new-find-python.js b/test/test-find-python.js similarity index 100% rename from test/test-new-find-python.js rename to test/test-find-python.js From 6ed7ed1d88c983943a057ab103c8e1aa6cc0ed7f Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Thu, 24 Dec 2020 14:01:02 +0300 Subject: [PATCH 06/25] test: fix test-find-python-script.js and test-find-python.js --- test/test-find-python-script.js | 13 +++++++------ test/test-find-python.js | 4 +++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/test/test-find-python-script.js b/test/test-find-python-script.js index b1afd05b14..2ad0727769 100644 --- a/test/test-find-python-script.js +++ b/test/test-find-python-script.js @@ -22,7 +22,9 @@ require('npmlog').level = 'warn' const checks = [ { path: process.env.PYTHON, name: 'env var PYTHON' }, { path: process.env.python2, name: 'env var python2' }, - { path: 'python3', name: 'env var python3' } + { path: 'python3', name: 'python3 in PATH' }, + { path: 'python2', name: 'python2 in PATH' }, + { path: 'python', name: 'pyhton in PATH' } ] const args = [path.resolve('./lib/find-python-script.py')] const options = { @@ -45,22 +47,21 @@ const options = { function check (err, stdout, stderr) { const { t, exec } = this if (!err && !stderr) { - t.strictEqual( + t.true( stdout.trim(), - exec.path, `${exec.name}: check path ${exec.path} equals ${stdout.trim()}` ) } else { // @ts-ignore - if (err.code === 9009) { + if (err.code === 9009 || err.code === 'ENOENT') { t.skip(`skipped: ${exec.name} file not found`) } else { - t.fail(`error: ${err}\n\nstderr: ${stderr}`) + t.skip(`error: ${err}\n\nstderr: ${stderr}`) } } } -test('find-python-script', (t) => { +test('find-python-script', { buffered: false }, (t) => { t.plan(checks.length) // context for check functions diff --git a/test/test-find-python.js b/test/test-find-python.js index d9c0a50737..f4ffb1948a 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -208,12 +208,14 @@ test('new-find-python', { buffered: true }, (t) => { execFile(path, ['-V'], (err, stdout, stderr) => { t.false(err) + console.log('stdout:' + stdout) + console.log('stderr:' + stderr) if (stderr.includes('Python 2')) { t.strictEqual(stdout, '') t.ok(stderr.includes('Python 2')) } else { - t.ok(stderr.includes('Python 3')) + t.ok(stdout.includes('Python 3')) t.strictEqual(stderr, '') } From 462fcc44db324a2203aea535fc4ab22a1275ec40 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Thu, 24 Dec 2020 18:26:01 +0300 Subject: [PATCH 07/25] gyp: fix utf8 encoding --- gyp/pylib/gyp/input.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gyp/pylib/gyp/input.py b/gyp/pylib/gyp/input.py index ca7ce44eab..354958bfb2 100644 --- a/gyp/pylib/gyp/input.py +++ b/gyp/pylib/gyp/input.py @@ -225,7 +225,7 @@ def LoadOneBuildFile(build_file_path, data, aux_data, includes, is_target, check return data[build_file_path] if os.path.exists(build_file_path): - build_file_contents = open(build_file_path).read() + build_file_contents = open(build_file_path, encoding='utf-8').read() else: raise GypError(f"{build_file_path} not found (cwd: {os.getcwd()})") From 93cf5ed61af6deb5c4bb73ccf9aca27818630703 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Wed, 27 Jan 2021 23:09:05 +0300 Subject: [PATCH 08/25] lib: improve comments --- lib/find-python.js | 53 +++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index 260c973a8f..de2565fb4c 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -311,7 +311,9 @@ class PythonFinder { )}" to get Python executable path` ) - const result = await check.checkFunc.apply(this, [check ? check.arg : null]) + const result = await check.checkFunc.apply(this, [ + check ? check.arg : null + ]) fail = false this.succeed(result.path, result.version) @@ -347,7 +349,10 @@ class PythonFinder { } return new Promise((resolve, reject) => { - this.run(exec, args, shell).then(this.checkExecPath).then(resolve).catch(reject) + this.run(exec, args, shell) + .then(this.checkExecPath) + .then(resolve) + .catch(reject) }) } @@ -384,6 +389,7 @@ class PythonFinder { // to pass both path and version return new Promise((resolve, reject) => { this.log.verbose(`- executing "${execPath}" to get version`) + this.run(execPath, this.argsVersion, false) .then((ver) => { // ? may be better code for version check @@ -415,9 +421,11 @@ class PythonFinder { colorizeOutput(RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') ) // object with error passed for conveniences - // (becouse we can also pass stderr or some additional staff) + // (becouse we may want to pass also stderr or some additional stuff) // eslint-disable-next-line prefer-promise-reject-errors - reject({ err: new Error(`Found unsupported Python version ${ver}`) }) + reject({ + err: new Error(`Found unsupported Python version ${ver}`) + }) } resolve({ path: execPath, version: ver }) @@ -445,6 +453,7 @@ class PythonFinder { function (resolve, reject) { const env = extend({}, this.env) env.TERM = 'dumb' + /** @type cp.ExecFileOptionsWithStringEncoding */ const opts = { env: env, shell: shell } this.log.verbose( @@ -457,24 +466,27 @@ class PythonFinder { ) this.log.silly('execFile: opts = ', JSON.stringify(opts, null, 2)) - //* assume that user use utf8 compatible termnal - - //* prosible outcomes with error messages (err.message, error.stack, stderr) - //! on windows: + //* prosible outcomes with error messages on windows (err.message, error.stack?, stderr) // issue of encoding (garbage in terminal ) when 866 or any other locale code // page is setted // possible solutions: - // use "cmd" command with flag "/U" and "/C" (for more informatiom help cmd) + // 1. let it be as it is and just warn user that it should use utf8 + // (already done in this.catchErros's info statement) + // 2. somehow determine user's terminal encdoing and use utils.TextDecoder + // with getting raw buffer from execFile. + // Requires to correct error.message becouse garbage persists there + // 3. Forse user's terminal to use utf8 encoding via e.g. run "chcp 65001". May broke some user's programs + // 4. use "cmd" command with flag "/U" and "/C" (for more informatiom run "help cmd") // which "Causes the output of - // internal commands to a pipe or file to be Unicode" (utf16le) - //* note: find-python-script.py send output in utf8 then may become necessary - //* to reencoded string with Buffer.from(stderr).toString() or something + // internal commands ... to be Unicode" (utf16le) + //* note: find-python-script.py already send output in utf8 then may become necessary + //* to reencode string with Buffer.from(stderr).toString() or something //* similar (if needed) - // for this solution all args should be passed as SINGLE string in quotes - // becouse cmd has such signature: CMD [/A | /U] [/Q] [/D] [/E:ON | /E:OFF] - // [/F:ON | /F:OFF] [/V:ON | /V:OFF] [[/S] [/C | /K] string] + // for this solution all execFile call should look like execFile("cmd /U /C", [command to run, arg1, arg2, ...]) //* all pathes/commands and each argument must be in quotes becouse if they have - //* spaces they will broke everything + //* spaces inside they will broke everything + + //* assume that user use utf8 compatible terminal this.execFile(exec, args, opts, execFileCallback.bind(this)) /** @@ -493,7 +505,7 @@ class PythonFinder { if (err || stderr) { reject({ err: err || null, stderr: stderr || null }) } else { - // trim function removing endings which couse bugs when comparing strings + // trim() function remove string endings that break string comparsion const stdoutTrimed = stdout.trim() resolve(stdoutTrimed) } @@ -549,7 +561,9 @@ class PythonFinder { this.log.silly(err ? err.stack : '') if (stderr) { - this.addLog(`${colorizeOutput(RED, 'STDERR:')} ${stderr ? stderr.trim() : ''}`) + this.addLog( + `${colorizeOutput(RED, 'STDERR:')} ${stderr ? stderr.trim() : ''}` + ) } } this.addLog('--------------------------------------------') @@ -586,7 +600,8 @@ class PythonFinder { // X const info = [ '**********************************************************', - 'If you have non-displayed characters set "UTF-8" encoding.', + 'If you have non-displayed characters, please set "UTF-8"', + 'encoding.', 'You need to install the latest version of Python.', 'Node-gyp should be able to find and use Python. If not,', 'you can try one of the following options:', From 79439534b7c50cb9e2c43ad86880c565f3eed6f2 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Fri, 29 Jan 2021 09:51:23 +0300 Subject: [PATCH 09/25] lib: fixed spelling errors in comments Co-authored-by: Christian Clauss --- lib/find-python.js | 57 +++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index de2565fb4c..928883601e 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -65,8 +65,8 @@ function colorizeOutput (color, string) { } //! on windows debug running with locale cmd (e. g. chcp 866) encoding -// to avoid that uncoment next lines -// locale encdoings couse issues. See run func for more info +// to avoid that uncomment next lines +// locale encodings cause issues. See run func for more info // this lines only for testing // win ? cp.execSync("chcp 65001") : null // log.level = "silly"; @@ -78,7 +78,7 @@ class PythonFinder { /** * * @param {string} configPython force setted from terminal or npm config python path - * @param {(err:Error, found:string) => void} callback succsed/error callback from where result + * @param {(err:Error, found:string) => void} callback succeed/error callback from where result * is available */ constructor (configPython, callback) { @@ -137,7 +137,7 @@ class PythonFinder { } /** - * Getting list of checks which should be cheked + * Getting list of checks which should be checked * * @private * @returns {check[]} @@ -279,7 +279,7 @@ class PythonFinder { * @argument {check[]} checks */ async runChecks (checks) { - // using this flag becouse Fail is happen when ALL checks fail + // using this flag because Fail is happen when ALL checks fail let fail = true for (const check of checks) { @@ -342,7 +342,7 @@ class PythonFinder { let args = this.argsExecutable let shell = false if (this.win) { - // Arguments have to be manually quoted to avoid bugs with spaces in pathes + // Arguments have to be manually quoted to avoid bugs with spaces in paths exec = `"${exec}"` args = args.map((a) => `"${a}"`) shell = true @@ -377,8 +377,8 @@ class PythonFinder { /** * - * Check if a getted path is correct and - * Python executable hase the correct version to use. + * Check if a gotten path is correct and + * Python executable has the correct version to use. * * @private * @argument {string} execPath path to check @@ -393,7 +393,7 @@ class PythonFinder { this.run(execPath, this.argsVersion, false) .then((ver) => { // ? may be better code for version check - // ? may be move log messgaes to catchError func + // ? may be move log messages to catchError func const range = new semver.Range(this.semverRange) let valid = false @@ -421,7 +421,7 @@ class PythonFinder { colorizeOutput(RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') ) // object with error passed for conveniences - // (becouse we may want to pass also stderr or some additional stuff) + // (because we may also want to pass stderr or other variables) // eslint-disable-next-line prefer-promise-reject-errors reject({ err: new Error(`Found unsupported Python version ${ver}`) @@ -466,27 +466,26 @@ class PythonFinder { ) this.log.silly('execFile: opts = ', JSON.stringify(opts, null, 2)) - //* prosible outcomes with error messages on windows (err.message, error.stack?, stderr) + //* possible outcomes with error messages on Windows (err.message, error.stack?, stderr) // issue of encoding (garbage in terminal ) when 866 or any other locale code // page is setted // possible solutions: - // 1. let it be as it is and just warn user that it should use utf8 - // (already done in this.catchErros's info statement) - // 2. somehow determine user's terminal encdoing and use utils.TextDecoder - // with getting raw buffer from execFile. - // Requires to correct error.message becouse garbage persists there - // 3. Forse user's terminal to use utf8 encoding via e.g. run "chcp 65001". May broke some user's programs - // 4. use "cmd" command with flag "/U" and "/C" (for more informatiom run "help cmd") + // 1. leave it as is and just warn the user that it should use utf8 + // (already done in this.catchError's info statement) + // 2. somehow determine the user's terminal encoding and use utils.TextDecoder + // with the raw buffer from execFile. + // Requires to correct error.message because garbage persists there + // 3. Force the user's terminal to use utf8 encoding via e.g. run "chcp 65001". May break user's programs + // 4. use "cmd" command with flag "/U" and "/C" (for more information run "help cmd") // which "Causes the output of // internal commands ... to be Unicode" (utf16le) //* note: find-python-script.py already send output in utf8 then may become necessary //* to reencode string with Buffer.from(stderr).toString() or something //* similar (if needed) - // for this solution all execFile call should look like execFile("cmd /U /C", [command to run, arg1, arg2, ...]) - //* all pathes/commands and each argument must be in quotes becouse if they have - //* spaces inside they will broke everything + // for this solution all execFile call should look like execFile("cmd", ["/U", "/C", command to run, arg1, arg2, ...]) + //* all paths/commands and each argument must be in quotes if they contain spaces - //* assume that user use utf8 compatible terminal + //* assume we have a utf8 compatible terminal this.execFile(exec, args, opts, execFileCallback.bind(this)) /** @@ -501,13 +500,13 @@ class PythonFinder { (err && err.stack) || err ) - // executed script shouldn't pass anythong to stderr if successful + // executed script shouldn't pass anything to stderr if successful if (err || stderr) { reject({ err: err || null, stderr: stderr || null }) } else { - // trim() function remove string endings that break string comparsion - const stdoutTrimed = stdout.trim() - resolve(stdoutTrimed) + // trim() function removes string endings that would break string comparison + const stdoutTrimmed = stdout.trim() + resolve(stdoutTrimmed) } } }.bind(this) @@ -532,7 +531,7 @@ class PythonFinder { // array of error codes (type of errors) that we handling const catchedErrorsCods = ['ENOENT', 9009] - // dont know type of terminal errors + // don't know type of terminal errors // @ts-ignore if (catchedErrorsCods.includes(err ? err.code : undefined)) { // @ts-ignore @@ -619,7 +618,7 @@ class PythonFinder { process.nextTick( this.callback.bind( null, - // if changing error message dont forget also change it test file too + // if changing error message don't forget also change it test file too new Error('Could not find any Python installation to use') ) ) @@ -629,7 +628,7 @@ class PythonFinder { /** * * @param {string} configPython force setted from terminal or npm config python path - * @param {(err:Error, found:string)=> void} callback succsed/error callback from where result + * @param {(err:Error, found:string)=> void} callback succeed/error callback from where result * is available */ function findPython (configPython, callback) { From e8829bb53747ef12c7e3a2471d60f444a6ac4b92 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Mon, 12 Apr 2021 21:42:44 +0300 Subject: [PATCH 10/25] test, lib: fix spelling errors, minor changes swap args and name in find-python. reason in comment add ".js" to couple imports due to import errors --- lib/find-python.js | 11 +++++++---- test/test-configure-python.js | 2 +- test/test-find-accessible-sync.js | 2 +- test/test-find-python.js | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index 928883601e..ff846df006 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -64,7 +64,7 @@ function colorizeOutput (color, string) { return color + string + RESET } -//! on windows debug running with locale cmd (e. g. chcp 866) encoding +//! on windows debug running with locale cmd encoding (e. g. chcp 866) // to avoid that uncomment next lines // locale encodings cause issues. See run func for more info // this lines only for testing @@ -307,7 +307,8 @@ class PythonFinder { this.log.verbose( `executing "${colorizeOutput( GREEN, - check.name || check.arg + // DONE: swap in favor of arg (user want to see what we actually will run not how it is named) + check.arg || check.name )}" to get Python executable path` ) @@ -464,6 +465,7 @@ class PythonFinder { `${colorizeOutput(GREEN, 'execFile')}: args = %j`, args ) + // TODO: make beauty print of PATH property (new line by semicolon) this.log.silly('execFile: opts = ', JSON.stringify(opts, null, 2)) //* possible outcomes with error messages on Windows (err.message, error.stack?, stderr) @@ -499,6 +501,7 @@ class PythonFinder { `${colorizeOutput(RED, 'execFile result')}: err =`, (err && err.stack) || err ) + // TODO: add silly logs as in previous version // executed script shouldn't pass anything to stderr if successful if (err || stderr) { @@ -640,8 +643,8 @@ function findPython (configPython, callback) { /* findPython(null, (err, found) => { console.log('found:', '\x1b[31m', found) console.log('\x1b[0m') -}) - */ +}) +*/ module.exports = findPython module.exports.test = { PythonFinder: PythonFinder, diff --git a/test/test-configure-python.js b/test/test-configure-python.js index ac25f7972e..6a52668502 100644 --- a/test/test-configure-python.js +++ b/test/test-configure-python.js @@ -5,7 +5,7 @@ const path = require('path') const devDir = require('./common').devDir() const gyp = require('../lib/node-gyp') const requireInject = require('require-inject') -const configure = requireInject('../lib/configure', { +const configure = requireInject('../lib/configure.js', { 'graceful-fs': { openSync: function () { return 0 }, closeSync: function () { }, diff --git a/test/test-find-accessible-sync.js b/test/test-find-accessible-sync.js index 0a2e584c4f..9bffd338ce 100644 --- a/test/test-find-accessible-sync.js +++ b/test/test-find-accessible-sync.js @@ -3,7 +3,7 @@ const test = require('tap').test const path = require('path') const requireInject = require('require-inject') -const configure = requireInject('../lib/configure', { +const configure = requireInject('../lib/configure.js', { 'graceful-fs': { closeSync: function () { return undefined }, openSync: function (path) { diff --git a/test/test-find-python.js b/test/test-find-python.js index f4ffb1948a..82c4d80e10 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -95,7 +95,7 @@ test('new-find-python', { buffered: true }, (t) => { const pythonFinderInstance = new PythonFinder(null, (err, path) => { if (err) { t.fail( - `musn't produce any errors if execFile doesn't produced error. ${err}` + `mustn't produce any errors if execFile doesn't produced error. ${err}` ) } else { t.strictEqual(path, testString) From dc580b9e74a86e823f5df087bbb9b5f93dc220cb Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Mon, 12 Apr 2021 23:51:11 +0300 Subject: [PATCH 11/25] lib: fix lint error on ubuntu --- lib/find-python-script.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/find-python-script.py b/lib/find-python-script.py index 5cf2979d25..acf26cca28 100644 --- a/lib/find-python-script.py +++ b/lib/find-python-script.py @@ -1,7 +1,7 @@ -import sys; +import sys if (sys.stdout.encoding != "utf-8" and sys.platform == "win32"): sys.stdout.reconfigure(encoding='utf-8') print(sys.executable) else: - print(sys.executable) \ No newline at end of file + print(sys.executable) From f26edde6bf5b2ecef19ce814e951858170aa3243 Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Tue, 13 Apr 2021 10:38:08 +0300 Subject: [PATCH 12/25] lib: remove unnecessary "else" --- lib/find-python-script.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/find-python-script.py b/lib/find-python-script.py index acf26cca28..dae48e0e95 100644 --- a/lib/find-python-script.py +++ b/lib/find-python-script.py @@ -1,7 +1,4 @@ import sys - -if (sys.stdout.encoding != "utf-8" and sys.platform == "win32"): +if sys.stdout.encoding != "utf-8" and sys.platform == "win32": sys.stdout.reconfigure(encoding='utf-8') - print(sys.executable) -else: - print(sys.executable) +print(sys.executable) From 461fa2ca2fd588b203649e1a88f3191261f02dad Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Tue, 13 Apr 2021 19:49:22 +0300 Subject: [PATCH 13/25] test: remove python2 support in tests --- test/test-find-python-script.js | 16 +++---- test/test-find-python.js | 76 ++++++++++++++++++--------------- 2 files changed, 49 insertions(+), 43 deletions(-) diff --git a/test/test-find-python-script.js b/test/test-find-python-script.js index 2ad0727769..c2e26977d1 100644 --- a/test/test-find-python-script.js +++ b/test/test-find-python-script.js @@ -8,23 +8,22 @@ const path = require('path') require('npmlog').level = 'warn' -//* can use name as short descriptions +//* name can be used as short descriptions /** * @typedef Check - * @property {string} path - path to execurtable or command + * @property {string} path - path to executable or command * @property {string} name - very little description */ +// TODO: add symlinks to python which will contain utf-8 chars /** * @type {Check[]} */ const checks = [ { path: process.env.PYTHON, name: 'env var PYTHON' }, - { path: process.env.python2, name: 'env var python2' }, { path: 'python3', name: 'python3 in PATH' }, - { path: 'python2', name: 'python2 in PATH' }, - { path: 'python', name: 'pyhton in PATH' } + { path: 'python', name: 'python in PATH' } ] const args = [path.resolve('./lib/find-python-script.py')] const options = { @@ -34,7 +33,7 @@ const options = { /** Getting output from find-python-script.py, compare it to path provided to terminal. - If equale - test pass + If equals - test pass runs for all checks @@ -42,7 +41,7 @@ const options = { @argument {Error} err - exec error @argument {string} stdout - stdout buffer of child process @argument {string} stderr - @this {{t, exec: Check}} + @this {{t: Tap, exec: Check}} */ function check (err, stdout, stderr) { const { t, exec } = this @@ -64,6 +63,7 @@ function check (err, stdout, stderr) { test('find-python-script', { buffered: false }, (t) => { t.plan(checks.length) + //? may be more elegant way to copy and pass context // context for check functions const ctx = { t: t, @@ -74,7 +74,7 @@ test('find-python-script', { buffered: false }, (t) => { // checking if env var exist if (!(exec.path === undefined || exec.path === null)) { ctx.exec = exec - // passing ctx as coppied object to make properties immutable from here + // passing ctx as copied object to make properties immutable from here const boundedCheck = check.bind(Object.assign({}, ctx)) execFile(exec.path, args, options, boundedCheck) } else { diff --git a/test/test-find-python.js b/test/test-find-python.js index 82c4d80e10..1d16502cf9 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -8,27 +8,27 @@ const PythonFinder = findPython.test.PythonFinder const npmlog = require('npmlog') npmlog.level = 'silent' -// what chould final error message displayed in terminal contain +// what final error message displayed in terminal should contain const finalErrorMessage = 'Could not find any Python' -//! dont forget manually call pythonFinderInstance.findPython() +//! don't forget manually call pythonFinderInstance.findPython() -// String emmulating path coomand or anything else with spaces -// and UTF-8 charcters. +// String emulating path command or anything else with spaces +// and UTF-8 characters. // Is returned by execFile //! USE FOR ALL STRINGS const testString = 'python one love♥' const testVersions = { outdated: '2.0.0', - normal: '3.7.0', - testError: new Error('test error') + normal: '3.9.0', + testError: new Error('test error'), } /** * @typedef OptionsObj - * @property {boolean} shouldProduceError + * @property {boolean} shouldProduceError pass test error to callback * @property {boolean} checkingPyLauncher - * @property {boolean} isPythonOutdated + * @property {boolean} isPythonOutdated return outdated version * @property {boolean} checkingWinDefaultPathes * */ @@ -37,12 +37,12 @@ const testVersions = { * * @param {OptionsObj} optionsObj */ -function TestExecFile (optionsObj) { +function TestExecFile(optionsObj) { /** * * @this {PythonFinder} */ - return function testExecFile (exec, args, options, callback) { + return function testExecFile(exec, args, options, callback) { if (!(optionsObj ? optionsObj.shouldProduceError : false)) { if (args === this.argsVersion) { if (optionsObj ? optionsObj.checkingWinDefaultPathes : false) { @@ -56,9 +56,14 @@ function TestExecFile (optionsObj) { } else { callback(null, testVersions.normal, null) } - } else if (args === this.win ? `"${this.argsExecutable}"` : this.argsExecutable) { + } else if ( + args === this.win ? `"${this.argsExecutable}"` : this.argsExecutable + ) { if (optionsObj ? optionsObj.checkingPyLauncher : false) { - if (exec === 'py.exe' || exec === (this.win ? '"python"' : 'python')) { + if ( + exec === 'py.exe' || + exec === (this.win ? '"python"' : 'python') + ) { callback(null, testString, null) } else { callback(new Error('not found')) @@ -85,7 +90,7 @@ are: ${args};\n\nValid are: \n${this.argsExecutable}\n${this.argsVersion}` /** * - * @param {boolean} isPythonOutdated if true will return outadet version of python + * @param {boolean} isPythonOutdated if true will return outdated version of python * @param {OptionsObj} optionsObj */ @@ -110,7 +115,7 @@ test('new-find-python', { buffered: true }, (t) => { t.test('outdated version of python found', (t) => { const pythonFinderInstance = new PythonFinder(null, (err, path) => { if (!err) { - t.fail("mustn't return path of outdated") + t.fail("mustn't return path for outdated version") } else { t.end() } @@ -128,13 +133,13 @@ test('new-find-python', { buffered: true }, (t) => { }) pythonFinderInstance.execFile = TestExecFile({ - shouldProduceError: true + shouldProduceError: true, }) pythonFinderInstance.findPython() }) - t.test('no python2, no python, unix', (t) => { + t.test('no python, unix', (t) => { const pythonFinderInstance = new PythonFinder(null, (err, path) => { t.false(path) @@ -147,7 +152,7 @@ test('new-find-python', { buffered: true }, (t) => { pythonFinderInstance.checkPyLauncher = t.fail pythonFinderInstance.execFile = TestExecFile({ - shouldProduceError: true + shouldProduceError: true, }) pythonFinderInstance.findPython() @@ -165,24 +170,29 @@ test('new-find-python', { buffered: true }, (t) => { pythonFinderInstance.win = true pythonFinderInstance.execFile = TestExecFile({ - checkingPyLauncher: true + checkingPyLauncher: true, }) pythonFinderInstance.findPython() }) - t.test('no python, no python launcher, checking win default locations', (t) => { - const pythonFinderInstance = new PythonFinder(null, (err, path) => { - t.strictEqual(err, null) - t.true(pythonFinderInstance.winDefaultLocations.includes(path)) - t.end() - }) + t.test( + 'no python, no python launcher, checking win default locations', + (t) => { + const pythonFinderInstance = new PythonFinder(null, (err, path) => { + t.strictEqual(err, null) + t.true(pythonFinderInstance.winDefaultLocations.includes(path)) + t.end() + }) - pythonFinderInstance.win = true + pythonFinderInstance.win = true - pythonFinderInstance.execFile = TestExecFile({ checkingWinDefaultPathes: true }) - pythonFinderInstance.findPython() - }) + pythonFinderInstance.execFile = TestExecFile({ + checkingWinDefaultPathes: true, + }) + pythonFinderInstance.findPython() + } + ) t.test('python is setted from config', (t) => { const pythonFinderInstance = new PythonFinder(testString, (err, path) => { @@ -202,6 +212,7 @@ test('new-find-python', { buffered: true }, (t) => { t.end() }) + // TODO: make symlink to python with utf-8 chars t.test('real testing (trying to find real python exec)', (t) => { const pythonFinderInstance = new PythonFinder(null, (err, path) => { t.strictEqual(err, null) @@ -211,13 +222,8 @@ test('new-find-python', { buffered: true }, (t) => { console.log('stdout:' + stdout) console.log('stderr:' + stderr) - if (stderr.includes('Python 2')) { - t.strictEqual(stdout, '') - t.ok(stderr.includes('Python 2')) - } else { - t.ok(stdout.includes('Python 3')) - t.strictEqual(stderr, '') - } + t.ok(stdout.includes('Python 3')) + t.strictEqual(stderr, '') t.end() }) From 96b411948695ff902cf37662224f57b7261b458d Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Thu, 15 Apr 2021 14:37:42 +0300 Subject: [PATCH 14/25] lib, test: general improvements related to find-python.js - test/test-find-python-script.js rephrase comment - test/test-find-python.js fix equality expression in testExecFile func, which led to wrong function behavior - lib/find-python.js - organize colors for highlighting output in single object - change the way errors are passed: now there is new class ErrorWithData to fit the propose to pass additional data along the error. Now any error capable object can be thrown - general improvements to comments: add additional explanations, expand documentation, provide thoughts about checkPyLauncher, outline potential bug with powershell and quoted string on windows, - fix error message in "checkExecPath", - add version in "checkExecPath" to silly log and return silly logs from previous version of "run" function - --- lib/find-python.js | 222 +++++++++++++++++++++----------- test/test-find-python-script.js | 2 +- test/test-find-python.js | 54 +++++--- 3 files changed, 185 insertions(+), 93 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index ff846df006..dcd2872d61 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -1,5 +1,5 @@ -'use strict' // @ts-check +'use strict' const path = require('path') const log = require('npmlog') @@ -50,18 +50,21 @@ function getOsUserInfo () { // i hope i made not bad error handling but may be some improvements would be nice // TODO: better error handler on linux/macOS -const RED = '\x1b[31m' -const RESET = '\x1b[0m' -const GREEN = '\x1b[32m' +// DONE: make more beautiful solution for selector of color +const colorHighlight = { + RED: '\x1b[31m', + RESET: '\x1b[0m', + GREEN: '\x1b[32m' +} /** * Paint (not print, just colorize) string with selected color * - * @param color color to set: RED or GREEN + * @param color color to set: colorHighlight.RED or colorHighlight.GREEN * @param {string} string string to colorize */ function colorizeOutput (color, string) { - return color + string + RESET + return color + string + colorHighlight.RESET } //! on windows debug running with locale cmd encoding (e. g. chcp 866) @@ -120,6 +123,7 @@ class PythonFinder { * @private */ addLog (message) { + // write also to verbose for consistent output this.log.verbose(message) this.errorLog.push(message) } @@ -173,7 +177,7 @@ class PythonFinder { if (!this.configPython) { this.addLog( `${colorizeOutput( - GREEN, + colorHighlight.GREEN, 'Python is not set from command line or npm configuration' )}` ) @@ -186,7 +190,7 @@ class PythonFinder { ) this.addLog( '- "--python=" or "npm config get python" is ' + - `"${colorizeOutput(GREEN, this.configPython)}"` + `"${colorizeOutput(colorHighlight.GREEN, this.configPython)}"` ) }, checkFunc: this.checkCommand, @@ -197,7 +201,7 @@ class PythonFinder { if (!this.env.PYTHON) { this.addLog( `Python is not set from environment variable ${colorizeOutput( - GREEN, + colorHighlight.GREEN, 'PYTHON' )}` ) @@ -209,9 +213,9 @@ class PythonFinder { ) this.addLog( `${colorizeOutput( - GREEN, + colorHighlight.GREEN, 'process.env.PYTHON' - )} is "${colorizeOutput(GREEN, this.env.PYTHON)}"` + )} is "${colorizeOutput(colorHighlight.GREEN, this.env.PYTHON)}"` ) }, checkFunc: this.checkCommand, @@ -237,7 +241,7 @@ class PythonFinder { checks.push({ before: () => { this.addLog( - `checking if Python is ${colorizeOutput(GREEN, location)}` + `checking if Python is "${colorizeOutput(colorHighlight.GREEN, location)}"` ) }, checkFunc: this.checkExecPath, @@ -248,7 +252,7 @@ class PythonFinder { before: () => { this.addLog( `checking if the ${colorizeOutput( - GREEN, + colorHighlight.GREEN, 'py launcher' )} can be used to find Python` ) @@ -266,10 +270,11 @@ class PythonFinder { * * @typedef check * @type {object} - * @property {(name: string) => number|void} [before] - * @property {function} checkFunc - * @property {*} [arg] - * @property {string} [name] + * @property {(name: string) => number|void} [before] what to execute before running check itself + * @property {function} checkFunc function which will perform check + * @property {string} [arg] what will be executed + * @property {string} [name] how check is named. this name is displayed to user + * @property {{shell: boolean}} [options] additional data, may be extended later, if shell true, exec command as in shell */ /** @@ -284,12 +289,12 @@ class PythonFinder { for (const check of checks) { if (check.before) { - const beforeResult = check.before.apply(this) + const beforeResult = check.before.apply(this, [check.name]) // if pretask fail - skip if (beforeResult === this.SKIP || beforeResult === this.FAIL) { // ?optional - // TODO: write to result arr which tests is SKIPPED + // TODO: write to result arr which tests are SKIPPED continue } } @@ -297,16 +302,16 @@ class PythonFinder { try { if (!check.before) { this.addLog( - `checking if ${colorizeOutput( - GREEN, + `checking if "${colorizeOutput( + colorHighlight.GREEN, check.name || check.arg - )} can be used` + )}" can be used` ) } this.log.verbose( `executing "${colorizeOutput( - GREEN, + colorHighlight.GREEN, // DONE: swap in favor of arg (user want to see what we actually will run not how it is named) check.arg || check.name )}" to get Python executable path` @@ -342,11 +347,13 @@ class PythonFinder { let exec = command let args = this.argsExecutable let shell = false + + // TODO: add explanation why shell is needed if (this.win) { // Arguments have to be manually quoted to avoid bugs with spaces in paths + shell = true exec = `"${exec}"` args = args.map((a) => `"${a}"`) - shell = true } return new Promise((resolve, reject) => { @@ -367,6 +374,19 @@ class PythonFinder { * @private * @returns {Promise} */ + // theoretically this method can be removed in favor of checkCommand and getChecks. + // the only difference between checkCommand and checkPyLauncher is the shell arg for run function + // BUT! if we will use declarative style (would be cool i think) + // then we should somehow instruct checkCommand esp. on windows, that + // we do not want to execute command in the shell mode. + // Have tried to do this via "optional.shell" property of check object + // but have failed, because to support high modularity of file + // consistent interface across functions should be supported. + // Thus we have to pass check object not only in checkCommand but in + // every other function in conveyor. + // Passing check to every function from previous in promise chain would lead to + // hard to fix errors and overcomplicate structure of module + checkPyLauncher () { return new Promise((resolve, reject) => { this.run(this.pyLauncher, this.argsExecutable, false) @@ -389,10 +409,12 @@ class PythonFinder { // Returning new Promise instead of forwarding existing // to pass both path and version return new Promise((resolve, reject) => { - this.log.verbose(`- executing "${execPath}" to get version`) + this.log.verbose(`executing "${colorizeOutput(colorHighlight.GREEN, execPath)}" to get version`) + // do not wrap with quotes because executing without shell this.run(execPath, this.argsVersion, false) .then((ver) => { + this.log.silly(colorizeOutput(colorHighlight.GREEN, 'version got:'), ver) // ? may be better code for version check // ? may be move log messages to catchError func const range = new semver.Range(this.semverRange) @@ -404,29 +426,27 @@ class PythonFinder { } catch (err) { this.log.silly(`range.test() threw:\n${err.stack}`) this.addLog( - `"${colorizeOutput(RED, execPath)}" does not have a valid version` + `"${colorizeOutput(colorHighlight.RED, execPath)}" does not have a valid version` ) - this.addLog('is it a Python executable?') + this.addLog('Is it a Python executable?') - reject(err) + // if you need to pass additional data, use ErrorWithData class + // you can also use any Error capable object + return reject(err) } if (!valid) { this.addLog( `version is ${colorizeOutput( - RED, + colorHighlight.RED, ver - )} - should be ${colorizeOutput(RED, this.semverRange)}` + )} - should be ${colorizeOutput(colorHighlight.RED, this.semverRange)}` ) this.addLog( - colorizeOutput(RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') + colorizeOutput(colorHighlight.RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') ) - // object with error passed for conveniences - // (because we may also want to pass stderr or other variables) - // eslint-disable-next-line prefer-promise-reject-errors - reject({ - err: new Error(`Found unsupported Python version ${ver}`) - }) + // if you need to pass additional data, use ErrorWithData class + reject(new Error(`Found unsupported Python version ${ver}`)) } resolve({ path: execPath, version: ver }) @@ -454,19 +474,13 @@ class PythonFinder { function (resolve, reject) { const env = extend({}, this.env) env.TERM = 'dumb' - /** @type cp.ExecFileOptionsWithStringEncoding */ + /** @type {cp.ExecFileOptions} */ const opts = { env: env, shell: shell } - this.log.verbose( - `${colorizeOutput(GREEN, 'execFile')}: exec = %j`, - exec - ) - this.log.verbose( - `${colorizeOutput(GREEN, 'execFile')}: args = %j`, - args - ) + this.log.verbose(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: exec = `, exec) + this.log.verbose(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: args = `, args) // TODO: make beauty print of PATH property (new line by semicolon) - this.log.silly('execFile: opts = ', JSON.stringify(opts, null, 2)) + this.log.silly(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: opts = `, JSON.stringify(opts, null, 2), '\n\n') //* possible outcomes with error messages on Windows (err.message, error.stack?, stderr) // issue of encoding (garbage in terminal ) when 866 or any other locale code @@ -487,9 +501,17 @@ class PythonFinder { // for this solution all execFile call should look like execFile("cmd", ["/U", "/C", command to run, arg1, arg2, ...]) //* all paths/commands and each argument must be in quotes if they contain spaces + // ! potential bug + // if "shell" is true and is users default shell on windows is powershell then executables in PATH which name contain spaces will not work. + // it is feature of powershell which handle first arg in quotes as string + // thus if exec name has spaces, we can shield them (every space) with ` (backtick) + // or & (ampersand) can be placed before string in quotes, to tell to shell that + // it is executable, not string + //* assume we have a utf8 compatible terminal this.execFile(exec, args, opts, execFileCallback.bind(this)) + // ? may be better to use utils.promisify /** * * @param {Error} err @@ -497,15 +519,14 @@ class PythonFinder { * @param {string} stderr */ function execFileCallback (err, stdout, stderr) { - this.log.silly( - `${colorizeOutput(RED, 'execFile result')}: err =`, - (err && err.stack) || err - ) - // TODO: add silly logs as in previous version + // Done: add silly logs as in previous version + this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: err =`, (err && err.stack) || err) + this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stdout =`, stdout) + this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stderr =`, stderr) // executed script shouldn't pass anything to stderr if successful if (err || stderr) { - reject({ err: err || null, stderr: stderr || null }) + reject(new ErrorWithData({ data: { stderr: stderr || null }, messageOrError: err || null })) } else { // trim() function removes string endings that would break string comparison const stdoutTrimmed = stdout.trim() @@ -523,50 +544,58 @@ class PythonFinder { * * @private * TODO: figure out err type - * @param {{err: Error, stderr: string}} error + * @param {ErrorWithData} err * @param {check} check */ - catchErrors (error, check) { - const { err, stderr } = error - - this.addLog(colorizeOutput(RED, `FAIL: ${check.name || check.arg}`)) + catchErrors (err, check) { + this.addLog(colorizeOutput(colorHighlight.RED, `FAIL: ${check.name || check.arg}`)) // array of error codes (type of errors) that we handling const catchedErrorsCods = ['ENOENT', 9009] // don't know type of terminal errors // @ts-ignore - if (catchedErrorsCods.includes(err ? err.code : undefined)) { + if (catchedErrorsCods.includes(err.error ? err.error.code : undefined)) { + const { error } = err // @ts-ignore - switch (err ? err.code : undefined) { + switch (error ? error.code : undefined) { case 'ENOENT': this.addLog( `${colorizeOutput( - RED, + colorHighlight.RED, 'ERROR:' // @ts-ignore - )} No such file or directory: ${colorizeOutput(RED, err.path)}` + )} No such file or directory: "${colorizeOutput(colorHighlight.RED, error.path)}"` ) break case 9009: this.addLog( `${colorizeOutput( - RED, + colorHighlight.RED, 'ERROR:' )} Command failed: file not found or not in PATH` ) break } } else { - this.addLog(`${colorizeOutput(RED, 'ERROR:')} ${err ? err.message : ''}`) - this.log.silly(err ? err.stack : '') + this.addLog( + `${colorizeOutput(colorHighlight.RED, 'ERROR:')} ${ + err ? (err.message ? err.message : err) : '' + }` + ) + this.log.silly(colorizeOutput(colorHighlight.RED, 'FULL ERROR:'), err ? (err.stack ? err.stack : err) : '') - if (stderr) { - this.addLog( - `${colorizeOutput(RED, 'STDERR:')} ${stderr ? stderr.trim() : ''}` - ) + // map through data object to print it as KEY: value + for (const prop in err.data) { + if (err.data[prop]) { + this.addLog(`${colorizeOutput(colorHighlight.RED, `${prop.toUpperCase()}:`)} ${err.data[prop].trim()}`) + } } + + // if (error.data.stderr) { + // this.addLog(`${colorizeOutput(colorHighlight.RED, 'STDERR:')} ${error.data.stderr.trim()}`) + // } } this.addLog('--------------------------------------------') } @@ -581,9 +610,9 @@ class PythonFinder { succeed (execPath, version) { this.log.info( `using Python version ${colorizeOutput( - GREEN, + colorHighlight.GREEN, version - )} found at "${colorizeOutput(GREEN, execPath)}"` + )} found at "${colorizeOutput(colorHighlight.GREEN, execPath)}"` ) process.nextTick(this.callback.bind(null, null, execPath)) } @@ -628,6 +657,49 @@ class PythonFinder { } } +/** + * Error with additional data. + * If you do not want to pass any additional data use regular Error + * + * !ALL MEMBERS EXCEPT "DATA" ARE OPTIONAL! + * @see Error + * + * @class + * @extends Error +*/ +class ErrorWithData extends Error { + // DONE: give to user possibility to pass existing error for which provide additional data + /** + * + * @typedef ErrorConstructor + * @property {{[key:string]: any}} data additional data to pass in data property of error object + * @property {string|Error} [messageOrError] + * @private + */ + /** + * @constructor + * @param {ErrorConstructor} [options] + */ + constructor (options) { + if (typeof options.messageOrError === 'string') { + const message = options.messageOrError + super(message) + } else if (options.messageOrError instanceof Error) { + const error = options.messageOrError + super(error.message) + this.error = error + } else { + super() + } + + if (!options.data) { + throw new TypeError('"data" property is required. If you do not want pass any additional data use regular Error instead this one') + } + + this.data = options.data + } +} + /** * * @param {string} configPython force setted from terminal or npm config python path @@ -641,10 +713,10 @@ function findPython (configPython, callback) { // function for tests /* findPython(null, (err, found) => { - console.log('found:', '\x1b[31m', found) - console.log('\x1b[0m') -}) -*/ + console.log('found:', colorizeOutput(colorHighlight.GREEN, found)) + console.log('err:', err) +}) + */ module.exports = findPython module.exports.test = { PythonFinder: PythonFinder, diff --git a/test/test-find-python-script.js b/test/test-find-python-script.js index c2e26977d1..65a82379f3 100644 --- a/test/test-find-python-script.js +++ b/test/test-find-python-script.js @@ -63,7 +63,7 @@ function check (err, stdout, stderr) { test('find-python-script', { buffered: false }, (t) => { t.plan(checks.length) - //? may be more elegant way to copy and pass context + // ? may be more elegant way to pass context // context for check functions const ctx = { t: t, diff --git a/test/test-find-python.js b/test/test-find-python.js index 1d16502cf9..02400f257d 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -1,6 +1,7 @@ 'use strict' -const test = require('tap').test +const tap = require('tap') +const { test } = tap const findPython = require('../lib/find-python') const execFile = require('child_process').execFile const PythonFinder = findPython.test.PythonFinder @@ -21,28 +22,45 @@ const testString = 'python one love♥' const testVersions = { outdated: '2.0.0', normal: '3.9.0', - testError: new Error('test error'), + testError: new Error('test error') +} + +function strictDeepEqual (received, wanted) { + let result = false + + for (let i = 0; i < received.length; i++) { + if (Array.isArray(received[i]) && Array.isArray(wanted[i])) { + result = strictDeepEqual(received[i], wanted[i]) + } else { + result = received[i] === wanted[i] + } + + if (!result) { + return result + } + } + + return result } /** * @typedef OptionsObj - * @property {boolean} shouldProduceError pass test error to callback - * @property {boolean} checkingPyLauncher - * @property {boolean} isPythonOutdated return outdated version - * @property {boolean} checkingWinDefaultPathes + * @property {boolean} [shouldProduceError] pass test error to callback + * @property {boolean} [checkingPyLauncher] + * @property {boolean} [isPythonOutdated] return outdated version + * @property {boolean} [checkingWinDefaultPathes] * */ /** - * - * @param {OptionsObj} optionsObj + * @param {OptionsObj} [optionsObj] */ -function TestExecFile(optionsObj) { +function TestExecFile (optionsObj) { /** * * @this {PythonFinder} */ - return function testExecFile(exec, args, options, callback) { + return function testExecFile (exec, args, options, callback) { if (!(optionsObj ? optionsObj.shouldProduceError : false)) { if (args === this.argsVersion) { if (optionsObj ? optionsObj.checkingWinDefaultPathes : false) { @@ -57,7 +75,8 @@ function TestExecFile(optionsObj) { callback(null, testVersions.normal, null) } } else if ( - args === this.win ? `"${this.argsExecutable}"` : this.argsExecutable + // DONE: map through argsExecutable to check that all args are equals + strictDeepEqual(args, this.argsExecutable.map((arg) => `"${arg}"`)) ) { if (optionsObj ? optionsObj.checkingPyLauncher : false) { if ( @@ -71,7 +90,8 @@ function TestExecFile(optionsObj) { } else if (optionsObj ? optionsObj.checkingWinDefaultPathes : false) { callback(new Error('not found')) } else { - callback(null, testString, null) + // should be trimmed + callback(null, testString + '\n', null) } } else { throw new Error( @@ -94,7 +114,7 @@ are: ${args};\n\nValid are: \n${this.argsExecutable}\n${this.argsVersion}` * @param {OptionsObj} optionsObj */ -test('new-find-python', { buffered: true }, (t) => { +test('find-python', { buffered: true }, (t) => { t.test('whole module tests', (t) => { t.test('python found', (t) => { const pythonFinderInstance = new PythonFinder(null, (err, path) => { @@ -133,7 +153,7 @@ test('new-find-python', { buffered: true }, (t) => { }) pythonFinderInstance.execFile = TestExecFile({ - shouldProduceError: true, + shouldProduceError: true }) pythonFinderInstance.findPython() @@ -152,7 +172,7 @@ test('new-find-python', { buffered: true }, (t) => { pythonFinderInstance.checkPyLauncher = t.fail pythonFinderInstance.execFile = TestExecFile({ - shouldProduceError: true, + shouldProduceError: true }) pythonFinderInstance.findPython() @@ -170,7 +190,7 @@ test('new-find-python', { buffered: true }, (t) => { pythonFinderInstance.win = true pythonFinderInstance.execFile = TestExecFile({ - checkingPyLauncher: true, + checkingPyLauncher: true }) pythonFinderInstance.findPython() @@ -188,7 +208,7 @@ test('new-find-python', { buffered: true }, (t) => { pythonFinderInstance.win = true pythonFinderInstance.execFile = TestExecFile({ - checkingWinDefaultPathes: true, + checkingWinDefaultPathes: true }) pythonFinderInstance.findPython() } From ba150bc574ee8cd1cd8dfe5426e4ac82eda573ec Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Thu, 15 Apr 2021 15:09:01 +0300 Subject: [PATCH 15/25] test: fix quotes check on POSIX systems --- test/test-find-python.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-find-python.js b/test/test-find-python.js index 02400f257d..d6c1bbde87 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -76,7 +76,7 @@ function TestExecFile (optionsObj) { } } else if ( // DONE: map through argsExecutable to check that all args are equals - strictDeepEqual(args, this.argsExecutable.map((arg) => `"${arg}"`)) + strictDeepEqual(args, this.win ? this.argsExecutable.map((arg) => `"${arg}"`) : this.argsExecutable) ) { if (optionsObj ? optionsObj.checkingPyLauncher : false) { if ( From 7edd0dbb38148c28ce2b39e1c1e2ff3ff6763135 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Tue, 4 May 2021 17:10:48 +0300 Subject: [PATCH 16/25] test: add test with utf-8 string in path remove the use of deprecated tap aliases --- lib/find-python.js | 3 +- test/test-find-python-script.js | 2 +- test/test-find-python.js | 121 ++++++++++++++++++++++++++------ 3 files changed, 101 insertions(+), 25 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index dcd2872d61..b633b95a0e 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -517,11 +517,12 @@ class PythonFinder { * @param {Error} err * @param {string} stdout * @param {string} stderr + * @this {PythonFinder} */ function execFileCallback (err, stdout, stderr) { // Done: add silly logs as in previous version this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: err =`, (err && err.stack) || err) - this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stdout =`, stdout) + this.log.verbose(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stdout =`, stdout) this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stderr =`, stderr) // executed script shouldn't pass anything to stderr if successful diff --git a/test/test-find-python-script.js b/test/test-find-python-script.js index 65a82379f3..2b4376c36e 100644 --- a/test/test-find-python-script.js +++ b/test/test-find-python-script.js @@ -46,7 +46,7 @@ const options = { function check (err, stdout, stderr) { const { t, exec } = this if (!err && !stderr) { - t.true( + t.ok( stdout.trim(), `${exec.name}: check path ${exec.path} equals ${stdout.trim()}` ) diff --git a/test/test-find-python.js b/test/test-find-python.js index d6c1bbde87..928f708dee 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -3,10 +3,12 @@ const tap = require('tap') const { test } = tap const findPython = require('../lib/find-python') -const execFile = require('child_process').execFile +const cp = require('child_process') const PythonFinder = findPython.test.PythonFinder - +const util = require('util') +const path = require('path') const npmlog = require('npmlog') +const fs = require('fs') npmlog.level = 'silent' // what final error message displayed in terminal should contain @@ -123,7 +125,7 @@ test('find-python', { buffered: true }, (t) => { `mustn't produce any errors if execFile doesn't produced error. ${err}` ) } else { - t.strictEqual(path, testString) + t.equal(path, testString) t.end() } }) @@ -161,9 +163,9 @@ test('find-python', { buffered: true }, (t) => { t.test('no python, unix', (t) => { const pythonFinderInstance = new PythonFinder(null, (err, path) => { - t.false(path) + t.notOk(path) - t.true(err) + t.ok(err) t.ok(err.message.includes(finalErrorMessage)) t.end() }) @@ -180,9 +182,9 @@ test('find-python', { buffered: true }, (t) => { t.test('no python, use python launcher', (t) => { const pythonFinderInstance = new PythonFinder(null, (err, path) => { - t.strictEqual(err, null) + t.equal(err, null) - t.strictEqual(path, testString) + t.equal(path, testString) t.end() }) @@ -200,8 +202,8 @@ test('find-python', { buffered: true }, (t) => { 'no python, no python launcher, checking win default locations', (t) => { const pythonFinderInstance = new PythonFinder(null, (err, path) => { - t.strictEqual(err, null) - t.true(pythonFinderInstance.winDefaultLocations.includes(path)) + t.equal(err, null) + t.ok(pythonFinderInstance.winDefaultLocations.includes(path)) t.end() }) @@ -216,9 +218,9 @@ test('find-python', { buffered: true }, (t) => { t.test('python is setted from config', (t) => { const pythonFinderInstance = new PythonFinder(testString, (err, path) => { - t.strictEqual(err, null) + t.equal(err, null) - t.strictEqual(path, testString) + t.equal(path, testString) t.end() }) @@ -232,24 +234,97 @@ test('find-python', { buffered: true }, (t) => { t.end() }) - // TODO: make symlink to python with utf-8 chars - t.test('real testing (trying to find real python exec)', (t) => { - const pythonFinderInstance = new PythonFinder(null, (err, path) => { - t.strictEqual(err, null) + // DONE: make symlink to python with utf-8 chars + t.test('real testing', async (t) => { + const paths = { + python: '', + pythonDir: '', + testDir: '', + baseDir: __dirname + } - execFile(path, ['-V'], (err, stdout, stderr) => { - t.false(err) - console.log('stdout:' + stdout) - console.log('stderr:' + stderr) + const execFile = util.promisify(cp.execFile) - t.ok(stdout.includes('Python 3')) - t.strictEqual(stderr, '') + // a bit tricky way to make PythonFinder promisified + function promisifyPythonFinder (config) { + let pythonFinderInstance - t.end() + const result = new Promise((resolve, reject) => { + pythonFinderInstance = new PythonFinder(config, (err, path) => { + if (err) { + reject(err) + } else { + resolve(path) + } + }) }) + + return { pythonFinderInstance, result } + } + + async function testPythonPath (t, pythonPath) { + try { + const { stderr, stdout } = await execFile(pythonPath, ['-V']) + + console.log('stdout:', stdout) + console.log('stderr:', stderr) + + if (t.ok(stdout.includes('Python 3'), 'is it python with major version 3') && + t.equal(stderr, '', 'is stderr empty')) { + return true + } + + return false + } catch (err) { + t.equal(err, null, 'is error null') + return false + } + } + + // await is needed because test func is async + await t.test('trying to find real python exec', async (t) => { + const { pythonFinderInstance, result } = promisifyPythonFinder(null) + + try { + pythonFinderInstance.findPython() + + const pythonPath = await result + + if (t.ok(await testPythonPath(t, pythonPath), 'is path valid')) { + // stdout contain output of "python -V" command, not python path + // using found path as trusted + paths.python = pythonPath + paths.pythonDir = path.join(paths.python, '../') + } + } catch (err) { + t.notOk(err, 'are we having error') + } + + t.end() }) - pythonFinderInstance.findPython() + await t.test(`test with path contain "${testString}"`, async (t) => { + // making fixture + paths.testDir = fs.mkdtempSync(path.resolve(paths.baseDir, 'node_modules', 'pythonFindTestFolder-')) + + // using "junction" to avoid permission error + fs.symlinkSync(paths.pythonDir, path.resolve(paths.testDir, testString), 'junction') + + const { pythonFinderInstance, result } = promisifyPythonFinder(path.resolve(paths.testDir, 'python')) + + pythonFinderInstance.findPython() + + const pythonPath = await result + + t.ok(await testPythonPath(t, pythonPath), 'is path valid') + + t.end() + }) + + // remove fixture + fs.rmSync(paths.testDir, { recursive: true }) + + t.end() }) t.end() From f4cd261939716d78b240c7b575d34cc625405c17 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Wed, 5 May 2021 16:30:05 +0300 Subject: [PATCH 17/25] test: fix "`fs.rmSync` is not a function" for old node.js versions add simplest polyfill for `fs.rmSync(path, {recursive: true})` --- test/rm.js | 20 ++++++++++++++++++++ test/test-find-python.js | 9 ++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 test/rm.js diff --git a/test/rm.js b/test/rm.js new file mode 100644 index 0000000000..c32bfe0553 --- /dev/null +++ b/test/rm.js @@ -0,0 +1,20 @@ +const fs = require('fs') +const path = require('path') + +/** recursively delete files, symlinks (without following them) and dirs */ +module.exports = function rmRecSync (pth) { + pth = path.normalize(pth) + + rm(pth) + + function rm (pth) { + const pathStat = fs.statSync(pth) + // trick with lstat is used to avoid following symlinks (especially junctions on windows) + if (pathStat.isDirectory() && !fs.lstatSync(pth).isSymbolicLink()) { + fs.readdirSync(pth).forEach((nextPath) => rm(path.join(pth, nextPath))) + fs.rmdirSync(pth) + } else { + fs.unlinkSync(pth) + } + } +} diff --git a/test/test-find-python.js b/test/test-find-python.js index 928f708dee..d6cbdf83f3 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -309,6 +309,8 @@ test('find-python', { buffered: true }, (t) => { // using "junction" to avoid permission error fs.symlinkSync(paths.pythonDir, path.resolve(paths.testDir, testString), 'junction') + console.log('🚀 ~ file: test-find-python.js ~ line 312 ~ awaitt.test ~ path.resolve(paths.testDir, testString)', path.resolve(paths.testDir, testString)) + console.log('🚀 ~ file: test-find-python.js ~ line 312 ~ awaitt.test ~ paths.pythonDir', paths.pythonDir) const { pythonFinderInstance, result } = promisifyPythonFinder(path.resolve(paths.testDir, 'python')) @@ -322,7 +324,12 @@ test('find-python', { buffered: true }, (t) => { }) // remove fixture - fs.rmSync(paths.testDir, { recursive: true }) + if (fs.rmSync) { + fs.rmSync(paths.testDir, { recursive: true }) + } else { + // + require('./rm.js')(paths.testDir) + } t.end() }) From 3ed64183ac9c82b7fad5665596374260280f0bc3 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Tue, 4 May 2021 12:12:46 +0300 Subject: [PATCH 18/25] chore: remove unnecessary comment, fix spelling errors --- lib/find-python.js | 4 ---- test/test-find-python.js | 6 +++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index b633b95a0e..d9e600afc2 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -593,10 +593,6 @@ class PythonFinder { this.addLog(`${colorizeOutput(colorHighlight.RED, `${prop.toUpperCase()}:`)} ${err.data[prop].trim()}`) } } - - // if (error.data.stderr) { - // this.addLog(`${colorizeOutput(colorHighlight.RED, 'STDERR:')} ${error.data.stderr.trim()}`) - // } } this.addLog('--------------------------------------------') } diff --git a/test/test-find-python.js b/test/test-find-python.js index d6cbdf83f3..14eaed09cc 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -303,14 +303,14 @@ test('find-python', { buffered: true }, (t) => { t.end() }) - await t.test(`test with path contain "${testString}"`, async (t) => { + await t.test(`test with path containing "${testString}"`, async (t) => { // making fixture paths.testDir = fs.mkdtempSync(path.resolve(paths.baseDir, 'node_modules', 'pythonFindTestFolder-')) // using "junction" to avoid permission error fs.symlinkSync(paths.pythonDir, path.resolve(paths.testDir, testString), 'junction') - console.log('🚀 ~ file: test-find-python.js ~ line 312 ~ awaitt.test ~ path.resolve(paths.testDir, testString)', path.resolve(paths.testDir, testString)) - console.log('🚀 ~ file: test-find-python.js ~ line 312 ~ awaitt.test ~ paths.pythonDir', paths.pythonDir) + console.log('🚀 ~ file: test-find-python.js ~ line 312 ~ await.test ~ path.resolve(paths.testDir, testString)', path.resolve(paths.testDir, testString)) + console.log('🚀 ~ file: test-find-python.js ~ line 312 ~ await.test ~ paths.pythonDir', paths.pythonDir) const { pythonFinderInstance, result } = promisifyPythonFinder(path.resolve(paths.testDir, 'python')) From 2a2ad82abf8ef55679d2ef23fc54dd994f7c431f Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Fri, 21 May 2021 00:22:40 +0300 Subject: [PATCH 19/25] chore: use `LF` endings --- lib/find-python.js | 1442 ++++++++++++++++++++++---------------------- 1 file changed, 721 insertions(+), 721 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index d9e600afc2..686e91a03a 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -1,721 +1,721 @@ -// @ts-check -'use strict' - -const path = require('path') -const log = require('npmlog') -const semver = require('semver') -const cp = require('child_process') -const extend = require('util')._extend // eslint-disable-line -const win = process.platform === 'win32' -const logWithPrefix = require('./util').logWithPrefix - -const systemDrive = process.env.SystemDrive || 'C:' -const username = process.env.USERNAME || process.env.USER || getOsUserInfo() -const localAppData = process.env.LOCALAPPDATA || `${systemDrive}\\${username}\\AppData\\Local` -const foundLocalAppData = process.env.LOCALAPPDATA || username -const programFiles = process.env.ProgramW6432 || process.env.ProgramFiles || `${systemDrive}\\Program Files` -const programFilesX86 = process.env['ProgramFiles(x86)'] || `${programFiles} (x86)` - -const winDefaultLocationsArray = [] -for (const majorMinor of ['39', '38', '37', '36']) { - if (foundLocalAppData) { - winDefaultLocationsArray.push( - `${localAppData}\\Programs\\Python\\Python${majorMinor}\\python.exe`, - `${programFiles}\\Python${majorMinor}\\python.exe`, - `${localAppData}\\Programs\\Python\\Python${majorMinor}-32\\python.exe`, - `${programFiles}\\Python${majorMinor}-32\\python.exe`, - `${programFilesX86}\\Python${majorMinor}-32\\python.exe` - ) - } else { - winDefaultLocationsArray.push( - `${programFiles}\\Python${majorMinor}\\python.exe`, - `${programFiles}\\Python${majorMinor}-32\\python.exe`, - `${programFilesX86}\\Python${majorMinor}-32\\python.exe` - ) - } -} - -function getOsUserInfo () { - try { - return require('os').userInfo().username - } catch {} -} - -//! after editing file dont forget run "npm test" and -//! change tests for this file if needed - -// ? may be some addition info in silly and verbose levels -// ? add safety to colorizeOutput function. E.g. when terminal doesn't -// ? support colorizing, just disable it (return given string) -// i hope i made not bad error handling but may be some improvements would be nice -// TODO: better error handler on linux/macOS - -// DONE: make more beautiful solution for selector of color -const colorHighlight = { - RED: '\x1b[31m', - RESET: '\x1b[0m', - GREEN: '\x1b[32m' -} - -/** - * Paint (not print, just colorize) string with selected color - * - * @param color color to set: colorHighlight.RED or colorHighlight.GREEN - * @param {string} string string to colorize - */ -function colorizeOutput (color, string) { - return color + string + colorHighlight.RESET -} - -//! on windows debug running with locale cmd encoding (e. g. chcp 866) -// to avoid that uncomment next lines -// locale encodings cause issues. See run func for more info -// this lines only for testing -// win ? cp.execSync("chcp 65001") : null -// log.level = "silly"; - -/** - * @class - */ -class PythonFinder { - /** - * - * @param {string} configPython force setted from terminal or npm config python path - * @param {(err:Error, found:string) => void} callback succeed/error callback from where result - * is available - */ - constructor (configPython, callback) { - this.callback = callback - this.configPython = configPython - this.errorLog = [] - - this.catchErrors = this.catchErrors.bind(this) - this.checkExecPath = this.checkExecPath.bind(this) - this.succeed = this.succeed.bind(this) - - this.SKIP = 0 - this.FAIL = 1 - - this.log = logWithPrefix(log, 'find Python') - - this.argsExecutable = [path.resolve(__dirname, 'find-python-script.py')] - this.argsVersion = [ - '-c', - 'import sys; print("%s.%s.%s" % sys.version_info[:3]);' - // for testing - // 'print("2.1.1")' - ] - this.semverRange = '>=3.6.0' - // These can be overridden for testing: - this.execFile = cp.execFile - this.env = process.env - this.win = win - this.pyLauncher = 'py.exe' - this.winDefaultLocations = winDefaultLocationsArray - } - - /** - * 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. - * - * ?message is array or one string - * - * @private - */ - addLog (message) { - // write also to verbose for consistent output - this.log.verbose(message) - this.errorLog.push(message) - } - - /** - * Find Python by trying a sequence of possibilities. - * Ignore errors, keep trying until Python is found. - * - * @public - */ - findPython () { - this.toCheck = this.getChecks() - - this.runChecks(this.toCheck) - } - - /** - * Getting list of checks which should be checked - * - * @private - * @returns {check[]} - */ - getChecks () { - if (this.env.NODE_GYP_FORCE_PYTHON) { - /** - * @type {check[]} - */ - return [ - { - before: () => { - this.addLog( - 'checking Python explicitly set from NODE_GYP_FORCE_PYTHON' - ) - this.addLog( - '- process.env.NODE_GYP_FORCE_PYTHON is ' + - `"${this.env.NODE_GYP_FORCE_PYTHON}"` - ) - }, - checkFunc: this.checkCommand, - arg: this.env.NODE_GYP_FORCE_PYTHON - } - ] - } - - /** - * @type {check[]} - */ - const checks = [ - { - before: (name) => { - if (!this.configPython) { - this.addLog( - `${colorizeOutput( - colorHighlight.GREEN, - 'Python is not set from command line or npm configuration' - )}` - ) - this.addLog('') - return this.SKIP - } - this.addLog( - 'checking Python explicitly set from command line or ' + - 'npm configuration' - ) - this.addLog( - '- "--python=" or "npm config get python" is ' + - `"${colorizeOutput(colorHighlight.GREEN, this.configPython)}"` - ) - }, - checkFunc: this.checkCommand, - arg: this.configPython - }, - { - before: (name) => { - if (!this.env.PYTHON) { - this.addLog( - `Python is not set from environment variable ${colorizeOutput( - colorHighlight.GREEN, - 'PYTHON' - )}` - ) - return this.SKIP - } - this.addLog( - 'checking Python explicitly set from environment ' + - 'variable PYTHON' - ) - this.addLog( - `${colorizeOutput( - colorHighlight.GREEN, - 'process.env.PYTHON' - )} is "${colorizeOutput(colorHighlight.GREEN, this.env.PYTHON)}"` - ) - }, - checkFunc: this.checkCommand, - arg: this.env.PYTHON, - // name used as very short description - name: 'process.env.PYTHON' - }, - { - checkFunc: this.checkCommand, - name: 'python3', - arg: 'python3' - }, - { - checkFunc: this.checkCommand, - name: 'python', - arg: 'python' - }, - ] - - if (this.win) { - for (let i = 0; i < this.winDefaultLocations.length; ++i) { - const location = this.winDefaultLocations[i] - checks.push({ - before: () => { - this.addLog( - `checking if Python is "${colorizeOutput(colorHighlight.GREEN, location)}"` - ) - }, - checkFunc: this.checkExecPath, - arg: location - }) - } - checks.push({ - before: () => { - this.addLog( - `checking if the ${colorizeOutput( - colorHighlight.GREEN, - 'py launcher' - )} can be used to find Python` - ) - }, - checkFunc: this.checkPyLauncher, - name: 'py Launcher' - }) - } - - return checks - } - - /** - * Type for possible place where python is - * - * @typedef check - * @type {object} - * @property {(name: string) => number|void} [before] what to execute before running check itself - * @property {function} checkFunc function which will perform check - * @property {string} [arg] what will be executed - * @property {string} [name] how check is named. this name is displayed to user - * @property {{shell: boolean}} [options] additional data, may be extended later, if shell true, exec command as in shell - */ - - /** - * - * - * @private - * @argument {check[]} checks - */ - async runChecks (checks) { - // using this flag because Fail is happen when ALL checks fail - let fail = true - - for (const check of checks) { - if (check.before) { - const beforeResult = check.before.apply(this, [check.name]) - - // if pretask fail - skip - if (beforeResult === this.SKIP || beforeResult === this.FAIL) { - // ?optional - // TODO: write to result arr which tests are SKIPPED - continue - } - } - - try { - if (!check.before) { - this.addLog( - `checking if "${colorizeOutput( - colorHighlight.GREEN, - check.name || check.arg - )}" can be used` - ) - } - - this.log.verbose( - `executing "${colorizeOutput( - colorHighlight.GREEN, - // DONE: swap in favor of arg (user want to see what we actually will run not how it is named) - check.arg || check.name - )}" to get Python executable path` - ) - - const result = await check.checkFunc.apply(this, [ - check ? check.arg : null - ]) - fail = false - this.succeed(result.path, result.version) - - break - } catch (err) { - this.catchErrors(err, check) - } - } - - if (fail) { - this.fail() - } - } - - /** - * 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. - * - * @private - * @argument {string} command command which will be executed in shell - * @returns {Promise} - */ - checkCommand (command) { - let exec = command - let args = this.argsExecutable - let shell = false - - // TODO: add explanation why shell is needed - if (this.win) { - // Arguments have to be manually quoted to avoid bugs with spaces in paths - shell = true - exec = `"${exec}"` - args = args.map((a) => `"${a}"`) - } - - return new Promise((resolve, reject) => { - this.run(exec, args, shell) - .then(this.checkExecPath) - .then(resolve) - .catch(reject) - }) - } - - /** - * 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. - * - * @private - * @returns {Promise} - */ - // theoretically this method can be removed in favor of checkCommand and getChecks. - // the only difference between checkCommand and checkPyLauncher is the shell arg for run function - // BUT! if we will use declarative style (would be cool i think) - // then we should somehow instruct checkCommand esp. on windows, that - // we do not want to execute command in the shell mode. - // Have tried to do this via "optional.shell" property of check object - // but have failed, because to support high modularity of file - // consistent interface across functions should be supported. - // Thus we have to pass check object not only in checkCommand but in - // every other function in conveyor. - // Passing check to every function from previous in promise chain would lead to - // hard to fix errors and overcomplicate structure of module - - checkPyLauncher () { - return new Promise((resolve, reject) => { - this.run(this.pyLauncher, this.argsExecutable, false) - .then(this.checkExecPath) - .then(resolve) - .catch(reject) - }) - } - - /** - * - * Check if a gotten path is correct and - * Python executable has the correct version to use. - * - * @private - * @argument {string} execPath path to check - * @returns {Promise} - */ - checkExecPath (execPath) { - // Returning new Promise instead of forwarding existing - // to pass both path and version - return new Promise((resolve, reject) => { - this.log.verbose(`executing "${colorizeOutput(colorHighlight.GREEN, execPath)}" to get version`) - - // do not wrap with quotes because executing without shell - this.run(execPath, this.argsVersion, false) - .then((ver) => { - this.log.silly(colorizeOutput(colorHighlight.GREEN, 'version got:'), ver) - // ? may be better code for version check - // ? may be move log messages to catchError func - const range = new semver.Range(this.semverRange) - let valid = false - - try { - valid = range.test(ver) - // throw new Error("test error") - } catch (err) { - this.log.silly(`range.test() threw:\n${err.stack}`) - this.addLog( - `"${colorizeOutput(colorHighlight.RED, execPath)}" does not have a valid version` - ) - this.addLog('Is it a Python executable?') - - // if you need to pass additional data, use ErrorWithData class - // you can also use any Error capable object - return reject(err) - } - - if (!valid) { - this.addLog( - `version is ${colorizeOutput( - colorHighlight.RED, - ver - )} - should be ${colorizeOutput(colorHighlight.RED, this.semverRange)}` - ) - this.addLog( - colorizeOutput(colorHighlight.RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') - ) - // if you need to pass additional data, use ErrorWithData class - reject(new Error(`Found unsupported Python version ${ver}`)) - } - - resolve({ path: execPath, version: ver }) - }) - .catch(reject) - }) - } - - /** - * Run an executable or shell command, trimming the output. - * - * @private - * @argument {string} exec command or path without arguments to execute - * @argument {string[]} args command args - * @argument {boolean} shell need be documented - * @returns {Promise} - */ - run (exec, args, shell) { - return new Promise( - /** - * @this {PythonFinder} - * @argument {function} resolve - * @argument {function} reject - */ - function (resolve, reject) { - const env = extend({}, this.env) - env.TERM = 'dumb' - /** @type {cp.ExecFileOptions} */ - const opts = { env: env, shell: shell } - - this.log.verbose(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: exec = `, exec) - this.log.verbose(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: args = `, args) - // TODO: make beauty print of PATH property (new line by semicolon) - this.log.silly(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: opts = `, JSON.stringify(opts, null, 2), '\n\n') - - //* possible outcomes with error messages on Windows (err.message, error.stack?, stderr) - // issue of encoding (garbage in terminal ) when 866 or any other locale code - // page is setted - // possible solutions: - // 1. leave it as is and just warn the user that it should use utf8 - // (already done in this.catchError's info statement) - // 2. somehow determine the user's terminal encoding and use utils.TextDecoder - // with the raw buffer from execFile. - // Requires to correct error.message because garbage persists there - // 3. Force the user's terminal to use utf8 encoding via e.g. run "chcp 65001". May break user's programs - // 4. use "cmd" command with flag "/U" and "/C" (for more information run "help cmd") - // which "Causes the output of - // internal commands ... to be Unicode" (utf16le) - //* note: find-python-script.py already send output in utf8 then may become necessary - //* to reencode string with Buffer.from(stderr).toString() or something - //* similar (if needed) - // for this solution all execFile call should look like execFile("cmd", ["/U", "/C", command to run, arg1, arg2, ...]) - //* all paths/commands and each argument must be in quotes if they contain spaces - - // ! potential bug - // if "shell" is true and is users default shell on windows is powershell then executables in PATH which name contain spaces will not work. - // it is feature of powershell which handle first arg in quotes as string - // thus if exec name has spaces, we can shield them (every space) with ` (backtick) - // or & (ampersand) can be placed before string in quotes, to tell to shell that - // it is executable, not string - - //* assume we have a utf8 compatible terminal - this.execFile(exec, args, opts, execFileCallback.bind(this)) - - // ? may be better to use utils.promisify - /** - * - * @param {Error} err - * @param {string} stdout - * @param {string} stderr - * @this {PythonFinder} - */ - function execFileCallback (err, stdout, stderr) { - // Done: add silly logs as in previous version - this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: err =`, (err && err.stack) || err) - this.log.verbose(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stdout =`, stdout) - this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stderr =`, stderr) - - // executed script shouldn't pass anything to stderr if successful - if (err || stderr) { - reject(new ErrorWithData({ data: { stderr: stderr || null }, messageOrError: err || null })) - } else { - // trim() function removes string endings that would break string comparison - const stdoutTrimmed = stdout.trim() - resolve(stdoutTrimmed) - } - } - }.bind(this) - ) - } - - /** - * Main error handling function in module - * Promises should throw errors up to this function - * Also used for logging - * - * @private - * TODO: figure out err type - * @param {ErrorWithData} err - * @param {check} check - */ - catchErrors (err, check) { - this.addLog(colorizeOutput(colorHighlight.RED, `FAIL: ${check.name || check.arg}`)) - - // array of error codes (type of errors) that we handling - const catchedErrorsCods = ['ENOENT', 9009] - - // don't know type of terminal errors - // @ts-ignore - if (catchedErrorsCods.includes(err.error ? err.error.code : undefined)) { - const { error } = err - // @ts-ignore - switch (error ? error.code : undefined) { - case 'ENOENT': - this.addLog( - `${colorizeOutput( - colorHighlight.RED, - 'ERROR:' - // @ts-ignore - )} No such file or directory: "${colorizeOutput(colorHighlight.RED, error.path)}"` - ) - break - - case 9009: - this.addLog( - `${colorizeOutput( - colorHighlight.RED, - 'ERROR:' - )} Command failed: file not found or not in PATH` - ) - break - } - } else { - this.addLog( - `${colorizeOutput(colorHighlight.RED, 'ERROR:')} ${ - err ? (err.message ? err.message : err) : '' - }` - ) - this.log.silly(colorizeOutput(colorHighlight.RED, 'FULL ERROR:'), err ? (err.stack ? err.stack : err) : '') - - // map through data object to print it as KEY: value - for (const prop in err.data) { - if (err.data[prop]) { - this.addLog(`${colorizeOutput(colorHighlight.RED, `${prop.toUpperCase()}:`)} ${err.data[prop].trim()}`) - } - } - } - this.addLog('--------------------------------------------') - } - - /** - * Function which is called if python path founded - * - * @private - * @param {string} execPath founded path - * @param {string} version python version - */ - succeed (execPath, version) { - this.log.info( - `using Python version ${colorizeOutput( - colorHighlight.GREEN, - version - )} found at "${colorizeOutput(colorHighlight.GREEN, execPath)}"` - ) - process.nextTick(this.callback.bind(null, null, execPath)) - } - - /** - * @private - */ - fail () { - const errorLog = this.errorLog.map((str) => str.trim()).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 = [ - '**********************************************************', - 'If you have non-displayed characters, please set "UTF-8"', - 'encoding.', - 'You need to install the latest version of Python.', - '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, - // if changing error message don't forget also change it test file too - new Error('Could not find any Python installation to use') - ) - ) - } -} - -/** - * Error with additional data. - * If you do not want to pass any additional data use regular Error - * - * !ALL MEMBERS EXCEPT "DATA" ARE OPTIONAL! - * @see Error - * - * @class - * @extends Error -*/ -class ErrorWithData extends Error { - // DONE: give to user possibility to pass existing error for which provide additional data - /** - * - * @typedef ErrorConstructor - * @property {{[key:string]: any}} data additional data to pass in data property of error object - * @property {string|Error} [messageOrError] - * @private - */ - /** - * @constructor - * @param {ErrorConstructor} [options] - */ - constructor (options) { - if (typeof options.messageOrError === 'string') { - const message = options.messageOrError - super(message) - } else if (options.messageOrError instanceof Error) { - const error = options.messageOrError - super(error.message) - this.error = error - } else { - super() - } - - if (!options.data) { - throw new TypeError('"data" property is required. If you do not want pass any additional data use regular Error instead this one') - } - - this.data = options.data - } -} - -/** - * - * @param {string} configPython force setted from terminal or npm config python path - * @param {(err:Error, found:string)=> void} callback succeed/error callback from where result - * is available - */ -function findPython (configPython, callback) { - const finder = new PythonFinder(configPython, callback) - finder.findPython() -} - -// function for tests -/* findPython(null, (err, found) => { - console.log('found:', colorizeOutput(colorHighlight.GREEN, found)) - console.log('err:', err) -}) - */ -module.exports = findPython -module.exports.test = { - PythonFinder: PythonFinder, - findPython: findPython -} +// @ts-check +'use strict' + +const path = require('path') +const log = require('npmlog') +const semver = require('semver') +const cp = require('child_process') +const extend = require('util')._extend // eslint-disable-line +const win = process.platform === 'win32' +const logWithPrefix = require('./util').logWithPrefix + +const systemDrive = process.env.SystemDrive || 'C:' +const username = process.env.USERNAME || process.env.USER || getOsUserInfo() +const localAppData = process.env.LOCALAPPDATA || `${systemDrive}\\${username}\\AppData\\Local` +const foundLocalAppData = process.env.LOCALAPPDATA || username +const programFiles = process.env.ProgramW6432 || process.env.ProgramFiles || `${systemDrive}\\Program Files` +const programFilesX86 = process.env['ProgramFiles(x86)'] || `${programFiles} (x86)` + +const winDefaultLocationsArray = [] +for (const majorMinor of ['39', '38', '37', '36']) { + if (foundLocalAppData) { + winDefaultLocationsArray.push( + `${localAppData}\\Programs\\Python\\Python${majorMinor}\\python.exe`, + `${programFiles}\\Python${majorMinor}\\python.exe`, + `${localAppData}\\Programs\\Python\\Python${majorMinor}-32\\python.exe`, + `${programFiles}\\Python${majorMinor}-32\\python.exe`, + `${programFilesX86}\\Python${majorMinor}-32\\python.exe` + ) + } else { + winDefaultLocationsArray.push( + `${programFiles}\\Python${majorMinor}\\python.exe`, + `${programFiles}\\Python${majorMinor}-32\\python.exe`, + `${programFilesX86}\\Python${majorMinor}-32\\python.exe` + ) + } +} + +function getOsUserInfo () { + try { + return require('os').userInfo().username + } catch {} +} + +//! after editing file dont forget run "npm test" and +//! change tests for this file if needed + +// ? may be some addition info in silly and verbose levels +// ? add safety to colorizeOutput function. E.g. when terminal doesn't +// ? support colorizing, just disable it (return given string) +// i hope i made not bad error handling but may be some improvements would be nice +// TODO: better error handler on linux/macOS + +// DONE: make more beautiful solution for selector of color +const colorHighlight = { + RED: '\x1b[31m', + RESET: '\x1b[0m', + GREEN: '\x1b[32m' +} + +/** + * Paint (not print, just colorize) string with selected color + * + * @param color color to set: colorHighlight.RED or colorHighlight.GREEN + * @param {string} string string to colorize + */ +function colorizeOutput (color, string) { + return color + string + colorHighlight.RESET +} + +//! on windows debug running with locale cmd encoding (e. g. chcp 866) +// to avoid that uncomment next lines +// locale encodings cause issues. See run func for more info +// this lines only for testing +// win ? cp.execSync("chcp 65001") : null +// log.level = "silly"; + +/** + * @class + */ +class PythonFinder { + /** + * + * @param {string} configPython force setted from terminal or npm config python path + * @param {(err:Error, found:string) => void} callback succeed/error callback from where result + * is available + */ + constructor (configPython, callback) { + this.callback = callback + this.configPython = configPython + this.errorLog = [] + + this.catchErrors = this.catchErrors.bind(this) + this.checkExecPath = this.checkExecPath.bind(this) + this.succeed = this.succeed.bind(this) + + this.SKIP = 0 + this.FAIL = 1 + + this.log = logWithPrefix(log, 'find Python') + + this.argsExecutable = [path.resolve(__dirname, 'find-python-script.py')] + this.argsVersion = [ + '-c', + 'import sys; print("%s.%s.%s" % sys.version_info[:3]);' + // for testing + // 'print("2.1.1")' + ] + this.semverRange = '>=3.6.0' + // These can be overridden for testing: + this.execFile = cp.execFile + this.env = process.env + this.win = win + this.pyLauncher = 'py.exe' + this.winDefaultLocations = winDefaultLocationsArray + } + + /** + * 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. + * + * ?message is array or one string + * + * @private + */ + addLog (message) { + // write also to verbose for consistent output + this.log.verbose(message) + this.errorLog.push(message) + } + + /** + * Find Python by trying a sequence of possibilities. + * Ignore errors, keep trying until Python is found. + * + * @public + */ + findPython () { + this.toCheck = this.getChecks() + + this.runChecks(this.toCheck) + } + + /** + * Getting list of checks which should be checked + * + * @private + * @returns {check[]} + */ + getChecks () { + if (this.env.NODE_GYP_FORCE_PYTHON) { + /** + * @type {check[]} + */ + return [ + { + before: () => { + this.addLog( + 'checking Python explicitly set from NODE_GYP_FORCE_PYTHON' + ) + this.addLog( + '- process.env.NODE_GYP_FORCE_PYTHON is ' + + `"${this.env.NODE_GYP_FORCE_PYTHON}"` + ) + }, + checkFunc: this.checkCommand, + arg: this.env.NODE_GYP_FORCE_PYTHON + } + ] + } + + /** + * @type {check[]} + */ + const checks = [ + { + before: (name) => { + if (!this.configPython) { + this.addLog( + `${colorizeOutput( + colorHighlight.GREEN, + 'Python is not set from command line or npm configuration' + )}` + ) + this.addLog('') + return this.SKIP + } + this.addLog( + 'checking Python explicitly set from command line or ' + + 'npm configuration' + ) + this.addLog( + '- "--python=" or "npm config get python" is ' + + `"${colorizeOutput(colorHighlight.GREEN, this.configPython)}"` + ) + }, + checkFunc: this.checkCommand, + arg: this.configPython + }, + { + before: (name) => { + if (!this.env.PYTHON) { + this.addLog( + `Python is not set from environment variable ${colorizeOutput( + colorHighlight.GREEN, + 'PYTHON' + )}` + ) + return this.SKIP + } + this.addLog( + 'checking Python explicitly set from environment ' + + 'variable PYTHON' + ) + this.addLog( + `${colorizeOutput( + colorHighlight.GREEN, + 'process.env.PYTHON' + )} is "${colorizeOutput(colorHighlight.GREEN, this.env.PYTHON)}"` + ) + }, + checkFunc: this.checkCommand, + arg: this.env.PYTHON, + // name used as very short description + name: 'process.env.PYTHON' + }, + { + checkFunc: this.checkCommand, + name: 'python3', + arg: 'python3' + }, + { + checkFunc: this.checkCommand, + name: 'python', + arg: 'python' + }, + ] + + if (this.win) { + for (let i = 0; i < this.winDefaultLocations.length; ++i) { + const location = this.winDefaultLocations[i] + checks.push({ + before: () => { + this.addLog( + `checking if Python is "${colorizeOutput(colorHighlight.GREEN, location)}"` + ) + }, + checkFunc: this.checkExecPath, + arg: location + }) + } + checks.push({ + before: () => { + this.addLog( + `checking if the ${colorizeOutput( + colorHighlight.GREEN, + 'py launcher' + )} can be used to find Python` + ) + }, + checkFunc: this.checkPyLauncher, + name: 'py Launcher' + }) + } + + return checks + } + + /** + * Type for possible place where python is + * + * @typedef check + * @type {object} + * @property {(name: string) => number|void} [before] what to execute before running check itself + * @property {function} checkFunc function which will perform check + * @property {string} [arg] what will be executed + * @property {string} [name] how check is named. this name is displayed to user + * @property {{shell: boolean}} [options] additional data, may be extended later, if shell true, exec command as in shell + */ + + /** + * + * + * @private + * @argument {check[]} checks + */ + async runChecks (checks) { + // using this flag because Fail is happen when ALL checks fail + let fail = true + + for (const check of checks) { + if (check.before) { + const beforeResult = check.before.apply(this, [check.name]) + + // if pretask fail - skip + if (beforeResult === this.SKIP || beforeResult === this.FAIL) { + // ?optional + // TODO: write to result arr which tests are SKIPPED + continue + } + } + + try { + if (!check.before) { + this.addLog( + `checking if "${colorizeOutput( + colorHighlight.GREEN, + check.name || check.arg + )}" can be used` + ) + } + + this.log.verbose( + `executing "${colorizeOutput( + colorHighlight.GREEN, + // DONE: swap in favor of arg (user want to see what we actually will run not how it is named) + check.arg || check.name + )}" to get Python executable path` + ) + + const result = await check.checkFunc.apply(this, [ + check ? check.arg : null + ]) + fail = false + this.succeed(result.path, result.version) + + break + } catch (err) { + this.catchErrors(err, check) + } + } + + if (fail) { + this.fail() + } + } + + /** + * 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. + * + * @private + * @argument {string} command command which will be executed in shell + * @returns {Promise} + */ + checkCommand (command) { + let exec = command + let args = this.argsExecutable + let shell = false + + // TODO: add explanation why shell is needed + if (this.win) { + // Arguments have to be manually quoted to avoid bugs with spaces in paths + shell = true + exec = `"${exec}"` + args = args.map((a) => `"${a}"`) + } + + return new Promise((resolve, reject) => { + this.run(exec, args, shell) + .then(this.checkExecPath) + .then(resolve) + .catch(reject) + }) + } + + /** + * 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. + * + * @private + * @returns {Promise} + */ + // theoretically this method can be removed in favor of checkCommand and getChecks. + // the only difference between checkCommand and checkPyLauncher is the shell arg for run function + // BUT! if we will use declarative style (would be cool i think) + // then we should somehow instruct checkCommand esp. on windows, that + // we do not want to execute command in the shell mode. + // Have tried to do this via "optional.shell" property of check object + // but have failed, because to support high modularity of file + // consistent interface across functions should be supported. + // Thus we have to pass check object not only in checkCommand but in + // every other function in conveyor. + // Passing check to every function from previous in promise chain would lead to + // hard to fix errors and overcomplicate structure of module + + checkPyLauncher () { + return new Promise((resolve, reject) => { + this.run(this.pyLauncher, this.argsExecutable, false) + .then(this.checkExecPath) + .then(resolve) + .catch(reject) + }) + } + + /** + * + * Check if a gotten path is correct and + * Python executable has the correct version to use. + * + * @private + * @argument {string} execPath path to check + * @returns {Promise} + */ + checkExecPath (execPath) { + // Returning new Promise instead of forwarding existing + // to pass both path and version + return new Promise((resolve, reject) => { + this.log.verbose(`executing "${colorizeOutput(colorHighlight.GREEN, execPath)}" to get version`) + + // do not wrap with quotes because executing without shell + this.run(execPath, this.argsVersion, false) + .then((ver) => { + this.log.silly(colorizeOutput(colorHighlight.GREEN, 'version got:'), ver) + // ? may be better code for version check + // ? may be move log messages to catchError func + const range = new semver.Range(this.semverRange) + let valid = false + + try { + valid = range.test(ver) + // throw new Error("test error") + } catch (err) { + this.log.silly(`range.test() threw:\n${err.stack}`) + this.addLog( + `"${colorizeOutput(colorHighlight.RED, execPath)}" does not have a valid version` + ) + this.addLog('Is it a Python executable?') + + // if you need to pass additional data, use ErrorWithData class + // you can also use any Error capable object + return reject(err) + } + + if (!valid) { + this.addLog( + `version is ${colorizeOutput( + colorHighlight.RED, + ver + )} - should be ${colorizeOutput(colorHighlight.RED, this.semverRange)}` + ) + this.addLog( + colorizeOutput(colorHighlight.RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') + ) + // if you need to pass additional data, use ErrorWithData class + reject(new Error(`Found unsupported Python version ${ver}`)) + } + + resolve({ path: execPath, version: ver }) + }) + .catch(reject) + }) + } + + /** + * Run an executable or shell command, trimming the output. + * + * @private + * @argument {string} exec command or path without arguments to execute + * @argument {string[]} args command args + * @argument {boolean} shell need be documented + * @returns {Promise} + */ + run (exec, args, shell) { + return new Promise( + /** + * @this {PythonFinder} + * @argument {function} resolve + * @argument {function} reject + */ + function (resolve, reject) { + const env = extend({}, this.env) + env.TERM = 'dumb' + /** @type {cp.ExecFileOptions} */ + const opts = { env: env, shell: shell } + + this.log.verbose(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: exec = `, exec) + this.log.verbose(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: args = `, args) + // TODO: make beauty print of PATH property (new line by semicolon) + this.log.silly(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: opts = `, JSON.stringify(opts, null, 2), '\n\n') + + //* possible outcomes with error messages on Windows (err.message, error.stack?, stderr) + // issue of encoding (garbage in terminal ) when 866 or any other locale code + // page is setted + // possible solutions: + // 1. leave it as is and just warn the user that it should use utf8 + // (already done in this.catchError's info statement) + // 2. somehow determine the user's terminal encoding and use utils.TextDecoder + // with the raw buffer from execFile. + // Requires to correct error.message because garbage persists there + // 3. Force the user's terminal to use utf8 encoding via e.g. run "chcp 65001". May break user's programs + // 4. use "cmd" command with flag "/U" and "/C" (for more information run "help cmd") + // which "Causes the output of + // internal commands ... to be Unicode" (utf16le) + //* note: find-python-script.py already send output in utf8 then may become necessary + //* to reencode string with Buffer.from(stderr).toString() or something + //* similar (if needed) + // for this solution all execFile call should look like execFile("cmd", ["/U", "/C", command to run, arg1, arg2, ...]) + //* all paths/commands and each argument must be in quotes if they contain spaces + + // ! potential bug + // if "shell" is true and is users default shell on windows is powershell then executables in PATH which name contain spaces will not work. + // it is feature of powershell which handle first arg in quotes as string + // thus if exec name has spaces, we can shield them (every space) with ` (backtick) + // or & (ampersand) can be placed before string in quotes, to tell to shell that + // it is executable, not string + + //* assume we have a utf8 compatible terminal + this.execFile(exec, args, opts, execFileCallback.bind(this)) + + // ? may be better to use utils.promisify + /** + * + * @param {Error} err + * @param {string} stdout + * @param {string} stderr + * @this {PythonFinder} + */ + function execFileCallback (err, stdout, stderr) { + // Done: add silly logs as in previous version + this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: err =`, (err && err.stack) || err) + this.log.verbose(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stdout =`, stdout) + this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stderr =`, stderr) + + // executed script shouldn't pass anything to stderr if successful + if (err || stderr) { + reject(new ErrorWithData({ data: { stderr: stderr || null }, messageOrError: err || null })) + } else { + // trim() function removes string endings that would break string comparison + const stdoutTrimmed = stdout.trim() + resolve(stdoutTrimmed) + } + } + }.bind(this) + ) + } + + /** + * Main error handling function in module + * Promises should throw errors up to this function + * Also used for logging + * + * @private + * TODO: figure out err type + * @param {ErrorWithData} err + * @param {check} check + */ + catchErrors (err, check) { + this.addLog(colorizeOutput(colorHighlight.RED, `FAIL: ${check.name || check.arg}`)) + + // array of error codes (type of errors) that we handling + const catchedErrorsCods = ['ENOENT', 9009] + + // don't know type of terminal errors + // @ts-ignore + if (catchedErrorsCods.includes(err.error ? err.error.code : undefined)) { + const { error } = err + // @ts-ignore + switch (error ? error.code : undefined) { + case 'ENOENT': + this.addLog( + `${colorizeOutput( + colorHighlight.RED, + 'ERROR:' + // @ts-ignore + )} No such file or directory: "${colorizeOutput(colorHighlight.RED, error.path)}"` + ) + break + + case 9009: + this.addLog( + `${colorizeOutput( + colorHighlight.RED, + 'ERROR:' + )} Command failed: file not found or not in PATH` + ) + break + } + } else { + this.addLog( + `${colorizeOutput(colorHighlight.RED, 'ERROR:')} ${ + err ? (err.message ? err.message : err) : '' + }` + ) + this.log.silly(colorizeOutput(colorHighlight.RED, 'FULL ERROR:'), err ? (err.stack ? err.stack : err) : '') + + // map through data object to print it as KEY: value + for (const prop in err.data) { + if (err.data[prop]) { + this.addLog(`${colorizeOutput(colorHighlight.RED, `${prop.toUpperCase()}:`)} ${err.data[prop].trim()}`) + } + } + } + this.addLog('--------------------------------------------') + } + + /** + * Function which is called if python path founded + * + * @private + * @param {string} execPath founded path + * @param {string} version python version + */ + succeed (execPath, version) { + this.log.info( + `using Python version ${colorizeOutput( + colorHighlight.GREEN, + version + )} found at "${colorizeOutput(colorHighlight.GREEN, execPath)}"` + ) + process.nextTick(this.callback.bind(null, null, execPath)) + } + + /** + * @private + */ + fail () { + const errorLog = this.errorLog.map((str) => str.trim()).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 = [ + '**********************************************************', + 'If you have non-displayed characters, please set "UTF-8"', + 'encoding.', + 'You need to install the latest version of Python.', + '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, + // if changing error message don't forget also change it test file too + new Error('Could not find any Python installation to use') + ) + ) + } +} + +/** + * Error with additional data. + * If you do not want to pass any additional data use regular Error + * + * !ALL MEMBERS EXCEPT "DATA" ARE OPTIONAL! + * @see Error + * + * @class + * @extends Error +*/ +class ErrorWithData extends Error { + // DONE: give to user possibility to pass existing error for which provide additional data + /** + * + * @typedef ErrorConstructor + * @property {{[key:string]: any}} data additional data to pass in data property of error object + * @property {string|Error} [messageOrError] + * @private + */ + /** + * @constructor + * @param {ErrorConstructor} [options] + */ + constructor (options) { + if (typeof options.messageOrError === 'string') { + const message = options.messageOrError + super(message) + } else if (options.messageOrError instanceof Error) { + const error = options.messageOrError + super(error.message) + this.error = error + } else { + super() + } + + if (!options.data) { + throw new TypeError('"data" property is required. If you do not want pass any additional data use regular Error instead this one') + } + + this.data = options.data + } +} + +/** + * + * @param {string} configPython force setted from terminal or npm config python path + * @param {(err:Error, found:string)=> void} callback succeed/error callback from where result + * is available + */ +function findPython (configPython, callback) { + const finder = new PythonFinder(configPython, callback) + finder.findPython() +} + +// function for tests +/* findPython(null, (err, found) => { + console.log('found:', colorizeOutput(colorHighlight.GREEN, found)) + console.log('err:', err) +}) + */ +module.exports = findPython +module.exports.test = { + PythonFinder: PythonFinder, + findPython: findPython +} From 3c106d7dd740054c18a77d3a0ad15cdb505f0ffd Mon Sep 17 00:00:00 2001 From: owl Date: Thu, 3 Jun 2021 17:39:11 +0300 Subject: [PATCH 20/25] chore: clarification comments --- test/test-find-python.js | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/test/test-find-python.js b/test/test-find-python.js index 14eaed09cc..6aa0579f19 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -55,6 +55,12 @@ function strictDeepEqual (received, wanted) { */ /** + * implement custom childProcess.execFile for testing proposes + * + * ! ***DO NOT FORGET TO OVERRIDE DEFAULT `PythonFinder.execFile` AFTER INSTANCING `PythonFinder`*** + * + * TODO: do overriding if automotive way + * * @param {OptionsObj} [optionsObj] */ function TestExecFile (optionsObj) { @@ -63,15 +69,16 @@ function TestExecFile (optionsObj) { * @this {PythonFinder} */ return function testExecFile (exec, args, options, callback) { - if (!(optionsObj ? optionsObj.shouldProduceError : false)) { + if (!(optionsObj && optionsObj.shouldProduceError)) { + // when checking version in checkExecPath, thus need to use PythonFinder.argsVersion if (args === this.argsVersion) { - if (optionsObj ? optionsObj.checkingWinDefaultPathes : false) { + if (optionsObj && optionsObj.checkingWinDefaultPathes) { if (this.winDefaultLocations.includes(exec)) { callback(null, testVersions.normal) } else { callback(new Error('not found')) } - } else if (optionsObj ? optionsObj.isPythonOutdated : false) { + } else if (optionsObj && optionsObj.isPythonOutdated) { callback(null, testVersions.outdated, null) } else { callback(null, testVersions.normal, null) @@ -80,7 +87,7 @@ function TestExecFile (optionsObj) { // DONE: map through argsExecutable to check that all args are equals strictDeepEqual(args, this.win ? this.argsExecutable.map((arg) => `"${arg}"`) : this.argsExecutable) ) { - if (optionsObj ? optionsObj.checkingPyLauncher : false) { + if (optionsObj && optionsObj.checkingPyLauncher) { if ( exec === 'py.exe' || exec === (this.win ? '"python"' : 'python') @@ -89,10 +96,13 @@ function TestExecFile (optionsObj) { } else { callback(new Error('not found')) } - } else if (optionsObj ? optionsObj.checkingWinDefaultPathes : false) { + } else if (optionsObj && optionsObj.checkingWinDefaultPathes) { + // return "not found" for regular checks (env-vars etc.) + // which are running twice: + // first to get path, second to check it callback(new Error('not found')) } else { - // should be trimmed + // returned string should be trimmed callback(null, testString + '\n', null) } } else { From 3a4cc8697ab66e95816dd1dea958c634adb0a824 Mon Sep 17 00:00:00 2001 From: owl Date: Thu, 3 Jun 2021 18:15:39 +0300 Subject: [PATCH 21/25] chore: use `&&` or `||` instead of `?` --- lib/find-python.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index 686e91a03a..434c69063d 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -556,10 +556,10 @@ class PythonFinder { // don't know type of terminal errors // @ts-ignore - if (catchedErrorsCods.includes(err.error ? err.error.code : undefined)) { + if (catchedErrorsCods.includes(err.error && err.error.code)) { const { error } = err // @ts-ignore - switch (error ? error.code : undefined) { + switch (error && error.code) { case 'ENOENT': this.addLog( `${colorizeOutput( @@ -582,10 +582,10 @@ class PythonFinder { } else { this.addLog( `${colorizeOutput(colorHighlight.RED, 'ERROR:')} ${ - err ? (err.message ? err.message : err) : '' + err ? (err.message || err) : '' }` ) - this.log.silly(colorizeOutput(colorHighlight.RED, 'FULL ERROR:'), err ? (err.stack ? err.stack : err) : '') + this.log.silly(colorizeOutput(colorHighlight.RED, 'FULL ERROR:'), err ? (err.stack || err) : '') // map through data object to print it as KEY: value for (const prop in err.data) { From d56bec1527fe514a46b099370e1f568dc3e96119 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Thu, 6 May 2021 12:31:11 +0300 Subject: [PATCH 22/25] lib: reinvent log system in find-python.js now here is Logger class with all logging functions and all errors except regular Error class should implement log method --- lib/find-python.js | 270 +++++++++++++++++++++++++++------------------ 1 file changed, 165 insertions(+), 105 deletions(-) diff --git a/lib/find-python.js b/lib/find-python.js index 434c69063d..4697275409 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -2,7 +2,7 @@ 'use strict' const path = require('path') -const log = require('npmlog') +const npmlog = require('npmlog') const semver = require('semver') const cp = require('child_process') const extend = require('util')._extend // eslint-disable-line @@ -50,21 +50,60 @@ function getOsUserInfo () { // i hope i made not bad error handling but may be some improvements would be nice // TODO: better error handler on linux/macOS -// DONE: make more beautiful solution for selector of color -const colorHighlight = { - RED: '\x1b[31m', - RESET: '\x1b[0m', - GREEN: '\x1b[32m' -} +class Logger { + constructor () { + /** @private */ + this._npmlog = npmlog + this.log = logWithPrefix(this._npmlog, 'find Python') -/** + /** @private */ + this._errorLog = [] + } + + /** + * Logs a message at verbose level, but also saves it to internal buffer + * from which they can be accessed later by calling `this.dumpErrorLog()`. + * This should help diagnose the problem. + * + * ?message is array or one string + */ + addLog (message) { + // write also to verbose for consistent output + this.log.verbose(message) + this._errorLog.push(message) + } + + /** + * Clear buffer with error messages and return colleted messages about error as a string + * To add messages to error buffer use `this.addLog()` + * + * @returns string contain all collected error messages + */ + dumpErrorLog () { + const errString = this._errorLog.map((str) => str.trim()).join('\n') + this._errorLog = [] + return errString + } + + /** * Paint (not print, just colorize) string with selected color * * @param color color to set: colorHighlight.RED or colorHighlight.GREEN * @param {string} string string to colorize */ -function colorizeOutput (color, string) { - return color + string + colorHighlight.RESET + colorizeOutput (color, string) { + return color + string + Logger.colors.RESET + } +} + +// `standard` package version we using doesn't work +// with class property declaration syntax. +// As soon as we upgrade standard, this should be +// declared as static property of `Logger` class +Logger.colors = { + RED: '\x1b[31m', + GREEN: '\x1b[32m', + RESET: '\x1b[0m' } //! on windows debug running with locale cmd encoding (e. g. chcp 866) @@ -72,7 +111,7 @@ function colorizeOutput (color, string) { // locale encodings cause issues. See run func for more info // this lines only for testing // win ? cp.execSync("chcp 65001") : null -// log.level = "silly"; +// npmlog.level = "silly"; /** * @class @@ -87,7 +126,6 @@ class PythonFinder { constructor (configPython, callback) { this.callback = callback this.configPython = configPython - this.errorLog = [] this.catchErrors = this.catchErrors.bind(this) this.checkExecPath = this.checkExecPath.bind(this) @@ -96,7 +134,7 @@ class PythonFinder { this.SKIP = 0 this.FAIL = 1 - this.log = logWithPrefix(log, 'find Python') + this.logger = new Logger() this.argsExecutable = [path.resolve(__dirname, 'find-python-script.py')] this.argsVersion = [ @@ -114,20 +152,6 @@ class PythonFinder { this.winDefaultLocations = winDefaultLocationsArray } - /** - * 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. - * - * ?message is array or one string - * - * @private - */ - addLog (message) { - // write also to verbose for consistent output - this.log.verbose(message) - this.errorLog.push(message) - } - /** * Find Python by trying a sequence of possibilities. * Ignore errors, keep trying until Python is found. @@ -154,10 +178,10 @@ class PythonFinder { return [ { before: () => { - this.addLog( + this.logger.addLog( 'checking Python explicitly set from NODE_GYP_FORCE_PYTHON' ) - this.addLog( + this.logger.addLog( '- process.env.NODE_GYP_FORCE_PYTHON is ' + `"${this.env.NODE_GYP_FORCE_PYTHON}"` ) @@ -175,22 +199,22 @@ class PythonFinder { { before: (name) => { if (!this.configPython) { - this.addLog( - `${colorizeOutput( - colorHighlight.GREEN, + this.logger.addLog( + `${this.logger.colorizeOutput( + Logger.colors.GREEN, 'Python is not set from command line or npm configuration' )}` ) - this.addLog('') + this.logger.addLog('') return this.SKIP } - this.addLog( + this.logger.addLog( 'checking Python explicitly set from command line or ' + 'npm configuration' ) - this.addLog( + this.logger.addLog( '- "--python=" or "npm config get python" is ' + - `"${colorizeOutput(colorHighlight.GREEN, this.configPython)}"` + `"${this.logger.colorizeOutput(Logger.colors.GREEN, this.configPython)}"` ) }, checkFunc: this.checkCommand, @@ -199,23 +223,23 @@ class PythonFinder { { before: (name) => { if (!this.env.PYTHON) { - this.addLog( - `Python is not set from environment variable ${colorizeOutput( - colorHighlight.GREEN, + this.logger.addLog( + `Python is not set from environment variable ${this.logger.colorizeOutput( + Logger.colors.GREEN, 'PYTHON' )}` ) return this.SKIP } - this.addLog( + this.logger.addLog( 'checking Python explicitly set from environment ' + 'variable PYTHON' ) - this.addLog( - `${colorizeOutput( - colorHighlight.GREEN, + this.logger.addLog( + `${this.logger.colorizeOutput( + Logger.colors.GREEN, 'process.env.PYTHON' - )} is "${colorizeOutput(colorHighlight.GREEN, this.env.PYTHON)}"` + )} is "${this.logger.colorizeOutput(Logger.colors.GREEN, this.env.PYTHON)}"` ) }, checkFunc: this.checkCommand, @@ -240,8 +264,8 @@ class PythonFinder { const location = this.winDefaultLocations[i] checks.push({ before: () => { - this.addLog( - `checking if Python is "${colorizeOutput(colorHighlight.GREEN, location)}"` + this.logger.addLog( + `checking if Python is "${this.logger.colorizeOutput(Logger.colors.GREEN, location)}"` ) }, checkFunc: this.checkExecPath, @@ -250,9 +274,9 @@ class PythonFinder { } checks.push({ before: () => { - this.addLog( - `checking if the ${colorizeOutput( - colorHighlight.GREEN, + this.logger.addLog( + `checking if the ${this.logger.colorizeOutput( + Logger.colors.GREEN, 'py launcher' )} can be used to find Python` ) @@ -301,17 +325,17 @@ class PythonFinder { try { if (!check.before) { - this.addLog( - `checking if "${colorizeOutput( - colorHighlight.GREEN, + this.logger.addLog( + `checking if "${this.logger.colorizeOutput( + Logger.colors.GREEN, check.name || check.arg )}" can be used` ) } - this.log.verbose( - `executing "${colorizeOutput( - colorHighlight.GREEN, + this.logger.log.verbose( + `executing "${this.logger.colorizeOutput( + Logger.colors.GREEN, // DONE: swap in favor of arg (user want to see what we actually will run not how it is named) check.arg || check.name )}" to get Python executable path` @@ -409,12 +433,12 @@ class PythonFinder { // Returning new Promise instead of forwarding existing // to pass both path and version return new Promise((resolve, reject) => { - this.log.verbose(`executing "${colorizeOutput(colorHighlight.GREEN, execPath)}" to get version`) + this.logger.log.verbose(`executing "${this.logger.colorizeOutput(Logger.colors.GREEN, execPath)}" to get version`) // do not wrap with quotes because executing without shell this.run(execPath, this.argsVersion, false) .then((ver) => { - this.log.silly(colorizeOutput(colorHighlight.GREEN, 'version got:'), ver) + this.logger.log.silly(this.logger.colorizeOutput(Logger.colors.GREEN, 'got version:'), ver) // ? may be better code for version check // ? may be move log messages to catchError func const range = new semver.Range(this.semverRange) @@ -424,11 +448,9 @@ class PythonFinder { valid = range.test(ver) // throw new Error("test error") } catch (err) { - this.log.silly(`range.test() threw:\n${err.stack}`) - this.addLog( - `"${colorizeOutput(colorHighlight.RED, execPath)}" does not have a valid version` - ) - this.addLog('Is it a Python executable?') + this.logger.log.silly(`semver.satisfies(${ver}, ${this.semverRange}) threw:\n${err.stack}`) + this.logger.addLog(`"${this.logger.colorizeOutput(Logger.colors.RED, execPath)}" does not have a valid version`) + this.logger.addLog('Is it a Python executable?') // if you need to pass additional data, use ErrorWithData class // you can also use any Error capable object @@ -436,17 +458,7 @@ class PythonFinder { } if (!valid) { - this.addLog( - `version is ${colorizeOutput( - colorHighlight.RED, - ver - )} - should be ${colorizeOutput(colorHighlight.RED, this.semverRange)}` - ) - this.addLog( - colorizeOutput(colorHighlight.RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED') - ) - // if you need to pass additional data, use ErrorWithData class - reject(new Error(`Found unsupported Python version ${ver}`)) + reject(new PythonVersionError({ received: ver, wanted: this.semverRange }, `Found unsupported Python version ${ver}`)) } resolve({ path: execPath, version: ver }) @@ -477,10 +489,10 @@ class PythonFinder { /** @type {cp.ExecFileOptions} */ const opts = { env: env, shell: shell } - this.log.verbose(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: exec = `, exec) - this.log.verbose(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: args = `, args) + this.logger.log.verbose(`${this.logger.colorizeOutput(Logger.colors.GREEN, 'execFile')}: exec = `, exec) + this.logger.log.verbose(`${this.logger.colorizeOutput(Logger.colors.GREEN, 'execFile')}: args = `, args) // TODO: make beauty print of PATH property (new line by semicolon) - this.log.silly(`${colorizeOutput(colorHighlight.GREEN, 'execFile')}: opts = `, JSON.stringify(opts, null, 2), '\n\n') + this.logger.log.silly(`${this.logger.colorizeOutput(Logger.colors.GREEN, 'execFile')}: opts = `, JSON.stringify(opts, null, 2), '\n\n') //* possible outcomes with error messages on Windows (err.message, error.stack?, stderr) // issue of encoding (garbage in terminal ) when 866 or any other locale code @@ -521,9 +533,9 @@ class PythonFinder { */ function execFileCallback (err, stdout, stderr) { // Done: add silly logs as in previous version - this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: err =`, (err && err.stack) || err) - this.log.verbose(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stdout =`, stdout) - this.log.silly(`${colorizeOutput(colorHighlight.RED, 'execFile result')}: stderr =`, stderr) + this.logger.log.silly(`${this.logger.colorizeOutput(Logger.colors.RED, 'execFile result')}: err =`, (err && err.stack) || err) + this.logger.log.verbose(`${this.logger.colorizeOutput(Logger.colors.RED, 'execFile result')}: stdout =`, stdout) + this.logger.log.silly(`${this.logger.colorizeOutput(Logger.colors.RED, 'execFile result')}: stderr =`, stderr) // executed script shouldn't pass anything to stderr if successful if (err || stderr) { @@ -549,7 +561,7 @@ class PythonFinder { * @param {check} check */ catchErrors (err, check) { - this.addLog(colorizeOutput(colorHighlight.RED, `FAIL: ${check.name || check.arg}`)) + this.logger.addLog(this.logger.colorizeOutput(Logger.colors.RED, `FAIL: ${check.name || check.arg}`)) // array of error codes (type of errors) that we handling const catchedErrorsCods = ['ENOENT', 9009] @@ -561,40 +573,39 @@ class PythonFinder { // @ts-ignore switch (error && error.code) { case 'ENOENT': - this.addLog( - `${colorizeOutput( - colorHighlight.RED, + this.logger.addLog( + `${this.logger.colorizeOutput( + Logger.colors.RED, 'ERROR:' // @ts-ignore - )} No such file or directory: "${colorizeOutput(colorHighlight.RED, error.path)}"` + )} No such file or directory: "${this.logger.colorizeOutput(Logger.colors.RED, error.path)}"` ) break case 9009: - this.addLog( - `${colorizeOutput( - colorHighlight.RED, + this.logger.addLog( + `${this.logger.colorizeOutput( + Logger.colors.RED, 'ERROR:' )} Command failed: file not found or not in PATH` ) break } + } else if (err instanceof PythonVersionError) { + err.log(this.logger) } else { - this.addLog( - `${colorizeOutput(colorHighlight.RED, 'ERROR:')} ${ + this.logger.addLog( + `${this.logger.colorizeOutput(Logger.colors.RED, 'ERROR:')} ${ err ? (err.message || err) : '' }` ) - this.log.silly(colorizeOutput(colorHighlight.RED, 'FULL ERROR:'), err ? (err.stack || err) : '') + this.logger.log.silly(this.logger.colorizeOutput(Logger.colors.RED, 'FULL ERROR:'), err ? (err.stack || err) : '') - // map through data object to print it as KEY: value - for (const prop in err.data) { - if (err.data[prop]) { - this.addLog(`${colorizeOutput(colorHighlight.RED, `${prop.toUpperCase()}:`)} ${err.data[prop].trim()}`) - } + if (err.log) { + err.log(this.logger) } } - this.addLog('--------------------------------------------') + this.logger.addLog('--------------------------------------------') } /** @@ -605,11 +616,11 @@ class PythonFinder { * @param {string} version python version */ succeed (execPath, version) { - this.log.info( - `using Python version ${colorizeOutput( - colorHighlight.GREEN, + this.logger.log.info( + `using Python version ${this.logger.colorizeOutput( + Logger.colors.GREEN, version - )} found at "${colorizeOutput(colorHighlight.GREEN, execPath)}"` + )} found at "${this.logger.colorizeOutput(Logger.colors.GREEN, execPath)}"` ) process.nextTick(this.callback.bind(null, null, execPath)) } @@ -618,7 +629,7 @@ class PythonFinder { * @private */ fail () { - const errorLog = this.errorLog.map((str) => str.trim()).join('\n') + const errorLog = this.logger.dumpErrorLog() const pathExample = this.win ? 'C:\\Path\\To\\python.exe' @@ -643,7 +654,7 @@ class PythonFinder { '**********************************************************' ].join('\n') - this.log.error(`\n${errorLog}\n\n${info}\n`) + this.logger.log.error(`\n${errorLog}\n\n${info}\n`) process.nextTick( this.callback.bind( null, @@ -676,6 +687,7 @@ class ErrorWithData extends Error { /** * @constructor * @param {ErrorConstructor} [options] + * @throws {TypeError} */ constructor (options) { if (typeof options.messageOrError === 'string') { @@ -695,6 +707,54 @@ class ErrorWithData extends Error { this.data = options.data } + + /** + * + * @param {Logger} logger + */ + log (logger) { + // map through data object to print it as KEY: value + for (const prop in this.data) { + if (this.data[prop]) { + logger.addLog(`${logger.colorizeOutput(Logger.colors.RED, `${prop.toUpperCase()}:`)} ${this.data[prop].trim()}`) + } + } + } +} + +class PythonVersionError extends ErrorWithData { + /** + * + * @param {{wanted: string, received: string}} ver + * @param {string} message + */ + constructor (ver, message) { + super({ messageOrError: message, data: { version: ver.received } }) + + /** @private */ + this._ver = ver + } + + /** + * Log error + * @param {Logger} logger + */ + log (logger) { + if (!this.data.version) { + logger.addLog(this.message) + } else { + logger.addLog( + `version is ${logger.colorizeOutput( + Logger.colors.RED, + this.data.version + )} - should be ${logger.colorizeOutput(Logger.colors.GREEN, this._ver.wanted)}` + ) + + logger.addLog('') + logger.addLog(logger.colorizeOutput(Logger.colors.RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED')) + logger.addLog('') + } + } } /** @@ -710,10 +770,10 @@ function findPython (configPython, callback) { // function for tests /* findPython(null, (err, found) => { - console.log('found:', colorizeOutput(colorHighlight.GREEN, found)) + console.log('found:', (new Logger()).colorizeOutput(Logger.colors.GREEN, found)) console.log('err:', err) -}) - */ +}) */ + module.exports = findPython module.exports.test = { PythonFinder: PythonFinder, From 826f3919bc3bed577fb22a59a659e7894d7d02eb Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Fri, 4 Jun 2021 12:57:12 +0300 Subject: [PATCH 23/25] fix: properly display `optionObj` --- test/test-find-python.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-find-python.js b/test/test-find-python.js index 6aa0579f19..2fa90917bf 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -113,7 +113,7 @@ are: ${args};\n\nValid are: \n${this.argsExecutable}\n${this.argsVersion}` } } else { const testError = new Error( - `test error ${testString}; optionsObj: ${optionsObj}` + `test error ${testString}; optionsObj: ${JSON.stringify(optionsObj)}` ) callback(testError) } From 00d477399e8d5066bff0af3c5dd81545cd66434e Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Fri, 4 Jun 2021 12:59:52 +0300 Subject: [PATCH 24/25] chore: clarification comment --- test/test-find-python.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/test-find-python.js b/test/test-find-python.js index 2fa90917bf..02f5985751 100644 --- a/test/test-find-python.js +++ b/test/test-find-python.js @@ -9,6 +9,9 @@ const util = require('util') const path = require('path') const npmlog = require('npmlog') const fs = require('fs') +// just comment this line to see log output +// useful to not to test output by hand +// ! keep uncommented when committing npmlog.level = 'silent' // what final error message displayed in terminal should contain @@ -317,7 +320,7 @@ test('find-python', { buffered: true }, (t) => { // making fixture paths.testDir = fs.mkdtempSync(path.resolve(paths.baseDir, 'node_modules', 'pythonFindTestFolder-')) - // using "junction" to avoid permission error + // using "junction" to avoid permission error on windows (ignored on other platforms) fs.symlinkSync(paths.pythonDir, path.resolve(paths.testDir, testString), 'junction') console.log('🚀 ~ file: test-find-python.js ~ line 312 ~ await.test ~ path.resolve(paths.testDir, testString)', path.resolve(paths.testDir, testString)) console.log('🚀 ~ file: test-find-python.js ~ line 312 ~ await.test ~ paths.pythonDir', paths.pythonDir) From c561192271f63becd4efae7cdf1154f1b749cab3 Mon Sep 17 00:00:00 2001 From: owl-from-hogvarts Date: Fri, 4 Jun 2021 13:08:54 +0300 Subject: [PATCH 25/25] chore: remove comma --- lib/find-python.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/find-python.js b/lib/find-python.js index 4697275409..16885bb4c8 100644 --- a/lib/find-python.js +++ b/lib/find-python.js @@ -256,7 +256,7 @@ class PythonFinder { checkFunc: this.checkCommand, name: 'python', arg: 'python' - }, + } ] if (this.win) {