fix(stage): skip zig-pkg/.labelle when hardlinking a local package - #421
Merged
Conversation
…hase 6c) bgfx is now resolved out-of-tree (`.backend = .bgfx` → the labelle-bgfx provider, flipped in #419 and on-device-validated), so the in-tree `backends/bgfx/` slot is dead weight. Delete it (~the whole src/templates/libs/manifest tree) and the coverage that built it in-tree: - Remove the `bgfx-build` CI job (`cd backends/bgfx && zig build test` + the standalone example). The bgfx backend's unit tests now run in labelle-bgfx's own CI (labelle-bgfx#1); its desktop+android builds are still exercised here via the examples-integration `bgfx-external` step and the `bgfx-android-build` job (both fetch the package). - `bgfx-android-build` cache key now hashes the example + config (it pinned the deleted `backends/bgfx/build.zig.zon`). - Drop the desktop bgfx-artifacts unit test (no in-tree package to resolve; coverage moved to labelle-bgfx + the splice tests) and the bgfx case from the in-tree-template preview regression list. Other backends keep their bundled slots; only bgfx leaves. First backend fully out of the assembler bundle.
hardlinkTree skipped .zig-cache/zig-out/.git but copied zig-pkg/ and .labelle/.
Staging a local backend package that's a real repo checkout (e.g. a backend repo
shipping an example inside itself) then walked:
- zig-pkg/<hash>/upstream/.../node_modules/... — deep enough that copyFile hit
NameTooLong, which crashed staging (and tripped an invalid free on the way out).
- .labelle/ — the in-repo example's own generated output, copied back into the stage.
Neither is part of a package's importable modules, so skip both alongside the
existing build/VCS dirs. Unblocks examples that live inside a backend package repo
(labelle-bgfx/examples/*) resolving the backend via its local checkout.
📝 WalkthroughWalkthroughThe PR removes the entire in-tree Changesbgfx backend extraction
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the dependency linker in src/deps_linker.zig to skip zig-pkg and .labelle directories during staging, preventing potential path limit errors and redundant file copying. A new unit test has been added to verify this behavior. Additionally, tests and template checks for the bgfx backend have been removed from test/build_zig_tests.zig and test/preview_mode_tests.zig because bgfx has been extracted into an out-of-tree repository (labelle-bgfx). There are no review comments, so 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/deps_linker.zig`:
- Around line 388-391: The staging logic in deps_linker.zig is treating every
zig-pkg subtree as disposable, which breaks declared path dependencies
referenced by build.zig.zon. Update the directory-skip/copy behavior in the
staging flow so it is manifest-aware: inspect the package manifest and preserve
or selectively copy any zig-pkg entries that are referenced, rather than
excluding the entire zig-pkg tree. Keep the fix localized to the entry
filtering/rewrite path around the directory walk and rewriteZonPaths handling.
🪄 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: cce2df5f-7de6-4672-929e-c454380a13c9
📒 Files selected for processing (52)
.github/workflows/ci.ymlbackends/bgfx/backend.manifest.zonbackends/bgfx/build.zigbackends/bgfx/build.zig.zonbackends/bgfx/build_fragments/backend_dep.txtbackends/bgfx/build_fragments/link.txtbackends/bgfx/example/android_video.zigbackends/bgfx/example/apk_video/AndroidManifest.xmlbackends/bgfx/example/build.zigbackends/bgfx/example/build.zig.zonbackends/bgfx/example/gamepad_overlay.zigbackends/bgfx/example/main.zigbackends/bgfx/libs/miniaudio/miniaudio.cbackends/bgfx/libs/miniaudio/miniaudio.hbackends/bgfx/src/android.zigbackends/bgfx/src/android_app.zigbackends/bgfx/src/audio.zigbackends/bgfx/src/audio_device.zigbackends/bgfx/src/audio_device_android.zigbackends/bgfx/src/gfx.zigbackends/bgfx/src/gfx/astc.zigbackends/bgfx/src/gfx/draw.zigbackends/bgfx/src/gfx/font.zigbackends/bgfx/src/gfx/programs.zigbackends/bgfx/src/gfx/state.zigbackends/bgfx/src/gfx/texture.zigbackends/bgfx/src/gfx/types.zigbackends/bgfx/src/input.zigbackends/bgfx/src/platform.zigbackends/bgfx/src/shaders.zigbackends/bgfx/src/stb_image.hbackends/bgfx/src/stb_image_impl.cbackends/bgfx/src/stb_shim.hbackends/bgfx/src/video/android.zigbackends/bgfx/src/video/android_audio.zigbackends/bgfx/src/video/apk/AndroidManifest.xmlbackends/bgfx/src/video/apk/build_apk.shbackends/bgfx/src/video/apk/decode_shim.hbackends/bgfx/src/video/apk/native.zigbackends/bgfx/src/video/backend.zigbackends/bgfx/src/video/desktop.zigbackends/bgfx/src/video/fit.zigbackends/bgfx/src/video/planes.zigbackends/bgfx/src/video/player.zigbackends/bgfx/src/video/test_decode.zigbackends/bgfx/src/video/yuv.zigbackends/bgfx/src/window.zigbackends/bgfx/templates/android.txtbackends/bgfx/templates/desktop.txtsrc/deps_linker.zigtest/build_zig_tests.zigtest/preview_mode_tests.zig
💤 Files with no reviewable changes (46)
- backends/bgfx/build_fragments/link.txt
- backends/bgfx/src/gfx.zig
- backends/bgfx/src/video/apk/build_apk.sh
- backends/bgfx/src/gfx/types.zig
- backends/bgfx/src/video/test_decode.zig
- backends/bgfx/example/build.zig.zon
- backends/bgfx/example/main.zig
- backends/bgfx/libs/miniaudio/miniaudio.c
- backends/bgfx/example/gamepad_overlay.zig
- backends/bgfx/example/build.zig
- backends/bgfx/src/video/apk/AndroidManifest.xml
- backends/bgfx/src/stb_shim.h
- backends/bgfx/example/android_video.zig
- backends/bgfx/build.zig.zon
- backends/bgfx/src/gfx/astc.zig
- backends/bgfx/src/gfx/state.zig
- backends/bgfx/src/video/android.zig
- backends/bgfx/src/video/apk/decode_shim.h
- backends/bgfx/src/platform.zig
- backends/bgfx/backend.manifest.zon
- backends/bgfx/example/apk_video/AndroidManifest.xml
- backends/bgfx/src/audio_device_android.zig
- backends/bgfx/src/video/player.zig
- backends/bgfx/src/video/android_audio.zig
- backends/bgfx/build_fragments/backend_dep.txt
- backends/bgfx/src/shaders.zig
- backends/bgfx/src/gfx/draw.zig
- backends/bgfx/src/video/planes.zig
- backends/bgfx/src/gfx/font.zig
- backends/bgfx/src/video/yuv.zig
- backends/bgfx/src/video/desktop.zig
- backends/bgfx/src/video/fit.zig
- backends/bgfx/src/video/apk/native.zig
- backends/bgfx/templates/desktop.txt
- backends/bgfx/src/gfx/programs.zig
- backends/bgfx/src/stb_image_impl.c
- backends/bgfx/templates/android.txt
- backends/bgfx/src/android.zig
- backends/bgfx/src/video/backend.zig
- backends/bgfx/src/input.zig
- backends/bgfx/src/android_app.zig
- backends/bgfx/build.zig
- backends/bgfx/src/audio.zig
- backends/bgfx/src/window.zig
- backends/bgfx/src/gfx/texture.zig
- backends/bgfx/src/audio_device.zig
Comment on lines
388
to
+391
| if (std.mem.eql(u8, entry.name, ".zig-cache") or | ||
| std.mem.eql(u8, entry.name, "zig-out") or | ||
| std.mem.eql(u8, entry.name, "zig-pkg") or | ||
| std.mem.eql(u8, entry.name, ".labelle") or |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve declared zig-pkg/* path deps when staging.
This now drops every zig-pkg/ subtree, but staged packages still need whatever their build.zig.zon points at. If a local package vendors a dep under zig-pkg/*, the staged manifest will reference content that no longer exists, and rewriteZonPaths cannot repair that because it only rewrites paths. We already use that layout in CI for labelle-cli/zig-pkg/labelle-gui, so the new assumption that zig-pkg/ is always disposable is too strong. Please make this skip manifest-aware, or copy only the referenced zig-pkg entries instead of dropping the whole directory.
🤖 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/deps_linker.zig` around lines 388 - 391, The staging logic in
deps_linker.zig is treating every zig-pkg subtree as disposable, which breaks
declared path dependencies referenced by build.zig.zon. Update the
directory-skip/copy behavior in the staging flow so it is manifest-aware:
inspect the package manifest and preserve or selectively copy any zig-pkg
entries that are referenced, rather than excluding the entire zig-pkg tree. Keep
the fix localized to the entry filtering/rewrite path around the directory walk
and rewriteZonPaths handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Local-package staging crashed on real repo checkouts
hardlinkTree(the local-package stager) skipped.zig-cache/zig-out/.gitbut copiedzig-pkg/and.labelle/. Staging a local backend package that's an actual repo checkout then recursed into:zig-pkg/<hash>/upstream/…/node_modules/…— deep enough thatcopyFilehitNameTooLong, which crashed staging (and tripped an invalid free unwinding the error)..labelle/— if a backend repo ships an example inside itself (labelle-bgfx/examples/*), staging the package while generating that example copied the example's own generated output back into the stage.Neither is part of a package's importable modules. Skip both alongside the existing build/VCS dirs.
Why
Unblocks bgfx's examples living in labelle-bgfx (per the extraction) and resolving the backend via the local checkout —
labelle generateforlabelle-bgfx/examples/{bgfx,bgfx-android}now succeeds (was: assembler panic).Adds a
hardlinkTreeregression test (zig-pkg/.labelle/.git/.zig-cache/zig-out skipped; real files staged).zig build testgreen.Summary by CodeRabbit
Chores
Refactor
Tests