feat!: targets are names; drop the CLI's Platform and AssetCompression mirrors (cli#471 P3) - #519
Conversation
…s (cli#471 I5) The ios target is owned by labelle-toolkit/labelle-ios (v0.1.0, in the provider registry): app bundling after build, simulator launch as the run replacement, and the bundle replacement. The core keeps nothing iOS-specific: - src/cli/ios.zig is deleted, along with the `labelle ios` command, its parsing and help, and Command.ios_cmd. - Deleted: the legacy own-build branch, the forced sokol backend (no silent substitution, D5), the ios Bypass arms, legacyRunBranch, and the simulator launch branch. - `ios` is no longer a reserved provider namespace. - IosConfig/Orientation are gone. project.labelle is read with ignore_unknown_fields, so an old `.ios` block still parses and is ignored. `labelle ios ...` now gives the unknown-command diagnostic with nothing generated. `run --platform=ios` without an owning provider gives the no-provider diagnostic, or NoRunReplacement when the provider has no `replace run`. The e2e scripts and docs are updated. The agnostic guard drops five entries. BREAKING CHANGE: `labelle ios` is removed; use labelle-ios with `labelle build|run|bundle --platform=ios`. Targets for ios generate into <project backend>_ios, since the backend is no longer forced.
…n mirrors (cli#471 P3) The CLI no longer mirrors the project schema's platform enum or its asset-compression table. A target is a name end to end: `desktop` is core, every other name is declared by a pinned provider, and the assembler says what it can do with it. - project_config: `Platform`, `AssetCompression` (+ `formatFor`) and the `platform`/`asset_compression` fields are gone. `.platform` is read as a name (`declaredTarget`, any enum literal) into `declared_target`, which `config.readProjectConfig` fills after the tolerant parse; it is still the default target when no `--platform` is given. `.asset_compression` is skipped by `ignore_unknown_fields`; the assembler reads both. - provider_targets: `Provisional.legacy` / `Resolved.legacy` are gone. - The labelle-assembler#378 schema-target gate is replaced by the assembler's own verdict: unless a provider replaces `generate`, the pipeline (and a watched rebuild) asks `describe --target <t>` after the install and refuses `supported: false` with describe's reason. An unknown target reads "backend 'X' cannot build target 'T': backend 'X' has no target 'T': this assembler generates for ...", plus a line telling a provider target's owner it declares no `replace generate`. - The ASTC prepass keys on `describe`'s `asset_format` for the resolved target name (passed to `labelle astc --platform <t>`), asked on every generation so a watched rebuild follows an edited `.asset_compression`. - `parsed.platform` is gone from the pipeline; args_resolve no longer derives it, and the session key, provider command context and `labelle astc` default read `declared_target`. - Agnosticism guard: cli.zig, cli/pipeline.zig, cli/pipeline/build.zig and cli/project_config.zig came clean; 2 entries left. - docs/provider-targets.md: "What the assembler generates" replaces the #378 boundary section; the targets e2e fake assembler answers like the real `describe` (unsupported with a reason for unknown names, `asset_format` from `.asset_compression`). BREAKING CHANGE: the CLI no longer knows the schema platforms. A provider target whose provider does not replace `generate` is refused with the assembler's `describe` reason instead of the #378 message, and whether the ASTC prepass runs comes from the assembler's `asset_format` for the target (so it needs an assembler that answers `describe`, protocol >= 7, which CLI 4.0 already requires). `.platform` accepts any target name as an enum literal; a non-enum value is a parse error.
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 104 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (32)
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: 55c9d228d4
ℹ️ 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".
| const described = opts.describer.query(scratch.allocator(), opts.target); | ||
| if (described != null and std.mem.eql(u8, described.?.asset_format, "astc")) { |
There was a problem hiding this comment.
If this additional describe call fails during a watched rebuild or a concurrent project-file update, query returns null and the code silently skips cmdAstc while continuing to generate. For a target configured for ASTC, an existing stale .astc sibling can then be selected by the assembler without being refreshed or removed, producing a successful build with outdated assets. Treat a missing description as a rebuild/build failure rather than as PNG.
Useful? React with 👍 / 👎.
docs/migrating-to-4.0.md covers what a project needs for CLI 4.0: - assembler 0.118.0+ (protocol 7); - labelle-ios for the ios target; - the sdl2 provider (or LABELLE_SDL2_LIB, or .gamepad = .none) for SDL2 on Windows; - smaller changes: the describe-named target dir, unknown targets, astc --backend, the .platform enum literal, doctor --fix, labelle test, and --docker on a Mac. `labelle help` points at it.
…kipping it A null describe answer used to read as PNG, so a target configured for ASTC could generate against a stale .astc sibling without refreshing it. The prepass now requires the answer: the cold pipeline stops with a clear message, and a watched rebuild fails only itself (the session lives on).
…drop-platform-enum
…drop-platform-enum
2 participants
Part of #471, item P3. Breaking: for CLI 4.0. Stacked on #518 (S4), which sits on #517 (D4). It also merges #515 (I5), so the 4.0 stack carries every removal. This PR is the tip of the 4.0 train.
P3
project_config.Platform,AssetCompression/formatFor, andprovider_targetstarget.legacy..platforminproject.labellestill picks the default target, now read as a plain name (declared_target)..asset_compressionstill parses. The assembler keys it by target, and the CLI skips it.asset_formatfor the resolved target name. It is asked on every generation, so watched rebuilds pick up edits. This also closes D3's asset-format follow-up.parsed.platformchecks in build/run/docker. The pipeline, the session key, provider commands andlabelle astcuse the target name.supported: falsewith a reason, and the CLI stops with, for example:labelle: backend 'bgfx' cannot build target 'probe-target': backend 'bgfx' has no target 'probe-target': this assembler generates for desktop ios android wasm. For a provider target it adds that the package declares the target but has noreplacehook ongenerate.Merge of I5
cli/ios.zigandcli/sdl_provision.zigare both deleted. The code path stays, and a comment says so.Agnostic guard: 2 entries left, the guard file itself and
cli/doctor.zig(its SDL2 advice line).Tests
zig build testpasses 13/13 steps (1154 tests).provider_hooks/provider_targetsskip their macOS--dockerblock, which X3 refuses on a Mac host; CI runs them on Linux.Open points
docs/migrating-to-4.0.md, which goes with the release..platformgiven as a string rather than an enum literal is now a CLI parse error.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.