From f7415059cb183fd0e3ab5eab43d1232978afc7d4 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Thu, 1 Oct 2026 13:29:26 +0000 Subject: [PATCH 1/4] ci: run clippy on Windows so cfg(windows) code is lint-gated too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The clippy job runs on Linux only, so it compiles the cfg(unix) side of every platform conditional and never sees the cfg(windows) side. Windows -gated code was outside the warnings-as-errors gate entirely, not merely under-linted: a lint could land there and no check in the set could fail. Add both of the Linux job's clippy invocations to windows-build-and-test, before the build so a lint failure costs seconds rather than the full build/test/package cycle. That job is already a required check, so the gate becomes load-bearing without a branch-protection change. Turning it on surfaced 21 existing violations, fixed here. Most are mechanical: `&mut x` passed to a Win32 call becomes `&raw mut x`, which also avoids materialising a reference just to take a pointer from it; `if c { 1 } else { 0 }` becomes `i32::from(c)`; `map(..).unwrap_or(..)` becomes `map_or_else`/`is_ok_and`. The rest are `missing_const_for_fn` and `unused_async` on platform stubs, and those are deliberately silenced rather than "fixed". Making only the off-unix arm of a cfg-split pair `const` would give the two platforms different signatures — two of these are `pub` — and would push the same lint onto every caller in turn, which is how the burn-down kept growing. The `async` stubs must keep their signature because callers `.await` them either way. Each site carries the reason inline. Verified by cross-compiling the whole workspace to x86_64-pc-windows-gnu (the msvc target cannot be checked off Windows: ring and aws-lc-sys need a Windows C toolchain to build their build scripts). Both clippy invocations are clean for that target and for Linux, and the test suite passes. Signed-off-by: Roman Inflianskas --- .github/workflows/ci.yml | 17 +++++++++++ apps/rocm/src/dash.rs | 4 +++ apps/rocm/src/main.rs | 8 +++++ apps/rocm/src/remote/transport.rs | 4 +++ apps/rocmd/src/lib.rs | 8 +++++ crates/rocm-core/src/disk_space.rs | 4 +++ crates/rocm-core/src/examine.rs | 4 +++ crates/rocm-core/src/lib.rs | 29 ++++++++++++------- crates/rocm-core/src/proc_lifecycle.rs | 8 +++++ crates/rocm-core/src/uv.rs | 6 ++++ crates/rocm-dash-daemon/src/server.rs | 3 ++ crates/rocm-dash-tui/src/client.rs | 6 ++-- tests/e2e-cucumber/src/capability.rs | 4 +++ .../e2e-cucumber/tests/e2e/lifecycle_steps.rs | 3 +- 14 files changed, 93 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ace17f0d3..79769cc04 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -611,6 +611,23 @@ jobs: with: cache-on-failure: true + # The `clippy` job runs on Linux only, so it compiles the `cfg(unix)` side + # of every platform conditional and never sees the `cfg(windows)` side. + # Windows-gated code was therefore outside the warnings-as-errors gate + # entirely — not merely under-linted — so a lint could land there and no + # check in the set could fail. This mirrors the two invocations the Linux + # `clippy` job runs; the second is separate because the `e2e` target sets + # `test = false`, so `--all-targets` does not build it. + # + # Runs before the build so a lint failure costs seconds rather than the + # full build/test/package cycle, matching why this job `needs: clippy`. + - name: Clippy (Windows targets) + if: needs.changes.outputs.heavy == 'true' + shell: pwsh + run: | + cargo clippy --locked --workspace --all-targets -- -D warnings + cargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings + - name: Build if: needs.changes.outputs.heavy == 'true' shell: pwsh diff --git a/apps/rocm/src/dash.rs b/apps/rocm/src/dash.rs index e3af6494d..a92e8a75c 100644 --- a/apps/rocm/src/dash.rs +++ b/apps/rocm/src/dash.rs @@ -912,6 +912,10 @@ const fn should_spawn_daemon(args: &ResolvedArgs) -> bool { /// listening, so `rocm dash` works without a separate `rocm daemon` terminal. /// Returns the task handle + socket to clean up on exit, or `None` when an /// existing daemon was found (we connect to it instead). +// One function with `#[cfg]` blocks inside, not a cfg-split pair: the unix +// body awaits, the Windows body does not. Callers `.await` this either way, +// so dropping `async` off unix would not compile. +#[allow(clippy::unused_async)] async fn maybe_spawn_embedded_daemon( connect: &str, config: &RocmCliConfig, diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index d486ee8bc..65b01b226 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -1293,6 +1293,10 @@ fn reset_sigpipe() { } #[cfg(not(unix))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn reset_sigpipe() {} /// Run `f` with SIGPIPE temporarily ignored, restoring the previous @@ -3965,6 +3969,10 @@ impl PrivilegeEscalation { /// /// Only ever called on the production path; every plan builder takes the /// escalation as a parameter so both branches are testable on any host. + // `running_as_root()` is a non-const syscall on unix and a trivial + // constant off it, so Clippy sees this as const-able only on Windows. + // Making it `const` would stop the unix build compiling. + #[allow(clippy::missing_const_for_fn)] fn detect() -> Self { if rocm_core::openmpi::running_as_root() { Self::AlreadyRoot diff --git a/apps/rocm/src/remote/transport.rs b/apps/rocm/src/remote/transport.rs index 4d2a00368..5fe134298 100644 --- a/apps/rocm/src/remote/transport.rs +++ b/apps/rocm/src/remote/transport.rs @@ -150,6 +150,10 @@ fn control_socket_path() -> Option { /// warnings at best. Returning `None` keeps the argument builder honest instead /// of emitting options the platform ignores. #[cfg(not(unix))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn control_socket_path() -> Option { None } diff --git a/apps/rocmd/src/lib.rs b/apps/rocmd/src/lib.rs index 7c7d45efb..8b3bf4bc4 100644 --- a/apps/rocmd/src/lib.rs +++ b/apps/rocmd/src/lib.rs @@ -2589,6 +2589,10 @@ fn descendant_pids_for_roots(root_pids: &[u32]) -> Result> { } #[cfg(not(unix))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn descendant_pids_for_roots(_root_pids: &[u32]) -> Result> { Ok(Vec::new()) } @@ -2735,6 +2739,10 @@ fn force_terminate_remaining_processes(pids: &[u32]) -> Result> { } #[cfg(not(unix))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn force_terminate_remaining_processes(_pids: &[u32]) -> Result> { Ok(Vec::new()) } diff --git a/crates/rocm-core/src/disk_space.rs b/crates/rocm-core/src/disk_space.rs index da5dc36fc..b928ac669 100644 --- a/crates/rocm-core/src/disk_space.rs +++ b/crates/rocm-core/src/disk_space.rs @@ -202,6 +202,10 @@ fn mount_owns_path(path: &Path, mount_point: &Path) -> bool { } #[cfg(not(unix))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn mount_owns_path(_path: &Path, _mount_point: &Path) -> bool { true } diff --git a/crates/rocm-core/src/examine.rs b/crates/rocm-core/src/examine.rs index 13a85a71d..531bfb95c 100644 --- a/crates/rocm-core/src/examine.rs +++ b/crates/rocm-core/src/examine.rs @@ -2825,6 +2825,10 @@ fn filesystem_size(path: &str) -> Option<(u64, u64)> { } #[cfg(not(target_os = "linux"))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn filesystem_size(_path: &str) -> Option<(u64, u64)> { None } diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index add7b61b6..cd369548a 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -1242,7 +1242,7 @@ pub fn spawn_hidden_console_with_log( current_process, source, current_process, - &mut stdout_handle, + &raw mut stdout_handle, 0, 1, DUPLICATE_SAME_ACCESS, @@ -1258,7 +1258,7 @@ pub fn spawn_hidden_console_with_log( current_process, source, current_process, - &mut stderr_handle, + &raw mut stderr_handle, 0, 1, DUPLICATE_SAME_ACCESS, @@ -1308,7 +1308,7 @@ pub fn wait_for_process_exit(pid: u32) -> Result { unsafe { WaitForSingleObject(handle, INFINITE); let mut exit_code = 0; - if GetExitCodeProcess(handle, &mut exit_code) == 0 { + if GetExitCodeProcess(handle, &raw mut exit_code) == 0 { CloseHandle(handle); bail!( "failed to read process {pid} exit code: {}", @@ -1491,7 +1491,7 @@ pub fn process_is_running(pid: u32) -> bool { return false; } let mut exit_code = 0; - let ok = unsafe { GetExitCodeProcess(handle, &mut exit_code) != 0 }; + let ok = unsafe { GetExitCodeProcess(handle, &raw mut exit_code) != 0 }; unsafe { CloseHandle(handle); } @@ -1587,6 +1587,10 @@ pub fn detach_command_session(command: &mut Command) { } #[cfg(not(unix))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] pub fn detach_command_session(_command: &mut Command) {} #[cfg(windows)] @@ -1630,12 +1634,12 @@ fn spawn_windows_no_inherit( command_line.as_mut_ptr(), null(), null(), - if std_handles.is_some() { 1 } else { 0 }, + i32::from(std_handles.is_some()), creation_flags, environment.as_mut_ptr().cast(), null(), - &startup_info, - &mut process_info, + &raw const startup_info, + &raw mut process_info, ) }; if created == 0 { @@ -2116,12 +2120,12 @@ struct WindowsDisplayAdapter { impl WindowsExamineInventory { #[cfg(windows)] - fn is_empty(&self) -> bool { + const fn is_empty(&self) -> bool { self.cpu_model.is_none() && self.system_ram_gib.is_none() && self.displays.is_empty() } #[cfg(windows)] - fn merge_missing_from(&mut self, mut other: WindowsExamineInventory) { + fn merge_missing_from(&mut self, mut other: Self) { if self.cpu_model.is_none() { self.cpu_model = other.cpu_model.take(); } @@ -3535,8 +3539,7 @@ fn append_windows_probe_diagnostics( result .program .as_ref() - .map(|path| path.display().to_string()) - .unwrap_or_else(|| program.to_owned()), + .map_or_else(|| program.to_owned(), |path| path.display().to_string()), args.join(" ") ); if let Some(error) = result.error.as_deref() { @@ -5432,6 +5435,10 @@ fn probe_usable_amd_gpu_indices() -> Option> { } #[cfg(not(target_os = "linux"))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn probe_usable_amd_gpu_indices() -> Option> { None } diff --git a/crates/rocm-core/src/proc_lifecycle.rs b/crates/rocm-core/src/proc_lifecycle.rs index 61f422f91..529132299 100644 --- a/crates/rocm-core/src/proc_lifecycle.rs +++ b/crates/rocm-core/src/proc_lifecycle.rs @@ -311,6 +311,10 @@ pub fn process_start_ticks(pid: u32) -> Option { #[cfg(not(target_os = "linux"))] #[must_use] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] pub fn process_start_ticks(_pid: u32) -> Option { None } @@ -332,6 +336,10 @@ fn process_has_exited(pid: u32) -> bool { } #[cfg(not(target_os = "linux"))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn process_has_exited(_pid: u32) -> bool { false } diff --git a/crates/rocm-core/src/uv.rs b/crates/rocm-core/src/uv.rs index 0a1fcee66..a1a5b023a 100644 --- a/crates/rocm-core/src/uv.rs +++ b/crates/rocm-core/src/uv.rs @@ -558,6 +558,12 @@ fn find_binary_in(dir: &Path, name: &str) -> Option { None } +// Off unix the body is a no-op, so Clippy's nursery `missing_const_for_fn` +// fires there — but this is one function with `#[cfg]` blocks inside, not a +// cfg-split pair, and the unix arm calls `set_permissions`, which is not +// const. Making it `const` to satisfy the Windows lint would stop the unix +// build compiling. +#[allow(clippy::missing_const_for_fn)] fn make_executable(path: &Path) -> Result<()> { #[cfg(unix)] { diff --git a/crates/rocm-dash-daemon/src/server.rs b/crates/rocm-dash-daemon/src/server.rs index a34e12bdc..c5246b267 100644 --- a/crates/rocm-dash-daemon/src/server.rs +++ b/crates/rocm-dash-daemon/src/server.rs @@ -205,6 +205,9 @@ async fn run_unix(path: PathBuf, opts: RunnerOptions) -> anyhow::Result<()> { } #[cfg(windows)] +// Mirrors the signature of the `cfg(unix)` arm, which callers `.await`; +// dropping `async` here would not compile on Windows. +#[allow(clippy::unused_async)] async fn run_unix(_path: PathBuf, _opts: RunnerOptions) -> anyhow::Result<()> { Err(anyhow!( "rocm-dash daemon requires Unix domain sockets; not supported on Windows yet" diff --git a/crates/rocm-dash-tui/src/client.rs b/crates/rocm-dash-tui/src/client.rs index 2104cb458..bd5e50b3f 100644 --- a/crates/rocm-dash-tui/src/client.rs +++ b/crates/rocm-dash-tui/src/client.rs @@ -179,9 +179,11 @@ async fn connect_and_run(connect: &str, tx: UnboundedSender) -> anyho } #[cfg(windows)] +// Mirrors the signature of the `cfg(unix)` arm, which callers `.await`; +// dropping `async` here would not compile on Windows. +#[allow(clippy::unused_async)] async fn connect_and_run(connect: &str, _tx: UnboundedSender) -> anyhow::Result<()> { Err(anyhow!( - "rocm-dash TUI requires Unix domain sockets; not supported on Windows yet (connect={})", - connect + "rocm-dash TUI requires Unix domain sockets; not supported on Windows yet (connect={connect})" )) } diff --git a/tests/e2e-cucumber/src/capability.rs b/tests/e2e-cucumber/src/capability.rs index a7fe4dbfa..0c0c19d7f 100644 --- a/tests/e2e-cucumber/src/capability.rs +++ b/tests/e2e-cucumber/src/capability.rs @@ -521,6 +521,10 @@ fn probe_amd_gpu_count() -> Option { /// Off Linux the product's own probe returns `None` too, so the count is /// unknown and every `@requires-multi-gpu` scenario resolves to skip. #[cfg(not(target_os = "linux"))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn probe_amd_gpu_count() -> Option { None } diff --git a/tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs b/tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs index 36e3e11aa..cc970d690 100644 --- a/tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs @@ -263,8 +263,7 @@ fn which_ok(name: &str) -> bool { .arg("-Command") .arg("exit 0") .status() - .map(|s| s.success()) - .unwrap_or(false) + .is_ok_and(|s| s.success()) } /// Capture the current Windows user PATH (registry `HKCU\Environment\Path`). From 09b81be4fe60cf3f9b1983a225637d1b8046e512 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 5 Oct 2026 07:00:41 +0000 Subject: [PATCH 2/4] ci: run one cargo command per Windows step so a failure cannot be masked The Windows clippy step ran two `cargo clippy` invocations in one `shell: pwsh` block. pwsh does not stop on a failing native command, and the step's result is the last command's exit code, so a first clippy that failed was turned green by a second that passed. Reproduced with pwsh 7.4: two commands where the first exits 3 leave the step at 0; the failing command alone exits 3. Split it into one step per invocation, the way the Linux `clippy` job already does. The Build step had the identical two-command shape, so it is split the same way. Signed-off-by: Roman Inflianskas --- .github/workflows/ci.yml | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79769cc04..606f62bd5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -621,19 +621,30 @@ jobs: # # Runs before the build so a lint failure costs seconds rather than the # full build/test/package cycle, matching why this job `needs: clippy`. + # + # One cargo command per step, here and in Build below: `shell: pwsh` does + # not stop on a failing native command, and the step's result is only the + # LAST command's exit code — so a second command that passes would turn a + # failing first one green. - name: Clippy (Windows targets) if: needs.changes.outputs.heavy == 'true' shell: pwsh - run: | - cargo clippy --locked --workspace --all-targets -- -D warnings - cargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings + run: cargo clippy --locked --workspace --all-targets -- -D warnings + + - name: Clippy (Windows, e2e harness + steps) + if: needs.changes.outputs.heavy == 'true' + shell: pwsh + run: cargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings - name: Build if: needs.changes.outputs.heavy == 'true' shell: pwsh - run: | - cargo build --workspace --all-targets - cargo build --release -p rocm -p rocmd -p rocm-engine-lemonade -p rocm-engine-vllm -p xtask + run: cargo build --workspace --all-targets + + - name: Build (release) + if: needs.changes.outputs.heavy == 'true' + shell: pwsh + run: cargo build --release -p rocm -p rocmd -p rocm-engine-lemonade -p rocm-engine-vllm -p xtask - name: Test if: needs.changes.outputs.heavy == 'true' From e79b527900b06089dbc72c11c0df05fdb399a97d Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 5 Oct 2026 07:00:41 +0000 Subject: [PATCH 3/4] fix(core): keep running_as_root's off-Linux stub a plain fn The off-Linux stub of `running_as_root` was a `pub const fn` while the Linux implementation is not const: the two platforms had different signatures, which is the pattern this change argues against. It also pushed `missing_const_for_fn` onto its caller, which is why `PrivilegeEscalation::detect` needed an unconditional allow. Make the stub a plain `fn` with the same inline reason and `#[allow(clippy::missing_const_for_fn)]` as the other platform stubs, and drop the allow on `detect`. Checked against a scratch crate under the workspace's `nursery` level for the Windows target: without the allow the stub fails clippy, with it clippy passes. Signed-off-by: Roman Inflianskas --- apps/rocm/src/main.rs | 4 ---- crates/rocm-core/src/openmpi.rs | 6 +++++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index 65b01b226..b0a61ebd9 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -3969,10 +3969,6 @@ impl PrivilegeEscalation { /// /// Only ever called on the production path; every plan builder takes the /// escalation as a parameter so both branches are testable on any host. - // `running_as_root()` is a non-const syscall on unix and a trivial - // constant off it, so Clippy sees this as const-able only on Windows. - // Making it `const` would stop the unix build compiling. - #[allow(clippy::missing_const_for_fn)] fn detect() -> Self { if rocm_core::openmpi::running_as_root() { Self::AlreadyRoot diff --git a/crates/rocm-core/src/openmpi.rs b/crates/rocm-core/src/openmpi.rs index cf835803d..0cd3d8fb1 100644 --- a/crates/rocm-core/src/openmpi.rs +++ b/crates/rocm-core/src/openmpi.rs @@ -181,7 +181,11 @@ pub fn running_as_root() -> bool { /// Whether the current process is running as root. Always false off Linux. #[cfg(not(target_os = "linux"))] -pub const fn running_as_root() -> bool { +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] +pub fn running_as_root() -> bool { false } From 82fa27b8951744464cf6d5e9563d62647bd60d9b Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Tue, 6 Oct 2026 12:21:17 +0000 Subject: [PATCH 4/4] fix(rocm): keep gpu_vram_usage_sysfs's off-Linux stub a plain fn The VRAM-usage fallback added on main has a Linux implementation that reads DRM sysfs counters and an off-Linux stub that returns `None`. On Windows, clippy's `missing_const_for_fn` flags the stub, which fails the Windows clippy gate this branch adds. Give the stub the same inline reason and `#[allow(clippy::missing_const_for_fn)]` as the other platform stubs rather than making it `const`, so both platforms keep one signature. A full Windows-target clippy cross-check (x86_64-pc-windows-gnu, both CI invocations) is clean after this change. Signed-off-by: Roman Inflianskas --- apps/rocm/src/main.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index b0a61ebd9..629ad5760 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -22205,6 +22205,10 @@ fn gpu_vram_usage_sysfs() -> Option> { } #[cfg(not(target_os = "linux"))] +// Platform stub. The `cfg` sibling calls non-const code, so making only this +// arm `const` would give the two platforms different signatures and push +// `missing_const_for_fn` onto every caller in turn. +#[allow(clippy::missing_const_for_fn)] fn gpu_vram_usage_sysfs() -> Option> { None }