Skip to content

Commit 5f9cfef

Browse files
committed
build: Remove which and isexe with vendored simplifications
We depended on which 5.0.0, which depended on isexe `^3.1.1`. The recent isexe 3.1.3 release is significantly bigger than its previous releases and pushes us over the install size threshold we set for ourselves, this causing CI to start failing on fresh installs. Even before this recent change it was arguably too large at over 10K, and that's for the tarball. The actual uncompressed size for isexe was 58kB. Replace all this with a simple 20-line version derived from it, which does exactly what we need, is obviously correct and secure, and is not likely to need a change for a long time.
1 parent 5439843 commit 5f9cfef

5 files changed

Lines changed: 87 additions & 23 deletions

File tree

package.json

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,10 @@
2525
"dependencies": {
2626
"commander": "12.1.0",
2727
"tap-parser": "18.0.0",
28-
"which": "5.0.0",
29-
"yaml": "^2.8.1"
28+
"yaml": "^2.8.3"
3029
},
3130
"devDependencies": {
3231
"@types/node": "22.10.5",
33-
"@types/which": "3.0.4",
3432
"eslint": "~8.57.1",
3533
"eslint-config-semistandard": "~17.0.0",
3634
"eslint-plugin-qunit": "^8.1.2",

src/browsers.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import fs from 'node:fs';
44
import path from 'node:path';
55

6-
import which from 'which';
6+
import which from './which.js';
77
import safari from './safari.js';
88
import { concatGenFn, CommandNotFoundError, LocalBrowser } from './util.js';
99
/** @import { Logger, Browser } from './qtap.js' */
@@ -41,7 +41,7 @@ function * getFirefoxPaths () {
4141
//
4242
// Example: /usr/bin/firefox
4343
yield process.env.FIREFOX_BIN;
44-
yield which.sync('firefox', { nothrow: true });
44+
yield which('firefox');
4545

4646
if (process.platform === 'darwin') {
4747
const appPath = '/Applications/Firefox.app/Contents/MacOS/firefox';
@@ -56,8 +56,8 @@ function * getFirefoxPaths () {
5656

5757
function * getChromePaths () {
5858
yield process.env.CHROME_BIN;
59-
yield which.sync('google-chrome', { nothrow: true });
60-
yield which.sync('google-chrome-stable', { nothrow: true });
59+
yield which('google-chrome');
60+
yield which('google-chrome-stable');
6161

6262
if (process.platform === 'darwin') {
6363
const appPath = '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome';
@@ -73,8 +73,8 @@ function * getChromePaths () {
7373
function * getChromiumPaths () {
7474
// Try 'chromium-browser' first to avoid conflict with 'chromium' from chromium-bsu on Debian
7575
yield process.env.CHROMIUM_BIN;
76-
yield which.sync('chromium-browser', { nothrow: true });
77-
yield which.sync('chromium', { nothrow: true });
76+
yield which('chromium-browser');
77+
yield which('chromium');
7878

7979
if (process.platform === 'darwin') {
8080
const appPath = '/Applications/Chromium.app/Contents/MacOS/Chromium';
@@ -93,8 +93,8 @@ function * getEdgePaths () {
9393
// https://github.com/actions/runner-images/blob/1ffc99a7ae/images/ubuntu/scripts/build/install-microsoft-edge.sh#L11
9494
// https://github.com/microsoft/playwright/blob/v1.49.1/packages/playwright-core/src/server/registry/index.ts#L560
9595
yield process.env.EDGE_BIN;
96-
yield which.sync('microsoft-edge', { nothrow: true });
97-
yield which.sync('microsoft-edge-stable', { nothrow: true });
96+
yield which('microsoft-edge');
97+
yield which('microsoft-edge-stable');
9898
yield '/opt/microsoft/msedge/msedge';
9999

100100
if (process.platform === 'darwin') {

src/safari.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import which from 'which';
1+
import which from './which.js';
22
import { LocalBrowser, QTapError, CommandNotFoundError } from './util.js';
33
/** @import { Logger } from './qtap.js' */
44

@@ -48,7 +48,7 @@ let sharedSafariDriverPort = null;
4848
async function safariOne (url, signals, logger) {
4949
// Step 1: Start safaridriver
5050
if (!sharedSafariDriverPort) {
51-
const safaridriverBin = process.env.SAFARIDRIVER_BIN || which.sync('safaridriver', { nothrow: true });
51+
const safaridriverBin = process.env.SAFARIDRIVER_BIN || which('safaridriver');
5252
if (process.platform !== 'darwin' || !safaridriverBin) {
5353
throw new CommandNotFoundError('Safari requires macOS and safaridriver');
5454
}

src/which.js

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/**
2+
* The ISC License
3+
*
4+
* Copyright (c) Isaac Z. Schlueter and Contributors
5+
*
6+
* Permission to use, copy, modify, and/or distribute this software for any
7+
* purpose with or without fee is hereby granted, provided that the above
8+
* copyright notice and this permission notice appear in all copies.
9+
*
10+
* THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
11+
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
12+
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
13+
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
14+
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
15+
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR
16+
* IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
17+
*
18+
* --
19+
*
20+
* This is based on https://github.com/npm/node-which/blob/v7.0.0/lib/index.js with
21+
* the following modifications for QTap:
22+
* - Fork node-which to inline isexe 3.1.1, because isexe 3.1.3 and later are too large.
23+
* The increase would fail our structure test for the "Lean" policy (ARCHITECTURE.md).
24+
* The essential logic for node-which has been constant for years, easy to understand,
25+
* easy to test, and adds more cost than benefit when maintained as a dependency.
26+
* - Simplify:
27+
* - Remove async version, use sync always.
28+
* - Remove "nothrow" option, enable always.
29+
* - Remove unused "all", "path", "pathExt", "delimiter" options.
30+
* - Remove isexe's unused "uid", "gid" options. Use plain `fs.accessSync(,X_OK)` and `stat.isFile()`.
31+
* - Restrict feature set to just searching for commands in the pathenv:
32+
* - Remove unneeded support for absolute paths (i.e. command with a slash like "/bin/foo").
33+
* This responsibility is handled by /src/util.js#spawn.
34+
* - Remove unneeded support for relative paths (i.e. "./foo").
35+
* - Remove isexe "ignoreErrors" option, enable always.
36+
* - Remove isexe feature for locating Windows-executable PATHEXT suffixes.
37+
* Programs (e.g. Firefox) don't have suffixes that are user-dependent or otherwise not
38+
* knowable in advance. To locate "FOO.COM", just specify that directly to keep code simple,
39+
* greppable, and unsurprising. We don't need to emulate or predict everything Windows shell
40+
* can find (i.e. locate "FOO.COM" given "FOO"). We only need to search the PATH choices,
41+
* and use the absolute path after that. Besides, browsers on Windows aren't installed as
42+
* shell commands, they're in PROGRAMFILES (see WINDOWS_DIRS in /src/browsers.js). To run a
43+
* custom shell program in a QTap browser plugin, specify the absolute path to your bundled
44+
* program or (if installed globally) include your program's suffix.
45+
*/
46+
'use strict';
47+
48+
import path from 'node:path';
49+
import fs from 'node:fs';
50+
51+
// On Windows, this treats all links or files as executable.
52+
function isexeSync (fullpath) {
53+
try {
54+
fs.accessSync(fullpath, fs.constants.X_OK);
55+
// Skip executable directories. https://github.com/npm/node-which/pull/46
56+
const stat = fs.statSync(fullpath);
57+
return stat.isSymbolicLink() || stat.isFile();
58+
} catch (er) {
59+
return false;
60+
}
61+
}
62+
63+
function whichSync (cmd) {
64+
const pathEnv = (process.env.PATH || '').split(path.delimiter);
65+
for (const pathEnvPart of pathEnv) {
66+
const cmdPath = path.join(pathEnvPart, cmd);
67+
if (isexeSync(cmdPath)) {
68+
return cmdPath;
69+
}
70+
}
71+
return null;
72+
}
73+
74+
export default whichSync;

test/structure.test.js

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ QUnit.module('structure', function () {
7373
const directDeps = Object.keys(lock.packages[''].dependencies);
7474
const allDeps = await resolveDeps(lock, '');
7575

76-
assert.strictEqual(directDeps.length, 4, 'upto 5 direct dependencies');
76+
assert.strictEqual(directDeps.length, 3, 'upto 5 direct dependencies');
7777
assert.deepEqual(allDeps, {
78-
_total: '248 KiB',
78+
_total: '238 KiB',
7979
commander: {
8080
packageSize: '47 KiB'
8181
},
@@ -98,14 +98,6 @@ QUnit.module('structure', function () {
9898
}
9999
}
100100
},
101-
which: {
102-
packageSize: '3 KiB',
103-
dependencies: {
104-
isexe: {
105-
packageSize: '7 KiB'
106-
}
107-
}
108-
},
109101
yaml: {
110102
packageSize: null
111103
}

0 commit comments

Comments
 (0)