diff --git a/stasis-core/src/state.js b/stasis-core/src/state.js index ae2d27ae..56898c49 100644 --- a/stasis-core/src/state.js +++ b/stasis-core/src/state.js @@ -40,6 +40,31 @@ function readPackageJSON(pkgAbsolute) { // be unsound. const VERSION = corePackage.version +// Set `format` for `file` in a formats map, applying the weak-stat-record priority +// rule shared by the LIVE map writers (addFsStat, addImport -- addFile/addFsDir drop +// a stale stat record in place before their own set) and the lockfile merge +// (#mergedFormats): a payload-free 'stat:*' record (addFsStat) YIELDS to a real +// format for the same file and never displaces one. Both directions arise +// legitimately in one capture: an import may reveal the real format of a path that +// was first only stat'd, and -- the reverse -- a stat may land on a path addImport +// already format-attested at RESOLVE time with no byte record for addFsStat's +// content skip to key off (stasis-core's own preload-cached modules are exactly +// that shape: the resolve hook records 'module', the load hook never fires, and a +// bundler's enhanced-resolve then stats the file). Everything else keeps noupsert's +// conflict-is-fatal stance: two divergent REAL formats, or a stat kind flip +// (stat:file vs stat:directory). +function upsertFormat(map, file, format) { + const existing = map.get(file) + if (existing !== undefined && existing !== format) { + if (isStatFormat(existing) && !isStatFormat(format)) { + map.set(file, format) + return + } + if (isStatFormat(format) && !isStatFormat(existing)) return + } + noupsert(map, file, format) +} + // TODO: stricter format validation // Process-wide registry of every live State, kept on globalThis (not a module-local @@ -1419,19 +1444,27 @@ export class State { // listing, whether captured this run or seeded from the lockfile/bundle): that // record already answers getFsStat, and a weak stat record must never sit beside // it (check-then-read -- lstatSync before readFileSync -- is the common pattern). - // The REVERSE order (stat first, content later) is handled by addFile/addFsDir - // dropping the stale stat record in place and by #mergedFormats' upgrade rule. A - // kind flip WITHIN the surviving stat records (stat'd as a file, later stat'd as - // a directory) conflicts at the noupsert and taints the capture, exactly like a - // mid-run byte conflict. + // A path with a REAL format but no content record -- addImport attests the format + // of a resolve-hook target whose load hook never fires (a preload-cached module) -- + // is likewise left alone: upsertFormat drops the stat record, the real format + // wins (a plain noupsert here aborted real captures: enhanced-resolve stat'ing + // stasis-core's own src/util.js under --fs + a bundler plugin). The REVERSE order + // (stat first, real format later) is handled by addFile/addFsDir dropping the + // stale stat record in place, addImport's upsertFormat, and #mergedFormats' + // upgrade rule. A kind flip WITHIN the surviving stat records (stat'd as a file, + // later stat'd as a directory) still conflicts and taints the capture, exactly + // like a mid-run byte conflict. addFsStat(url, kind) { assert.ok(kind === 'file' || kind === 'directory', `addFsStat: unsupported kind '${kind}'`) const file = this.#canonicalFile(url) if (this.hashes.has(file) || this.sources.has(file) || this.resources.has(file)) return - noupsert(this.formats, file, `stat:${kind}`) + upsertFormat(this.formats, file, `stat:${kind}`) // Forwarded to the root via shardSnapshot's #observed-filtered formats, like - // every other capture; only a child's shardSnapshot reads it, skip otherwise. - if (this.config.childProcess) this.#observed.add(file) + // every other capture; only a child's shardSnapshot reads it, skip otherwise -- + // and only when a stat record actually landed (upsertFormat yields to an + // existing real format, and forwarding THAT would ship an entry mergeShard's + // stat replay ignores). + if (this.config.childProcess && isStatFormat(this.formats.get(file))) this.#observed.add(file) } // Serve an `fs.readFileSync` from the bundle (`stasis run --fs`, bundle=load). @@ -1697,7 +1730,11 @@ export class State { if (!imports.has(parent)) imports.set(parent, new Map()) const specifiers = imports.get(parent) noupsert(specifiers, specifier, file) - if (format) noupsert(this.formats, file, format) + // upsertFormat (not a bare noupsert): the resolve hook may attest the format of + // a target the program already lstat/stat'd under --fs (a payload-free + // 'stat:file' record) -- the real format replaces the weak stat record instead + // of aborting the capture as a conflict. + if (format) upsertFormat(this.formats, file, format) } getImport(parentURL, specifier, { conditions = '*', importAttributes } = {}) { @@ -1821,22 +1858,15 @@ export class State { // WEAK by design -- real content observed for the same file (a lock=add re-run // READING a path a prior capture only stat'd, or a sidecar bundling a file this // run also stat'd) UPGRADES the record instead of conflicting, and a stat record - // never displaces a real format. Two REAL formats -- and two divergent stat - // kinds (stat:file vs stat:directory) -- stay fatal. + // never displaces a real format (upsertFormat, the same rule the live-map + // writers apply). Two REAL formats -- and two divergent stat kinds (stat:file + // vs stat:directory) -- stay fatal. #mergedFormats() { const merged = new Map() - const mergeFormat = (file, format) => { - const existing = merged.get(file) - if (existing !== undefined && existing !== format) { - if (isStatFormat(existing) && !isStatFormat(format)) { merged.set(file, format); return } - if (isStatFormat(format) && !isStatFormat(existing)) return - } - noupsert(merged, file, format) - } if (this.#lockFormats) for (const [file, format] of this.#lockFormats) merged.set(file, format) - for (const [file, format] of this.formats) mergeFormat(file, format) + for (const [file, format] of this.formats) upsertFormat(merged, file, format) for (const sidecar of this.#sidecars) { - for (const [file, format] of sidecar.formats) mergeFormat(file, format) + for (const [file, format] of sidecar.formats) upsertFormat(merged, file, format) } return merged } diff --git a/tests/cli-fs.test.js b/tests/cli-fs.test.js index a97502be..174fd935 100644 --- a/tests/cli-fs.test.js +++ b/tests/cli-fs.test.js @@ -1305,6 +1305,36 @@ test('run --fs --child-process forwards a child-only stat record through the sha } }) +test('run --fs: a module stat\'d before it is ever imported upgrades to the real format (no conflict)', withTmp((t, tmp) => { + // The stat lands first ('stat:file'); the dynamic import's resolve hook then attests + // the REAL format via addImport -- for a target whose format arrives at RESOLVE time, + // not through addFile. The real format must replace the weak stat record; a bare + // noupsert used to abort the whole capture as a Conflict (the crash seen with + // enhanced-resolve stat'ing stasis-core's own resolve-attested src/util.js). + writeFileSync(join(tmp, 'src', 'dep.js'), 'export const k = "v"\n') + writeFileSync(join(tmp, 'src', 'entry.js'), [ + "import { lstatSync } from 'node:fs'", + 'const dep = new URL("./dep.js", import.meta.url)', + 'console.log(`stat:${lstatSync(dep).isFile()}`)', // stat BEFORE dep.js is resolved/loaded + 'const { k } = await import("./dep.js")', // resolve hook now records format "module" + 'console.log(`k:${k}`)', + '', + ].join('\n')) + const bundlePath = join(tmp, 'snapshot.br') + const save = run(['run', '--lock=add', '--bundle=add', `--bundle-file=${bundlePath}`, '--fs=sync', 'src/entry.js'], { cwd: tmp }) + t.assert.equal(save.status, 0, `save stderr: ${save.stderr}`) + t.assert.equal(save.stdout, 'stat:true\nk:v\n') + t.assert.doesNotMatch(save.stderr, /capture aborted|Conflict/, 'the real format must upgrade the stat record, not conflict') + const bundle = decode(bundlePath) + t.assert.equal(bundle.formats['src/dep.js'], 'module', 'the import upgraded the stat record to the real format') + t.assert.equal(JSON.parse(readFileSync(join(tmp, 'stasis.lock.json'), 'utf-8')).formats['src/dep.js'], 'module') + + rmSync(join(tmp, 'src', 'dep.js')) // gone; both the stat and the import serve from the bundle + const load = run(['run', '--lock=frozen', '--bundle=load', `--bundle-file=${bundlePath}`, '--fs=sync', 'src/entry.js'], { cwd: tmp }) + t.assert.equal(load.status, 0, `load stderr: ${load.stderr}`) + t.assert.equal(load.stdout, 'stat:true\nk:v\n') +})) + test('run --fs only captures the single-argument lstatSync/statSync (options pass through)', withTmp((t, tmp) => { // Same scope rule as readdirSync: an options form (bigint, throwIfNoEntry) is out of // scope -- passed through untouched and NOT recorded. diff --git a/tests/state.test.js b/tests/state.test.js index 2337ea53..8b18f4cc 100644 --- a/tests/state.test.js +++ b/tests/state.test.js @@ -109,6 +109,45 @@ test('getImport throws for unknown specifier', (t) => { t.assert.throws(() => state.getImport(parentURL, './does-not-exist.js')) }) +// --- payload-free stat records vs real formats (the weak-record priority rule) -------- +// addImport attests a resolve-hook target's format WITHOUT a byte record when the load +// hook never fires for it (a preload-cached module -- stasis-core's own files, under a +// bundler whose enhanced-resolve then stats them), so addFsStat cannot key its skip off +// hashes/sources alone. The two must never conflict in EITHER order: the real format +// always wins and the weak stat record always yields. Regression: a bare noupsert here +// aborted whole captures -- `Conflict for "node_modules/@exodus/stasis-core/src/util.js"`, +// 'module' vs 'stat:file'. Fresh State instances so the shared `state` above stays clean. + +test('addFsStat yields to a real format addImport recorded for a byte-less target', (t) => { + const s = new State(root) + const parentURL = pathToFileURL(fileAbs2).toString() + const childURL = pathToFileURL(fileAbs).toString() + s.addImport(parentURL, './foo.js', childURL, { format: 'module' }) // resolve-time format, no addFile + t.assert.ok(!s.hashes.has('src/foo.js') && !s.sources.has('src/foo.js'), 'precondition: no byte record') + s.addFsStat(childURL, 'file') // must neither throw nor displace 'module' + t.assert.equal(s.formats.get('src/foo.js'), 'module') +}) + +test('addImport upgrades a stat record recorded before the target was ever resolved', (t) => { + const s = new State(root) + const parentURL = pathToFileURL(fileAbs2).toString() + const childURL = pathToFileURL(fileAbs).toString() + s.addFsStat(childURL, 'file') // stat first (check-before-import) + t.assert.equal(s.formats.get('src/foo.js'), 'stat:file') + s.addImport(parentURL, './foo.js', childURL, { format: 'module' }) // must upgrade, not conflict + t.assert.equal(s.formats.get('src/foo.js'), 'module') +}) + +test('divergent stat kinds and divergent real formats still conflict', (t) => { + const s = new State(root) + const parentURL = pathToFileURL(fileAbs2).toString() + const childURL = pathToFileURL(fileAbs).toString() + s.addFsStat(childURL, 'file') + t.assert.throws(() => s.addFsStat(childURL, 'directory'), /Conflict/) // kind flip stays fatal + s.addImport(parentURL, './foo.js', childURL, { format: 'module' }) + t.assert.throws(() => s.addImport(parentURL, './foo.js', childURL, { format: 'commonjs' }), /Conflict/) +}) + // CJS `require(require.resolve('./foo.js'))` (e.g. webpack's loader-runner doing // `require(loader.path)`) hands the resolve hook the already-resolved ABSOLUTE // path as the specifier. Recorded verbatim it bakes a machine-specific absolute