Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Bump which to v4 #139

Merged
merged 2 commits into from
Aug 30, 2024
Merged

Bump which to v4 #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1cd6975
Update `which`
fregante Aug 11, 2024
c92c14e
Fix mocks
fregante Aug 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ var which = require("which");
* @return {Promise}
*/
function normalizeBinary (binaryPath, platform, arch) {
return new Promise(function(resolve, reject) {
return new Promise(function(resolve) {
platform = platform || os.platform();
arch = arch || os.arch();
binaryPath = binaryPath || process.env.JPM_FIREFOX_BINARY || "firefox";
Expand Down Expand Up @@ -66,13 +66,7 @@ function normalizeBinary (binaryPath, platform, arch) {
// On linux but no path yet, use which to try and find the binary
else if (platform.indexOf("linux") !== -1) {
binaryPath = normalizeBinary.appNames[binaryPath + " on linux"] || binaryPath;
which(binaryPath, function(err, resolvedPath) {
if (err) {
reject(err);
} else {
resolve(resolvedPath);
}
});
resolve(which(binaryPath));
return;
}
// No action needed on windows if it's an executable already
Expand Down
28 changes: 7 additions & 21 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"dependencies": {
"commander": "2.9.0",
"shell-quote": "1.8.1",
"which": "1.2.4",
"which": "^4.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Version 4.0.0 drops Node 14 support, and does not have other significant changes, per https://github.com/npm/node-which/blob/main/CHANGELOG.md#400-2023-08-29

If we are planning to release a major version then this lgtm. Otherwise we should use 3.0.1 instead.

Considering that we are indeed preparing for a major version in #89, 4.0.0 here is acceptable.

"winreg": "0.0.12"
},
"devDependencies": {
Expand Down
12 changes: 6 additions & 6 deletions test/run/test.utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ var sandbox = require('sandboxed-module');
var binary = utils.normalizeBinary;

sandbox.configure({
requires: {
// sandboxed-module is unable to find this module directly
Copy link
Member

Choose a reason for hiding this comment

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

... because it is not listed at https://github.com/felixge/node-sandboxed-module/blob/v2.0.4/lib/builtin_modules.json and would therefore result in the module being resolved as a relative file path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened a PR upstream. This PR can be merged in the meanwhile because that package hasn't seen updates in 4 years.

"fs/promises": require("fs/promises"),
},
globals: {
// As of Node 12.x, the process global is no longer an enumerable property of the global.
// https://github.com/nodejs/node/pull/26882
Expand Down Expand Up @@ -160,9 +164,7 @@ describe("lib/utils", function () {

var binary = sandbox.require("../../lib/utils", {
requires: {
which: function(bin, callback) {
callback(null, "/usr/bin/" + bin);
}
which: async (bin) => "/usr/bin/" + bin
}
}).normalizeBinary;

Expand Down Expand Up @@ -247,9 +249,7 @@ describe("lib/utils", function () {

var binary = sandbox.require("../../lib/utils", {
requires: {
which: function(bin, callback) {
callback(null, "/usr/bin/" + bin);
}
which: async (bin) => "/usr/bin/" + bin
}
}).normalizeBinary;

Expand Down