Skip to content

ci: pin rust-toolchain + add clippy_report at release (active toolchain mgmt)#112

Merged
TigerInYourDream merged 4 commits intomainfrom
ci/clippy-report-at-release
Apr 21, 2026
Merged

ci: pin rust-toolchain + add clippy_report at release (active toolchain mgmt)#112
TigerInYourDream merged 4 commits intomainfrom
ci/clippy-report-at-release

Conversation

@TigerInYourDream
Copy link
Copy Markdown

@TigerInYourDream TigerInYourDream commented Apr 20, 2026

Summary

Shifts robrix2 from passive Rust toolchain drift to active toolchain management by:

  1. Pinning rust-toolchain.toml to 1.94.0 (exact version, not stable).
  2. Swapping all 14 CI toolchain-install steps from dtolnay/rust-toolchain@stable to actions-rust-lang/setup-rust-toolchain@v1, which honors rust-toolchain.toml as the single source of truth.
  3. Adding a non-blocking clippy_report job to release.yml — runs cargo clippy, renders a categorized warning summary to $GITHUB_STEP_SUMMARY, uploads the full log as a 30-day artifact.
  4. Documenting the upgrade cadence inline in rust-toolchain.toml as TOML comments, so anyone opening the file to bump the version sees the checklist.

Why

Companion to #111, which retired RUSTFLAGS: -D warnings. Together, these two PRs close the full loop:

  • ci: retire -D warnings, adopt Cargo [lints] as sole lint policy #111 removes the randomness: upstream lint drift can no longer fail a random PR author's work.
  • This PR replaces passivity with deliberate cadence: the toolchain is whatever the maintainer pinned. Upgrades are scheduled maintainer-opened PRs, not background events.
  • clippy_report gives each release a moment of review: the lint state of the pinned toolchain is visible in the release run's summary panel, so the bump-review cycle has clear trigger points.

Upgrade process (written into rust-toolchain.toml)

1. Find the latest stable: https://forge.rust-lang.org/
2. Update `channel` in rust-toolchain.toml
3. Locally run cargo build --profile fast and cargo clippy --workspace --all-features
4. Fix any new lint hits (edit code, or adjust [lints.clippy] in Cargo.toml)
5. Open a "chore: bump rust-toolchain to X.Y.Z" PR

File changes

File Change
rust-toolchain.toml Pinned to 1.94.0; upgrade process documented as inline comments
.github/workflows/main.yml 1 toolchain action swap (clippy job)
.github/workflows/builds.yml 7 toolchain action swaps; iOS job cleanup (redundant toolchain: stable removed)
.github/workflows/release.yml 6 toolchain action swaps + new clippy_report job

clippy_report mechanics

  • continue-on-error: true at job level + || true on the cargo invocation — never blocks a release.
  • Warning count sourced from #[warn(...)] annotations (not top-level warning: header lines), so cargo meta-warnings (e.g., duplicate-package notices) do not inflate the number.
  • Writes a markdown summary to $GITHUB_STEP_SUMMARY (rendered at top of the Actions run page).
  • Uploads full clippy.log as a 30-day artifact for diff-over-time review.

Test plan

  • Local rustup show respects the pin (downloads 1.94.0, marks it as the active override)
  • Local clippy-summary extraction pipeline verified against synthetic log; categorizes lints correctly (unused_variables, clippy::collapsible_if, clippy::useless_format)
  • CI on this PR is green (main.yml clippy + builds.yml x 7 builds)
  • After merge, trigger workflow_dispatch on release.yml with all build inputs false and create_release: false, confirm clippy_report runs and the summary renders
  • First future "bump rust-toolchain" PR uses the process documented in the TOML comment

Companion PR

#111 retires RUSTFLAGS: -D warnings and adds unused/dead_code denies to Cargo.toml [lints.rust]. Land that one first for smallest blast radius.

Run cargo clippy on every tag push / workflow_dispatch. Non-blocking:
continue-on-error: true at job level plus "|| true" on the cargo
invocation. The job writes a categorized warning summary to
$GITHUB_STEP_SUMMARY (displayed at top of the Actions run page) and
uploads the full clippy log as a 30-day artifact.

Purpose: visibility into clippy state at release time, so new warnings
introduced by upstream Rust/clippy releases can be reviewed during the
regular packaging cycle rather than blocking random PR authors.

Warning counts in the summary come from the #[warn(...)] annotations
emitted by rustc/clippy (not the top-level "warning:" line), so cargo
meta-warnings such as duplicate-package notices do not inflate the
count.

Companion to the CI policy change that retires RUSTFLAGS=-D warnings
in favor of Cargo.toml [lints] as the sole lint policy source.
Switches from floating dtolnay/rust-toolchain@stable to
actions-rust-lang/setup-rust-toolchain@v1 across all 14 workflow
invocations (main.yml x1, builds.yml x7, release.yml x6). The new
action honors rust-toolchain.toml as the single source of truth
for the Rust version used by CI and local development.

rust-toolchain.toml is pinned to 1.94.0 (current stable). The upgrade
process is documented inline as TOML comments, so anyone opening the
file to bump the version sees the checklist.

Combined with retiring -D warnings and the clippy_report job in the
preceding commit, robrix2 moves from passive toolchain drift to active
control: CI uses the exact compiler specified in rust-toolchain.toml,
local dev auto-installs the same version via rustup, toolchain bumps
are deliberate maintainer-opened PRs, and clippy_report at release
surfaces the lint state of the pinned toolchain.
@TigerInYourDream TigerInYourDream changed the title ci: add clippy_report job to release workflow ci: pin rust-toolchain + add clippy_report at release (active toolchain mgmt) Apr 20, 2026
…t CI

actions-rust-lang/setup-rust-toolchain@v1 defaults to rustflags: "-D warnings"
and exports that to $GITHUB_ENV, which silently re-injects strict mode into
every subsequent step including cargo install of third-party crates.

This was caught on PR #112's first CI run: cargo-makepad itself has an
unused_variable warning (ndk_version at compile.rs:368) which normally is
just a warning, but got promoted to an error by the injected RUSTFLAGS,
failing all 4 mobile builds.

Explicitly setting rustflags: "" on all 14 invocations disables the default
injection. Lint policy remains entirely in Cargo.toml [lints.rust] and
[lints.clippy], which is the whole point of the migration.
The original clippy_report job added in 9a02b40 was non-blocking:
`continue-on-error: true` plus `|| true` on the cargo invocation, so
a dirty clippy state at release time would show up only as a summary
panel while the release publish steps still produced artifacts. That
turned the job into pure visibility without discipline — warnings
could accumulate on main indefinitely and still ship.

This commit upgrades the semantics:

  * Removes `continue-on-error: true` at the job level and `|| true`
    on the cargo command; any warning or hard error now fails the job.

  * Splits the single "run + summarize" step into two: the run step
    is allowed to fail normally, while the summarize step carries
    `if: always()` so the GitHub step-summary panel and the
    clippy.log artifact are produced even on failure. Without the
    split, a dirty run would hide the categorized warning report
    that makes cleanup tractable.

  * Adds a final gate check in the summarize step that exits 1 with
    an `::error::` annotation when warn_count > 0 OR error_count > 0.
    Belt and suspenders alongside `set -o pipefail`.

  * Adds `clippy_report` to `create_release.needs` and extends its
    `if:` expression with `needs.clippy_report.result == 'success'`.
    Because the surrounding `always()` still evaluates the condition
    even when upstream jobs are skipped, the `result == 'success'`
    check is what actually cascades the block. A failed clippy_report
    skips create_release, which skips every downstream for_* build
    and publish job — no artifacts are produced with dirty clippy.

  * Renames the display to `Clippy Gate` (the job key stays
    `clippy_report` so the Swatinem cache key and upload-artifact
    name remain unchanged — no CI cache churn).

Rationale: the CI policy retirement of `-D warnings` in #111 moved
lint enforcement from "every PR, for every lint" to "Cargo.toml
per-lint denies for author self-defects". That is intentionally
lenient on style/perf/complexity/suspicious clippy warnings, which
would otherwise make every Rust toolchain bump a PR-carpet-bombing
event. The unavoidable consequence is that those warnings can
accumulate on main between releases. The release hard gate is the
scheduled cleanup trigger: it forces the maintainer to clear the
backlog at the exact moment it matters — before shipping.

Locally verified on the branch prior to push: current robrix code
compiles clippy-clean (warn_count=0, error_count=0), so this change
does not immediately block the next release. The first toolchain
bump that promotes a lint from allow to warn in the `unused` or
correctness group will fail the bump PR itself (by the layer-2
lint policy); anything lower-priority will surface at the next
release and require a deliberate cleanup PR.
@TigerInYourDream TigerInYourDream merged commit c15c45b into main Apr 21, 2026
11 checks passed
@TigerInYourDream TigerInYourDream deleted the ci/clippy-report-at-release branch April 21, 2026 03:16
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