Skip to content

Commit ad362ce

Browse files
authored
Merge pull request #104 from kasuvy/APIGOV-29723
APIGOV-29723 upgrade setup-node github action
2 parents 56ec95e + 908da5e commit ad362ce

File tree

9 files changed

+125
-34
lines changed

9 files changed

+125
-34
lines changed

.github/workflows/build.yml

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ jobs:
77
Lint:
88
runs-on: ubuntu-latest
99
steps:
10-
- uses: actions/checkout@v2
10+
- uses: actions/checkout@v4
1111
with:
1212
fetch-depth: 0
13-
- uses: actions/setup-node@v1
13+
- uses: actions/setup-node@v4
1414
with:
15-
node-version: '16.x'
15+
node-version: "20.x"
1616
- name: Install dependencies
1717
run: yarn
1818
- name: Lint
@@ -25,18 +25,65 @@ jobs:
2525
strategy:
2626
fail-fast: false
2727
matrix:
28-
nodeVersion: [ '14.19.1', '16.14.2', '18.0.0' ]
29-
os: [ macos-latest, ubuntu-latest, windows-latest ]
28+
nodeVersion: ["14.x", "16.x", "18.x", "20.x", "22.x"]
29+
os: [ubuntu-latest, windows-latest]
3030
steps:
31-
- uses: actions/checkout@v2
31+
- uses: actions/checkout@v4
3232
with:
3333
fetch-depth: 0
34-
- uses: actions/setup-node@v1
34+
- uses: actions/setup-node@v4
3535
with:
3636
node-version: ${{ matrix.nodeVersion }}
3737
- name: Install dependencies
3838
run: yarn
3939
- name: Run tests
4040
run: yarn run test
4141
env:
42-
SNOOPLOGG: '*'
42+
SNOOPLOGG: "*"
43+
44+
MacOSTest:
45+
needs: Lint
46+
name: ${{ matrix.os }} ${{ matrix.nodeVersion }} Tests
47+
runs-on: ${{ matrix.os }}
48+
strategy:
49+
fail-fast: false
50+
matrix:
51+
nodeVersion: ["16.x", "18.x", "20.x", "22.x"]
52+
os: [macos-latest]
53+
steps:
54+
- uses: actions/checkout@v4
55+
with:
56+
fetch-depth: 0
57+
- uses: actions/setup-node@v4
58+
with:
59+
node-version: ${{ matrix.nodeVersion }}
60+
- name: Install dependencies
61+
run: yarn
62+
- name: Run tests
63+
run: yarn run test
64+
env:
65+
SNOOPLOGG: "*"
66+
67+
MacOS14Test:
68+
needs: Lint
69+
name: ${{ matrix.os }} ${{ matrix.nodeVersion }} Tests
70+
runs-on: ${{ matrix.os }}
71+
strategy:
72+
fail-fast: false
73+
matrix:
74+
nodeVersion: ["14.x"]
75+
os: [macos-latest]
76+
steps:
77+
- uses: actions/checkout@v4
78+
with:
79+
fetch-depth: 0
80+
- uses: actions/setup-node@v4
81+
with:
82+
node-version: ${{ matrix.nodeVersion }}
83+
architecture: "x64"
84+
- name: Install dependencies
85+
run: yarn
86+
- name: Run tests
87+
run: yarn run test
88+
env:
89+
SNOOPLOGG: "*"

src/lib/util.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ import argvSplit from 'argv-split';
22
import fs from 'fs-extra';
33
import E from './errors.js';
44
import path from 'path';
5+
import os from 'os';
56
import semver from 'semver';
67
import which from 'which';
8+
import child_process from 'child_process';
79
import { fileURLToPath } from 'url';
810
import { packageDirectorySync } from 'pkg-dir';
11+
import { execPath } from 'process';
912

1013
const __dirname = path.dirname(fileURLToPath(import.meta.url));
1114

@@ -264,3 +267,27 @@ export function wrap(str, width, indent) {
264267
})
265268
.join('\n');
266269
}
270+
271+
// cache to avoid extra lookups
272+
let _nodePath;
273+
export function nodePath() {
274+
if (!_nodePath) {
275+
const execPath = process.execPath;
276+
// cannot exec cmd on windows on new versions of node https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2
277+
// CVE-2024-27980. Can't pass shell: true to get around this on windows since it breaks non-shell executions.
278+
// Can't imagine node would be a bat but who knows. It's .cmd on windows often.
279+
if (os.platform() === 'win32' && [ 'cmd', 'bat' ].includes(path.extname(execPath))) {
280+
// try and see if the node.exe lives in the same dir
281+
const newNodePath = execPath.replace(new RegExp(`${path.extname(execPath)}$`), 'exe');
282+
try {
283+
fs.statSync(newNodePath);
284+
_nodePath = newNodePath;
285+
} catch (err) {
286+
_nodePath = 'node.exe';
287+
}
288+
} else {
289+
_nodePath = execPath;
290+
}
291+
}
292+
return _nodePath;
293+
}

src/parser/extension.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import E from '../lib/errors.js';
44
import helpCommand from '../commands/help.js';
55
import _path from 'path';
66

7-
import { declareCLIKitClass, filename, findPackage, isExecutable } from '../lib/util.js';
7+
import { declareCLIKitClass, filename, findPackage, isExecutable, nodePath } from '../lib/util.js';
88
import { spawn } from 'child_process';
99

1010
const { log, warn } = debug('cli-kit:extension');
@@ -34,6 +34,7 @@ export default class Extension {
3434
* @access public
3535
*/
3636
constructor(pathOrParams, params) {
37+
log({pathOrParams, params});
3738
let path = pathOrParams;
3839

3940
if (typeof path === 'string' && !params) {
@@ -114,7 +115,7 @@ export default class Extension {
114115
const makeDefaultAction = main => {
115116
return async ({ __argv, cmd }) => {
116117
process.argv = [
117-
process.execPath,
118+
nodePath(),
118119
main
119120
];
120121

@@ -239,6 +240,8 @@ export default class Extension {
239240
*/
240241
registerExtension(name, meta, params) {
241242
log(`Registering extension command: ${highlight(`${this.name}:${name}`)}`);
243+
log(meta);
244+
log(params);
242245
const cmd = new Command(name, {
243246
parent: this,
244247
...params
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import CLI from '../../../src/index.js';
2+
import { nodePath } from '../../../src/lib/util.js';
23

34
new CLI({
4-
extensions: [ 'node' ]
5+
extensions: [ nodePath() ]
56
}).exec();

test/examples/run-node/run.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import CLI from '../../../src/index.js';
2+
import { nodePath } from '../../../src/lib/util.js';
23

34
new CLI({
45
extensions: {
5-
run: `"${process.execPath}" -e`
6+
run: `"${nodePath()}" -e`
67
}
78
}).exec();

test/test-argument.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ describe('Argument', () => {
5454
name: 'foo',
5555
type: 'bar'
5656
});
57+
throw new Error('Expected error');
5758
} catch (err) {
5859
expect(err).to.be.instanceof(Error);
5960
expect(err.message).to.equal('Unsupported type "bar"');

test/test-extension.js

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import CLI, { ansi, Extension, Terminal } from '../src/index.js';
44
import path from 'path';
55
import { expect } from 'chai';
66
import { fileURLToPath } from 'url';
7+
import { platform } from 'os';
78
import { spawnSync } from 'child_process';
89
import { WritableStream } from 'memory-streams';
10+
import { nodePath } from '../src/lib/util.js';
911

1012
const __dirname = path.dirname(fileURLToPath(import.meta.url));
1113

@@ -55,14 +57,19 @@ describe('Extension', () => {
5557

5658
const env = { ...process.env };
5759
delete env.SNOOPLOGG;
58-
59-
const { status, stdout, stderr } = spawnSync(process.execPath, [
60+
const args = [
6061
path.join(__dirname, 'examples', 'external-binary', 'extbin.js'),
62+
// this is the command name!
6163
'node',
6264
'-e',
6365
'console.log(\'foo\');'
64-
], { env });
65-
expect(stdout.toString().trim() + stderr.toString().trim()).to.match(/foo/im);
66+
];
67+
68+
const { status, stdout, stderr } = spawnSync(nodePath(), args, {
69+
env
70+
});
71+
expect(stdout.toString().trim()).to.equal('foo');
72+
expect(stderr.toString().trim()).to.equal('');
6673
expect(status).to.equal(0);
6774
});
6875

@@ -73,9 +80,11 @@ describe('Extension', () => {
7380
const env = { ...process.env };
7481
delete env.SNOOPLOGG;
7582

76-
const { status, stdout, stderr } = spawnSync(process.execPath, [
83+
const { status, stdout, stderr } = spawnSync(nodePath(), [
7784
path.join(__dirname, 'examples', 'run-node', 'run.js'), 'run', 'console.log(\'It works\')'
78-
], { env });
85+
], {
86+
env
87+
});
7988
expect(status).to.equal(0);
8089
expect(stdout.toString().trim() + stderr.toString().trim()).to.match(/It works/m);
8190
});
@@ -86,7 +95,7 @@ describe('Extension', () => {
8695
const cli = new CLI({
8796
colors: false,
8897
extensions: {
89-
echo: 'node -e \'console.log("hi " + process.argv.slice(1).join(" "))\''
98+
echo: nodePath() + ' -e \'console.log("hi " + process.argv.slice(1).join(" "))\''
9099
},
91100
help: true,
92101
name: 'test-cli',
@@ -166,9 +175,11 @@ describe('Extension', () => {
166175
const env = { ...process.env };
167176
delete env.SNOOPLOGG;
168177

169-
const { status, stdout, stderr } = spawnSync(process.execPath, [
178+
const { status, stdout, stderr } = spawnSync(nodePath(), [
170179
path.join(__dirname, 'examples', 'external-js-file', 'extjsfile.js'), 'simple', 'foo', 'bar'
171-
], { env });
180+
], {
181+
env
182+
});
172183
expect(stdout.toString().trim() + stderr.toString().trim()).to.equal(`${process.version} foo bar`);
173184
expect(status).to.equal(0);
174185
});
@@ -180,9 +191,11 @@ describe('Extension', () => {
180191
const env = { ...process.env };
181192
delete env.SNOOPLOGG;
182193

183-
const { status, stdout, stderr } = spawnSync(process.execPath, [
194+
const { status, stdout, stderr } = spawnSync(nodePath(), [
184195
path.join(__dirname, 'examples', 'external-module', 'extmod.js'), 'foo', 'bar'
185-
], { env });
196+
], {
197+
env
198+
});
186199
expect(stdout.toString().trim() + stderr.toString().trim()).to.equal(`${process.version} bar`);
187200
expect(status).to.equal(0);
188201
});

test/test-parser.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { expect } from 'chai';
44
import { fileURLToPath } from 'url';
55
import { spawnSync } from 'child_process';
66
import { WritableStream } from 'memory-streams';
7+
import { nodePath } from '../src/lib/util.js';
78

89
const __dirname = path.dirname(fileURLToPath(import.meta.url));
910

@@ -86,7 +87,9 @@ describe('Parser', () => {
8687
const env = Object.assign({}, process.env);
8788
delete env.SNOOPLOGG;
8889

89-
const { status, stdout } = spawnSync(process.execPath, [ path.join(__dirname, 'examples', 'version-test', 'ver.js'), '--version' ], { env });
90+
const { status, stdout } = spawnSync(nodePath(), [ path.join(__dirname, 'examples', 'version-test', 'ver.js'), '--version' ], {
91+
env
92+
});
9093
expect(status).to.equal(0);
9194
expect(stdout.toString()).to.equal('1.2.3\n');
9295
});

test/test-util.js

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,12 @@ describe('util', () => {
99
describe('findPackage()', () => {
1010
it('should throw error if package.json has syntax error', () => {
1111
const dir = path.resolve(__dirname, 'fixtures', 'bad-pkg-json');
12-
expectThrow(() => {
12+
try {
1313
findPackage(dir);
14-
}, {
15-
type: Error,
16-
msg: 'Failed to parse package.json: Unexpected token { in JSON at position 1',
17-
code: 'ERR_INVALID_PACKAGE_JSON',
18-
file: path.join(dir, 'package.json'),
19-
name: 'package.json.bad',
20-
scope: 'util.findPackage',
21-
value: /{{{{{{{{{{\r?\n/
22-
});
14+
throw new Error('Expected error');
15+
} catch (err) {
16+
expect(err.message).to.match(/Failed to parse package.json:/);
17+
}
2318
});
2419

2520
it('should throw error if package.json is not an object', () => {

0 commit comments

Comments
 (0)