feat(#386): flip sokol → labelle-sokol — assembler now fully backend-agnostic - #452
Conversation
…end-agnostic
Sokol was the last bundled backend. `builtinProvider(.sokol)` now resolves to
the published `labelle-sokol@0.1.0` package, so every Backend enum tag maps to
an external provider and there are NO remaining `=> null` arms. Production
codegen no longer bundles any backend.
`backends/sokol` is RETAINED purely as the offline in-tree test fixture /
reference manifest (the generic codegen tests need a real backend manifest
readable on disk). Tests that generate with sokol are repointed at the fixture
via an explicit `.backend_package = .{ .name = "sokol", .repo =
"local:backends/sokol" }` + `project_dir = "."`; the manifest splice renders
byte-identically to the pre-flip enum path.
- src/config.zig: flip `.sokol => null` to the labelle-sokol provider; refresh
the now-stale `builtinProvider` doc (no null arms remain).
- src/root.zig: export `BuildZigOptions` for the test helper.
- test/helpers.zig: add `sokol_fixture_package` + `genSokolBuildZig` helper;
repoint the 24 sokol build_zig_tests call sites to it.
- src/backend_registry.zig, src/deps_linker.zig: rewrite enum-logic tests that
asserted sokol was "bundled/non-external" to the new all-external reality
(the built-in-staging switch arms are now defensive dead code).
- src/codegen/manifest_splice.zig: repoint the built-in no-op test to verify
the retained fixture's manifest is accepted.
- test/main_zig_tests.zig: drop the "built-in emits NO contract guard" test
(the no-guard branch is unreachable now every backend is external) with a
NOTE; external-guard coverage already lives directly above.
- test/flow_scanner/integration_tests.zig: repoint the two sokol generations
at the fixture.
CI unchanged: the `backends/sokol && zig build test` fixture self-test stays;
no sokol example-integration jobs exist to remove.
Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughWalkthroughThe built-in ChangesSokol external backend flip
Sequence Diagram(s)sequenceDiagram
participant Test as Test Code
participant Config as ProjectConfig
participant Resolver as effectiveBackendPackage
participant Provider as builtinProvider
participant Manifest as requireManifestIfExternal
Test->>Config: set backend = .sokol
Config->>Resolver: effectiveBackendPackage()
Resolver->>Provider: builtinProvider(.sokol)
Provider-->>Resolver: provider package (labelle-sokol)
Resolver-->>Config: non-null package, isExternal = true
Test->>Manifest: requireManifestIfExternal(project_dir=".")
Manifest-->>Test: success (manifest found via backend_package)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2376028026
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .raylib => .{ .name = "raylib", .repo = "github.com/labelle-toolkit/labelle-raylib", .version = "0.1.0" }, | ||
| // Still bundled. | ||
| .sokol => null, | ||
| .sokol => .{ .name = "sokol", .repo = "github.com/labelle-toolkit/labelle-sokol", .version = "0.1.0" }, |
There was a problem hiding this comment.
Flipping the bare Sokol enum to an external provider makes cfg.isExternal() true for every .backend = .sokol project. On .platform = .wasm or .ios, the manifest path is disabled because manifests are desktop-only, and generateBuildZig only allows external enum fallback on Android (externalUsesEnumPath), so generation returns ExternalBackendNeedsManifest before the existing backend_sokol_wasm / backend_sokol_ios sections can run. This breaks current Sokol web/iOS projects that don't set an explicit backend_package; either keep Sokol non-external for those targets or add manifest/enum support for them before this flip.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/main_zig_tests.zig (1)
431-437: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep a positive contract-guard test for the enum-shorthand path.
The remaining assertion only covers an explicit
.backend_package. This PR’s actual behavior change is that plain.backend = .<tag>now becomes external viabuiltinProvider(), so a regression where the guard still keys offcfg.backend_package != nullwould slip through.🧪 Suggested test
+ test "enum-shorthand built-in backend emits the core-contract verification guard" { + const main_zig = try generate.generateMainZigFromTemplate(std.testing.allocator, engine_template, .{ .y_axis = .up, + .name = "test-game", + .backend = .raylib, + .ecs = .mock, + }, raylib_lifecycle, empty_entries, empty_names, empty_names, empty_scene_manifests, empty_names, empty_names, empty_names, empty_names, empty_names, empty_names, empty_names, empty_plugin_events, empty_plugin_flow_nodes, empty_plugin_pin_styles, empty_plugin_coercions); + defer std.testing.allocator.free(main_zig); + + try std.testing.expect(std.mem.indexOf(u8, main_zig, "assertBackend(`@import`(\"backend_gfx\"))") != null); + try std.testing.expect(std.mem.indexOf(u8, main_zig, "assertWindow(`@import`(\"backend_window\"))") != null); + try std.testing.expect(std.mem.indexOf(u8, main_zig, "assertInput(`@import`(\"backend_input\"))") != null); + }🤖 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 `@test/main_zig_tests.zig` around lines 431 - 437, Add a positive contract-guard test that exercises the enum-shorthand `.backend = .<tag>` path, not just an explicit `.backend_package`, so `main_zig_tests.zig` verifies the new `builtinProvider()` behavior. Update the relevant test case(s) to assert that a plain backend tag now resolves as external and still emits the contract guard, ensuring the check is not accidentally tied only to `cfg.backend_package != null`.test/helpers.zig (1)
351-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake
genSokolBuildZigforce the sokol tag.Line 352 only rewires
backend_package. If a caller forgets.backend = .sokol, this helper still builds a mixed config even though its contract is “generate a sokol build.zig”. Setting the tag here (or asserting it) would make the helper self-consistent.Proposed change
pub fn genSokolBuildZig( allocator: std.mem.Allocator, cfg_in: generate.ProjectConfig, opts_in: generate.BuildZigOptions, ) ![]const u8 { var cfg = cfg_in; + cfg.backend = .sokol; cfg.backend_package = sokol_fixture_package; var opts = opts_in; opts.project_dir = "."; return generate.generateBuildZig(allocator, cfg, opts); }🤖 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 `@test/helpers.zig` around lines 351 - 355, genSokolBuildZig currently only swaps backend_package, so it can still produce a mixed config if the caller does not set the sokol backend explicitly. Update genSokolBuildZig in helpers.zig to force the cfg.backend tag to .sokol (or assert it before calling generate.generateBuildZig), alongside the existing backend_package and opts.project_dir setup, so the helper always matches its sokol-specific contract.
🤖 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.
Nitpick comments:
In `@test/helpers.zig`:
- Around line 351-355: genSokolBuildZig currently only swaps backend_package, so
it can still produce a mixed config if the caller does not set the sokol backend
explicitly. Update genSokolBuildZig in helpers.zig to force the cfg.backend tag
to .sokol (or assert it before calling generate.generateBuildZig), alongside the
existing backend_package and opts.project_dir setup, so the helper always
matches its sokol-specific contract.
In `@test/main_zig_tests.zig`:
- Around line 431-437: Add a positive contract-guard test that exercises the
enum-shorthand `.backend = .<tag>` path, not just an explicit
`.backend_package`, so `main_zig_tests.zig` verifies the new `builtinProvider()`
behavior. Update the relevant test case(s) to assert that a plain backend tag
now resolves as external and still emits the contract guard, ensuring the check
is not accidentally tied only to `cfg.backend_package != null`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 696d8815-bd85-4cdc-96e0-ac7e96883104
📒 Files selected for processing (9)
src/backend_registry.zigsrc/codegen/manifest_splice.zigsrc/config.zigsrc/deps_linker.zigsrc/root.zigtest/build_zig_tests.zigtest/flow_scanner/integration_tests.zigtest/helpers.zigtest/main_zig_tests.zig
1 participant
The final backend extraction (#386). Flips
builtinProvider(.sokol)fromnullto the publishedlabelle-sokol@0.1.0. With this, every backend (bgfx, wgpu, null, sdl, raylib, sokol) resolves to an external package —builtinProviderhas zero=> nullarms and the assembler's production codegen is 100% backend-agnostic.Design note —
backends/sokolis retained as a test fixtureBecause sokol is the last backend, the ~24 generic codegen tests (which call
generateBuildZigand inspect the generated string, reading the backend manifest offline) need some real in-tree manifest.backends/sokolstays purely as the reference/test fixture, pinned explicitly via.backend_package = local:backends/sokol+.project_dir = ".". Production never bundles it — it resolves the external package like every other backend. This keeps CI offline/fast without compromising agnosticism.Changes (9 files, +133/−111)
src/config.zig—.sokol => labelle-sokol@0.1.0; refreshed thebuiltinProviderdoc (no null arms remain).src/root.zig— exportBuildZigOptionsfor the typed test helper.test/helpers.zig— newgenSokolBuildZighelper injecting the fixturebackend_package+project_dir.test/build_zig_tests.zig— 24 sokol call sites →h.genSokolBuildZig(bgfx + error-expecting calls untouched). Splice renders byte-identically, so all string assertions still hold.src/backend_registry.zig,src/deps_linker.zig— rewrote enum-logic tests that assumed sokol was bundled/non-external (now impossible — all backends external).src/codegen/manifest_splice.zig— repointed the built-in no-op test to verify the retained fixture's manifest is accepted.test/main_zig_tests.zig— dropped a now-unreachable "built-in emits no contract guard" test (replaced with a NOTE).test/flow_scanner/integration_tests.zig— 2 sokol generations → fixture.zig build testandzig buildboth exit 0 locally. Completes the pluggable-backends epic #386.https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
Summary by CodeRabbit
Bug Fixes
sokolbackend.Tests