ReviewOS
Reload

also looking at this

craft-native/craft

fix(dev): validate the pinned Zig toolchain before builds

#299
Merged glennmichael123 wants to merge codex/pinned-zig-toolchain into main
5 files +137 -14

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

.github/CONTRIBUTING.mdmodified+14-6
@@ -6,9 +6,17 @@ Thank you for your interest in contributing to Craft! This document provides gui
66
77### Prerequisites
88
9- **Pantry-managed stable Zig**: run `eval "$(pantry env | sed -n '/^export /,$p')"`, then invoke `zig` (locally only — CI does not: the pantry action already puts `zig` on PATH, and `pantry env` runs a workspace setup that re-installs JS deps with its own `--linker`, rewriting `node_modules` mid-job)
9- **Pinned Zig toolchain**: use the exact snapshot in `pantry.lock`. Root `bun run build:core`, `test`, `fmt`, `run`, and `zig` scripts validate the selected compiler and print its path/version before use. Activate your Pantry environment locally, or set `CRAFT_ZIG=/absolute/path/to/zig` to a matching compiler. The scripts do not invoke `pantry env` automatically because it can reinstall workspace dependencies. CI receives the pinned toolchain from the Pantry action.
1010- **Bun**: Install from [bun.sh](https://bun.sh)
1111
12To intentionally test another Zig snapshot, use both an explicit compiler and the opt-in:
13
14```bash
15CRAFT_ZIG=/absolute/path/to/experimental/zig CRAFT_ALLOW_UNPINNED_ZIG=1 bun run build:core
16```
17
18This override is for compatibility testing; it does not change the checked-in lockfile.
19
1220#### Platform-specific Dependencies
1321
1422**macOS:**
@@ -41,19 +49,19 @@ cd craft
4149
4250```bash
4351# Debug build
44zig build
52bun run build:debug
4553
4654# Release build
47zig build -Doptimize=ReleaseSafe
55bun run build:core
4856
4957# Run tests
50zig build test
58bun run test
5159
5260# Run example
53zig build run
61bun run run:demo
5462
5563# Run the craft CLI
56zig build run
64bun run run
5765```
5866
5967## Project Structure
.github/workflows/ci.ymlmodified+3-0
@@ -267,6 +267,9 @@ jobs:
267267 - name: Install dependencies
268268 run: bun install
269269
270 - name: Test pinned Zig toolchain selection
271 run: bun test scripts/zig.test.ts
272
270273 - name: Check Zig formatting
271274 working-directory: packages/zig
272275 run: zig fmt --check src/ build.zig
package.jsonmodified+8-8
@@ -14,8 +14,8 @@
1414 ],
1515 "scripts": {
1616 "build": "bun run build:core && bun run build:sdk",
17 "build:core": "command -v zig >/dev/null 2>&1 || eval \"$(pantry env | sed -n '/^export /,$p')\"; cd packages/zig && zig build -Doptimize=ReleaseSafe -Dversion=$(bun -e \"console.log(require('../../package.json').version)\")",
18 "build:debug": "command -v zig >/dev/null 2>&1 || eval \"$(pantry env | sed -n '/^export /,$p')\"; cd packages/zig && zig build",
17 "build:core": "bun scripts/zig.ts --core --versioned build -Doptimize=ReleaseSafe",
18 "build:debug": "bun scripts/zig.ts --core build",
1919 "build:sdk": "cd packages/typescript && bun run build",
2020 "build:ios": "cd packages/ios && bun run build",
2121 "build:android": "cd packages/android && bun run build",
@@ -35,14 +35,14 @@
3535 "verify:create": "bun scripts/verify-create-craft.ts",
3636 "verify:cimports": "bun scripts/audit-cimports.ts",
3737 "verify:docs": "bun scripts/verify-doc-links.ts",
38 "test": "command -v zig >/dev/null 2>&1 || eval \"$(pantry env | sed -n '/^export /,$p')\"; cd packages/zig && zig build test",
38 "test": "bun scripts/zig.ts --core build test",
3939 "test:sdk": "cd packages/typescript && bun test",
4040 "test:native-lifecycle": "bun scripts/native-lifecycle.ts",
4141 "test:mobile-e2e:ios": "bun scripts/mobile-e2e.ts --platform ios --ios-runtime packages/zig/zig-out/lib",
4242 "test:mobile-e2e:android": "bun scripts/mobile-e2e.ts --platform android --android-runtime packages/zig/zig-out/android",
4343 "typecheck": "cd packages/typescript && bun run typecheck && cd ../create-craft && bun run typecheck && cd ../ts-maps && bun run typecheck",
44 "run": "command -v zig >/dev/null 2>&1 || eval \"$(pantry env | sed -n '/^export /,$p')\"; cd packages/zig && zig build run",
45 "run:demo": "command -v zig >/dev/null 2>&1 || eval \"$(pantry env | sed -n '/^export /,$p')\"; cd packages/zig && zig build run-demo",
44 "run": "bun scripts/zig.ts --core build run",
45 "run:demo": "bun scripts/zig.ts --core build run-demo",
4646 "create": "cd packages/create-craft && bun bin/cli.ts",
4747 "clean": "rm -rf packages/zig/zig-cache packages/zig/zig-out && bun run clean:packages",
4848 "clean:packages": "rm -rf packages/*/dist packages/*/node_modules",
@@ -50,10 +50,10 @@
5050 "lint:fix": "bun x --bun pickier . --fix",
5151 "format": "bun x --bun pickier . --format",
5252 "format:fix": "bun x --bun pickier . --format --write",
53 "fmt": "command -v zig >/dev/null 2>&1 || eval \"$(pantry env | sed -n '/^export /,$p')\"; cd packages/zig && zig fmt src/ build.zig",
54 "fmt:check": "command -v zig >/dev/null 2>&1 || eval \"$(pantry env | sed -n '/^export /,$p')\"; cd packages/zig && zig fmt --check src/ build.zig",
53 "fmt": "bun scripts/zig.ts --core fmt src/ build.zig",
54 "fmt:check": "bun scripts/zig.ts --core fmt --check src/ build.zig",
5555 "pantry:env": "pantry env",
56 "zig": "command -v zig >/dev/null 2>&1 || eval \"$(pantry env | sed -n '/^export /,$p')\"; zig",
56 "zig": "bun scripts/zig.ts",
5757 "dev": "cd packages/typescript && bun run dev",
5858 "changelog": "bun x logsmith --verbose",
5959 "changelog:generate": "bun x logsmith --output CHANGELOG.md",
scripts/zig.test.tsadded+51-0
@@ -0,0 +1,51 @@
1import { afterEach, expect, test } from 'bun:test'
2import { chmodSync, mkdtempSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
3import { tmpdir } from 'node:os'
4import { join } from 'node:path'
5import { pinnedZigVersion, resolveZig, runZig } from './zig'
6
7const roots: string[] = []
8afterEach(() => { for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }) })
9
10function fixture(version = '0.17.0-dev.1963+e00c6c439') {
11 const root = realpathSync(mkdtempSync(join(tmpdir(), 'craft-zig-toolchain-')))
12 roots.push(root)
13 mkdirSync(join(root, 'packages/zig'), { recursive: true })
14 writeFileSync(join(root, 'pantry.lock'), JSON.stringify({ workspaces: { '': { system: { 'ziglang.org': '0.17.0-dev.1963_e00c6c439' } } } }))
15 writeFileSync(join(root, 'package.json'), JSON.stringify({ version: '1.2.3' }))
16 const path = join(root, 'zig')
17 writeFileSync(path, `#!${process.execPath}\nif (process.argv[2] === 'version') console.log(${JSON.stringify(version)}); else { await Bun.write(process.env.ZIG_TEST_LOG, JSON.stringify({ cwd: process.cwd(), args: process.argv.slice(2) })); process.exit(23); }\n`)
18 chmodSync(path, 0o755)
19 return { root, path, env: { ...process.env, CRAFT_ZIG: '', CRAFT_ALLOW_UNPINNED_ZIG: '', PATH: root, ZIG_TEST_LOG: join(root, 'invocation.json') } }
20}
21
22test('accepts only the locked snapshot by default, even when PATH shadows it', () => {
23 const f = fixture('0.17.0-dev.2163+89ff10d56')
24 expect(pinnedZigVersion(f.root)).toBe('0.17.0-dev.1963+e00c6c439')
25 expect(() => resolveZig(f.root, f.env)).toThrow('version mismatch')
26 expect(() => resolveZig(f.root, f.env)).toThrow('CRAFT_ALLOW_UNPINNED_ZIG=1')
27 const pinned = fixture()
28 expect(resolveZig(f.root, { ...f.env, CRAFT_ZIG: pinned.path })).toEqual({ path: pinned.path, version: '0.17.0-dev.1963+e00c6c439', overridden: false })
29})
30
31test('requires an explicit opt-in for unpinned snapshots', () => {
32 const f = fixture('0.17.0-dev.2163+89ff10d56')
33 expect(() => resolveZig(f.root, { ...f.env, CRAFT_ALLOW_UNPINNED_ZIG: 'true' })).toThrow('version mismatch')
34 expect(resolveZig(f.root, { ...f.env, CRAFT_ALLOW_UNPINNED_ZIG: '1' }).overridden).toBe(true)
35})
36
37test('fails early for absent executables, invalid lock versions, and invalid compiler output', () => {
38 const f = fixture('not a version')
39 expect(() => resolveZig(f.root, { ...f.env, CRAFT_ZIG: join(f.root, 'missing') })).toThrow('was not found')
40 expect(() => resolveZig(f.root, f.env)).toThrow('Invalid Zig version')
41 writeFileSync(join(f.root, 'pantry.lock'), JSON.stringify({ workspaces: { '': { system: { 'ziglang.org': 'latest' } } } }))
42 expect(() => pinnedZigVersion(f.root)).toThrow('exact Zig version')
43})
44
45test('forwards arguments, core working directory, package version and compiler exit status', () => {
46 const f = fixture()
47 expect(runZig(f.root, ['--core', '--versioned', 'build', '-Doptimize=ReleaseSafe', '-Dfoo=a b'], f.env)).toBe(23)
48 expect(JSON.parse(readFileSync(f.env.ZIG_TEST_LOG, 'utf8'))).toEqual({ cwd: join(f.root, 'packages/zig'), args: ['build', '-Doptimize=ReleaseSafe', '-Dfoo=a b', '-Dversion=1.2.3'] })
49 expect(runZig(f.root, ['fmt', 'src/'], f.env)).toBe(23)
50 expect(JSON.parse(readFileSync(f.env.ZIG_TEST_LOG, 'utf8'))).toEqual({ cwd: f.root, args: ['fmt', 'src/'] })
51})
scripts/zig.tsadded+61-0
@@ -0,0 +1,61 @@
1import { readFileSync } from 'node:fs'
2import { join, resolve } from 'node:path'
3
4export function pinnedZigVersion(root: string): string {
5 const lock = JSON.parse(readFileSync(join(root, 'pantry.lock'), 'utf8'))
6 const version = lock.workspaces?.['']?.system?.['ziglang.org']
7 if (typeof version !== 'string' || !/^\d+\.\d+\.\d+(?:-dev\.\d+_[a-f0-9]+)?$/.test(version))
8 throw new Error('pantry.lock must contain an exact Zig version in the root workspace system dependencies')
9 return version.replace('_', '+')
10}
11
12export function resolveZig(root: string, env: NodeJS.ProcessEnv = process.env): { path: string, version: string, overridden: boolean } {
13 const expected = pinnedZigVersion(root)
14 const executable = env.CRAFT_ZIG || 'zig'
15 const path = Bun.which(executable, { PATH: env.PATH, cwd: root })
16 const remedy = 'Activate the pinned Pantry toolchain, or set CRAFT_ZIG to its executable. To intentionally test a different snapshot, also set CRAFT_ALLOW_UNPINNED_ZIG=1.'
17 if (!path)
18 throw new Error(`Zig ${expected} was not found (${executable}). ${remedy}`)
19 const result = Bun.spawnSync([path, 'version'], { cwd: root, env, stdout: 'pipe', stderr: 'pipe' })
20 if (result.exitCode !== 0)
21 throw new Error(`Cannot read Zig version from ${path}: ${result.stderr.toString().trim()}. ${remedy}`)
22 const version = result.stdout.toString().trim()
23 if (!/^\d+\.\d+\.\d+(?:-[0-9A-Za-z.+-]+)?$/.test(version))
24 throw new Error(`Invalid Zig version from ${path}: ${JSON.stringify(version)}`)
25 const overridden = version !== expected
26 if (overridden && env.CRAFT_ALLOW_UNPINNED_ZIG !== '1')
27 throw new Error(`Zig version mismatch: ${path} reports ${version}; pantry.lock requires ${expected}. ${remedy}`)
28 return { path, version, overridden }
29}
30
31export function runZig(root: string, args: string[], env: NodeJS.ProcessEnv = process.env): number {
32 const command = [...args]
33 const core = command[0] === '--core'
34 if (core) command.shift()
35 const versioned = command[0] === '--versioned'
36 if (versioned) command.shift()
37 if (versioned) {
38 const { version } = JSON.parse(readFileSync(join(root, 'package.json'), 'utf8'))
39 command.push(`-Dversion=${version}`)
40 }
41 const zig = resolveZig(root, env)
42 console.error(`Zig ${zig.version}: ${zig.path}${zig.overridden ? ' (explicit unpinned override)' : ''}`)
43 const result = Bun.spawnSync([zig.path, ...command], {
44 cwd: core ? join(root, 'packages/zig') : root,
45 env,
46 stdin: 'inherit',
47 stdout: 'inherit',
48 stderr: 'inherit',
49 })
50 return result.exitCode
51}
52
53if (import.meta.main) {
54 try {
55 process.exitCode = runZig(resolve(import.meta.dir, '..'), process.argv.slice(2))
56 }
57 catch (error) {
58 console.error(error instanceof Error ? error.message : String(error))
59 process.exitCode = 1
60 }
61}