Skip to content

build(ci): pin the Rust toolchain to 1.98.0 - #1620

Merged
justinchuby merged 1 commit into
mainfrom
justinchuby-pin-rust-toolchain
Aug 21, 2026
Merged

justinchuby merged 1 commit into
mainfrom
justinchuby-pin-rust-toolchain

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

Closes the root cause behind the 1.98.0 CI incidents tracked in #1600.

What the failure was

Every blocking gate in ci.yml runs under -D warnings — either as RUSTFLAGS or as a trailing -- -D warnings on clippy. CI resolved stable at job time, so when runner images rolled forward to rustc 1.98.0 (88d9e12ae 2026-08-18), every lint that release made warn-by-default became an instant failure on code that was clean the day before. No commit to blame, and a contributor on 1.97.0 could not reproduce it — clippy 0.1.97 does not carry those lints at all, so it does not even print them.

Four consecutive incidents, all the same cause:

PR Lint Code from
#1604 cargo fmt drift, manual_slice_fill, needless_late_init assorted
#1609 collapsible_if ×5 in qmoe.rs + ep-cpu lints #1602
#1615 collapsible_if in runtime.rs #1612
#1603 chunks_exact_to_as_chunks, 12 crates assorted

Each fixed symptoms; none could prevent the next. Formatting is the self-perpetuating one — an unpinned rustfmt means whoever formats next re-flows the file back the other way, so the tree oscillates.

How I reproduced it

I read the version out of a failing job log rather than guessing, then installed it:

rustup toolchain install 1.98.0    # -> rustc 1.98.0 (88d9e12ae 2026-08-18), byte-identical build hash

That is what turned "unreproducible locally" into reproducible, and it is what made #1609 and #1603 diagnosable at all.

What I changed

rust-toolchain.toml (new) — channel = "1.98.0", profile = "minimal", components = ["clippy", "rustfmt", "llvm-tools-preview"]. No targets key: Rust (Windows ARM64) runs natively on windows-11-arm and needs no cross target, and scripts/check_cross_compile.sh adds its own.

ci.yml — the eight setup steps collapse from rustup toolchain install stable --profile minimal --component … + rustup default stable to a bare rustup toolchain install, which reads the file and installs the declared components. The version= cache-key output is unchanged. Policy comment rewritten.

ci.yml, changes job — separate bug, fixed while here. It was gated on github.event_name == 'pull_request' || 'push', with a comment claiming schedule and workflow_dispatch runs would skip only that job and still get full CI. They did not. Every job below is needs: changes with a plain if: (no always()/!cancelled()), and GitHub skips a job whose needs dependency was skipped regardless of its own if:. So the gate skipped the entire workflow: the nightly cron and every manual dispatch reported success without running a single check — a green with no verdict behind it, which is the same class of problem this PR is about. The classify step already leaves docs_only=false for any event it does not diff, so removing the gate makes the documented behaviour real.

How I verified it

All local, on this branch, with my rustup default left at 1.97.0 so the file is doing the work:

Negative control — the pin is what changes the resolution:

without rust-toolchain.toml:  rustc 1.97.0   clippy 0.1.97   rustfmt 1.9.0 (2d8144b788)
with    rust-toolchain.toml:  rustc 1.98.0   clippy 0.1.98   rustfmt 1.9.0 (88d9e12ae1)

clippy 0.1.97 → 0.1.98 is precisely the gap that made these failures invisible to contributors.

Both gates, run with bare commands (no +1.98.0), which is the point — a 1.97.0 contributor now gets CI's behaviour by default:

  • cargo fmt --all -- --check → clean
  • the verbatim 30-package clippy gate from ci.yml ~326, including the trailing -- -D warnings → exit 0

Component auto-install: llvm-tools was absent from my 1.98.0 install and rustup fetched it on first use inside the repo, so dropping the per-job --component flags is safe. Confirmed end-to-end with cargo llvm-cov --locked -p onnx-runtime-cpuinfo → exit 0, which is the one job whose component requirement changed.

rustup override semantics, empirically checked (I had this wrong earlier and said so on #1600): rustup component add and rustup target add respect the directory pin — I expected them to hit the default toolchain and silently break the coverage and cross-compile jobs. They do not; both landed on 1.98.0. Explicit +toolchain still beats the file (rustc +stable -Vv → 1.97.0 inside the repo), so miri's cargo +nightly is unaffected.

YAML: all workflow files parse; changes confirmed to have no if: key.

What I could not verify

  • Everything about the CI runners themselves. I am on macOS aarch64. That a Linux or Windows runner materializes 1.98.0 from this file, and that the cache key still resolves correctly there, is only checkable in CI. This PR's own run is the oracle.
  • The workflow_dispatch/schedule fix. I verified the mechanism by reading the gating (all 8 downstream jobs are needs: changes with plain if:), but I have not observed a dispatch run execute the matrix. Worth triggering one after merge to confirm.
  • Other workflows. audit.yml, publish.yml, publish-ep-plugins.yml, wheels.yml still say rustup default stable. They inherit the pin anyway — any cargo run inside the repo resolves through the file — so their behaviour is already correct and their explicit install is merely redundant. I left them rather than widen the blast radius onto release workflows. One exception worth flagging: benchmark.yml uses dtolnay/rust-toolchain@stable, which sets RUSTUP_TOOLCHAIN — that env var overrides the file, so benchmark is genuinely not pinned. It is non-blocking, but it is a real gap, not an oversight.

Related, not fixed here

No CI job runs clippy on macOS. rust-coverage matrixes macOS but only installs clippy (~line 458) and never runs it; all six clippy-running jobs are Linux/Windows. That means #1609's macOS-only fixes in accelerate_gemm.rs and matmul.rs (both #[cfg(target_os = "macos")]) were structurally unverifiable by CI. I ran CI's exact clippy package list on macOS under 1.98.0 → exit 0, so this is a prevention gap rather than an outstanding defect. Filing separately.

Conflict risk with #1579

None. This PR touches only .github/workflows/ci.yml and a new root file. No overlap with crates/onnx-runtime-ep-cpu, crates/onnx-runtime-ir, or crates/onnx-genai-engine/src/{decode,native_decode}/.

Every blocking gate in ci.yml runs under `-D warnings`. CI resolved `stable`
at job time, so the moment a runner image rolled forward to 1.98.0, any lint
newly warn-by-default in that release turned `main` red with no commit to
blame -- and a contributor on 1.97.0 could not reproduce it, or even see it.

That happened four times in a row: #1604 (fmt drift, manual_slice_fill,
needless_late_init), #1609 (collapsible_if x5 in qmoe.rs from #1602, plus
ep-cpu lints), #1615 (collapsible_if in runtime.rs from #1612), and #1603
(chunks_exact_to_as_chunks across 12 crates). Each fixed the symptom; none
could stop the next one. Formatting is the self-perpetuating case, since
whoever formats next re-flows the file back the other way.

Add rust-toolchain.toml pinning channel 1.98.0 -- the exact release CI was
already resolving to, `rustc 1.98.0 (88d9e12ae 2026-08-18)`, read out of the
job logs -- with clippy, rustfmt and llvm-tools-preview as components.

rustup installs the components listed in that file when it materializes the
toolchain, and a directory pin takes precedence over the default toolchain
for `rustup component add` and `rustup target add` as well, so CI no longer
has to install components per-job and can no longer attach them to a
different toolchain than the one cargo will use. The eight setup steps in
ci.yml therefore collapse to a bare `rustup toolchain install`, which reads
the file. The `version=` cache-key output is unchanged and still records the
resolved release. An explicit `+toolchain` still wins, so miri.yml's
`cargo +nightly` is unaffected.

This pins the toolchain, not dependencies: `cargo update` and audit.yml are
untouched, so advisories are still surfaced and fixable. The previous policy
comment justified tracking `stable` as avoiding "freezing security fixes",
which conflated the two -- security fixes arrive via dependencies, while the
recurring breakage came from new warn-by-default lints.

Also fix the `changes` job, which was gated on the event being pull_request
or push. The comment said schedule and workflow_dispatch runs would skip only
that job and still get full CI. They did not: every job below is
`needs: changes` with a plain `if:`, and GitHub skips a job whose `needs`
dependency was skipped regardless of its own `if:`. Gating it skipped the
whole workflow, so the nightly cron and every manual dispatch reported
success without running a check. The classify step already leaves
docs_only=false for any event it does not diff, so removing the gate makes
the documented behaviour real.

Refs #1600

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d
@codecov

codecov Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.53%. Comparing base (2f0d08a) to head (9b3d116).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1620      +/-   ##
==========================================
+ Coverage   81.49%   81.53%   +0.03%     
==========================================
  Files         383      383              
  Lines      179257   179257              
  Branches   179257   179257              
==========================================
+ Hits       146086   146157      +71     
+ Misses      28231    28161      -70     
+ Partials     4940     4939       -1     
Flag Coverage Δ
cli-ort-linux 82.60% <ø> (ø)
cli-ort-windows 82.11% <ø> (ø)
mlas 85.19% <ø> (ø)
offline 81.43% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@justinchuby
justinchuby merged commit e90603c into main Aug 21, 2026
16 checks passed
@justinchuby
justinchuby deleted the justinchuby-pin-rust-toolchain branch August 21, 2026 03:36
justinchuby added a commit that referenced this pull request Aug 21, 2026
`main` is red at `843b0bf7` on `cargo fmt --all -- --check` (exit 1, 2
diffs),
which reddens `Fast (Linux x86_64)` and `Rust quality`. Both sites came
in with
#1644:

  crates/onnx-genai-engine/src/native_decode/mod.rs:1115
`snapshot_recurrent_state_public` signature folded across three lines;
    it fits on one at 98 columns.

  crates/onnx-genai-engine/src/native_decode/tests.rs:1412
`spec.rewind(base_len).expect(...)` on one line at 62 columns; rustfmt's
    default `use_small_heuristics` breaks a chain over 60.

Whitespace only -- `git diff -w` is empty.

Verified under the pinned toolchain (`rustfmt 1.9.0-stable (88d9e12ae1
2026-08-18)`, resolved from `rust-toolchain.toml`, no explicit `rustup
run`):

  cargo fmt --all -- --check                                   1 -> 0
clippy -p onnx-genai-engine -F native-backend --all-targets 0
(unchanged)

Reproduced on a clean `origin/main` worktree first, so the diffs are
#1644's
and not inherited from my tree.

One observation, since #1620 pinned the toolchain specifically to stop
this:
this is not toolchain drift. Both directions here are what 1.98.0's
rustfmt
produces from a default config, and the pin is being honoured -- the
code
simply was never run through `cargo fmt`. The pin removed the class
where two
contributors format the same file two ways; it cannot do anything about
code
that no formatter has touched. That is a merge-gating question, not a
toolchain one: #1644 merged with `Rust quality` red.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d
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