diff --git a/.github/actions/rust-setup/action.yml b/.github/actions/rust-setup/action.yml index 1ba1509b..275dd901 100644 --- a/.github/actions/rust-setup/action.yml +++ b/.github/actions/rust-setup/action.yml @@ -7,8 +7,12 @@ description: >- inputs: toolchain: - description: "Rust toolchain (e.g. stable, 1.86.0)." - default: stable + description: >- + Rust toolchain to install. Leave empty (the default) to use the channel + from `rust-toolchain.toml`, which is the single source of truth for the + project toolchain — set it there, not here. Only pass a value to install + something deliberately different from the project pin. + default: "" targets: description: "Comma-separated extra rustup targets (e.g. wasm32-unknown-unknown)." default: "" @@ -37,6 +41,64 @@ inputs: runs: using: composite steps: + # Resolve the toolchain from `rust-toolchain.toml` unless the caller asked + # for a specific one. This keeps ONE version literal in the repo: bumping + # the project toolchain is a single edit to `rust-toolchain.toml` and every + # workflow follows, with no chance of a call site drifting behind. + # + # This used to default to `stable`, which was misleading rather than wrong: + # `rust-toolchain.toml` is a directory override and outranks the `rustup + # default` this action performs, so every job was ALREADY compiling on the + # pinned channel (rustup logs "toolchain '' is currently in use + # (overridden by .../rust-toolchain.toml)"). The only effect of the + # `stable` default was to download a second toolchain that nothing then + # used — and to make the workflows read as though they tested on latest + # stable, which they never did. + # + # Extraction is scoped to the `[toolchain]` TABLE, not "the first `channel` + # key in the file". A bare `channel = ...` match would take the wrong value + # if any other table ever gained a `channel` key ahead of `[toolchain]` — + # silently installing whatever that said, which is exactly the kind of + # guess the fail-closed design exists to prevent. + # + # Only TOML basic (double-quoted) strings are accepted. A literal + # single-quoted channel is valid TOML but is not used here, and failing + # loudly on one beats guessing at its value. + # + # Fail-closed: an unparseable `rust-toolchain.toml` aborts the job rather + # than silently falling back to some other channel. + - name: Resolve Rust toolchain + id: resolve + shell: bash + env: + REQUESTED: ${{ inputs.toolchain }} + run: | + set -euo pipefail + if [ -n "$REQUESTED" ]; then + resolved="$REQUESTED" + echo "Toolchain requested explicitly by the caller: ${resolved}" + else + resolved="$(awk ' + /^[[:space:]]*\[/ { + in_toolchain = /^[[:space:]]*\[toolchain\][[:space:]]*$/ + next + } + in_toolchain && match($0, /^[[:space:]]*channel[[:space:]]*=[[:space:]]*"[^"]*"/) { + v = substr($0, RSTART, RLENGTH) + sub(/^[^"]*"/, "", v) + sub(/"$/, "", v) + print v + exit + } + ' rust-toolchain.toml)" + if [ -z "$resolved" ]; then + echo "::error::Could not parse [toolchain].channel (as a double-quoted string) from rust-toolchain.toml" + exit 1 + fi + echo "Toolchain resolved from rust-toolchain.toml: ${resolved}" + fi + echo "toolchain=${resolved}" >> "$GITHUB_OUTPUT" + # Pinned to a commit SHA, not `@master`. Every other action in this repo is # referenced by a `@vN` tag, which is not expected to move; `@master` is a # BRANCH that advances on every upstream commit, so each run silently @@ -55,7 +117,7 @@ runs: - name: Install Rust toolchain uses: dtolnay/rust-toolchain@e97e2d8cc328f1b50210efc529dca0028893a2d9 # v1 with: - toolchain: ${{ inputs.toolchain }} + toolchain: ${{ steps.resolve.outputs.toolchain }} targets: ${{ inputs.targets }} components: ${{ inputs.components }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65ef911e..87d099a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -182,7 +182,6 @@ jobs: persist-credentials: false - uses: ./.github/actions/rust-setup with: - toolchain: 1.96.0 components: rustfmt,clippy - name: rustfmt run: cargo fmt --all -- --check @@ -367,16 +366,11 @@ jobs: - uses: actions/checkout@v7 with: persist-credentials: false - # Pinned to the project toolchain like the `lint` job, rather than the - # action's `stable` default. Not a correctness fix — `rust-toolchain.toml` - # already wins over the action's `rustup default stable` (rustup logs - # "toolchain '1.96.0' is currently in use (overridden by - # .../rust-toolchain.toml)"), so this job was always building on 1.96.0. - # It avoids installing a `stable` toolchain that nothing then uses, and - # makes the toolchain this job rehearses explicit at the call site. + # No `toolchain:` input anywhere in this repo: `rust-setup` resolves it + # from `rust-toolchain.toml`, so this job rehearses the buildbot on the + # exact channel the buildbot uses. - uses: ./.github/actions/rust-setup with: - toolchain: 1.96.0 apt: "false" cache-key: libretro-${{ matrix.target }} # Point bindgen at the NDK sysroot, which is what `cargo ndk` does for us diff --git a/.github/workflows/pgo.yml b/.github/workflows/pgo.yml index 13e4da52..807c9ad8 100644 --- a/.github/workflows/pgo.yml +++ b/.github/workflows/pgo.yml @@ -83,7 +83,6 @@ jobs: # full `rustynes-frontend` binary). - uses: ./.github/actions/rust-setup with: - toolchain: 1.96.0 components: llvm-tools-preview cache-key: pgo @@ -199,7 +198,6 @@ jobs: persist-credentials: false - uses: ./.github/actions/rust-setup with: - toolchain: 1.96.0 components: llvm-tools-preview cache-key: pgo-bolt - name: Install cargo-pgo diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index a152767b..6ddf4cad 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -89,7 +89,6 @@ jobs: persist-credentials: false - uses: ./.github/actions/rust-setup with: - toolchain: 1.96.0 components: clippy cache-key: security-clippy # The main CI `lint` job already runs `clippy -D warnings` on the DEFAULT diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index eec14fb1..94931223 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -191,55 +191,62 @@ libretro-build-ios-arm64: # tvOS # -# The tvOS template overrides `script` with -# `cargo +nightly build -Zbuild-std --target aarch64-apple-tvos --release` -# (aarch64-apple-tvos needs build-std). That explicit `+nightly` outranks -# BOTH `rust-toolchain.toml` and `RUSTUP_TOOLCHAIN`, so this is the one job -# that does not run on our pinned 1.96.0 — and the image's pre-installed -# nightly is 1.94.0-nightly, which trips cargo's MSRV gate against the -# workspace's `rust-version = "1.96"` before compilation even starts: +# Unlike the other three Apple jobs, the upstream tvOS template also overrides +# `script`, replacing the shared Apple build line with # -# error: rustc 1.94.0-nightly is not supported by the following packages: -# rustynes-core@2.2.1 requires rustc 1.96 (+ apu/cpu/libretro/mappers/ppu) +# cargo +nightly build -Zbuild-std --target aarch64-apple-tvos --release # -# Refreshing the nightly channel is the surgical fix. Lowering `rust-version` -# on those six crates would also silence it, but `.cargo/config.toml` sets -# `[resolver] incompatible-rust-versions = "fallback"`, which keys off the -# LOWEST workspace-member MSRV — so that route risks silently downgrading -# dependencies workspace-wide on the next `cargo update`. Not worth it for -# one job. +# That dates from when `aarch64-apple-tvos` was a tier-3 target with no +# distributed `rust-std`, making `-Zbuild-std` — and therefore nightly — the +# only way to build it. The target has since been promoted and rustup now +# ships a complete prebuilt std for it, `panic_abort` included, so the +# override is obsolete. Verified against the pinned 1.96.0: +# `rustup target add aarch64-apple-tvos` installs 26 rlibs (`panic_abort` +# among them) and `cargo check --release -p rustynes-libretro --target +# aarch64-apple-tvos` compiles the entire graph, bindgen included. # -# Uninstall-then-install rather than `rustup update nightly`: a stale extra -# component carried by the image's nightly may not be published for the new -# nightly date, which aborts an in-place update. `rust-src` is required by -# `-Zbuild-std`. The container is ephemeral, so this is job-local. +# Restoring the shared script puts tvOS on the same pinned 1.96.0 as every +# other job, and dissolves three separate workarounds this job used to need +# rather than accumulating a fourth: # -# Refreshing the nightly then exposes a SECOND tvOS-only blocker. A bare -# `-Zbuild-std` builds `std` but NOT `panic_abort`, and this workspace sets -# `[profile.release] panic = "abort"`, so linking the cdylib fails with -# `E0463: can't find crate for panic_abort`. The usual fix is -# `-Zbuild-std=std,panic_abort` — but the flag is hardcoded in the template's -# `script`, and `CARGO_UNSTABLE_BUILD_STD` does NOT help (verified: the CLI -# `-Z` flag wins over the config/env form, so the crate list stays the -# default). Overriding the profile's panic strategy for this job is the -# remaining lever, and it uses the same `CARGO_PROFILE_RELEASE_*` mechanism -# the template itself already uses for `CARGO_PROFILE_RELEASE_DEBUG`. +# 1. MSRV. `+nightly` outranks BOTH `rust-toolchain.toml` and +# `RUSTUP_TOOLCHAIN`, pinning the job to the image's stale +# 1.94.0-nightly — below the workspace's `rust-version = "1.96"`, so +# cargo's MSRV gate failed before compiling anything. The job had to +# uninstall and reinstall the nightly channel to get past it. +# 2. `panic_abort`. A bare `-Zbuild-std` builds `std` but NOT +# `panic_abort`, which `[profile.release] panic = "abort"` requires, so +# the cdylib failed to link with `E0463`. The crate list is hardcoded in +# the template and `CARGO_UNSTABLE_BUILD_STD` cannot override it (the +# CLI `-Z` flag wins), so the job had to force `panic = "unwind"` via +# `CARGO_PROFILE_RELEASE_PANIC`. The prebuilt std ships `panic_abort`, +# so the profile is now honoured as written and tvOS matches every +# other platform. +# 3. `-C ar`. Refreshing to a current nightly then failed with +# `error: `-C ar`: this option has been removed`. The build image +# injects `-Car=,Clink-arg=...` into every Apple job, and that +# long-deprecated no-op became a hard error in Rust 1.97 (bisected: +# 1.96.1 warns, 1.97.1 errors). On the pinned 1.96.0 it is still only a +# warning — exactly as it already was for the three Apple jobs that +# were green throughout. # -# `unwind` is safe here: since Rust 1.81 a panic that reaches an `extern "C"` -# boundary aborts rather than unwinding into C frames, so a libretro core -# behaves the same on a panic either way — this costs only slightly larger -# code and unwind tables, on tvOS alone. Every other platform keeps -# `panic = "abort"` untouched. +# `!reference [.libretro-rust-apple-base, script]` pulls in the generic Apple +# build/package steps instead of duplicating them here, so upstream fixes to +# that script keep reaching this job. It drives off `TARGET_ARCH`, which the +# tvOS template already sets to `aarch64-apple-tvos`, and honours the same +# `STRIP_CORE_LIB` / `DSYM_CORE_LIB` flags the template sets. # -# GitLab concatenates `before_script` and `script` into ONE shell invocation -# (that is how the template's own `export SDKROOT=...` reaches its cargo -# call), so exporting here does reach the build. +# NOTE: `-C ar` is still a latent hazard for ALL FOUR Apple jobs, now +# including this one. Read `rust-toolchain.toml` before bumping `channel` to +# 1.97 or newer. libretro-build-tvos-arm64: extends: - .libretro-rust-tvos-arm64-default - .core-defs + variables: + RUST_TARGET: aarch64-apple-tvos before_script: - !reference [.libretro-rust-tvos-arm64-default, before_script] - - rustup toolchain uninstall nightly || true - - rustup toolchain install nightly --profile minimal --component rust-src - - export CARGO_PROFILE_RELEASE_PANIC=unwind + - rustup target add ${RUST_TARGET} + script: + - !reference [.libretro-rust-apple-base, script] diff --git a/AGENTS.md b/AGENTS.md index f63682df..c462d016 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -190,10 +190,14 @@ These cross-cutting decisions span multiple files. Reading individual chip docs - **Symlinked Agent Configs:** Ensure symlinked agent files (like `GEMINI.md` -> `AGENTS.md`) are explicitly removed from `.gitignore` so they are correctly tracked by version control. - **Libretro buildbot CI (issue #311) shipped in PR #312 (2026-07-19/20, `b49dd1e0`) — issue #311 stays OPEN by design.** `.gitlab-ci.yml` + a `[lib] name = "rustynes"` naming-collision fix + `[workspace] default-members` + RA memory-maps + an FDS load-path fix/disk-control + native Game Genie cheats all shipped. TAS and Netplay needed no new libretro-side wiring — RetroArch's own rollback/movie machinery already rides the existing `on_serialize`/`on_unserialize` hooks. Companion upstream PRs `libretro/docs#1164` and `libretro/libretro-super#2021` are open but not yet merged by the libretro team; the remaining step (mirroring the repo onto `git.libretro.com` + enabling the buildbot) is entirely on their side. **Never close #311** with a "Closes #311" commit-body keyword — it already auto-closed prematurely once this way and had to be reopened; only close it once the upstream team confirms the buildbot is live. - **GENERAL RULE (learned twice, the hard way): never put a GitHub magic closing keyword — `Closes #N` / `Fixes #N` / `Resolves #N` — in a commit or PR body when the issue tracks ANY work the change does not finish.** GitHub closes the issue the instant the PR merges, marks it `stateReason: COMPLETED`, and the tracker then reports finished work that was never done. This has now fired twice: #311 (PR #312), and again on #318, which auto-closed one second after PR #319 merged (`02:42:15Z` merge → `02:42:16Z` close) even though #319's own body and review replies explicitly deferred two of its items. Write "addresses #N" / "partially addresses #N" in prose instead, and close the issue by hand only once its last item actually lands. When an issue *looks* completed, verify against the tree (`git log`, grep the code) rather than trusting the CLOSED/COMPLETED state — a keyword-closed issue is indistinguishable from a genuinely finished one in the UI. + - **It fired a THIRD time, in the subtlest way: even QUOTING the keyword triggers it.** PR #321's commit body carefully said "Addresses #318" — but it also *explained the earlier mistake* with the sentence "#318 had auto-closed ... purely from a `` `Closes #318.` `` keyword in that commit body". GitHub's parser ignores backticks, quotation, and surrounding context: it saw `Closes #318` and closed the issue on merge. **Writing about the anti-pattern reproduced the anti-pattern.** So the check is purely lexical — before pushing, grep the whole message for `Closes|Fixes|Resolves` followed by `#`, including inside code spans and narrative, and rewrite any such quotation (e.g. "a closing keyword" / `Close​s #NNN`) if the issue must stay open. - **CI security hardening (PRs #319 + #320, 2026-07-21, merged `85ee20db` / `a69200ef`).** `persist-credentials: false` on **all 19** `actions/checkout` sites (18 in #319; the last one, `release-auto.yml`, once its tag check stopped needing Git credentials — see the next bullet), because build scripts / proc macros / test binaries / Gradle scripts / MkDocs all execute unreviewed PR code that could read the token out of `.git/config`. Facts worth not re-deriving: `.github/actions/rust-setup` performs **no checkout of its own** (so call-site hardening is complete coverage); `persist-credentials` does **not** affect the `gh` CLI or API calls, only git network ops using the stored credential — which is why `gh release create`, `softprops/action-gh-release`, and `fastlane match` (a *different* repo, own `MATCH_GIT_*` secrets) all look like they need it and don't; and the highest-exposure job is **`web.yml`'s `build`**, not any `ci.yml` job, because `web.yml` declares `pages: write` + `id-token: write` at *workflow* level. There are now **no exceptions**: `release-auto.yml`'s `prepare` was the last holdout (it needed `git ls-remote origin` for the tag check), and that check is now a `gh api` call, so its checkout joined the sweep. - **The release tag-existence check is FAIL-CLOSED — keep it that way.** `release-auto.yml`'s `decide` step queries `gh api repos/$GITHUB_REPOSITORY/git/matching-refs/tags/`, NOT `git/ref/tags/`: `matching-refs` answers "absent" with HTTP 200 + an empty array, so a genuine miss can never be confused with a lookup failure and no error-body parsing is needed. It matches by **prefix**, so the exact ref is compared in `jq` — this is load-bearing, not defensive: `v2.2` prefix-matches two real tags (`v2.2.0`, `v2.2.1`) while exact-matching none. Under `shell: bash` + `set -euo pipefail` both a `gh` failure and a non-array body abort the job (verified: exit 1 and exit 5 respectively). The old `git ls-remote ... >/dev/null 2>&1` read *any* non-zero exit as "tag absent", so a blip would try to re-release a shipped version. **Never reintroduce a form where a failed lookup is indistinguishable from "absent."** (Note when testing shell behavior locally: this harness's shell is zsh, whose `set -e` semantics for `var="$(cmd)"` differ from bash's — test with `bash -c` or you will get a false result.) +- **`dtolnay/rust-toolchain` is SHA-pinned in `.github/actions/rust-setup` — never put it back on a branch ref.** It is `@e97e2d8cc328f1b50210efc529dca0028893a2d9 # v1`; it used to be `@master`, a branch that advances on every upstream commit, so each run silently resolved to whatever HEAD was. That composite feeds 12 of the repo's 19 checkouts, including `release.yml` (`contents: write`, builds the shipped binaries) and `web.yml` (`pages: write` + `id-token: write`), and it is the action that *installs the compiler* — the hardest place to notice a compromise. The trailing `# v1` is the form Dependabot's `github-actions` ecosystem (enabled in `.github/dependabot.yml`) reads to keep the pin current, so don't strip it. The other ~12 actions remain on `@vN` tags by choice: those are not expected to move, and blanket SHA-pinning is a maintenance-policy decision, not a drive-by fix. - **`pre-commit run --all-files` REWRITES vendored/immutable trees — use `--files ` or a single named hook instead.** `trailing-whitespace` / `end-of-file-fixer` / `mixed-line-ending` *modify* files and, before PR #320, had no `exclude` at all: one `--all-files` run silently reformatted **41 files** across the vendored TriCNES C#, vendored rcheevos C, `ref-docs/`, an upstream font licence, and upstream test-ROM READMEs — destroying exactly the byte-identical-to-upstream property those trees exist for. `.markdownlintignore` covered them for markdownlint only. #320 added a shared `exclude` anchor across the three rewriting hooks, scoped deliberately **narrower** than `.markdownlintignore`: only content we did not author. Frozen-but-ours trees (`docs/archive/`, `to-dos/plans/`, `docs/monetization/`) stay in scope, since the invariant is "don't rewrite what we didn't write". If it happens anyway, revert **only** the unintended paths (never a blanket `git checkout`, and never including your own edits). -- **The libretro buildbot is a THIRD CI system with its own rules — and the pinned toolchain fights it.** The first real run (pipeline #91899, 2026-07-20) passed 1 of 10 jobs. `rust-toolchain.toml`'s `channel = "1.96.0"` makes rustup install a *fresh* toolchain inside libretro's build image, bypassing the image's pre-provisioned cross targets, so 8 jobs died on `E0463: can't find crate for core`; each job in `.gitlab-ci.yml` now runs `rustup target add ${RUST_TARGET}` (NOT added to `rust-toolchain.toml`'s `targets` — that would cost every contributor and GH Actions job ~8 extra `rust-std` downloads). The Apple jobs must use `!reference` rather than `extends` for that, because GitLab's `extends` REPLACES array keys and would silently drop the templates' `SDKROOT`/`STRIP`/`CC`/`CXX` exports. tvOS is special twice over: its template hardcodes `cargo +nightly build -Zbuild-std`, which bypasses our channel pin onto the image's stale nightly (below our MSRV) *and* omits `panic_abort` that `[profile.release] panic = "abort"` needs — handled by refreshing nightly + `CARGO_PROFILE_RELEASE_PANIC=unwind` in that job (`CARGO_UNSTABLE_BUILD_STD` does NOT work: the CLI `-Z` flag wins). **A green GitHub Actions run does not imply a green buildbot** — the new `libretro-cross` CI job (one triple for each buildbot ABI family a Linux runner can model — MinGW-Windows and Android/NDK; the Apple families are deliberately excluded, as bindgen needs a real per-target sysroot and there is no Apple SDK on a Linux runner) is the early-warning gate; before touching anything libretro-related, cross-check `cargo check --release -p rustynes-libretro --target ` locally. +- **The libretro buildbot is a THIRD CI system with its own rules — and the pinned toolchain fights it.** The first real run (pipeline #91899, 2026-07-20) passed 1 of 10 jobs. `rust-toolchain.toml`'s `channel = "1.96.0"` makes rustup install a *fresh* toolchain inside libretro's build image, bypassing the image's pre-provisioned cross targets, so 8 jobs died on `E0463: can't find crate for core`; each job in `.gitlab-ci.yml` now runs `rustup target add ${RUST_TARGET}` (NOT added to `rust-toolchain.toml`'s `targets` — that would cost every contributor and GH Actions job ~8 extra `rust-std` downloads). The Apple jobs must use `!reference` rather than `extends` for that, because GitLab's `extends` REPLACES array keys and would silently drop the templates' `SDKROOT`/`STRIP`/`CC`/`CXX` exports. **tvOS: the upstream template's `cargo +nightly build -Zbuild-std` override is OBSOLETE — don't reinstate it.** It dates from when `aarch64-apple-tvos` was tier 3 with no distributed `rust-std`; the target has since been promoted and rustup ships a complete prebuilt std **including `panic_abort`** (verified on the pinned 1.96.0: `rustup target add aarch64-apple-tvos` gives 26 rlibs and the crate `cargo check`s clean, bindgen included). Our job overrides `script` back to `!reference [.libretro-rust-apple-base, script]`, putting tvOS on the same pinned stable as every other job. That one change dissolved THREE stacked workarounds the `+nightly` path had forced: a nightly-channel reinstall (`+nightly` outranks both `rust-toolchain.toml` and `RUSTUP_TOOLCHAIN`, so the job rode the image's stale 1.94.0-nightly, below our MSRV); `CARGO_PROFILE_RELEASE_PANIC=unwind` (bare `-Zbuild-std` omits `panic_abort`, and `CARGO_UNSTABLE_BUILD_STD` does NOT override the hardcoded crate list — the CLI `-Z` flag wins); and clearing the image's `-C ar` (see the next bullet). Worth reporting upstream: every Rust core's tvOS job could drop `+nightly` the same way. **A green GitHub Actions run does not imply a green buildbot** — the new `libretro-cross` CI job (one triple for each buildbot ABI family a Linux runner can model — MinGW-Windows and Android/NDK; the Apple families are deliberately excluded, as bindgen needs a real per-target sysroot and there is no Apple SDK on a Linux runner) is the early-warning gate; before touching anything libretro-related, cross-check `cargo check --release -p rustynes-libretro --target ` locally. +- **The libretro build image injects `-C ar` into EVERY Apple job, and it is a hard error from Rust 1.97 — a bomb armed against the next MSRV bump.** The image (not the `rust-apple.yml` template, which sets no `RUSTFLAGS` at all, and not our `.cargo/config.toml`) adds `-Car=,Clink-arg=-undefined,Clink-arg=dynamic_lookup,-rpath=` to osx-x64 / osx-arm64 / ios-arm64 / tvos-arm64. `-C ar` was a deprecated no-op for years and became a **hard error in 1.97** (bisected locally: 1.93.0-nightly / 1.96.0 / 1.96.1 warn; 1.97.1 and 1.99.0-nightly error). No job trips it today — all four Apple jobs are on the pinned 1.96.0 and merely log the warning. **The day `rust-toolchain.toml` moves to 1.97+, all four fail together** — the warning lives in that file, at the line someone would edit. Discarding the flags is behaviour-preserving, not a gamble: rustc splits `-C` at the FIRST `=`, so the whole comma-joined string is swallowed as the `ar` value and those link args have never reached the linker for *any* core (cargo prints it as one argv token), and two upstream Rust cores have green tvOS jobs on the same image with the same dead token. The override works without knowing where the image sets it because cargo takes rustflags from exactly one source, first match wins: `CARGO_ENCODED_RUSTFLAGS` → `RUSTFLAGS` → `target..rustflags` → `build.rustflags` (verified locally against a global `~/.cargo/config.toml` `build.rustflags`: `RUSTFLAGS=""` removes every injected `-C`, and empty means zero flags, not one empty argument). +- **There is ONE toolchain, `rust-toolchain.toml`'s `channel`, and no version literal anywhere in `.github/` — don't add one.** `.github/actions/rust-setup` parses the channel out of that file and fails closed if it can't, so a toolchain bump is a one-line edit there. Pass the composite's `toolchain:` input only to install something *deliberately* different from the project pin. The old `stable` default was misleading rather than wrong: `rust-toolchain.toml` is a directory override that outranks the `rustup default` the action performs, so every job was already compiling on 1.96.0 (rustup logs `overridden by .../rust-toolchain.toml`) — `stable` just downloaded a second toolchain nothing used and made the workflows *read* as though they tested latest stable, which they never did. **Nightly is used in exactly two places, neither a gate:** `cargo fuzz` (hard requirement — libFuzzer's sanitizer flags are nightly-only) and the dormant `rustynes-monetization` crate's standalone `uniffi-bindgen`. If you think a CI job needs nightly, it doesn't. - **`rust-libretro 0.3.2` is unmaintained (no commit since 2023-02) and has a MinGW bug we work around.** It casts a keycode with `cfg(target_family = "windows")`, but C enum signedness follows the *ABI*: only **MSVC** gives plain enums `int` — under **MinGW** (`x86_64-pc-windows-gnu`, what the buildbot builds) bindgen emits `c_uint` and the crate fails `E0308`. `.cargo/config.toml`'s `[env] BINDGEN_EXTRA_CLANG_ARGS_x86_64_pc_windows_gnu = "--target=x86_64-pc-windows-msvc"` fixes it; the generated-bindings diff is 28 lines, all enum signedness. Don't "clean up" that env var without rebuilding for `x86_64-pc-windows-gnu`. - **CodeRabbit is now a 3rd automated PR review bot** (`.coderabbit.yaml`, added 2026-07-20 in PR #316), alongside gemini-code-assist and copilot-pull-request-reviewer — same reply-and-resolve-every-thread ceremony applies before any merge. Configured `profile: assertive` (not the "chill" default) and a `tools{}`/`path_instructions`/custom-checks set audited against this repo's actual file footprint, not guessed. `tone_instructions` has a hard 250-character schema limit that fails validation silently on the CodeRabbit side — after editing `.coderabbit.yaml`, verify with a `@coderabbitai configuration` PR comment and confirm every changed field shows `Source: Repository YAML (base)`. - **lz4_flex 0.14+ requires the crate's own `alloc` feature explicitly** for `compress_prepend_size`/`decompress_size_prepended` (used by `rewind.rs`/`zwinder.rs`) — it split real no_std support into an `alloc`-vs-`std` distinction that didn't exist in 0.13. A `cargo build --workspace` will NOT catch a missing `alloc` feature here because `rustynes-core`'s own default-on `std` feature implies it via cargo's feature unification; only a standalone `cargo build -p rustynes-core --target thumbv7em-none-eabihf --no-default-features` (the exact CI `no_std build` job) will. Run that command locally before pushing any bump that touches this dependency. diff --git a/CHANGELOG.md b/CHANGELOG.md index b3865d53..04e20daf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,24 @@ cycle-accurate core later replaced. ### Changed +- **One toolchain everywhere: `rust-toolchain.toml` is now the single source + of truth for CI.** `.github/actions/rust-setup` parses the `channel` from + that file and installs it, failing closed if it cannot be parsed, so there + is no longer a `toolchain:` version literal anywhere in `.github/` — a + toolchain bump is a one-line edit. Previously the composite defaulted to + `stable` and 5 of its 12 call sites overrode that with an explicit + `1.96.0`. That was misleading rather than broken: `rust-toolchain.toml` is + a directory override and outranks the `rustup default` the action + performs, so **every job was already compiling on 1.96.0** — the `stable` + default merely downloaded a second toolchain nothing used, and made the + workflows read as though they tested against latest stable, which they + never did. With the libretro tvOS job also moved onto the pin (see Fixed), + every build, test, lint, docs, release, and packaging path across GitHub + Actions, the libretro buildbot, and local builds now runs on the same + pinned 1.96.0. Nightly survives in exactly two places, neither of them a + gate: `cargo fuzz`, which requires it for its sanitizer flags, and the + dormant `rustynes-monetization` crate's standalone `uniffi-bindgen` + helper. - **Dependency version-bump consolidation (closes Dependabot #313–#315).** Rolled all three open Dependabot PRs into one reviewed change plus a full `cargo update --workspace` sweep of the rest of the tree, all with @@ -169,6 +187,31 @@ cycle-accurate core later replaced. lookalike rather than a rehearsal. There was previously *zero* libretro coverage in GitHub Actions, which is why all three defects reached a third-party buildbot we cannot push to or re-run. +- **Libretro buildbot: the last failing job (tvOS) now builds on the pinned + stable toolchain, dropping three workarounds.** The follow-up run + ([pipeline #91954](https://git.libretro.com/libretro/RustyNES/-/pipelines/91954)) + took the recipe from 1/10 to 9/10, leaving only + `libretro-build-tvos-arm64`. Its upstream template overrides `script` with + `cargo +nightly build -Zbuild-std`, which dates from when + `aarch64-apple-tvos` was a tier-3 target with no distributed `rust-std`. + The target has since been promoted and rustup now ships a complete + prebuilt std for it — `panic_abort` included — so the job now uses the + shared Apple build script (via `!reference`) on the same pinned 1.96.0 as + every other job. That removes all three workarounds the `+nightly` path + had forced, rather than adding a fourth: the nightly-channel reinstall + (needed because `+nightly` outranks the channel pin and the image's + 1.94.0-nightly is below the workspace `rust-version = "1.96"`); the + `CARGO_PROFILE_RELEASE_PANIC=unwind` override (needed because bare + `-Zbuild-std` omits the `panic_abort` that `[profile.release] panic = + "abort"` requires, and the hardcoded flag cannot be overridden by + `CARGO_UNSTABLE_BUILD_STD`); and a clearing of the image-injected + `-Car=,Clink-arg=...`, whose long-deprecated `-C ar` became a **hard + error in Rust 1.97** (bisected: 1.96.1 warns, 1.97.1 errors) and so broke + the refreshed nightly. tvOS now honours `panic = "abort"` exactly like + every other platform. All four Apple jobs still receive the `-C ar` flag + and are green only because 1.96.0 treats it as a warning, so + `rust-toolchain.toml` carries a warning for whoever bumps that pin to + 1.97+. ## [2.2.1] - 2026-07-15 - Housekeeping patch (dev-tooling archival + dependency consolidation + FDS test corpus) diff --git a/docs/build-and-tooling.md b/docs/build-and-tooling.md index bdd80107..69aaae14 100644 --- a/docs/build-and-tooling.md +++ b/docs/build-and-tooling.md @@ -6,7 +6,8 @@ - **Rust edition**: 2024. - **MSRV (minimum supported Rust version)**: 1.96.0. Pinned via `rust-toolchain.toml`. (Bumped from 1.86 in v1.3.0 "Bedrock" to unblock the edition-2024 + egui 0.34.3 / wgpu 29 / rfd 0.17.2 dependency tier.) -- **Channel**: stable. Nightly only for fuzz tests (`cargo +nightly fuzz`). +- **Channel**: the pinned `1.96.0` stable release — *not* a floating `stable`. `rust-toolchain.toml` is the single source of truth: every GitHub Actions job resolves its toolchain from that file (`.github/actions/rust-setup` parses the `channel` and fails closed if it cannot), the libretro buildbot builds all ten of its jobs on it, and local builds pick it up automatically as a directory override. There is no `toolchain:` version literal anywhere in `.github/`, so bumping the pin is a one-line edit here — but read the `-C ar` warning in `rust-toolchain.toml` before bumping to 1.97 or newer. +- **Nightly** is used for exactly two things, both outside CI and neither a gate: `cargo fuzz`, which requires it for the sanitizer flags it threads through `rustc` (`cargo +nightly fuzz run ` — see `fuzz/README.md`), and the dormant `rustynes-monetization` crate's standalone `uniffi-bindgen` helper, which needs it for metadata discovery. No build, test, lint, docs, release, or packaging path uses nightly. - **Targets supported**: `x86_64-unknown-linux-gnu`, `aarch64-apple-darwin`, `x86_64-pc-windows-msvc`. Tier 2: `aarch64-unknown-linux-gnu`. Cross-compile targets declared in `rust-toolchain.toml` (auto-installed): `thumbv7em-none-eabihf` (the `no_std` chip-stack gate) and `wasm32-unknown-unknown` (browser). Android arm64/arm/x86_64 via `cargo ndk` (see `docs/android.md`). The `x86_64-apple-darwin` release target was retired (ADR 0009). ## Workspace layout diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 82e78034..0ef487aa 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -25,5 +25,26 @@ components = ["rustfmt", "clippy"] # `rust-std` components on each toolchain install. They are installed # per-job by `.gitlab-ci.yml` instead, keeping that cost where the need # is. See the post-mortem comment at the top of `.gitlab-ci.yml`. +# +# BEFORE BUMPING `channel` TO 1.97 OR NEWER, read this. The libretro build +# image injects `-Car=,Clink-arg=...` into EVERY Apple job, and `-C ar` +# — a deprecated no-op for years — became a hard error in Rust 1.97: +# +# error: `-C ar`: this option has been removed +# +# No job trips it at 1.96.0 — all four Apple jobs merely log the warning. +# The moment this pin reaches 1.97+, all four (`libretro-build-osx-x64`, +# `-osx-arm64`, `-ios-arm64`, `-tvos-arm64`) fail together. The fix is two +# lines in each job's `before_script`: +# +# - unset CARGO_ENCODED_RUSTFLAGS +# - export RUSTFLAGS="" +# +# Cargo takes extra flags from exactly one source, first match wins +# (CARGO_ENCODED_RUSTFLAGS -> RUSTFLAGS -> target..rustflags -> +# build.rustflags), so that pair wins without needing to know where the +# image sets them. Discarding them is safe: rustc splits `-C` at the FIRST +# `=`, so the whole comma-joined string is consumed as the `ar` value and +# those link arguments have never reached the linker for any core. targets = ["thumbv7em-none-eabihf", "wasm32-unknown-unknown"] profile = "default"