-
Notifications
You must be signed in to change notification settings - Fork 10
fix(build:wasm): add preflight check with clear remediation #990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,85 @@ const grammarsDir = resolve(root, 'grammars'); | |
|
|
||
| if (!existsSync(grammarsDir)) mkdirSync(grammarsDir); | ||
|
|
||
| type PreflightFailure = { | ||
| reason: string; | ||
| remediation: string[]; | ||
| }; | ||
|
|
||
| function printBanner(title: string, lines: string[]): void { | ||
| const bar = '─'.repeat(Math.max(title.length + 4, 60)); | ||
| console.warn(bar); | ||
| console.warn(` ${title}`); | ||
| console.warn(bar); | ||
| for (const l of lines) console.warn(l); | ||
| console.warn(bar); | ||
| } | ||
|
|
||
| function runCaptured( | ||
| cmd: string, | ||
| args: string[], | ||
| cwd: string, | ||
| ): { ok: true; stdout: string } | { ok: false; stdout: string; stderr: string; message: string } { | ||
| try { | ||
| const stdout = execFileSync(cmd, args, { | ||
| cwd, | ||
| stdio: ['ignore', 'pipe', 'pipe'], | ||
| shell: true, | ||
| encoding: 'utf8', | ||
| }); | ||
| return { ok: true, stdout }; | ||
| } catch (err: any) { | ||
| return { | ||
| ok: false, | ||
| stdout: String(err.stdout ?? ''), | ||
| stderr: String(err.stderr ?? ''), | ||
| message: String(err.message ?? 'unknown error'), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| function preflightTreeSitterCli(): PreflightFailure | null { | ||
| const result = runCaptured('npx', ['tree-sitter', '--version'], root); | ||
| if (result.ok) return null; | ||
|
|
||
| const combined = `${result.stderr}\n${result.stdout}\n${result.message}`; | ||
| const missingBinary = | ||
| /ENOENT.*tree-sitter(\.exe)?/i.test(combined) || | ||
| /tree-sitter(\.exe)? was not found/i.test(combined) || | ||
| /spawn .*tree-sitter(\.exe)?.*ENOENT/i.test(combined); | ||
|
|
||
| if (missingBinary) { | ||
| return { | ||
| reason: "tree-sitter CLI binary is missing — can't build WASM grammars", | ||
| remediation: [ | ||
| 'The tree-sitter-cli npm package downloads a platform-specific binary', | ||
| 'at install time (see node_modules/tree-sitter-cli/install.js). That', | ||
| 'download appears to have failed or been skipped on this machine,', | ||
| 'likely due to a network/proxy block or a previous --ignore-scripts install.', | ||
| '', | ||
| 'Remediation — try one of:', | ||
| ' 1. npm rebuild tree-sitter-cli (re-run the binary download)', | ||
| ' 2. npm install -g tree-sitter-cli (install globally from PATH)', | ||
| ' 3. Download tree-sitter from', | ||
| ' https://github.com/tree-sitter/tree-sitter/releases', | ||
| ' and place it on PATH', | ||
| '', | ||
| 'Note: the native Rust engine handles parsing on its own. WASM grammars', | ||
| 'are only needed when running with --engine wasm or on platforms without', | ||
| 'a prebuilt native addon.', | ||
| ], | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| reason: `tree-sitter CLI is not runnable: ${result.message.trim()}`, | ||
| remediation: [ | ||
| 'Inspect node_modules/tree-sitter-cli/ to confirm the package installed cleanly.', | ||
| result.stderr.trim() ? `stderr: ${result.stderr.trim().split('\n').slice(0, 3).join(' | ')}` : '', | ||
| ].filter(Boolean), | ||
| }; | ||
| } | ||
|
|
||
| // Allowed WASM imports — pure C runtime / memory primitives only (no I/O, no syscalls) | ||
| const ALLOWED_WASM_IMPORTS = new Set([ | ||
| 'env.memory', | ||
|
|
@@ -124,8 +203,16 @@ const grammars = [ | |
| { name: 'tree-sitter-verilog', pkg: 'tree-sitter-verilog', sub: null }, | ||
| ]; | ||
|
|
||
| const preflight = preflightTreeSitterCli(); | ||
| if (preflight) { | ||
| printBanner(`WASM build skipped: ${preflight.reason}`, preflight.remediation); | ||
| console.warn(`\nSkipped building ${grammars.length} grammars. Exiting cleanly (non-fatal — native engine available).`); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| let failed = 0; | ||
| let rejected = 0; | ||
| let missingToolchain = false; | ||
|
|
||
| for (const g of grammars) { | ||
| let pkgDir: string; | ||
|
|
@@ -139,15 +226,15 @@ for (const g of grammars) { | |
| const grammarDir = g.sub ? resolve(pkgDir, g.sub) : pkgDir; | ||
|
|
||
| console.log(`Building ${g.name}.wasm from ${grammarDir}...`); | ||
| try { | ||
| execFileSync('npx', ['tree-sitter', 'build', '--wasm', grammarDir], { | ||
| cwd: grammarsDir, | ||
| stdio: 'inherit', | ||
| shell: true, | ||
| }); | ||
| } catch (err: any) { | ||
| const build = runCaptured('npx', ['tree-sitter', 'build', '--wasm', grammarDir], grammarsDir); | ||
| if (!build.ok) { | ||
| failed++; | ||
| console.warn(` WARN: Failed to build ${g.name}.wasm — ${err.message ?? 'unknown error'}`); | ||
| const combined = `${build.stderr}\n${build.stdout}`; | ||
| if (/emcc|emscripten|docker/i.test(combined) && /not found|no such|cannot find|missing/i.test(combined)) { | ||
| missingToolchain = true; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The toolchain regex is tested against
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 0820688. The toolchain-detection check now concatenates |
||
| const detail = build.stderr.trim().split('\n').slice(-2).join(' | ') || build.message; | ||
| console.warn(` WARN: Failed to build ${g.name}.wasm — ${detail}`); | ||
| continue; | ||
| } | ||
|
|
||
|
|
@@ -173,6 +260,18 @@ for (const g of grammars) { | |
| const total = failed + rejected; | ||
| if (total > 0) { | ||
| console.warn(`\n${failed} build failures, ${rejected} validation rejections out of ${grammars.length} grammars (non-fatal — native engine available)`); | ||
| if (missingToolchain) { | ||
| printBanner('WASM toolchain missing', [ | ||
| "tree-sitter's `build --wasm` needs either Docker or Emscripten (emcc) to", | ||
| 'compile grammars from C to WebAssembly. Neither appears to be available.', | ||
| '', | ||
| 'Remediation — install one of:', | ||
| ' - Docker Desktop: https://www.docker.com/products/docker-desktop', | ||
| ' - Emscripten SDK: https://emscripten.org/docs/getting_started/downloads.html', | ||
| '', | ||
| 'Then re-run: npm run build:wasm', | ||
| ]); | ||
| } | ||
| if (rejected > 0) { | ||
| console.error('SECURITY: Some grammars were rejected — inspect the source packages before retrying.'); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shell: truewith array args may mishandle paths with spacesexecFileSyncwithshell: truepasses the command to a shell, which joinscmdandargsinto a single string. AnygrammarDirpath containing spaces (common on Windows underC:\Users\First Last\...) will be split by the shell, causing a "no such directory" error rather than the expected WASM-toolchain or ENOENT message. This also existed in the original code but is now centralised inrunCaptured; wrapping path args in quotes whenshell: trueis set would harden both the preflight and the per-grammar builds.If
shell: trueis required fornpxresolution on Windows, consider usingspawnSyncwithshell: trueand quoting the path argument explicitly instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0820688. Added a
quoteShellArghelper that wraps whitespace-containing args in platform-appropriate quotes (cmd.exe double quotes on Windows, POSIX single quotes elsewhere) before handing them to the shell.runCapturednow maps every arg through it, sogrammarDirpaths underC:\Users\First Last\...won't be re-split by the shell.