Repository navigation
feat!: SDL2 provisioning leaves the core for the sdl2 provider (cli#471 S4) - #518
Conversation
…71 S4) labelle-sdl now ships the opt-in `sdl2` CLI provider: an env hook provisions SDL2 on Windows and writes LABELLE_SDL2_LIB, a stage hook copies the DLLs, and `labelle sdl2 doctor|install`. The core keeps nothing SDL-specific. - Deleted: src/cli/sdl_provision.zig, wants_sdl2 and the SDL env auto-wiring (preInstall no longer takes a backend), SDL2.dll staging, the doctor's SDL rows and its `--fix` provisioning, and LABELLE_SDL2_LIB in the unreserved-env list. - `labelle doctor` prints one advice line (never a failure) when the build links SDL2 and .plugins has no `sdl2`. `--fix` now points at `labelle <ns> doctor --fix`. Forwarding --fix to provider doctors (D10) is not wired yet, and doctor.zig says so. The agnostic guard drops cli/sdl_provision.zig. BREAKING CHANGE: the CLI no longer downloads SDL2 or stages SDL2.dll. Add the `sdl2` provider (labelle-sdl) to .plugins, set LABELLE_SDL2_LIB, or use `.gamepad = .none`.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: f2cc02265c
ℹ️ 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".
| //! The build and bundle stages: the `build` hook phases around the core | ||
| //! build (docker or host `zig build`, SDL2 DLL staging and the `labelle | ||
| //! build` packaging finalisation), and the `bundle` step. | ||
| //! build (docker or host `zig build` and the `labelle build` packaging |
There was a problem hiding this comment.
For Windows SDL2 builds, this change removes the core's runtime DLL staging, but docs/provider-contract-v1.md under Environment contributions → Scope still promises that the core stages SDL2.dll beside the executable using the contributed LABELLE_SDL2_LIB. Provider authors following that contract may omit the new stage hook and produce an executable that fails to start, so the normative contract must be updated to make provider-owned staging explicit.
Useful? React with 👍 / 👎.
| /// Advice, never a failure: SDL2 may be installed by other means. | ||
| fn sdl2Hint(cfg: Cfg, backend: ?[]const u8) ?[]const u8 { | ||
| if (!cfg.found or cfg.sdl2_provider or !linksSdl2(backend, cfg.gamepad_off)) return null; | ||
| return "SDL2: this build links SDL2, which the CLI no longer provisions: add the `sdl2` provider (labelle-sdl) to .plugins, set LABELLE_SDL2_LIB, or use `.gamepad = .none`."; |
There was a problem hiding this comment.
When the resolved backend is sdl, this hint still recommends .gamepad = .none as an alternative to installing SDL2, even though linksSdl2("sdl", true) correctly remains true because the renderer itself always links SDL2. A user following that advice will still get the same missing-library failure; the gamepad alternative should only be included for the raylib/sokol/bgfx gamepad-only cases.
Useful? React with 👍 / 👎.
…ce for sdl - docs/provider-contract-v1.md no longer promises that the core stages SDL2.dll from the contributed LABELLE_SDL2_LIB. The core only merges the env contribution, and staging a runtime library is a provider's after build hook (the sdl2 provider's stage hook, for example). - The doctor's SDL2 advice doesn't offer `.gamepad = .none` for the sdl backend, whose renderer always links SDL2. It still does for raylib/sokol/bgfx and for an unknown backend.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fba736b696
ℹ️ 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".
| .renderer => "SDL2: this build links SDL2, which the CLI no longer provisions: add the `sdl2` provider (labelle-sdl) to .plugins, or set LABELLE_SDL2_LIB.", | ||
| .gamepad => "SDL2: this build links SDL2, which the CLI no longer provisions: add the `sdl2` provider (labelle-sdl) to .plugins, set LABELLE_SDL2_LIB, or use `.gamepad = .none`.", |
There was a problem hiding this comment.
On Windows, setting LABELLE_SDL2_LIB alone only lets the build locate SDL2; the Windows loader does not inspect that variable, and this commit removes the core step that copied SDL2.dll beside the executable. A user following either hint without adding the provider can therefore compile successfully but still fail to run with a missing DLL. Recommend the provider, or explicitly require staging the DLL or adding its directory to PATH.
Useful? React with 👍 / 👎.
The SDL2 advice now says that with LABELLE_SDL2_LIB, SDL2.dll must also be on PATH or beside the exe on Windows. The loader never reads that variable, and the core no longer copies the DLL. The sdl2 provider stays the first recommendation.
2 participants
Part of #471, item S4. Breaking: for CLI 4.0. Stacked on #517 (D4). Retarget to
mainonce #517 lands.labelle-sdl now ships the opt-in
sdl2CLI provider (labelle-sdl#10, release v0.4.0):envhook that provisions SDL2 on Windows and writesLABELLE_SDL2_LIB;stagehook that copiesSDL2.dll/SDL2_mixer.dll;labelle sdl2 doctor|install.The core keeps nothing SDL-specific.
Removed
src/cli/sdl_provision.zig.wants_sdl2and the SDL env auto-wiring.preInstallno longer takes a backend.SDL2.dllstaging, the doctor's lib/dll/headers/mixer rows, and its--fixprovisioning.LABELLE_SDL2_LIBin the unreserved-env list, plus the SDL wording in help.Doctor now
.pluginshas nosdl2: "add thesdl2provider (labelle-sdl) to .plugins, set LABELLE_SDL2_LIB, or use.gamepad = .none".--fixsays the core has nothing to fix and points atlabelle <ns> doctor --fix.--fixto provider doctors (D10) is not wired yet (provider_doctor.runForRoottakes no fix flag). That is out of scope here and noted indoctor.zig.The guard goes 14 → 13 entries. The RFC's −6 doesn't come in one step: the other files still name iOS, wasm or backends, and I5/P3 remove those.
Tests
zig build testpasses 13/13 steps (1168 tests).test/provider_doctor_e2e.pyis updated to the new all-clear line. Locally it stops earlier, at adescribestep that fails the same way on the base branch.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.