Skip to content

feat(config): flip .backend = .null to the extracted package by default (#386 step 4) - #431

Merged
apotema merged 3 commits into
mainfrom
feat/386-flip-null-default
Jun 30, 2026
Merged

feat(config): flip .backend = .null to the extracted package by default (#386 step 4)#431
apotema merged 3 commits into
mainfrom
feat/386-flip-null-default

Conversation

@apotema

@apotema apotema commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

null extraction step 4 — the flip. builtinProvider(.null)labelle-null 0.1.0 (third default-external backend after bgfx/wgpu).

Per the design call we made: null is the assembler's internal test/headless backend, and generateTestsTarget (#83) forces it for every project's zig build test — so that universal test target now fetches labelle-null (cached + pure-Zig, so the no system libs / cross-compile toolchain property still holds). The null headless + plugin-controllers examples + the is_tests_target codegen now resolve external null; CI exercises those (build + run, asserting frame sequences).

Test rework (same class as bgfx/wgpu): backend_registry enum-shorthand + the 4 build_zig .backend=.null tests (local null pkg + project_dir so the manifest splice resolves). labelle-null published + CI green (ubuntu+macOS build-test). zig build test green locally.

Next: slim bundled backends/null + move its examples/CI to labelle-null.

Summary by CodeRabbit

  • Bug Fixes
    • Updated backend handling so the built-in .null backend is treated consistently as an extracted external backend.
    • Build and test generation now route .null setups through the external-backend path (with corresponding null-backend wiring expectations).
    • Improved cache validation and cache population to account for the “tests target” backend resolution, ensuring required .null backend plugins are present.
  • Tests
    • Adjusted build/test suite expectations to match the new extracted/external .null behavior and override/trim behavior.

…lt (#386 step 4)

builtinProvider(.null) → labelle-null 0.1.0 — null is the third default-external
backend (after bgfx #419, wgpu #428). Pure-Zig, zero deps, empty link fragment.

Notable: generateTestsTarget (#83) forces .backend=.null for every project's
`zig build test`, so that universal test target now fetches labelle-null (cached,
pure-Zig — the no-system-libs property still holds). The null headless +
plugin-controllers examples + the is_tests_target codegen now resolve external null.

Test rework (same class as the bgfx/wgpu flips): backend_registry enum-shorthand
(null joins bgfx/wgpu external), and the 4 build_zig `.backend=.null` tests
(null-wires-modules, no-override, the two is_tests_target tests) now point at a
local null pkg + project_dir so the manifest splice resolves. Inherited the cache
fetch-path + externalUsesEnumPath from the earlier flips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request extracts the null backend into an external package (github.com/labelle-toolkit/labelle-null), moving it out of the bundled backends list. It updates the backend registry and configuration to treat the null backend as external, and updates corresponding tests in test/build_zig_tests.zig to resolve the null backend via local paths. I have no feedback to provide.

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.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Backend.null now resolves to labelle-null as an external backend. Build tests, cache validation, and cache population were updated to use the null backend package flow.

Changes

Reclassify .null backend as external

Layer / File(s) Summary
builtinProvider and registry assertions for .null
src/config.zig, src/backend_registry.zig
Backend.null now resolves to labelle-null in builtinProvider, and the enum-as-shorthand test treats .null as part of the EXTRACTED set.
build_zig_tests updated for external-null setup
test/build_zig_tests.zig
The null-backend build tests now supply backend_package and project_dir, replace the backend_input override check, and update is_tests_target cases for the extracted null backend.
cache resolution and fetching for null
src/cache/resolve.zig, src/cache_cmd.zig
Cache validation now reports the null backend package as missing when absent, and cache population now fetches that null backend package for the tests target.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐇 I hop where null once stayed,
now labelle-null has found its way.
Cache and tests both hop along,
to the external-backend song.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: making .backend = .null use the extracted package by default.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/386-flip-null-default

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/build_zig_tests.zig (1)

202-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep one regression that exercises the implicit .null provider path.

These updates all inject backend_package explicitly, so they only cover the override branch. If ProjectConfig.builtinProvider(.null) drifts, generateBuildZig(... .backend = .null ...) would still pass here. A tiny assertion on effectiveBackendPackage() for .null would pin the default-external contract without requiring a network fetch.

Also applies to: 290-304, 392-399, 517-523

🤖 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/build_zig_tests.zig` around lines 202 - 211, The current tests only
cover the explicit backend_package override path, so add one regression that
exercises the implicit .null provider path through
ProjectConfig.builtinProvider(.null). Introduce a small assertion around
effectiveBackendPackage() for .null to verify the default external package
contract without relying on a network fetch, and keep the existing
generateBuildZig(...) cases focused on the override branch.
🤖 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/build_zig_tests.zig`:
- Around line 202-211: The current tests only cover the explicit backend_package
override path, so add one regression that exercises the implicit .null provider
path through ProjectConfig.builtinProvider(.null). Introduce a small assertion
around effectiveBackendPackage() for .null to verify the default external
package contract without relying on a network fetch, and keep the existing
generateBuildZig(...) cases focused on the override branch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d250280e-49e3-4ecf-bba8-cce48760505c

📥 Commits

Reviewing files that changed from the base of the PR and between ce8deaf and f57a5b1.

📒 Files selected for processing (3)
  • src/backend_registry.zig
  • src/config.zig
  • test/build_zig_tests.zig

apotema added 2 commits June 30, 2026 15:10
The flip surfaced this (CI: a raylib project's tests-target generate failed
ExternalBackendNeedsManifest): generateTestsTarget (#83) forces .backend=.null
for EVERY project's `zig build test`, but validateCache/ensureCache only
considered the project's OWN backend — so external labelle-null was never
reported missing or fetched for a non-null project. Account for the tests-target
null in both (dedup when the project already IS null). No-op while null is bundled.
…nal tests

The two external-backend validateCache tests now also see the tests-target's
external null in the missing set (every project validates null post-flip). Assert
the project backend specifically (local stub) + expect both fakebackend AND null
for the remote case.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/cache/resolve.zig (1)

334-336: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Compare the full backend package identity here, not just name.

This dedupe suppresses the tests-target labelle-null fetch for any project backend whose effective package is also named "null" but points at a different repo/version/local path. In that case validateCache() can return no missing entries, ensureCache() exits early, and zig build test never gets the real extracted null backend. Matching repo + version (or the whole PluginDep) would keep validateCache aligned with ensureCache.

Suggested fix
     if (tests_target_cfg.effectiveBackendPackage()) |null_bp| {
-        const already = cfg.effectiveBackendPackage() != null and
-            std.mem.eql(u8, cfg.effectiveBackendPackage().?.name, null_bp.name);
+        const already = if (cfg.effectiveBackendPackage()) |bp|
+            std.mem.eql(u8, bp.name, null_bp.name) and
+                std.mem.eql(u8, bp.repo, null_bp.repo) and
+                std.mem.eql(u8, bp.version, null_bp.version)
+        else
+            false;
         if (!already and !try isPluginCached(allocator, null_bp)) {
             try missing.append(allocator, try std.fmt.allocPrint(allocator, "backend {s} {s}", .{ null_bp.name, null_bp.version }));
         }
     }
🤖 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/cache/resolve.zig` around lines 334 - 336, The dedupe in the cache
resolution logic is only comparing the effective backend package’s name against
`null_bp.name`, which can incorrectly suppress the `labelle-null` fetch for
different packages that happen to share the same name. Update the `resolve.zig`
check around `cfg.effectiveBackendPackage()` and `isPluginCached()` to compare
the full backend package identity instead of just `name`—for example by matching
`repo` and `version` (or the complete `PluginDep` fields) so `validateCache()`
and `ensureCache()` stay consistent and the real null backend is fetched when
needed.
🤖 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.

Outside diff comments:
In `@src/cache/resolve.zig`:
- Around line 334-336: The dedupe in the cache resolution logic is only
comparing the effective backend package’s name against `null_bp.name`, which can
incorrectly suppress the `labelle-null` fetch for different packages that happen
to share the same name. Update the `resolve.zig` check around
`cfg.effectiveBackendPackage()` and `isPluginCached()` to compare the full
backend package identity instead of just `name`—for example by matching `repo`
and `version` (or the complete `PluginDep` fields) so `validateCache()` and
`ensureCache()` stay consistent and the real null backend is fetched when
needed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a5027ad-17cf-4290-a56e-58489f0f3273

📥 Commits

Reviewing files that changed from the base of the PR and between 2c0a44a and 3b31f75.

📒 Files selected for processing (1)
  • src/cache/resolve.zig

@apotema
apotema merged commit b7b48f6 into main Jun 30, 2026
4 checks passed
@apotema
apotema deleted the feat/386-flip-null-default branch June 30, 2026 18:22
apotema added a commit that referenced this pull request Jun 30, 2026
…hase 6c) (#432)

null resolves out-of-tree now (`.backend = .null` → the labelle-null provider,
flipped in #431), so the in-tree backends/null/ slot is dead weight. Delete it.

THREE backends now fully out of the assembler bundle (bgfx + wgpu + null). The
assembler ships no null backend, yet `.backend = .null` still works everywhere —
incl. the universal `zig build test` tests target (#83), which fetches labelle-null
(pure-Zig, cached; the no-system-libs property holds).

- Remove backends/null/ (src/templates/manifest/fragments).
- Tests: drop the 2 null-specific build_zig unit tests (no in-tree package to
  resolve — coverage moved to labelle-null's CI + the examples-integration null
  headless + plugin-controllers RUN steps, which build+run a project on the
  fetched external null) and the null case from the in-tree-template preview list.
  The two is_tests_target mechanism tests switch to the bundled raylib backend
  (the trim/lib-chaining behavior is backend-agnostic).
- Docs: point the null references at the labelle-null package.

examples/null + examples/plugin-controllers stay — they're headless/plugin
feature tests that now exercise the external null end-to-end in CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant