Skip to content

Commit 90352a3

Browse files
robertsLandoclaude
andcommitted
fix(sea): address copilot review — mixed-major guard, emitWarning leak, test cleanup
- lib/sea.ts: add assertSingleTargetMajor() and call from both sea() and seaEnhanced(). SEA prep blobs are Node-major specific, so mixing majors in one run (e.g. -t node22-linux-x64,node24-linux-x64) silently produced broken executables. Reject up front instead. - prelude/sea-bootstrap.js: restore the original process.emitWarning as soon as the SEA loader warning is suppressed, so user code does not observe a permanently wrapped emitWarning. - test/utils.js: filesAfter() gains { tolerateWindowsEbusy } option that only swallows EBUSY on win32 during cleanup. - test/test-85..92: drop copy-pasted try/catch noop around filesAfter and use the new option. test-91/92 also switch from inlined spawn+CRLF logic to the shared assertSeaOutput helper. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 2ac0177 commit 90352a3

10 files changed

Lines changed: 63 additions & 61 deletions

File tree

lib/sea.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,31 @@ function resolveMinTargetMajor(
441441
);
442442
}
443443

444+
/**
445+
* SEA prep blobs are Node-major specific (e.g. Node 25.8 added an
446+
* exec_argv_extension header field), so a single blob cannot be safely
447+
* baked into binaries of different Node majors. Reject mixed-major target
448+
* lists up front instead of silently producing broken executables.
449+
*/
450+
function assertSingleTargetMajor(
451+
targets: (NodeTarget & Partial<Target>)[],
452+
): void {
453+
const hostMajor = parseInt(process.version.slice(1), 10);
454+
const majors = new Set(
455+
targets.map((t) => {
456+
const v = parseInt(t.nodeRange.replace('node', ''), 10);
457+
return Number.isNaN(v) ? hostMajor : v;
458+
}),
459+
);
460+
if (majors.size > 1) {
461+
throw wasReported(
462+
`SEA mode cannot mix Node.js majors in a single run ` +
463+
`(got ${[...majors].sort((a, b) => a - b).join(', ')}). ` +
464+
`Run pkg once per Node major.`,
465+
);
466+
}
467+
}
468+
444469
/**
445470
* Pick the node binary used to generate the SEA prep blob.
446471
*
@@ -460,8 +485,9 @@ function resolveMinTargetMajor(
460485
* cross-major + cross-platform build will fail at spawn time — pkg
461486
* has no way to produce a host-platform binary of the target major.
462487
*
463-
* All targets share a single node major (validated in bin.ts), so
464-
* inspecting only nodePaths[0] is sufficient.
488+
* All targets share a single node major (enforced by
489+
* {@link assertSingleTargetMajor}), so inspecting only nodePaths[0] is
490+
* sufficient.
465491
*/
466492
function pickBlobGeneratorBinary(
467493
minTargetMajor: number,
@@ -516,6 +542,8 @@ export async function seaEnhanced(
516542
);
517543
}
518544

545+
assertSingleTargetMajor(opts.targets);
546+
519547
const minTargetMajor = resolveMinTargetMajor(opts.targets);
520548
if (minTargetMajor < 22) {
521549
throw wasReported(
@@ -632,6 +660,7 @@ export async function seaEnhanced(
632660
/** Create NodeJS executable using sea */
633661
export default async function sea(entryPoint: string, opts: SeaOptions) {
634662
assertHostSeaNodeVersion();
663+
assertSingleTargetMajor(opts.targets);
635664

636665
entryPoint = resolve(process.cwd(), entryPoint);
637666

prelude/sea-bootstrap.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,15 @@ if (manifest.entryIsESM) {
3838
var url = require('url');
3939

4040
// Suppress the ExperimentalWarning emitted once on first use of
41-
// vm.USE_MAIN_CONTEXT_DEFAULT_LOADER. We chain it onto the original
42-
// emitWarning so every other warning still reaches listeners.
41+
// vm.USE_MAIN_CONTEXT_DEFAULT_LOADER. Restore the original emitWarning
42+
// as soon as we swallow it so user code doesn't observe a wrapped
43+
// emitWarning for the rest of the process lifetime.
4344
var origEmitWarning = process.emitWarning;
4445
process.emitWarning = function (warning) {
4546
var msg =
4647
typeof warning === 'string' ? warning : warning && warning.message;
4748
if (msg && msg.indexOf('vm.USE_MAIN_CONTEXT_DEFAULT_LOADER') !== -1) {
49+
process.emitWarning = origEmitWarning;
4850
return;
4951
}
5052
return origEmitWarning.apply(this, arguments);

test/test-85-sea-enhanced/main.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,4 @@ const expected =
3434

3535
utils.assertSeaOutput('test-85-sea-enhanced', expected);
3636

37-
try {
38-
utils.filesAfter(before, newcomers);
39-
} catch (_error) {
40-
// noop — Windows EBUSY workaround
41-
}
37+
utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true });

test/test-86-sea-assets/main.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,4 @@ utils.assertSeaOutput(
2929
'config:test-value\ndata:hello world\n',
3030
);
3131

32-
try {
33-
utils.filesAfter(before, newcomers);
34-
} catch (_error) {
35-
// noop — Windows EBUSY workaround
36-
}
32+
utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true });

test/test-87-sea-esm/main.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,4 @@ utils.pkg.sync([input, '--sea'], { stdio: 'inherit' });
2828

2929
utils.assertSeaOutput('test-87-sea-esm', 'add:5\ngreeting:hello world\n');
3030

31-
try {
32-
utils.filesAfter(before, newcomers);
33-
} catch (_error) {
34-
// noop — Windows EBUSY workaround
35-
}
31+
utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true });

test/test-89-sea-fs-ops/main.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,4 @@ const expectedOutput =
3838

3939
utils.assertSeaOutput('test-89-sea-fs-ops', expectedOutput);
4040

41-
try {
42-
utils.filesAfter(before, newcomers);
43-
} catch (_error) {
44-
// noop — Windows EBUSY workaround
45-
}
41+
utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true });

test/test-90-sea-worker-threads/main.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,4 @@ const expected =
3232

3333
utils.assertSeaOutput('test-90-sea-worker-threads', expected);
3434

35-
try {
36-
utils.filesAfter(before, newcomers);
37-
} catch (_error) {
38-
// noop — Windows EBUSY workaround
39-
}
35+
utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true });

test/test-91-sea-esm-entry/main.js

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,6 @@ const before = utils.filesBefore(newcomers);
2424

2525
utils.pkg.sync([input, '--sea'], { stdio: 'inherit' });
2626

27-
const expected = 'add:5\ngreet:hello world\n';
28-
29-
const platformSuffix = { linux: 'linux', darwin: 'macos', win32: 'win.exe' };
30-
const suffix = platformSuffix[process.platform];
31-
if (suffix) {
32-
const actual = utils.spawn
33-
.sync(`./test-91-sea-esm-entry-${suffix}`, [])
34-
.replace(/\r\n/g, '\n');
35-
assert.equal(actual, expected, 'Output matches');
36-
}
27+
utils.assertSeaOutput('test-91-sea-esm-entry', 'add:5\ngreet:hello world\n');
3728

38-
try {
39-
utils.filesAfter(before, newcomers);
40-
} catch (_error) {
41-
// noop
42-
}
29+
utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true });

test/test-92-sea-tla/main.js

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,6 @@ const before = utils.filesBefore(newcomers);
2424

2525
utils.pkg.sync([input, '--sea'], { stdio: 'inherit' });
2626

27-
const expected = 'before-tla\nafter-tla:42\n';
28-
29-
const platformSuffix = { linux: 'linux', darwin: 'macos', win32: 'win.exe' };
30-
const suffix = platformSuffix[process.platform];
31-
if (suffix) {
32-
const actual = utils.spawn
33-
.sync(`./test-92-sea-tla-${suffix}`, [])
34-
.replace(/\r\n/g, '\n');
35-
assert.equal(actual, expected, 'Output matches');
36-
}
27+
utils.assertSeaOutput('test-92-sea-tla', 'before-tla\nafter-tla:42\n');
3728

38-
try {
39-
utils.filesAfter(before, newcomers);
40-
} catch (_error) {
41-
// noop
42-
}
29+
utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true });

test/utils.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,12 @@ module.exports.filesBefore = function (n) {
209209
* after the test are the same as before the test.
210210
* @param {string[]} before Files that should exist
211211
* @param {string[]} newComers New files produced by test that should be removed
212+
* @param {{ tolerateWindowsEbusy?: boolean }} [options] When
213+
* `tolerateWindowsEbusy` is true, EBUSY failures on win32 during cleanup
214+
* are swallowed. SEA tests need this because Windows briefly keeps a
215+
* handle on an executable after the spawned process has exited.
212216
*/
213-
module.exports.filesAfter = function (before, newComers) {
217+
module.exports.filesAfter = function (before, newComers, options) {
214218
// actual files in the directory
215219
const a = globSync('**/*').sort();
216220

@@ -241,8 +245,21 @@ module.exports.filesAfter = function (before, newComers) {
241245
}
242246

243247
// remove the files that should not exist
248+
const tolerateWindowsEbusy = options && options.tolerateWindowsEbusy === true;
244249
for (const ni of newComers) {
245-
module.exports.vacuum.sync(ni);
250+
try {
251+
module.exports.vacuum.sync(ni);
252+
} catch (error) {
253+
if (
254+
tolerateWindowsEbusy &&
255+
process.platform === 'win32' &&
256+
error &&
257+
error.code === 'EBUSY'
258+
) {
259+
continue;
260+
}
261+
throw error;
262+
}
246263
}
247264
};
248265

0 commit comments

Comments
 (0)