From d383f69267151d83261fb449efa08b008be18610 Mon Sep 17 00:00:00 2001 From: Ewan Harris Date: Sat, 24 Oct 2020 14:18:36 +0100 Subject: [PATCH] fix: if passed python option set environment variable Previously if passed the --python option node-gyp would always use it, now the only way to force using that value is to set an environment variable pointing to it --- lib/configure.js | 6 +++++- test/build.test.js | 15 +++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/configure.js b/lib/configure.js index a6e34382..0ea4229e 100644 --- a/lib/configure.js +++ b/lib/configure.js @@ -16,7 +16,11 @@ function configure(gyp, argv, callback) { known_gyp_args.forEach(function(key) { var val = gyp.opts[key] || gyp.opts[key.replace('-','_')]; if (val) { - final_args.push('--'+key+'='+val); + if (key === 'python') { + // node-gyp 5+ will use this if set + process.env.NODE_GYP_FORCE_PYTHON = val; + } + final_args.push('--'+key+'='+val); } }); // --ensure=false tell node-gyp to re-install node development headers diff --git a/test/build.test.js b/test/build.test.js index c2b4d85f..8e344fd9 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -179,6 +179,15 @@ test.Test.prototype.stringContains = function(actual, contents, message) { }); }; +test.Test.prototype.stringMatches = function(actual, regex, message) { + this._assert(regex.test(actual) > -1, { + message: message || 'should match '+regex, + operator: 'stringMatches', + actual: actual, + expected: regex + }); +}; + // Because the below tests only ensure that flags can be correctly passed to node-gyp is it not // likely they will behave differently for different apps. So we save time by avoiding running these for each app. var app = apps[0]; @@ -206,9 +215,11 @@ test(app.name + ' passes --nodedir down to node-gyp via npm' + app.args, functio // https://github.com/nodejs/node-gyp/blob/c84a54194781410743efe353d18ca7d20fc9d3a3/lib/configure.js#L396-L397 if (process.platform !== 'win32') { test(app.name + ' passes --python down to node-gyp via node-pre-gyp ' + app.args, function(t) { + // node-gyp@5 improved the python detection the the below wont be used if it's invalid, + // we can instead set an invalid NODE_GYP_FORCE_PYTHON environment variable to force a failure run('node-pre-gyp', 'configure', '--python=invalid-value', app, {}, function(err,stdout,stderr) { t.ok(err, 'Expected command to fail'); - t.stringContains(stderr,"Can't find Python executable"); + t.stringMatches(stderr,/(Can't find Python executable|Could not find any Python installation to use)/); t.end(); }); }); @@ -216,7 +227,7 @@ if (process.platform !== 'win32') { test(app.name + ' passes --python down to node-gyp via npm ' + app.args, function(t) { run('node-pre-gyp', 'configure', '--build-from-source --python=invalid-value', app, {}, function(err,stdout,stderr) { t.ok(err, 'Expected command to fail'); - t.stringContains(stderr,"Can't find Python executable"); + t.stringMatches(stderr,/(Can't find Python executable|Could not find any Python installation to use)/); t.end(); }); });