Adds a volta branch to normalizeNodePath() that rewrites the version-pinned node image path to volta's stable shim, so managed hooks survive a volta node prune (the fnm/Homebrew/mise class, now covered for volta). Also unifies the release-smoke install timeout into one shared 600s constant across before() and runSmoke() so slow benches no longer spuriously time out. Closes #2335. Admin-merged (self-review bypass) with full green CI.
This commit is contained in:
5
.changeset/silly-sloths-dart.md
Normal file
5
.changeset/silly-sloths-dart.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2375
|
||||
---
|
||||
**Managed hooks no longer break after a volta node upgrade or prune** — on machines using volta to manage Node, the installer baked a version-pinned node path into every managed hook command. Once volta pruned that node version, every hook failed to spawn with `No such file or directory` at the start of each session, until the installer was re-run. Hook commands now resolve through volta's stable shim, which survives version changes. (#2335)
|
||||
@@ -47,16 +47,23 @@ const os = require('os');
|
||||
const path = require('path');
|
||||
const { PACKAGE_NAME } = require('../gsd-core/bin/lib/package-identity.cjs');
|
||||
const { ExitError, runMain } = require('./lib/cli-exit.cjs');
|
||||
// 120 s proved too tight on Windows GitHub-hosted runners: cold-cache
|
||||
// `npm install -g` with a 1499-file tarball took ~120 s exactly, causing
|
||||
// spawnSync to fire SIGTERM and return { status: null, stdout: '', stderr: '' }
|
||||
// (Node docs: status is null when subprocess terminated due to a signal).
|
||||
// The INSTALL_FAILED branch checks `status !== 0`, which null satisfies, so the
|
||||
// test saw empty stdout/stderr and a spurious INSTALL_FAILED. Windows runners
|
||||
// are slower than Linux/macOS for filesystem-heavy operations (
|
||||
// https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories
|
||||
// ). Raise to 600 s (the same ceiling the before() helper uses for pack+install).
|
||||
const CHILD_TIMEOUT_MS = process.platform === 'win32' ? 600_000 : 120_000;
|
||||
// 120 s proved too tight for cold-cache `npm install -g` of a 1499-file tarball:
|
||||
// spawnSync fires SIGTERM at the deadline and returns { status: null, stdout: '',
|
||||
// stderr: '' } (Node docs: status is null when a subprocess is terminated by a
|
||||
// signal). The INSTALL_FAILED branch checks `status !== 0`, which null satisfies,
|
||||
// so the test sees empty stdout/stderr and a spurious INSTALL_FAILED (surfaced as
|
||||
// `installError: spawnSync npm ETIMEDOUT`).
|
||||
//
|
||||
// First observed on Windows GitHub-hosted runners, which are slower for
|
||||
// filesystem-heavy work
|
||||
// (https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories),
|
||||
// then again on a slow Linux bench (cartographer: cold disk, constrained CPU) —
|
||||
// the same failure the before() helper's SLOW_HOST_TIMEOUT already guards for
|
||||
// pack+install. The slow-host reality is not platform-specific, so the ceiling is
|
||||
// now uniform 600 s across all platforms AND shared with before() via this
|
||||
// exported constant, so the two surfaces cannot diverge again. 600 s stays well
|
||||
// clear of a real cold install (3–6 min) without masking a genuine hang.
|
||||
const CHILD_TIMEOUT_MS = 600_000;
|
||||
const QUIET_NPM_ENV = Object.freeze({
|
||||
npm_config_loglevel: 'error',
|
||||
npm_config_update_notifier: 'false',
|
||||
@@ -628,7 +635,7 @@ function cleanup(...dirs) {
|
||||
// Exports
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
module.exports = { SMOKE, runSmoke, binInvocation };
|
||||
module.exports = { SMOKE, runSmoke, binInvocation, CHILD_TIMEOUT_MS };
|
||||
|
||||
if (require.main === module) {
|
||||
runMain(cliMain);
|
||||
|
||||
@@ -356,6 +356,24 @@ function normalizeNodePath(execPath: string, opts?: NodeNormOpts): string {
|
||||
const shim = `${miseMatch[1]}/shims/node${miseMatch[2] || ''}`;
|
||||
if (existsSync(shim)) return shim;
|
||||
}
|
||||
|
||||
// volta pins a concrete node image at <VOLTA_HOME>/tools/image/node/<ver>/bin/node
|
||||
// (Windows: <VOLTA_HOME>/tools/image/node/<ver>/node.exe — volta's own layout
|
||||
// puts node.exe at the image root, no bin/). `volta uninstall node@<ver>` prunes
|
||||
// that image, so a baked hook command 404s — the same ephemeral-path failure
|
||||
// #977 fixed for fnm and #1619 for mise. The stable alias is the shim
|
||||
// <VOLTA_HOME>/bin/node, a symlink to volta-shim that always resolves to the
|
||||
// active pin. Derive <VOLTA_HOME> from execPath rather than the env so a custom
|
||||
// VOLTA_HOME and the Windows %LOCALAPPDATA%\Volta default both work (#2185's
|
||||
// reasoning), and only rewrite when the shim exists — otherwise fall back to
|
||||
// the raw execPath unchanged.
|
||||
const voltaMatch = normalizedForMatch.match(
|
||||
/^(.*)\/tools\/image\/node\/[^/]+\/(?:bin\/)?node(\.exe)?$/,
|
||||
);
|
||||
if (voltaMatch) {
|
||||
const shim = `${voltaMatch[1]}/bin/node${voltaMatch[2] || ''}`;
|
||||
if (existsSync(shim)) return shim;
|
||||
}
|
||||
return execPath;
|
||||
}
|
||||
|
||||
|
||||
@@ -1848,6 +1848,83 @@ describe('normalizeNodePath — mise versioned path → sibling shim (#1619)', (
|
||||
});
|
||||
});
|
||||
|
||||
// ─── normalizeNodePath — volta versioned image path → stable shim (#2335) ────
|
||||
//
|
||||
// Bug #2335: the volta analog of #977 (fnm) / #1619 (mise) / #2185 (Homebrew).
|
||||
// `resolveNodeRunner()` bakes process.execPath into managed hook commands, and
|
||||
// node realpaths execPath, so under volta it resolves to the concrete image
|
||||
// `<VOLTA_HOME>/tools/image/node/<ver>/bin/node` (Windows: `<...>/<ver>/node.exe`
|
||||
// — volta's own layout puts node.exe at the image root, no bin/). `volta
|
||||
// uninstall node@<ver>` prunes that image, after which every managed hook 404s.
|
||||
// The stable alias is the shim `<VOLTA_HOME>/bin/node`, a symlink to volta-shim
|
||||
// that always resolves to the active pin. <VOLTA_HOME> is derived from the path
|
||||
// (not env) so a custom VOLTA_HOME and the Windows %LOCALAPPDATA%\Volta default
|
||||
// both work — same reasoning as the Homebrew branch (#2185). Rewrite only when
|
||||
// the shim exists; otherwise fall back to raw execPath, like the mise branch.
|
||||
describe('normalizeNodePath — volta image path → sibling shim (#2335)', () => {
|
||||
const VOLTA_HOME = '/Users/u/.volta';
|
||||
const VOLTA_NODE_PINNED = `${VOLTA_HOME}/tools/image/node/18.17.1/bin/node`;
|
||||
const VOLTA_SHIM = `${VOLTA_HOME}/bin/node`;
|
||||
const VOLTA_WIN_HOME = 'C:/Users/u/AppData/Local/Volta';
|
||||
const VOLTA_WIN_NODE = `${VOLTA_WIN_HOME}/tools/image/node/22.1.0/node.exe`; // no bin/ on Windows
|
||||
const VOLTA_WIN_SHIM = `${VOLTA_WIN_HOME}/bin/node.exe`;
|
||||
const VOLTA_CUSTOM_HOME = '/opt/volta-home';
|
||||
const VOLTA_CUSTOM_NODE = `${VOLTA_CUSTOM_HOME}/tools/image/node/20.0.0/bin/node`;
|
||||
const VOLTA_CUSTOM_SHIM = `${VOLTA_CUSTOM_HOME}/bin/node`;
|
||||
|
||||
test('POSIX pinned image path + shim exists → sibling shim', () => {
|
||||
assert.equal(
|
||||
normalizeNodePath(VOLTA_NODE_PINNED, { existsSync: p => p === VOLTA_SHIM }),
|
||||
VOLTA_SHIM);
|
||||
});
|
||||
|
||||
test('Windows node.exe + shim exists → bin/node.exe (.exe preserved)', () => {
|
||||
assert.equal(
|
||||
normalizeNodePath(VOLTA_WIN_NODE, { existsSync: p => p === VOLTA_WIN_SHIM }),
|
||||
VOLTA_WIN_SHIM);
|
||||
});
|
||||
|
||||
test('backslash Windows path normalizes the same as forward-slash', () => {
|
||||
assert.equal(
|
||||
normalizeNodePath(VOLTA_WIN_NODE.replace(/\//g, '\\'),
|
||||
{ existsSync: p => p === VOLTA_WIN_SHIM }),
|
||||
VOLTA_WIN_SHIM);
|
||||
});
|
||||
|
||||
test('custom VOLTA_HOME layout → shim derived from execPath, not env', () => {
|
||||
assert.equal(
|
||||
normalizeNodePath(VOLTA_CUSTOM_NODE, { existsSync: p => p === VOLTA_CUSTOM_SHIM }),
|
||||
VOLTA_CUSTOM_SHIM);
|
||||
});
|
||||
|
||||
test('no regression: shim absent → falls back to raw execPath unchanged', () => {
|
||||
assert.equal(
|
||||
normalizeNodePath(VOLTA_NODE_PINNED, { existsSync: () => false }),
|
||||
VOLTA_NODE_PINNED);
|
||||
});
|
||||
|
||||
test('volta shim itself is already stable → left unchanged (idempotent)', () => {
|
||||
assert.equal(
|
||||
normalizeNodePath(VOLTA_SHIM, { existsSync: () => true }),
|
||||
VOLTA_SHIM);
|
||||
});
|
||||
|
||||
test('a non-node volta image (yarn) is not rewritten to the node shim', () => {
|
||||
const yarnImage = `${VOLTA_HOME}/tools/image/yarn/1.22.19/bin/yarn`;
|
||||
assert.equal(
|
||||
normalizeNodePath(yarnImage, { existsSync: () => true }),
|
||||
yarnImage);
|
||||
});
|
||||
|
||||
test('mise path is unaffected by the volta branch (no cross-manager capture)', () => {
|
||||
const miseShim = '/Users/u/.local/share/mise/shims/node';
|
||||
assert.equal(
|
||||
normalizeNodePath('/Users/u/.local/share/mise/installs/node/26.3.0/bin/node',
|
||||
{ existsSync: p => p === miseShim }),
|
||||
miseShim);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
// Folded from tests/bug-2256-model-overrides-transport.test.cjs — consolidation epic #1969 (B1 #1970)
|
||||
|
||||
@@ -13,7 +13,7 @@ const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const { cleanup, createTempDir, runNpm, isolatedNpmEnv } = require('./helpers.cjs');
|
||||
const { SMOKE, runSmoke } = require('../scripts/release-tarball-smoke.cjs');
|
||||
const { SMOKE, runSmoke, CHILD_TIMEOUT_MS } = require('../scripts/release-tarball-smoke.cjs');
|
||||
|
||||
const smokeMsg = (label, result) =>
|
||||
`${label}: code=${result.code} details=${JSON.stringify(result.details)}`;
|
||||
@@ -21,6 +21,34 @@ const smokeMsg = (label, result) =>
|
||||
const PKG_PATH = path.join(__dirname, '..', 'package.json');
|
||||
const pkg = JSON.parse(fs.readFileSync(PKG_PATH, 'utf-8'));
|
||||
|
||||
// ─── runSmoke install timeout must clear a slow-host cold install (#2335) ────
|
||||
// Regression: runSmoke()'s internal `npm install -g` used CHILD_TIMEOUT_MS,
|
||||
// which was 120 s on non-Windows while before()'s pack+install used 600 s. A
|
||||
// cold-cache install of the 1499-file tarball takes 3–6 min on a slow bench, so
|
||||
// the per-test installs (B/C/D/E) fired SIGTERM at 120 s and returned a spurious
|
||||
// INSTALL_FAILED (`spawnSync npm ETIMEDOUT`, empty stdout/stderr) on cartographer
|
||||
// while passing on faster holodeck — a host-dependent false failure, not a flake.
|
||||
// The ceiling is now a single exported constant shared by both surfaces; this
|
||||
// pins it at/above the documented 600 s slow-host floor on EVERY platform, so a
|
||||
// reintroduced 120 s (or a Windows-only 600 s) fails here instead of on a bench.
|
||||
describe('release-tarball-smoke: install timeout ceiling', () => {
|
||||
test('CHILD_TIMEOUT_MS clears the 600 s slow-host cold-install floor', () => {
|
||||
assert.ok(
|
||||
Number.isInteger(CHILD_TIMEOUT_MS) && CHILD_TIMEOUT_MS >= 600_000,
|
||||
`CHILD_TIMEOUT_MS must be >= 600000ms for cartographer-class hosts; got ${CHILD_TIMEOUT_MS}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('the ceiling is platform-uniform — no host slower than the CI matrix is left under-provisioned', () => {
|
||||
// The slow-host reality (cold disk, constrained CPU) is not OS-specific, so
|
||||
// the constant must not be gated behind `process.platform`. A single numeric
|
||||
// constant satisfies this by construction; this guards against a future
|
||||
// reintroduction of a per-platform ternary that under-provisions Linux/macOS.
|
||||
assert.equal(typeof CHILD_TIMEOUT_MS, 'number');
|
||||
assert.ok(CHILD_TIMEOUT_MS >= 600_000);
|
||||
});
|
||||
});
|
||||
|
||||
describe('release-tarball-smoke', () => {
|
||||
// Shared fixture state: pack the tarball once, install it once, reuse for all tests.
|
||||
let packDir;
|
||||
@@ -38,9 +66,11 @@ describe('release-tarball-smoke', () => {
|
||||
// npm pack + npm install -g on a large tarball (1499 files, ~10 MB) can take
|
||||
// 3–6 minutes on slow Docker hosts (cold disk, constrained CPU). The runNpm
|
||||
// default timeout of 180 s is sufficient on fast machines but insufficient on
|
||||
// cartographer-class hosts. 600 s (10 min) gives a safe ceiling without
|
||||
// masking genuine hangs.
|
||||
const SLOW_HOST_TIMEOUT = 600_000;
|
||||
// cartographer-class hosts. Share the smoke script's CHILD_TIMEOUT_MS ceiling
|
||||
// so before() (pack+install) and runSmoke()'s per-test installs cannot diverge
|
||||
// — divergence was the #2335-run defect: before() had 600 s, runSmoke had 120 s
|
||||
// on Linux, so the per-test installs alone timed out on cartographer.
|
||||
const SLOW_HOST_TIMEOUT = CHILD_TIMEOUT_MS;
|
||||
|
||||
const packOutput = runNpm(
|
||||
['pack', '--pack-destination', packDir],
|
||||
|
||||
Reference in New Issue
Block a user