feat(#461): manifest-v2 PR8 — null + wgpu (generic declarative desktop path) - #469
Conversation
Convert the null and wgpu backends to the manifest-v2 build-graph schema (epic #453 item 3, PR 8). Both are fully-declarative, HOOKLESS desktop backends — the first cells to exercise the v2 codegen's generic declarative desktop path (loop-form unifyCoreDiamond walk), as opposed to the sokol desktop byte anchor (unrolled sokol residual, kept 0-diff). - backends/null_v2/backend.manifest.v2.zon: headless, pure-Zig, no artifact, no frameworks, no dep_options, no build_hook. capabilities = {headless}. - backends/wgpu_v2/backend.manifest.v2.zon: glfw artifact + frameworks.desktop.macos = {Foundation, QuartzCore, Metal}, no build_hook. capabilities = {compressed_textures}. Codegen (manifest_v2_splice.zig): - Add a generic declarative desktop path (renderDesktopDepsDeclsV2, renderDesktopBackendDepGenericV2, renderDesktopLinkGenericV2 + emitDesktopOsLinks) mirroring the android/ios/wasm generic emitters: no sokol-specific prose, loop-form core-diamond walk, manifest-driven artifact + per-OS system-lib/framework switch. - isDesktopByteAnchor(m) routes the sokol fixture to the unrolled byte-anchor path and every other backend to the generic path, so the sokol-desktop 0-diff anchor is untouched. Wired via desktopUsesGenericV2 in build_files. Gate: reviewed golden snapshots (test/goldens/{null,wgpu}_v2.build.zig) — NOT a byte-anchor. Although the .backend_null/.backend_wgpu/.link_wgpu enum sections still exist in build_zig.txt, the v2 path deliberately emits the generic loop form (differs from the enum's unrolled overrides), so per design §7 the reviewed golden is the correct gate. Each golden is asserted AST-valid, hookless (no backend_build_hook import / resolve_target / post_wire), carries the generic walk, links wgpu's frameworks, and links none for null. zig build test: exit 0. zig build: exit 0. sokol byte anchor + all prior v2 goldens unchanged. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds v2 backend manifests for the ChangesManifest-v2 generic desktop backend support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request implements the generic declarative path for desktop builds under the manifest-v2 schema, introducing support for hookless backends like null and wgpu. It adds manifest files, updates the build generator to conditionally emit generic desktop dependency declarations and the unifyCoreDiamond walk, and includes comprehensive tests with golden files. The review feedback highlights a potential compilation issue in the generated build.zig where artifact names from the manifest are used directly as Zig variable identifiers. If these names contain hyphens or start with digits, they will produce invalid Zig syntax, so sanitizing these identifiers is recommended.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for (desktop.artifacts) |art| { | ||
| try w.print(" const {s} = backend_dep.artifact(\"{s}\");\n", .{ art.name, art.name }); | ||
| if (art.pic) try w.print(" {s}.root_module.pic = true;\n", .{art.name}); | ||
| } |
There was a problem hiding this comment.
Artifact names defined in the manifest (such as art.name) are used directly as Zig variable identifiers. If an artifact name starts with a digit (e.g., 123lib) or contains invalid characters like hyphens (e.g., glfw-native), the generated build.zig will fail to compile.
Following the general rule, Zig identifiers must not start with a digit and must be valid identifiers. We should sanitize the variable name (for example, by replacing hyphens with underscores and prefixing with an underscore if it starts with a digit) while keeping the original string for the backend_dep.artifact(...) call.
References
- Zig identifiers must not start with a digit. When converting strings (such as file paths) to Zig identifiers, prefix the result with an underscore if the input starts with a digit to ensure the generated code is valid.
| for (desktop.artifacts) |art| { | ||
| try w.print(" exe.root_module.linkLibrary({s});\n", .{art.name}); | ||
| } |
There was a problem hiding this comment.
Similarly to the artifact declaration, the variable name used here to link the library should be sanitized to ensure it matches the sanitized variable identifier, preventing compilation errors if the artifact name contains hyphens or starts with a digit.
References
- Zig identifiers must not start with a digit. When converting strings (such as file paths) to Zig identifiers, prefix the result with an underscore if the input starts with a digit to ensure the generated code is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/codegen/manifest_v2_splice.zig (1)
189-241: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffOptional: consolidate the four near-identical generic backend-dep emitters.
renderDesktopBackendDepGenericV2is a structural clone ofrenderAndroidBackendDepV2,renderIosBackendDepV2, andrenderWasmBackendDepV2, differing only in the.targetalias string and the platform-entry accessor. The core-diamond walk-call block (Lines 225-239) is byte-identical across all four. A single helper parameterized by(target_alias, platform_entry)would collapse the family and remove drift risk as more backends are added. Deferrable — golden snapshots protect the emitted output today, so this is maintainability-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codegen/manifest_v2_splice.zig` around lines 189 - 241, Consolidate the four near-identical generic backend-dep emitters into one shared helper to reduce drift risk. `renderDesktopBackendDepGenericV2` is structurally the same as `renderAndroidBackendDepV2`, `renderIosBackendDepV2`, and `renderWasmBackendDepV2`, so extract the common emission logic and parameterize only the target alias and platform-entry accessor. Keep the core-diamond walk-call block and module/artifact emission behavior unchanged while centralizing it behind the shared helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/goldens/wgpu_v2.build.zig`:
- Around line 66-108: The test build setup for test_root is missing the same
native linkage that exe.root_module already has, so backend imports can fail at
test link time. Mirror the glfw library and the macOS frameworks onto the
root_module passed to b.addTest in the test_root setup, using the same
conditional switch on target.result.os.tag so zig build test links the
wgpu-backed code correctly.
---
Nitpick comments:
In `@src/codegen/manifest_v2_splice.zig`:
- Around line 189-241: Consolidate the four near-identical generic backend-dep
emitters into one shared helper to reduce drift risk.
`renderDesktopBackendDepGenericV2` is structurally the same as
`renderAndroidBackendDepV2`, `renderIosBackendDepV2`, and
`renderWasmBackendDepV2`, so extract the common emission logic and parameterize
only the target alias and platform-entry accessor. Keep the core-diamond
walk-call block and module/artifact emission behavior unchanged while
centralizing it behind the shared helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 301dab20-7b6e-43d3-9612-1cc332f22b48
📒 Files selected for processing (8)
backends/null_v2/backend.manifest.v2.zonbackends/wgpu_v2/backend.manifest.v2.zonsrc/build_files.zigsrc/codegen/manifest_v2_splice.zigtest/build_zig_tests.zigtest/goldens/null_v2.build.zigtest/goldens/wgpu_v2.build.zigtest/helpers.zig
… test_root links Three valid findings on the generic declarative desktop path (PR 8, null/wgpu): - gemini m×2: manifest artifact names were emitted verbatim as Zig variable identifiers at both the decl site (`const <name> = backend_dep.artifact(...)`) and the link site. Names like `glfw-native` / `123lib` would emit invalid Zig. Reuse `scan.sanitizePluginIdent` (hyphens/dots/invalid → `_`, leading digit → `_` prefix) via a new `artifactIdent` helper at BOTH sites so the declared variable and the link reference match. Unit test added. - coderabbit Major: the generic desktop link attached linkLibrary(glfw) + the macOS frameworks to exe.root_module only, not test_root.root_module — so `zig build test` for a wgpu project could fail at link time. Factor the link emission into `emitDesktopLinkForTarget(target_var)` and emit it a second time onto test_root (after the test step declares it) via `renderDesktopTestLinkGenericV2`. Enum/sokol byte-anchor path links only exe; the generic path now links both. null emits nothing (no artifact/framework). wgpu golden updated to include the test_root links. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
PR 8 of the manifest-v2 epic (#461, #453 item 3). Converts null + wgpu — pure-declarative, HOOKLESS desktop backends. Beyond the fixtures, this adds the generic declarative desktop path that generalizes v2 desktop codegen beyond sokol.
Fixtures (mirrored from the real external backends)
backends/null_v2/backend.manifest.v2.zon—id="labelle.null",capabilities={headless}, 4 modules, no artifact/frameworks/dep_options/hook, desktop.loop/.native/.binary(matches null's empty link fragment).backends/wgpu_v2/backend.manifest.v2.zon—id="labelle.wgpu",capabilities={compressed_textures},glfwartifact,frameworks.desktop.macos={Foundation, QuartzCore, Metal}, no hook.Generic declarative desktop path (load-bearing)
The prior desktop v2 path was sokol-specific (hardcoded IOSurface frameworks, sdl_gamepad overrides,
with_imguiprose) to hold the byte anchor. AddedrenderDesktopDepsDeclsV2/renderDesktopBackendDepGenericV2/renderDesktopLinkGenericV2/emitDesktopOsLinks— a manifest-driven generic path (loop-formunifyCoreDiamond, manifest artifact link, per-OS syslib/frameworkswitch).isDesktopByteAnchor(m)(keyed ondep_name=="labelle_sokol") routes sokol → the unrolled anchor, everything else → the generic path.Golden cells + assertions
test/goldens/null_v2.build.zig+wgpu_v2.build.zig+ asserts: 0 AST parse errors; NObackend_build_hook/resolve_target/post_wire(hookless); genericunifyCoreDiamondpresent; null links nothing; wgpu linksglfw+ the macOS Metal/Foundation/QuartzCore block. + unit tests forisDesktopByteAnchor/emitDesktopOsLinks.Byte-anchor vs golden
The
.backend_null/.backend_wgpu/.link_wgpuenum sections DO still exist, but a byte-anchor is not the right gate here: the v2 path deliberately emits the generic loop form (§7), which differs from the enum's unrolled overrides — so a reviewed golden is correct.Sokol desktop byte anchor + android/ios/wasm goldens unchanged (sokol still routes to the anchor path). v1/enum unchanged (v2 via opt-in).
zig build test+zig buildexit 0.Ref #461, #453.
https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
Summary by CodeRabbit
New Features
Bug Fixes
Tests