From 0dda41a15ede15d778b0831d7ceaed93d50335c5 Mon Sep 17 00:00:00 2001 From: Eugene Volen Date: Thu, 3 Sep 2026 11:20:45 +0000 Subject: [PATCH 1/4] feat(doctor): diagnose and fix WSL2 hosts instead of routing out `rocm diagnose` treated WSL2 as out of scope and ran none of its checks, and `rocm examine` returned after the framework probe alone. The reasoning was sound -- WSL2 reaches the GPU through /dev/dxg and the Windows host driver, so the bare-metal checks for the amdgpu module, /dev/kfd and the render group would report confident nonsense -- but it left users on that platform with no help at all. Add a parallel WSL2 catalog rather than porting the bare-metal one. Selection moves to a platform family resolved from `is_wsl`, not from `os_family`, which stays "linux" because install, serve and the engines branch on it. Every bare-metal entry is tagged `linux` only and so stops applying on WSL automatically, which keeps the no-false-positives property the old wholesale skip bought. Entries that were always valid there -- the arch list, HSA_OVERRIDE_GFX_VERSION, PATH and the wheel/ROCm pairing -- opt in explicitly and now answer on WSL for the first time. `rocm fix` uses the same resolution, so a Linux-only recipe is refused there rather than running usermod for a group that governs nothing. Seven WSL entries cover the device, the DXCore handoff, ROCDXG and its linker entry, the distro floor, the Windows host driver, and WSL 1. All are print-only: each installs packages with sudo, edits loader config, or belongs to the Windows host, so none meets the bar the four auto-applicable fixes clear. Throughout, "could not ask" is kept distinct from "at fault", because the machines that cannot answer are the ones most likely to be misdiagnosed: the host-driver check abstains when the Windows host is unreachable, the distro floor when the release will not parse, and the linker check when ldconfig itself cannot be run. Symptom keywords only ever corroborate machine evidence -- never establish a finding on their own -- so a user describing a GPU failure cannot talk the catalog into blaming their driver. `examine` now runs the probes that do apply on WSL and reports the stack under a new `wsl` section. Fields the shared checks expect from the skipped GPU probe are filled from those facts; left at their defaults they did not read as "unknown" but as "absent", which made the PATH check score 50 on every WSL host with ROCm installed. Where the shared checks still run on less evidence than on bare metal the degradation is one-directional -- they may miss a fault, never invent one -- and a test pins that direction. Also fixes defects that would have produced false positives: ROCDXG was looked up only under /opt/rocm so a versioned install reported it missing; two WSL predicates disagreed about hosts identified only by $WSL_DISTRO_NAME; and ldconfig was resolved by bare name, which fails for a non-root user on Debian, where it lives in /sbin. `out_of_scope` is repurposed rather than deleted: it now marks a platform with no catalog entries at all, which previously fell through to "no known misconfiguration" and read as a clean bill of health. `run` used `read_to_string`, which errors on invalid UTF-8 with the error discarded, so one stray byte emptied an entire capture and every caller read that as "the command printed nothing". It now reads bytes and converts lossily. Replace the Python WSL preflight with `rocm diagnose --distro`. Its in-guest half was already redundant; the host-side half now collects the facts over `wsl.exe` using a POSIX shell and runs the same catalog, so the target distro needs neither rocm-cli nor python3 -- which matters, since the distro being checked is usually the one that is not set up yet. It sees no environment, so it says which checks did not run rather than letting silence read as health. `wsl.exe` output is decoded as UTF-16LE by declaration: sniffing cannot work, because that text in a Latin or Cyrillic script is made of bytes below 0x80 and so is valid UTF-8 that decodes into mojibake. The script's distro-list parser and release-floor rules keep their coverage as Rust tests. The two `@requires-wsl` scenarios run only on the `e2e-wsl` lane in e2e-selfhosted.yml; the third new scenario runs everywhere. Signed-off-by: Eugene Volen --- .github/workflows/ci.yml | 2 - apps/rocm/src/main.rs | 52 +- crates/rocm-core/src/diagnose.rs | 998 +++++++++++++++++- crates/rocm-core/src/examine.rs | 604 ++++++++++- crates/rocm-core/src/fix.rs | 204 +++- crates/rocm-core/src/lib.rs | 200 +++- docs/ci-hardware-testing.md | 11 +- docs/testing.md | 23 +- docs/wsl.md | 92 +- scripts/wsl_preflight.py | 483 --------- tests/e2e-cucumber/expectations.toml | 5 +- tests/e2e-cucumber/features/diagnose.feature | 86 +- tests/e2e-cucumber/src/expectation.rs | 38 + .../e2e-cucumber/tests/e2e/diagnose_steps.rs | 150 ++- 14 files changed, 2312 insertions(+), 636 deletions(-) delete mode 100644 scripts/wsl_preflight.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15e3bdc29..c9b677be8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -358,7 +358,6 @@ jobs: python scripts/comfyui_therock_gpu_test.py --self-test python scripts/local_assistant_therock_gpu_test.py --self-test python scripts/vllm_therock_gpu_test.py --self-test - python scripts/wsl_preflight.py --self-test - name: Portable WSL build deps self-test if: needs.changes.outputs.heavy == 'true' @@ -583,7 +582,6 @@ jobs: python .\scripts\comfyui_therock_gpu_test.py --self-test python .\scripts\local_assistant_therock_gpu_test.py --self-test python .\scripts\vllm_therock_gpu_test.py --self-test - python .\scripts\wsl_preflight.py --self-test - name: Release readiness self-test if: needs.changes.outputs.heavy == 'true' diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index f6c166207..137770475 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -146,6 +146,11 @@ enum Command { /// Emit the machine-readable diagnosis JSON. #[arg(long)] json: bool, + /// Diagnose a WSL distribution from the Windows host instead of this + /// machine. Needs nothing installed inside the distribution. Pass the + /// name only when more than one is installed. + #[arg(long, value_name = "NAME", num_args = 0..=1, default_missing_value = "")] + distro: Option, }, /// Apply a known fix by id (see `rocm diagnose`); run with no id to list fixes. /// @@ -1752,7 +1757,12 @@ fn dispatch(cli: Cli) -> Result<()> { match cli.command { Some(Command::Examine { json, framework }) => examine(json, framework.into()), - Some(Command::Diagnose { symptom, top, json }) => diagnose(symptom, top, json), + Some(Command::Diagnose { + symptom, + top, + json, + distro, + }) => diagnose(symptom, top, json, distro), // Keep this error chained rather than discarding it into a fresh // `anyhow!(...)` (e.g. via a `.map_err` that restringifies it) -- see // `FixExitCode`'s doc comment for why that would silently break its @@ -2334,23 +2344,50 @@ fn examine(json: bool, framework: rocm_core::FrameworkProbe) -> Result<()> { let (text, summary) = examine_human_report(&paths, &config)?; print!("{text}"); if summary.wsl.as_ref().is_some_and(|w| w.is_wsl) { - // Informational route-out guidance for humans (the verdict is also in + // Informational platform guidance for humans (the verdict is also in // the `status` field for `--json` consumers). - println!("\n{}", rocm_core::WSL_ROUTE_OUT_NOTE); + println!("\n{}", rocm_core::WSL_PLATFORM_NOTE); } Ok(()) } -fn diagnose(symptom: Option, top: usize, json: bool) -> Result<()> { +fn diagnose(symptom: Option, top: usize, json: bool, distro: Option) -> Result<()> { // `rocm diagnose` is a query: it exits 0 whether it matched, found nothing, // or is out of scope. Callers read `has_match` / `out_of_scope` / // `route_when_no_match` from `--json` rather than branching on the exit code. - let examination = rocm_core::Examination::probe(rocm_core::FrameworkProbe::Auto); + // + // `--distro` is the exception that still errors: the user named a machine to + // inspect, and silently reporting on a different one would be worse than + // failing. `--distro` with no value means "the only one installed". + let examination = match distro { + Some(name) => { + let selected = (!name.is_empty()).then_some(name); + rocm_core::probe_wsl_distro_from_host(selected.as_deref()) + .map_err(|reason| anyhow::anyhow!("{reason}"))? + } + None => rocm_core::Examination::probe(rocm_core::FrameworkProbe::Auto), + }; + let inspected_remotely = examination + .wsl + .as_ref() + .is_some_and(|wsl| !wsl.locally_probed); let report = rocm_core::run_diagnose(&examination, &symptom.unwrap_or_default()); if json { println!("{}", serde_json::to_string_pretty(&report)?); } else { print!("{}", rocm_core::render_diagnose_text(&report, top)); + // Inspecting a distribution from outside it collects no environment, so + // the checks that read one never run. Without saying so, "no known + // misconfiguration matched" reads as a clean bill of health for the + // whole installation, when it only covers the WSL GPU stack. + if inspected_remotely { + println!( + "\nNote: inspected from outside the distribution, which sees only its \ + WSL GPU stack.\nChecks that read the environment (HSA_OVERRIDE_GFX_VERSION, \ + PATH, the framework/ROCm pairing)\ndid not run. For those, run `rocm diagnose` \ + inside the distribution." + ); + } } Ok(()) } @@ -3121,7 +3158,10 @@ fn build_driver_install_plan( reason: "WSL uses the Windows host driver plus ROCDXG; run `scripts/wsl_setup_rocdxg.sh` inside WSL instead of installing Linux DKMS.".to_owned(), preflight_checks: Vec::new(), commands: Vec::new(), - checks: vec!["rocm examine".to_owned(), "scripts/wsl_preflight.py".to_owned()], + // `rocm diagnose` carries the WSL catalog, including the host-side + // form that inspects a distro over `wsl.exe` without needing anything + // installed inside it. + checks: vec!["rocm examine".to_owned(), "rocm diagnose".to_owned()], }; } diff --git a/crates/rocm-core/src/diagnose.rs b/crates/rocm-core/src/diagnose.rs index 5d269eb82..cc24729d4 100644 --- a/crates/rocm-core/src/diagnose.rs +++ b/crates/rocm-core/src/diagnose.rs @@ -15,7 +15,7 @@ //! data; the per-check logic mirrors `diagnose.py` field-for-field so the two //! stay behaviorally identical. -use crate::examine::Examination; +use crate::examine::{Examination, WslFacts}; use regex::Regex; use serde::{Deserialize, Serialize}; @@ -1378,18 +1378,455 @@ fn check_17_torch_dlpack_cuda_variant(_e: &Examination, symptom: &str) -> Diagno ) } +// --------------------------------------------------------------------------- +// WSL2 catalog +// +// A parallel catalog, not a port of the bare-metal one. WSL2 reaches the GPU +// through /dev/dxg and the Windows host driver (dxgkrnl), so the questions worth +// asking are about the DXCore handoff, the ROCDXG userspace, and the host — none +// of which the bare-metal checks know anything about. +// --------------------------------------------------------------------------- + +const KEYWORDS_WSL_NO_DEVICE: KeywordTable = &[ + ( + "no rocm-capable device", + 40, + "error mentions no ROCm-capable device", + ), + ( + "no hip-capable device", + 40, + "error mentions no HIP-capable device", + ), + (r"/dev/dxg", 45, "error mentions /dev/dxg"), + ( + "hsa_status_error_out_of_resources", + 25, + "error mentions HSA_STATUS_ERROR_OUT_OF_RESOURCES", + ), + ( + "no amd gpus? (?:were )?(?:found|detected)", + 35, + "error mentions no AMD GPU found", + ), +]; + +const KEYWORDS_WSL_LOADER: KeywordTable = &[ + (r"librocdxg\.so", 50, "error names librocdxg.so"), + (r"libdxcore\.so", 50, "error names libdxcore.so"), + ( + "cannot open shared object file", + 30, + "error mentions a shared object that could not be opened", + ), + ( + "error while loading shared libraries", + 35, + "error mentions a shared library load failure", + ), +]; + +/// The WSL facts, or a default set when the probe did not populate them. +/// +/// A WSL host whose `wsl` section is missing is a probe failure, not a healthy +/// machine, so every field reads false and the checks fire on the missing +/// plumbing rather than silently passing. +fn wsl_facts(e: &Examination) -> WslFacts { + e.wsl.clone().unwrap_or_default() +} + +fn check_wsl_1_gpu_not_exposed(e: &Examination, symptom: &str) -> Diagnosis { + let w = wsl_facts(e); + if w.dxg_device { + return zero("fix-wsl-1-gpu-not-exposed", "GPU not exposed to the distro"); + } + // WSL 1 has no GPU path at all; fix-wsl-7 says so in terms the user can act + // on, and two findings for one cause is noise. + if w.version == 1 { + return zero("fix-wsl-1-gpu-not-exposed", "GPU not exposed to the distro"); + } + let mut score = 55; + let mut evidence = vec!["/dev/dxg is missing, so the distro has no GPU path".to_owned()]; + let (kw_score, kw_ev) = keyword_score(symptom, KEYWORDS_WSL_NO_DEVICE); + score += kw_score; + evidence.extend(kw_ev); + + // Three different causes produce the same missing device, and they need + // different actions. Naming which one this is spares the user from updating a + // Windows driver that was never the problem. + let (cause, commands, notes) = if e.in_container { + ( + "this is a container, and containers only see /dev/dxg when it is passed in", + vec![ + "# Re-run the container with the WSL GPU device and libraries:".to_owned(), + "# --device=/dev/dxg -v /usr/lib/wsl:/usr/lib/wsl".to_owned(), + "# and add /usr/lib/wsl/lib to the loader path inside it.".to_owned(), + ], + vec![ + "The Windows host driver is probably fine here: the device is missing because this container was not given it, not because the host lacks GPU support.".to_owned(), + ], + ) + } else if !w.wsl_lib_dir { + ( + "/usr/lib/wsl is absent too, so this distro never had WSL GPU support wired in", + vec![ + "# Update WSL itself, then restart the distro from Windows:".to_owned(), + "# wsl --update".to_owned(), + "# wsl --shutdown".to_owned(), + ], + Vec::new(), + ) + } else { + ( + "/usr/lib/wsl is present but the device is not, which points at the Windows host driver or the WSL kernel", + vec![ + "# On the Windows host: install a WSL-capable AMD Adrenalin driver,".to_owned(), + "# then update the WSL kernel and restart the distro:".to_owned(), + "# wsl --update".to_owned(), + "# wsl --shutdown".to_owned(), + ], + vec![format!("Driver and WSL setup steps: {WSL_DOCS_URL}")], + ) + }; + evidence.push(cause.to_owned()); + + let fix = Fix { + summary: "Expose the GPU to the distro: /dev/dxg is how WSL reaches it, and nothing works until it is there.".to_owned(), + commands, + fix_id: "fix-wsl-1-gpu-not-exposed".to_owned(), + auto_applicable: false, + verify: "ls -l /dev/dxg".to_owned(), + notes, + ..Fix::default() + }; + finalize( + "fix-wsl-1-gpu-not-exposed", + "GPU not exposed to the distro (/dev/dxg missing)", + score, + evidence, + fix, + ) +} + +fn check_wsl_2_dxcore_missing(e: &Examination, symptom: &str) -> Diagnosis { + let w = wsl_facts(e); + // Without the device there is nothing for DXCore to talk to; fix-wsl-1 is the + // cause and this would only add a second finding for it. + if w.dxcore || !w.dxg_device { + return zero("fix-wsl-2-dxcore-missing", "WSL DXCore libraries missing"); + } + let mut score = 50; + let mut evidence = + vec!["/usr/lib/wsl/lib/libdxcore.so is missing, so the ROCm runtime cannot reach the host driver".to_owned()]; + if !w.wsl_lib_dir { + score += 15; + evidence.push("/usr/lib/wsl/lib does not exist at all".to_owned()); + } + let (kw_score, kw_ev) = keyword_score(symptom, KEYWORDS_WSL_LOADER); + score += kw_score; + evidence.extend(kw_ev); + + let fix = Fix { + summary: "Restore the WSL DXCore libraries, then make sure they are on the loader path." + .to_owned(), + commands: vec![ + "# From Windows, refresh the WSL runtime that ships these libraries:".to_owned(), + "# wsl --update".to_owned(), + "# wsl --shutdown".to_owned(), + "# Inside the distro, confirm the loader can see them:".to_owned(), + "echo /usr/lib/wsl/lib | sudo tee /etc/ld.so.conf.d/wsl.conf".to_owned(), + "sudo ldconfig".to_owned(), + ], + needs_sudo: true, + fix_id: "fix-wsl-2-dxcore-missing".to_owned(), + auto_applicable: false, + verify: "ls -l /usr/lib/wsl/lib/libdxcore.so && ldconfig -p | grep libdxcore".to_owned(), + notes: vec![ + "/usr/lib/wsl is mounted by WSL itself, not installed by the distro's package manager, so apt cannot repair it -- the fix is on the Windows side.".to_owned(), + ], + ..Fix::default() + }; + finalize( + "fix-wsl-2-dxcore-missing", + "WSL DXCore libraries missing or not on the loader path", + score, + evidence, + fix, + ) +} + +fn check_wsl_3_rocdxg_missing(e: &Examination, symptom: &str) -> Diagnosis { + let w = wsl_facts(e); + if w.librocdxg || !w.dxg_device { + return zero("fix-wsl-3-rocdxg-missing", "ROCDXG not installed"); + } + let mut score = 50; + let mut evidence = vec!["librocdxg.so was not found under any ROCm install".to_owned()]; + if w.dxcore { + // The host side is ready and only the distro-side package is absent, which + // is both the most common case and the one the user can fix alone. + score += 15; + evidence.push( + "the WSL DXCore handoff is present, so only the distro-side ROCDXG is missing" + .to_owned(), + ); + } + let (kw_score, kw_ev) = keyword_score(symptom, KEYWORDS_WSL_LOADER); + score += kw_score; + evidence.extend(kw_ev); + + let fix = Fix { + summary: "Install ROCDXG inside the distro: it is the ROCm-to-DXCore shim the WSL path runs on.".to_owned(), + commands: vec![ + "bash scripts/wsl_setup_rocdxg.sh".to_owned(), + "# Or, to pin the package you install:".to_owned(), + "# ROCDXG_SHA256=<64-hex-sha256> bash scripts/wsl_setup_rocdxg.sh".to_owned(), + ], + needs_sudo: true, + fix_id: "fix-wsl-3-rocdxg-missing".to_owned(), + auto_applicable: false, + verify: "ldconfig -p | grep librocdxg".to_owned(), + notes: vec![ + "This downloads and installs a .deb with sudo, so `rocm fix` prints it rather than running it. Set ROCDXG_SHA256 to verify the download against a digest you trust.".to_owned(), + ], + ..Fix::default() + }; + finalize( + "fix-wsl-3-rocdxg-missing", + "ROCDXG not installed in the distro", + score, + evidence, + fix, + ) +} + +fn check_wsl_4_rocdxg_not_linked(e: &Examination, symptom: &str) -> Diagnosis { + let w = wsl_facts(e); + // Only meaningful once the library is actually on disk: when it is not, + // fix-wsl-3 is the finding and the missing linker entry is a consequence. + // + // The device guard matches its siblings. librocdxg can be installed while + // /dev/dxg is absent, and running `ldconfig` fixes nothing then -- offering + // it beside the real cause just leaves the user to guess which to act on. + // `Some(false)` only: `None` means ldconfig could not be run, and an + // unreadable linker cache is not an unregistered library. + if !w.librocdxg || w.ldconfig_librocdxg != Some(false) || !w.dxg_device { + return zero( + "fix-wsl-4-rocdxg-not-linked", + "ROCDXG installed but not on the loader path", + ); + } + let mut score = 55; + let mut evidence = vec![ + "librocdxg.so is installed but does not appear in `ldconfig -p`, so the runtime will not load it".to_owned(), + ]; + let (kw_score, kw_ev) = keyword_score(symptom, KEYWORDS_WSL_LOADER); + score += kw_score; + evidence.extend(kw_ev); + + let fix = Fix { + summary: "Refresh the linker cache so the installed ROCDXG becomes loadable.".to_owned(), + commands: vec!["sudo ldconfig".to_owned()], + needs_sudo: true, + fix_id: "fix-wsl-4-rocdxg-not-linked".to_owned(), + auto_applicable: false, + verify: "ldconfig -p | grep librocdxg".to_owned(), + notes: vec![ + "If `ldconfig` alone does not fix it, the install went somewhere outside the linker's search path: add that directory under /etc/ld.so.conf.d/ and re-run.".to_owned(), + ], + ..Fix::default() + }; + finalize( + "fix-wsl-4-rocdxg-not-linked", + "ROCDXG installed but invisible to the dynamic linker", + score, + evidence, + fix, + ) +} + +fn check_wsl_5_distro_too_old(e: &Examination, _symptom: &str) -> Diagnosis { + let w = wsl_facts(e); + // `None` means the release could not be read. That is not evidence of an old + // distro, so this stays silent rather than guessing. + if w.distro_supported != Some(false) { + return zero("fix-wsl-5-distro-too-old", "Distro release below the floor"); + } + let (major, minor) = crate::examine::WSL_MIN_UBUNTU; + let evidence = vec![format!( + "distro is {} {}, below the {major}.{minor:02} floor the WSL path requires", + e.distro_id, e.distro_version + )]; + let fix = Fix { + summary: format!( + "Move to Ubuntu {major}.{minor:02} or newer: older releases ship a glibc the engines cannot run against." + ), + commands: vec![ + "# From Windows, install a supported distro alongside the current one:".to_owned(), + "# wsl --install -d Ubuntu-24.04".to_owned(), + ], + fix_id: "fix-wsl-5-distro-too-old".to_owned(), + auto_applicable: false, + verify: "grep VERSION_ID /etc/os-release".to_owned(), + notes: vec![ + "This is a hard floor, not a recommendation: Ubuntu 22.04 ships glibc 2.35, below the glibc 2.38 / GLIBCXX_3.4.32 that every published Lemonade embeddable is linked against, so the engine cannot start there at all.".to_owned(), + ], + ..Fix::default() + }; + finalize( + "fix-wsl-5-distro-too-old", + "Distro release is below the supported floor for WSL", + 70, + evidence, + fix, + ) +} + +fn check_wsl_6_host_driver_too_old(e: &Examination, symptom: &str) -> Diagnosis { + let w = wsl_facts(e); + // Abstain unless the host was actually reached. Treating "could not ask" as + // "driver is old" would fire on every host with interop switched off and on + // every container, where the question is unanswerable rather than answered. + // + // WSL 1 abstains too: it has no GPU path for any driver to serve, so the + // host's driver version cannot be the reason anything failed. Without this + // the symptom keywords alone were enough to raise it alongside fix-wsl-7, + // pointing the user at a Windows driver that could never have helped. + if !w.host_reachable || w.version == 1 { + return zero( + "fix-wsl-6-host-driver-too-old", + "Windows host GPU driver too old", + ); + } + let mut score = 0; + let mut evidence = Vec::new(); + match w.host_driver_version.as_deref() { + // Guarded on the device like its siblings: without /dev/dxg, fix-wsl-1 + // is the finding, and adding a second one for the same root cause just + // leaves the user choosing between them. + Some("") if w.dxg_device => { + score += 45; + evidence.push( + "the Windows host reports no AMD display adapter, so no WSL-capable AMD driver is installed there" + .to_owned(), + ); + } + // The plumbing is present but the runtime still cannot see a GPU, which + // on a machine with a working DXCore handoff points at the host driver. + // + // `rocm_sees_gpu`, not `has_amd_gpu`: the probes that populate the latter + // are skipped on WSL, so it reads false on every host here, healthy or + // not, and this check fired on a complete working stack. `Some(false)` + // specifically -- `None` means rocminfo was absent so the question went + // unasked, which is not evidence of anything. + Some(version) + if w.dxg_device && w.dxcore && w.librocdxg && w.rocm_sees_gpu == Some(false) => + { + score += 40; + evidence.push(format!( + "host AMD display driver {version} is installed and the distro plumbing is complete, but rocminfo enumerates no GPU" + )); + } + Some(_) | None => {} + } + + // Symptom keywords only ever CORROBORATE machine evidence here; they cannot + // stand alone. Every other WSL entry either carries a base score from a fact + // or returns before scoring keywords, so this was the one place where the + // words a user typed could clear the threshold by themselves -- and the + // table it shares with fix-wsl-1 is generic enough ("/dev/dxg", "no + // HIP-capable device") that an ordinary description of any GPU failure hit + // 85 on a completely healthy machine, outranking the check that had found + // the real fault. The WSL-1 guard above was one instance of this; the floor + // is the general fix. + if score <= 0 { + return zero( + "fix-wsl-6-host-driver-too-old", + "Windows host GPU driver too old", + ); + } + let (kw_score, kw_ev) = keyword_score(symptom, KEYWORDS_WSL_NO_DEVICE); + score += kw_score; + evidence.extend(kw_ev); + let fix = Fix { + summary: "Update the AMD driver on the Windows host: the GPU driver WSL uses lives there, not in the distro.".to_owned(), + commands: vec![ + "# On the Windows host, not in this distro:".to_owned(), + "# install a WSL-capable AMD Adrenalin driver, then `wsl --shutdown`.".to_owned(), + ], + fix_id: "fix-wsl-6-host-driver-too-old".to_owned(), + auto_applicable: false, + verify: "rocminfo | head -n 20".to_owned(), + notes: vec![ + format!("Driver and ROCm version pairing: {WSL_DOCS_URL}"), + "Nothing inside the distro can carry this out -- `rocm fix` prints the steps because the change belongs to the Windows host.".to_owned(), + ], + ..Fix::default() + }; + finalize( + "fix-wsl-6-host-driver-too-old", + "Windows host AMD driver missing or too old for ROCm on WSL", + score, + evidence, + fix, + ) +} + +fn check_wsl_7_wsl1(e: &Examination, _symptom: &str) -> Diagnosis { + if wsl_facts(e).version != 1 { + return zero("fix-wsl-7-wsl1", "Distro is running under WSL 1"); + } + let evidence = vec![format!( + "kernel '{}' is a WSL 1 kernel; WSL 1 translates syscalls and exposes no GPU device at all", + e.kernel_release + )]; + let fix = Fix { + summary: "Convert the distro to WSL 2: WSL 1 has no GPU path, so no amount of driver or package work will help.".to_owned(), + commands: vec![ + "# From Windows PowerShell:".to_owned(), + "# wsl --set-version 2".to_owned(), + "# wsl --set-default-version 2".to_owned(), + ], + fix_id: "fix-wsl-7-wsl1".to_owned(), + auto_applicable: false, + verify: "uname -r".to_owned(), + notes: vec![ + "Converting rewrites the distro's filesystem and can take a while on a large install; back up anything you cannot lose first.".to_owned(), + ], + ..Fix::default() + }; + finalize( + "fix-wsl-7-wsl1", + "Distro is running under WSL 1, which has no GPU support", + 80, + evidence, + fix, + ) +} + /// A checker plus the OS families it applies to. type Checker = (fn(&Examination, &str) -> Diagnosis, &'static [&'static str]); +// The `wsl` entries below run on less evidence than on bare metal: WSL collects +// no GPU topology (`gpus` is empty, because the probes that fill it read KFD and +// DRM), so the parts of those checks that compare a detected gfx target against a +// wheel's arch list cannot contribute. They still fire on the evidence WSL does +// have -- environment, ROCm install, symptom keywords -- which is strictly better +// than the previous behaviour of not running at all. +// +// The degradation is one-directional and must stay that way: less evidence means +// a check may miss a real fault, never that it invents one. Pinned by +// `the_shared_checks_under_report_on_wsl_rather_than_over_report`. const CHECKERS: &[Checker] = &[ - (check_1_arch_not_in_wheel, &["linux", "windows"]), - (check_2_hsa_override_unneeded, &["linux", "windows"]), + (check_1_arch_not_in_wheel, &["linux", "windows", "wsl"]), + (check_2_hsa_override_unneeded, &["linux", "windows", "wsl"]), (check_3_rocm_kernel_unsupported, &["linux"]), (check_4_render_group, &["linux"]), (check_5_amdgpu_blacklisted, &["linux"]), - (check_6_path_missing, &["linux", "windows"]), + (check_6_path_missing, &["linux", "windows", "wsl"]), (check_7_stale_repos, &["linux"]), - (check_8_wheel_rocm_mismatch, &["linux", "windows"]), + (check_8_wheel_rocm_mismatch, &["linux", "windows", "wsl"]), + // Not "wsl": WSL2 exposes no per-device topology to collide over. (check_9_igpu_dgpu_collision, &["linux", "windows"]), (check_10_container_devices, &["linux"]), (check_11_iommu_hang, &["linux"]), @@ -1402,19 +1839,48 @@ const CHECKERS: &[Checker] = &[ // out-of-memory entry reserves 16 on its own branch; the number is a stable // handle, so the two do not get to share one. (check_17_torch_dlpack_cuda_variant, &["linux"]), + (check_wsl_1_gpu_not_exposed, WSL_ONLY), + (check_wsl_2_dxcore_missing, WSL_ONLY), + (check_wsl_3_rocdxg_missing, WSL_ONLY), + (check_wsl_4_rocdxg_not_linked, WSL_ONLY), + (check_wsl_5_distro_too_old, WSL_ONLY), + (check_wsl_6_host_driver_too_old, WSL_ONLY), + (check_wsl_7_wsl1, WSL_ONLY), ]; +const WSL_ONLY: &[&str] = &["wsl"]; + +/// The platform family a catalog entry is selected by. +/// +/// This is deliberately NOT `Examination::os_family`. WSL2 reports an `os_family` +/// of `linux` and must keep doing so — `install`, `serve` and the engine crates +/// branch on it — but it is a different platform for diagnosis: it reaches the +/// GPU through `/dev/dxg` and the Windows host driver, with no `amdgpu` module +/// and no `/dev/kfd`. +/// +/// Resolving the family here rather than in each check is what makes the split +/// safe. Every bare-metal entry is tagged `linux` only, so it stops applying on +/// WSL automatically; an entry that genuinely applies to both opts in by naming +/// `wsl` as well. That preserves the property the old wholesale WSL short-circuit +/// bought — no `fix-4-render-group` on a healthy WSL box — without also +/// suppressing the checks that were always valid there. +const fn platform_family(e: &Examination) -> &str { + if e.is_wsl { + return "wsl"; + } + if e.os_family.is_empty() { + return "linux"; + } + e.os_family.as_str() +} + /// Run every applicable checker, drop zero-score results, sort by score /// descending (stable, so ties keep catalog order). fn run_all_checks(e: &Examination, symptom: &str) -> Vec { - let os_family = if e.os_family.is_empty() { - "linux" - } else { - e.os_family.as_str() - }; + let family = platform_family(e); let mut results: Vec = CHECKERS .iter() - .filter(|(_, applicable)| applicable.contains(&os_family)) + .filter(|(_, applicable)| applicable.contains(&family)) .map(|(check, _)| check(e, symptom)) .filter(|d| d.score > 0) .collect(); @@ -1423,6 +1889,14 @@ fn run_all_checks(e: &Examination, symptom: &str) -> Vec { results } +/// Whether any catalog entry at all applies to this platform. +fn catalog_covers(e: &Examination) -> bool { + let family = platform_family(e); + CHECKERS + .iter() + .any(|(_, applicable)| applicable.contains(&family)) +} + fn route_when_no_match(e: &Examination) -> Route { let target = match e.framework.as_str() { "pytorch" => "pytorch", @@ -1441,12 +1915,15 @@ fn route_when_no_match(e: &Examination) -> Route { /// Diagnose an examination against the closed catalog. #[must_use] pub fn diagnose(e: &Examination, symptom: &str) -> DiagnoseReport { - // WSL2 is a distinct platform (it uses /dev/dxg + the Windows host driver, - // not the in-tree amdgpu module or /dev/kfd), and `examine` already treats - // it as out of scope (exit_code() == 2). Mirror that here: skip the - // bare-metal Linux catalog entirely so we don't emit false positives like - // fix-4-render-group / fix-5-amdgpu-load on a healthy WSL2 box. - let out_of_scope = wsl_out_of_scope_message(e); + // WSL2 used to be short-circuited here as out of scope. It is a real platform + // in the catalog now, with its own entries; what keeps the bare-metal checks + // off it is `platform_family`, not a special case at this level. + // + // `out_of_scope` still exists, for the platforms that genuinely have no + // entries. That case used to fall through to an empty catalog and report "no + // known misconfiguration", which reads as "your machine looks fine" when the + // truth is that nothing was ever checked. + let out_of_scope = uncovered_platform_message(e); let matched = if out_of_scope.is_some() { Vec::new() } else { @@ -1465,15 +1942,19 @@ pub fn diagnose(e: &Examination, symptom: &str) -> DiagnoseReport { /// ROCm-on-WSL2 setup guidance (distinct from the bare-metal catalog). const WSL_DOCS_URL: &str = "https://rocm.docs.amd.com/projects/radeon-ryzen/en/latest/docs/install/installryz/wsl/howto_wsl.html"; -fn wsl_out_of_scope_message(e: &Examination) -> Option { - e.is_wsl.then(|| { - format!( - "ROCm on WSL2 is a distinct platform: it uses /dev/dxg and the Windows host \ - driver (dxgkrnl), not the in-tree amdgpu kernel module or /dev/kfd. This catalog \ - targets bare-metal Linux, so its checks (render group, /dev/kfd, modprobe amdgpu) \ - do not apply here. For ROCm-on-WSL2 setup, see {WSL_DOCS_URL}" - ) - }) +/// Say so when the catalog has no entries for the running platform. +/// +/// Returning `None` here means the platform is covered, not that it is healthy. +fn uncovered_platform_message(e: &Examination) -> Option { + if catalog_covers(e) { + return None; + } + Some(format!( + "rocm diagnose covers Linux, Windows and WSL2. This host reports '{}', which the \ + catalog has no entries for, so nothing was checked -- this is not a clean bill of \ + health. Run `rocm examine --json` and report the platform upstream.", + e.os_family + )) } /// Render the human-facing diagnosis view (mirrors `diagnose.py`'s text output). @@ -1953,29 +2434,474 @@ mod tests { assert!(report.matched.iter().all(|d| d.id != "fix-1-arch")); } - #[test] - fn wsl2_is_out_of_scope_and_emits_no_false_positives() { + /// A WSL2 host with the GPU stack fully in place. + fn wsl_base() -> Examination { let mut e = linux_base(); e.is_wsl = true; - // Signals that WOULD fire fix-4/fix-5/fix-3/fix-6 on bare-metal Linux — - // all normal/irrelevant on WSL2, so none must surface. + e.distro_id = "ubuntu".to_owned(); + e.distro_version = "24.04".to_owned(); + e.kernel_release = "6.6.87.2-microsoft-standard-WSL2".to_owned(); + e.wsl = Some(WslFacts { + version: 2, + dxg_device: true, + dxcore: true, + wsl_lib_dir: true, + librocdxg: true, + rocdxg_dids: true, + ldconfig_librocdxg: Some(true), + rocminfo: true, + rocm_sees_gpu: Some(true), + distro_supported: Some(true), + host_driver_version: Some("32.0.12033.1030".to_owned()), + host_reachable: true, + locally_probed: true, + }); + // The shared cross-platform checks read these, and on a real WSL host + // `probe_wsl` fills them from the WSL facts. A fixture that left them at + // their defaults would be a healthier machine than any real one. + e.rocminfo_present = true; + e.rocminfo_status = "ok".to_owned(); + e + } + + #[test] + fn wsl2_never_runs_the_bare_metal_catalog() { + // The property the old wholesale WSL short-circuit bought, kept after + // WSL became a real platform in the catalog. Every signal below WOULD + // fire a bare-metal check; none of them mean anything on WSL2, where + // there is no amdgpu module, no /dev/kfd and no render group. + let mut e = wsl_base(); e.in_render_group = Some(false); e.in_video_group = Some(false); e.amdgpu_loaded = Some(false); e.rocm_version = "6.4.1".to_owned(); - e.rocm_path = "/opt/rocm".to_owned(); - e.rocminfo_present = false; + e.amdgpu_blacklisted_in = vec!["/etc/modprobe.d/blacklist.conf".to_owned()]; + e.rocm_repos_seen = vec![ + "repo.radeon.com/rocm/6.2".to_owned(), + "repo.radeon.com/rocm/6.4".to_owned(), + ]; let report = diagnose(&e, "unable to open /dev/kfd permission denied"); + for bare_metal in [ + "fix-3-rocm-kernel", + "fix-4-render-group", + "fix-5-amdgpu-load", + "fix-7-stale-repos", + "fix-10-container", + "fix-11-iommu", + "fix-12-installer", + ] { + assert!( + !report.matched.iter().any(|d| d.id == bare_metal), + "{bare_metal} must not fire on WSL2" + ); + } + } + + #[test] + fn wsl_checks_never_fire_on_bare_metal() { + // The converse guard. A bare-metal host has no WSL facts at all, and the + // WSL checks read those facts as false -- so without the family gate they + // would report a missing /dev/dxg on every ordinary Linux box. + let mut e = linux_base(); + e.in_render_group = Some(false); + let report = diagnose(&e, "no ROCm-capable device is detected"); + assert!( + !report.matched.iter().any(|d| d.id.starts_with("fix-wsl-")), + "no WSL entry may fire on bare metal: {:?}", + report.matched.iter().map(|d| &d.id).collect::>() + ); + } + + #[test] + fn a_healthy_wsl_host_gets_no_diagnosis() { + // Scenario 2: the platform is covered, so `out_of_scope` stays clear, and + // a symptom the catalog does not recognise must not manufacture a cause. + let report = diagnose(&wsl_base(), "the model output looks wrong"); + assert!( + report.out_of_scope.is_none(), + "WSL2 is a covered platform now" + ); + // Emptiness, not just `!has_match()`. A sub-threshold finding is still + // printed and still tells the user something is wrong with their machine, + // so a healthy host must produce NO entries at all. Asserting only on the + // verdict let a permanent score-40 false positive through: the host-driver + // check read a field the WSL probe never populates. assert!( report.matched.is_empty(), - "WSL2 must not run the bare-metal catalog" + "a healthy WSL host must produce no findings at all, got: {:?}", + report + .matched + .iter() + .map(|d| (&d.id, d.score)) + .collect::>() + ); + assert!(!report.has_match()); + assert!(!report.route_when_no_match.url.is_empty()); + } + + #[test] + fn an_installed_rocm_is_not_reported_as_missing_from_path_on_wsl() { + // `rocminfo_present` is set by the bare-metal GPU probe, which WSL skips. + // Left at its default it read as "rocminfo is not on PATH", so fix-6 -- + // enabled on WSL because PATH problems are real there -- scored 50 on + // every WSL host that had ROCm installed. + let mut e = wsl_base(); + e.rocm_path = "/opt/rocm".to_owned(); + e.rocminfo_present = true; + e.env + .insert("PATH".to_owned(), "/opt/rocm/bin:/usr/bin:/bin".to_owned()); + let report = diagnose(&e, ""); + assert!( + !report.matched.iter().any(|d| d.id == "fix-6-path"), + "ROCm is installed and on PATH here: {:?}", + report.matched + ); + } + + #[test] + fn an_unlinked_rocdxg_is_not_reported_when_the_device_is_missing() { + // librocdxg can be installed while /dev/dxg is absent. Running `ldconfig` + // then fixes nothing, and offering it alongside the real cause leaves the + // user to guess which to act on -- the same reason the DXCore and ROCDXG + // checks already stand down without the device. + let mut e = wsl_base(); + let w = e.wsl.as_mut().expect("wsl facts"); + w.dxg_device = false; + w.ldconfig_librocdxg = Some(false); + let report = diagnose(&e, ""); + let ids: Vec<&str> = report.matched.iter().map(|d| d.id.as_str()).collect(); + assert_eq!(ids, vec!["fix-wsl-1-gpu-not-exposed"], "ids: {ids:?}"); + } + + #[test] + fn a_missing_dxg_device_is_the_top_finding() { + let mut e = wsl_base(); + e.wsl.as_mut().expect("wsl facts").dxg_device = false; + let report = diagnose(&e, "no ROCm-capable device is detected"); + let top = &report.matched[0]; + assert_eq!(top.id, "fix-wsl-1-gpu-not-exposed"); + assert!(top.score >= HIGH_CONFIDENCE, "score {}", top.score); + } + + #[test] + fn a_container_without_the_device_is_not_blamed_on_the_windows_driver() { + // A container on WSL2 reports itself as WSL but only sees /dev/dxg when + // it was started with it. Telling that user to update a Windows driver + // sends them to fix a machine that was never broken. + let mut e = wsl_base(); + e.wsl.as_mut().expect("wsl facts").dxg_device = false; + e.in_container = true; + e.container_kind = "docker".to_owned(); + let report = diagnose(&e, ""); + let top = &report.matched[0]; + assert_eq!(top.id, "fix-wsl-1-gpu-not-exposed"); + assert!( + top.evidence.iter().any(|line| line.contains("container")), + "evidence must name the container as the reason: {:?}", + top.evidence ); + let fix = top.fix.as_ref().expect("carries a fix"); + assert!( + fix.notes + .iter() + .any(|n| n.contains("host driver is probably fine")), + "must say the Windows driver is not the suspect: {:?}", + fix.notes + ); + } + + #[test] + fn only_the_root_cause_of_a_broken_stack_is_reported() { + // With no device, the missing DXCore shim and the missing ROCDXG package + // are consequences, not causes. Reporting all three would leave the user + // to guess which one to act on. + let mut e = wsl_base(); + let w = e.wsl.as_mut().expect("wsl facts"); + w.dxg_device = false; + w.dxcore = false; + w.librocdxg = false; + w.ldconfig_librocdxg = Some(false); + let report = diagnose(&e, ""); + let ids: Vec<&str> = report.matched.iter().map(|d| d.id.as_str()).collect(); + assert_eq!(ids, vec!["fix-wsl-1-gpu-not-exposed"], "ids: {ids:?}"); + } + + #[test] + fn a_missing_rocdxg_package_is_reported_when_the_host_side_is_ready() { + let mut e = wsl_base(); + let w = e.wsl.as_mut().expect("wsl facts"); + w.librocdxg = false; + w.ldconfig_librocdxg = Some(false); + let report = diagnose(&e, ""); + let top = &report.matched[0]; + assert_eq!(top.id, "fix-wsl-3-rocdxg-missing"); + let fix = top.fix.as_ref().expect("carries a fix"); + assert!( + !fix.auto_applicable, + "installing a .deb with sudo must stay print-only" + ); + assert!( + fix.notes.iter().any(|n| n.contains("ROCDXG_SHA256")), + "must offer the checksum option: {:?}", + fix.notes + ); + } + + #[test] + fn an_installed_but_unlinked_rocdxg_is_a_distinct_finding() { + let mut e = wsl_base(); + e.wsl.as_mut().expect("wsl facts").ldconfig_librocdxg = Some(false); + let report = diagnose(&e, "librocdxg.so: cannot open shared object file"); + let top = &report.matched[0]; + assert_eq!(top.id, "fix-wsl-4-rocdxg-not-linked"); + } + + #[test] + fn a_distro_below_the_floor_is_reported() { + let mut e = wsl_base(); + e.distro_version = "22.04".to_owned(); + e.wsl.as_mut().expect("wsl facts").distro_supported = Some(false); + let report = diagnose(&e, ""); + let top = &report.matched[0]; + assert_eq!(top.id, "fix-wsl-5-distro-too-old"); assert!( - report.out_of_scope.is_some(), - "WSL2 should be flagged out of scope" + top.evidence[0].contains("22.04"), + "evidence must name the release found: {:?}", + top.evidence ); + } + + #[test] + fn an_unreadable_distro_release_is_not_reported_as_too_old() { + // `None` means the release could not be parsed. That is not evidence of + // an old distro, and a finding here would send the user to reinstall a + // perfectly supported one. + let mut e = wsl_base(); + e.wsl.as_mut().expect("wsl facts").distro_supported = None; + let report = diagnose(&e, ""); + assert!( + !report + .matched + .iter() + .any(|d| d.id == "fix-wsl-5-distro-too-old"), + "must not guess: {:?}", + report.matched + ); + } + + #[test] + fn a_typed_symptom_alone_never_blames_the_windows_host_driver() { + // The host-driver check shares a generic keyword table with fix-wsl-1 + // ("/dev/dxg", "no HIP-capable device"), and unlike every other WSL entry + // it has no base score from a fact. Scoring keywords before checking that + // anything was actually measured let an ordinary description of a GPU + // problem reach 85 -- HIGH confidence -- on a fully healthy machine, + // with both evidence lines being the user's own words. + let symptom = "no hip-capable device found, see /dev/dxg"; + let report = diagnose(&wsl_base(), symptom); + assert!( + report.matched.is_empty(), + "a healthy host must stay silent whatever the user typed: {:?}", + report + .matched + .iter() + .map(|d| (&d.id, d.score)) + .collect::>() + ); + + // And the real cause must outrank a keyword-only guess. With ROCDXG + // missing, the driver check previously scored 85 against fix-wsl-3's 65 + // and sent the user to reinstall a current driver. + let mut broken = wsl_base(); + let w = broken.wsl.as_mut().expect("wsl facts"); + w.librocdxg = false; + w.ldconfig_librocdxg = Some(false); + let report = diagnose(&broken, symptom); + assert_eq!( + report.matched[0].id, + "fix-wsl-3-rocdxg-missing", + "the measured fault must rank first: {:?}", + report + .matched + .iter() + .map(|d| (&d.id, d.score)) + .collect::>() + ); + } + + #[test] + fn an_unreadable_linker_cache_is_not_an_unregistered_library() { + // `ldconfig` lives in /sbin, off a non-root user's PATH on Debian. When + // it cannot be run the cache is unknown, not empty -- reading it as empty + // told users with a correctly installed ROCDXG to re-run `ldconfig`. + let mut e = wsl_base(); + e.wsl.as_mut().expect("wsl facts").ldconfig_librocdxg = None; + let report = diagnose(&e, ""); + assert!( + !report + .matched + .iter() + .any(|d| d.id == "fix-wsl-4-rocdxg-not-linked"), + "unknown must not be reported as not-linked: {:?}", + report.matched + ); + } + + #[test] + fn a_host_with_no_amd_adapter_is_not_reported_twice_with_the_device_missing() { + // Without /dev/dxg, fix-wsl-1 is the cause. The driver check's + // no-adapter arm lacked the device guard its siblings carry, so both + // fired for one root cause. + let mut e = wsl_base(); + let w = e.wsl.as_mut().expect("wsl facts"); + w.dxg_device = false; + w.host_driver_version = Some(String::new()); + let report = diagnose(&e, ""); + let ids: Vec<&str> = report.matched.iter().map(|d| d.id.as_str()).collect(); + assert_eq!(ids, vec!["fix-wsl-1-gpu-not-exposed"], "ids: {ids:?}"); + } + + #[test] + fn a_missing_dxcore_shim_is_reported_when_the_device_is_present() { + // The only WSL entry with no fire-case test: both existing tests that + // clear `dxcore` also clear `dxg_device`, so they exercised the abstain + // path and an inverted condition here would have passed CI. + let mut e = wsl_base(); + let w = e.wsl.as_mut().expect("wsl facts"); + w.dxcore = false; + w.wsl_lib_dir = false; + let report = diagnose(&e, "libdxcore.so: cannot open shared object file"); + let top = &report.matched[0]; + assert_eq!(top.id, "fix-wsl-2-dxcore-missing"); + assert!(top.score >= MIN_SCORE_FOR_MATCH, "score {}", top.score); + } + + #[test] + fn an_unreachable_windows_host_never_blames_the_host_driver() { + // Scenario 6. Interop is off or this is a container, so the host driver + // is unknown -- and unknown must not read as "too old". + let mut e = wsl_base(); + let w = e.wsl.as_mut().expect("wsl facts"); + w.host_reachable = false; + w.host_driver_version = None; + let report = diagnose(&e, "no ROCm-capable device is detected"); + assert!( + !report + .matched + .iter() + .any(|d| d.id == "fix-wsl-6-host-driver-too-old"), + "the check must abstain when the host was never asked: {:?}", + report.matched + ); + } + + #[test] + fn a_host_with_no_amd_adapter_is_reported() { + let mut e = wsl_base(); + e.wsl.as_mut().expect("wsl facts").host_driver_version = Some(String::new()); + let report = diagnose(&e, ""); + let top = &report.matched[0]; + assert_eq!(top.id, "fix-wsl-6-host-driver-too-old"); + let fix = top.fix.as_ref().expect("carries a fix"); + assert!( + fix.notes + .iter() + .any(|n| n.contains("Nothing inside the distro")), + "must say the remedy belongs to the Windows host: {:?}", + fix.notes + ); + } + + #[test] + fn wsl1_is_reported_instead_of_a_missing_device() { + // WSL 1 has no GPU path at all, so "install a driver" is the wrong advice + // and fix-wsl-1 must stand down in favour of the conversion. + let mut e = wsl_base(); + e.kernel_release = "4.4.0-19041-Microsoft".to_owned(); + let w = e.wsl.as_mut().expect("wsl facts"); + w.version = 1; + w.dxg_device = false; + w.dxcore = false; + w.wsl_lib_dir = false; + w.librocdxg = false; + w.ldconfig_librocdxg = Some(false); + let report = diagnose(&e, "no ROCm-capable device is detected"); + let ids: Vec<&str> = report.matched.iter().map(|d| d.id.as_str()).collect(); + assert_eq!(ids, vec!["fix-wsl-7-wsl1"], "ids: {ids:?}"); + } + + #[test] + fn the_shared_checks_under_report_on_wsl_rather_than_over_report() { + // WSL does not collect GPU topology: `gpus` stays empty because the + // probes that fill it read KFD and DRM, which do not exist there. The + // cross-platform checks enabled on WSL read those fields, so they run on + // less evidence here than on bare metal. + // + // That degradation has to be one-directional. Missing topology must mean + // a check cannot reach its threshold on structure alone -- never that it + // invents a fault. This pins the direction: on a healthy host with a + // framework installed, no shared check may fire at all. + let mut e = wsl_base(); + e.framework = "pytorch".to_owned(); + e.framework_version = "2.6.0".to_owned(); + e.framework_rocm_version = "6.4".to_owned(); + e.rocm_version = "6.4.1".to_owned(); + e.framework_arch_list = vec!["gfx1100".to_owned(), "gfx1151".to_owned()]; + e.rocm_path = "/opt/rocm".to_owned(); + e.env + .insert("PATH".to_owned(), "/opt/rocm/bin:/usr/bin".to_owned()); + assert!(e.gpus.is_empty(), "WSL collects no GPU topology"); + + let report = diagnose(&e, ""); + assert!( + report.matched.is_empty(), + "no shared check may fire without topology: {:?}", + report + .matched + .iter() + .map(|d| (&d.id, d.score)) + .collect::>() + ); + } + + #[test] + fn an_environment_override_is_still_diagnosed_on_wsl() { + // The gain from the family split: HSA_OVERRIDE_GFX_VERSION has nothing to + // do with the kernel module, so it was always a valid question on WSL -- + // but the old wholesale skip meant it went unanswered there. + let mut e = wsl_base(); + e.env + .insert("HSA_OVERRIDE_GFX_VERSION".to_owned(), "11.0.0".to_owned()); + let report = diagnose(&e, "memory access fault page fault"); + assert!( + report + .matched + .iter() + .any(|d| d.id == "fix-2-unset-override"), + "matched: {:?}", + report.matched.iter().map(|d| &d.id).collect::>() + ); + } + + #[test] + fn a_platform_with_no_catalog_entries_says_nothing_was_checked() { + // Previously only WSL set this. An unsupported OS fell through to an + // empty catalog and reported "no known misconfiguration", which reads as + // a clean bill of health when in truth nothing ran. + let mut e = linux_base(); + e.os_family = "other".to_owned(); + let report = diagnose(&e, "anything at all"); + let reason = report + .out_of_scope + .as_deref() + .expect("an uncovered platform must say so"); + assert!(reason.contains("other"), "must name the platform: {reason}"); + assert!( + reason.contains("not a clean bill of health"), + "must not be mistaken for a pass: {reason}" + ); + assert!(report.matched.is_empty()); assert!(!report.has_match()); - assert!(report.out_of_scope.as_deref().unwrap().contains("WSL2")); } #[test] diff --git a/crates/rocm-core/src/examine.rs b/crates/rocm-core/src/examine.rs index 7db0c67b7..39ebd1420 100644 --- a/crates/rocm-core/src/examine.rs +++ b/crates/rocm-core/src/examine.rs @@ -29,6 +29,8 @@ const ENV_VALUE_MAX_CHARS: usize = 16_000; const TRACKED_ENV_VARS: &[&str] = &[ "HSA_OVERRIDE_GFX_VERSION", + // Legacy ROCm releases need this to find the GPU through DXG under WSL. + "HSA_ENABLE_DXG_DETECTION", "HIP_VISIBLE_DEVICES", "ROCR_VISIBLE_DEVICES", "CUDA_VISIBLE_DEVICES", @@ -93,9 +95,70 @@ pub struct Device { pub user_can_write: Option, } -/// Structured machine state consumed by the diagnosis catalog. Field order and -/// names mirror `examine.py`'s `Examination` dataclass so the JSON contract is -/// identical. +/// The oldest distro release the WSL path supports. +/// +/// Ubuntu 22.04 ships glibc 2.35, below the glibc 2.38 / `GLIBCXX_3.4.32` floor +/// every published Lemonade embeddable is linked against, so the engine cannot +/// start there. See `docs/wsl.md`. +pub const WSL_MIN_UBUNTU: (u32, u32) = (24, 4); + +/// WSL2-specific machine state. `None` on every other platform. +/// +/// WSL reaches the GPU through `/dev/dxg` and the Windows host driver rather than +/// the in-tree `amdgpu` module, so none of the bare-metal driver fields describe +/// it. These are the facts the WSL half of the catalog reasons over. +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +pub struct WslFacts { + /// `1` or `2`. A kernel release that names neither is read as `2`: WSL 2 has + /// been the default for years, and the cost of the two errors is not + /// symmetric — calling a WSL 2 host "WSL 1" tells the user to convert a + /// distro that is already converted. `0` only in the default value, which + /// stands for "the probe did not run". + pub version: u8, + pub dxg_device: bool, + pub dxcore: bool, + pub wsl_lib_dir: bool, + pub librocdxg: bool, + pub rocdxg_dids: bool, + /// Whether the linker cache lists ROCDXG. + /// + /// `None` when `ldconfig` could not be run at all — on Debian and its + /// derivatives it lives in `/sbin`, off a non-root user's `PATH`. An + /// unreadable cache is not an unregistered library, and reporting it as one + /// told users with a working install to re-run `ldconfig`. + pub ldconfig_librocdxg: Option, + /// Whether `rocminfo` is on PATH. + pub rocminfo: bool, + /// Whether ROCm can actually enumerate a GPU here. + /// + /// `None` when `rocminfo` is absent, so the question could not be asked. This + /// is the only WSL-collected evidence that the plumbing is complete yet no + /// device is reachable, which is what distinguishes an out-of-date Windows + /// host driver from a distro-side fault. The bare-metal `has_amd_gpu` cannot + /// stand in: the probes that populate it are skipped here, so it is always + /// false on WSL and reads as "no GPU" on a perfectly healthy machine. + pub rocm_sees_gpu: Option, + /// `None` when the distro release could not be parsed, which fails closed: + /// an unreadable release is not evidence of a supported one. + pub distro_supported: Option, + /// `None` when WSL interop could not reach the Windows host — distinct from + /// a host that answered and reported no AMD adapter, which is `Some("")`. + pub host_driver_version: Option, + pub host_reachable: bool, + /// Whether these facts were gathered from inside the distribution. + /// + /// `false` when inspected from the Windows host over `wsl.exe`, which sees + /// the GPU stack but no environment — so the checks that read one cannot + /// run, and a caller must say so rather than let "nothing matched" read as + /// a clean bill of health. + pub locally_probed: bool, +} + +/// Structured machine state consumed by the diagnosis catalog. +/// +/// Field order and names mirror `examine.py`'s `Examination` dataclass so the +/// JSON contract is identical, except for the `wsl` section, which has no +/// `examine.py` analogue. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Examination { // platform @@ -106,6 +169,8 @@ pub struct Examination { pub kernel_release: String, pub kernel_cmdline: String, pub is_wsl: bool, + /// Populated only when `is_wsl`; see [`WslFacts`]. + pub wsl: Option, // hardware pub cpu_vendor: String, @@ -183,6 +248,7 @@ impl Default for Examination { kernel_release: String::new(), kernel_cmdline: String::new(), is_wsl: false, + wsl: None, cpu_vendor: "unknown".to_owned(), cpu_model: String::new(), gpus: Vec::new(), @@ -230,9 +296,14 @@ impl Default for Examination { } } -/// Route-out guidance shown when WSL2 is detected (out of scope for this -/// catalog, which targets bare-metal Linux). Mirrors `examine.py`. -pub const WSL_ROUTE_OUT_NOTE: &str = "Detected WSL2. rocm examine does not cover the ROCm-on-WSL flow (it requires Adrenalin Pro + the WSL kernel update on the Windows host). Either run `rocm examine` on the native Linux host, or follow AMD's WSL guide directly: https://rocm.docs.amd.com/projects/radeon-ryzen/en/latest/docs/install/installryz/wsl/howto_wsl.html"; +/// Guidance shown when WSL2 is detected. +/// +/// Named for what it does. It used to be a route-out — `rocm examine` did not +/// cover the ROCm-on-WSL flow and sent the user elsewhere — and it kept that +/// name for a while after it stopped routing anyone anywhere. It now explains +/// which checks are skipped on this platform and points at the one that covers +/// it. +pub const WSL_PLATFORM_NOTE: &str = "Detected WSL2. The GPU is reached through /dev/dxg and the Windows host driver, so the bare-metal driver checks do not apply and are skipped. Run `rocm diagnose` for the WSL-specific checks. Setup guide: https://rocm.docs.amd.com/projects/radeon-ryzen/en/latest/docs/install/installryz/wsl/howto_wsl.html"; /// Which framework probe to run. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -252,20 +323,23 @@ impl Examination { let mut e = Self::default(); probe_os(&mut e); if e.is_wsl { - // WSL2 is out of scope for the *driver* probes: it uses /dev/dxg and - // the Windows host driver, not the in-tree amdgpu module or - // /dev/kfd, so asking about modprobe, the render group or /dev/kfd - // would only mislead. The "wsl" status carries that verdict. + // WSL2 keeps the *driver* probes skipped: it reaches the GPU through + // /dev/dxg and the Windows host driver, not the in-tree amdgpu module + // or /dev/kfd, so asking about modprobe, the render group or + // /dev/kfd would only mislead. // - // The frameworks are a different matter. PyTorch on WSL2 is a - // supported, documented configuration, and stopping before the - // framework probe meant `--json` could never tell a WSL user which - // ROCm build their torch was compiled against -- a question that has - // nothing to do with the kernel module. So run that one, and only - // that one, before routing out. + // Everything else applies. This used to return here after the + // framework probe alone, which left `env`, the container fields and + // the ROCm install at their defaults -- so the WSL half of the + // catalog had nothing to read and questions with no kernel-module + // component, like "is HSA_OVERRIDE_GFX_VERSION set", went unanswered + // on the one platform most likely to need them. + probe_wsl(&mut e); + probe_rocm_install(&mut e); + probe_env(&mut e); + probe_container(&mut e); probe_framework(&mut e, framework); - e.notes.push(WSL_ROUTE_OUT_NOTE.to_owned()); - e.status = "wsl".to_owned(); + e.status = e.compute_status(); return e; } if e.os_family == "linux" { @@ -328,6 +402,16 @@ impl Examination { /// Run a command with a timeout. Returns `(rc, stdout, stderr)`. `rc` is `127` /// when the program can't be spawned and `124` on timeout. pub(crate) fn run(program: &str, args: &[&str], timeout: Duration) -> (i32, String, String) { + let (rc, stdout, stderr) = run_raw(program, args, timeout); + ( + rc, + String::from_utf8_lossy(&stdout).into_owned(), + String::from_utf8_lossy(&stderr).into_owned(), + ) +} + +/// [`run`] without the UTF-8 assumption, for output that is not UTF-8. +fn run_raw(program: &str, args: &[&str], timeout: Duration) -> (i32, Vec, Vec) { let Ok(mut child) = Command::new(program) .args(args) .stdin(Stdio::null()) @@ -335,19 +419,23 @@ pub(crate) fn run(program: &str, args: &[&str], timeout: Duration) -> (i32, Stri .stderr(Stdio::piped()) .spawn() else { - return (127, String::new(), String::new()); + return (127, Vec::new(), Vec::new()); }; + // Bytes, then a lossy conversion at the end. `read_to_string` FAILS on + // invalid UTF-8 and the error was discarded, so a single stray byte silently + // emptied the whole capture — which reads downstream as "the command printed + // nothing", not as "the output could not be decoded". let stdout_handle = child.stdout.take().map(|mut stdout| { thread::spawn(move || { - let mut buf = String::new(); - let _ = stdout.read_to_string(&mut buf); + let mut buf = Vec::new(); + let _ = stdout.read_to_end(&mut buf); buf }) }); let stderr_handle = child.stderr.take().map(|mut stderr| { thread::spawn(move || { - let mut buf = String::new(); - let _ = stderr.read_to_string(&mut buf); + let mut buf = Vec::new(); + let _ = stderr.read_to_end(&mut buf); buf }) }); @@ -366,10 +454,10 @@ pub(crate) fn run(program: &str, args: &[&str], timeout: Duration) -> (i32, Stri Err(_) => break None, } }; - let stdout = stdout_handle + let stdout: Vec = stdout_handle .and_then(|handle| handle.join().ok()) .unwrap_or_default(); - let stderr = stderr_handle + let stderr: Vec = stderr_handle .and_then(|handle| handle.join().ok()) .unwrap_or_default(); let rc = match status { @@ -379,6 +467,28 @@ pub(crate) fn run(program: &str, args: &[&str], timeout: Duration) -> (i32, Stri (rc, stdout, stderr) } +/// Run a command whose output is UTF-16LE, as `wsl.exe`'s is. +/// +/// Decoding is by declaration, not detection. Sniffing the encoding cannot work +/// here: UTF-16LE text in a Latin or Cyrillic script is made entirely of bytes +/// below 0x80, so it is *valid UTF-8* and decodes without error straight into +/// mojibake — no NUL-density or validity test can tell the two apart. The one +/// reliable fact is which program produced the bytes. +fn run_utf16le(program: &str, args: &[&str], timeout: Duration) -> (i32, String) { + let (rc, stdout, _) = run_raw(program, args, timeout); + (rc, decode_utf16le(&stdout)) +} + +fn decode_utf16le(bytes: &[u8]) -> String { + let body = bytes.strip_prefix(&[0xFF, 0xFE]).unwrap_or(bytes); + // `chunks_exact` drops a trailing odd byte rather than panicking on it. + let units: Vec = body + .chunks_exact(2) + .map(|pair| u16::from_le_bytes([pair[0], pair[1]])) + .collect(); + String::from_utf16_lossy(&units) +} + fn read_text(path: &str) -> String { std::fs::read_to_string(path).unwrap_or_default() } @@ -458,6 +568,280 @@ fn probe_os(e: &mut Examination) { } } +/// Collect the WSL-specific facts the WSL half of the catalog reasons over. +/// +/// Reuses [`crate::detect_wsl_summary`] for the plumbing it already probes rather +/// than restating those paths, and adds the facts no existing caller needed: the +/// WSL major version, whether the distro release clears the supported floor, and +/// the Windows host driver version. +fn probe_wsl(e: &mut Examination) { + let summary = crate::detect_wsl_summary(); + let (host_reachable, host_driver_version) = host_driver_fields(crate::detect_wsl_host_driver()); + let rocminfo = which("rocminfo"); + e.wsl = Some(WslFacts { + version: if crate::is_wsl1_kernel(&e.kernel_release) { + 1 + } else { + 2 + }, + dxg_device: summary.as_ref().is_some_and(|s| s.dxg_device), + dxcore: summary.as_ref().is_some_and(|s| s.dxcore), + wsl_lib_dir: Path::new("/usr/lib/wsl/lib").is_dir(), + librocdxg: summary.as_ref().is_some_and(|s| s.librocdxg), + rocdxg_dids: summary.as_ref().is_some_and(|s| s.rocdxg_dids), + // `None` when ldconfig itself could not be run, which the summary's + // bool cannot express -- recover it from the same source the summary used. + ldconfig_librocdxg: crate::ldconfig_lists_librocdxg(), + rocminfo, + rocm_sees_gpu: rocminfo.then(probe_rocminfo_sees_gpu), + distro_supported: distro_clears_wsl_floor(&e.distro_id, &e.distro_version), + host_driver_version, + host_reachable, + locally_probed: true, + }); + sync_shared_fields_from_wsl(e); + e.notes.push(WSL_PLATFORM_NOTE.to_owned()); +} + +/// Flatten a host-driver probe into the two [`WslFacts`] fields that carry it. +/// +/// One place, so the in-guest and host-side probes cannot drift on the point +/// that matters: `None` means the question went unanswered, and `Some("")` means +/// it was answered with "no AMD adapter". Only the second is evidence. +fn host_driver_fields(probe: crate::WslHostDriverProbe) -> (bool, Option) { + match probe { + crate::WslHostDriverProbe::Unreachable => (false, None), + crate::WslHostDriverProbe::NoAmdDisplay => (true, Some(String::new())), + crate::WslHostDriverProbe::Version(version) => (true, Some(version)), + } +} + +/// Collect the WSL facts from *outside* the distro, over `wsl.exe`. +/// +/// Emits `key=value` lines rather than JSON so the guest side needs nothing but +/// a POSIX shell. The Python preflight this replaces injected a Python program +/// and so required `python3` in the distro — on a machine being checked precisely +/// because it is not set up yet. +/// +/// `librocdxg` is globbed across `/opt/rocm*` for the same reason the in-guest +/// probe resolves it across installs: a versioned root must not read as missing. +const WSL_REMOTE_PROBE: &str = r#" +echo "kernel=$(uname -r 2>/dev/null)" +if [ -e /dev/dxg ]; then echo dxg=1; else echo dxg=0; fi +if [ -e /usr/lib/wsl/lib/libdxcore.so ]; then echo dxcore=1; else echo dxcore=0; fi +if [ -d /usr/lib/wsl/lib ]; then echo wsllib=1; else echo wsllib=0; fi +if ls /opt/rocm*/lib/librocdxg.so >/dev/null 2>&1 \ + || ls /usr/local/rocm*/lib/librocdxg.so >/dev/null 2>&1; then echo librocdxg=1; else echo librocdxg=0; fi +if ls /opt/rocm*/share/rocdxg/dids.conf >/dev/null 2>&1 \ + || ls /usr/local/rocm*/share/rocdxg/dids.conf >/dev/null 2>&1; then echo dids=1; else echo dids=0; fi +for ldc in ldconfig /sbin/ldconfig /usr/sbin/ldconfig; do + if cache=$(command -v "$ldc" >/dev/null 2>&1 && "$ldc" -p 2>/dev/null); then + case "$cache" in *librocdxg.so*) echo ldconfig=1 ;; *) echo ldconfig=0 ;; esac + break + fi +done +if command -v rocminfo >/dev/null 2>&1; then + echo rocminfo=1 + if rocminfo 2>/dev/null | grep -qi gfx; then echo rocmgfx=1; else echo rocmgfx=0; fi +else + echo rocminfo=0 +fi +. /etc/os-release 2>/dev/null +echo "id=${ID}" +echo "version=${VERSION_ID}" +"#; + +/// Parse `wsl.exe -l -q` into distribution names. +/// +/// `-q` prints one bare name per line, so the whole line is the name. It is NOT +/// split on whitespace: `wsl --import "My Distro"` is legal, and truncating that +/// to `My` would both fail to match what the user asked for and hand a wrong +/// name to `wsl.exe -d`. +/// +/// The header and `*` handling below is for tolerance only — `-q` emits neither, +/// but a caller passing `-l -v` should not silently get its header row back as a +/// distribution. +#[must_use] +pub fn parse_wsl_distro_list(text: &str) -> Vec { + text.replace('\u{0}', "") + .lines() + .filter_map(|raw| { + let line = raw.trim(); + if line.is_empty() || line.to_uppercase().starts_with("NAME") { + return None; + } + let line = line.strip_prefix('*').map_or(line, str::trim); + (!line.is_empty()).then(|| line.to_owned()) + }) + .collect() +} + +fn parse_remote_flag(fields: &BTreeMap, key: &str) -> bool { + fields.get(key).is_some_and(|value| value == "1") +} + +/// A remote flag that can also report that the question went unanswered. +fn parse_remote_tristate(fields: &BTreeMap, key: &str) -> Option { + match fields.get(key).map(String::as_str) { + Some("1") => Some(true), + Some("0") => Some(false), + _ => None, + } +} + +/// Inspect a WSL distribution from the Windows host. +/// +/// Returns an [`Examination`] the ordinary catalog can be run against, so the +/// host-side check and the in-distro one share a single set of rules. Nothing +/// needs to be installed in the target distro. +/// +/// # Errors +/// +/// When `wsl.exe` is unavailable, no distribution matches, or the probe cannot +/// be run inside the selected distribution. +pub fn probe_wsl_distro_from_host(distro: Option<&str>) -> Result { + if !which("wsl.exe") { + return Err( + "wsl.exe was not found; inspecting a distribution this way only works from the Windows host" + .to_owned(), + ); + } + // `-q` prints names only. `-l -v` adds a header row that is localised, and a + // header the parser fails to recognise is not skipped -- it is taken for a + // distribution name. + let (rc, listed) = run_utf16le("wsl.exe", &["-l", "-q"], MEDIUM); + if rc != 0 { + return Err("could not list WSL distributions".to_owned()); + } + let distros = parse_wsl_distro_list(&listed); + let selected = match distro { + Some(name) => { + if !distros.iter().any(|d| d.eq_ignore_ascii_case(name)) { + return Err(format!( + "no WSL distribution named '{name}'; found: {}", + distros.join(", ") + )); + } + name.to_owned() + } + None => match distros.as_slice() { + [] => return Err("no WSL distributions were found".to_owned()), + [only] => only.clone(), + many => { + return Err(format!( + "several WSL distributions are installed; name one with --distro: {}", + many.join(", ") + )); + } + }, + }; + + let (rc, out, _) = run( + "wsl.exe", + &["-d", &selected, "--exec", "/bin/sh", "-c", WSL_REMOTE_PROBE], + Duration::from_secs(30), + ); + if rc != 0 { + return Err(format!( + "could not inspect '{selected}'; the distribution may be stopped or unreachable" + )); + } + let fields: BTreeMap = out + .lines() + .filter_map(|line| line.split_once('=')) + .map(|(k, v)| (k.trim().to_owned(), v.trim().to_owned())) + .collect(); + let (host_reachable, host_driver_version) = + host_driver_fields(crate::detect_local_windows_host_driver()); + + let mut e = Examination { + os_family: "linux".to_owned(), + is_wsl: true, + kernel_release: fields.get("kernel").cloned().unwrap_or_default(), + distro_id: fields.get("id").cloned().unwrap_or_default(), + distro_version: fields.get("version").cloned().unwrap_or_default(), + ..Examination::default() + }; + let rocminfo = parse_remote_flag(&fields, "rocminfo"); + e.wsl = Some(WslFacts { + version: if crate::is_wsl1_kernel(&e.kernel_release) { + 1 + } else { + 2 + }, + dxg_device: parse_remote_flag(&fields, "dxg"), + dxcore: parse_remote_flag(&fields, "dxcore"), + wsl_lib_dir: parse_remote_flag(&fields, "wsllib"), + librocdxg: parse_remote_flag(&fields, "librocdxg"), + rocdxg_dids: parse_remote_flag(&fields, "dids"), + ldconfig_librocdxg: parse_remote_tristate(&fields, "ldconfig"), + rocminfo, + rocm_sees_gpu: rocminfo.then(|| parse_remote_flag(&fields, "rocmgfx")), + distro_supported: distro_clears_wsl_floor(&e.distro_id, &e.distro_version), + // Running on the host, the driver is a local question rather than one + // that has to cross the interop boundary. It can still go unanswered — + // the inventory query can fail — and that must stay distinguishable from + // "the host has no AMD adapter", which is a finding. + host_driver_version, + host_reachable, + locally_probed: false, + }); + sync_shared_fields_from_wsl(&mut e); + e.status = "wsl".to_owned(); + Ok(e) +} + +/// Whether `rocminfo` enumerates a GPU agent. +/// +/// Only the yes/no answer is taken. Parsing the agents into `gpus` is the job of +/// the bare-metal probe, which stays skipped here — this exists so the WSL +/// catalog can tell "the plumbing is complete but no device is reachable" from +/// "the plumbing is incomplete", which is the difference between blaming the +/// Windows host driver and blaming the distro. +fn probe_rocminfo_sees_gpu() -> bool { + let (rc, out, _) = run("rocminfo", &[], MEDIUM); + rc == 0 && out.to_lowercase().contains("gfx") +} + +/// Mirror the WSL facts onto the shared fields the cross-platform checks read. +/// +/// Those checks (PATH, the wheel/ROCm pairing) are valid on WSL and enabled +/// there, but they read fields the bare-metal GPU probe populates — and that +/// probe is skipped here. Left at their defaults they do not read as "unknown", +/// they read as "absent": `rocminfo_present: false` made the PATH check score 50 +/// on every WSL host that had ROCm installed. +fn sync_shared_fields_from_wsl(e: &mut Examination) { + let Some(wsl) = e.wsl.as_ref() else { + return; + }; + e.rocminfo_present = wsl.rocminfo; + e.rocminfo_status = match (wsl.rocminfo, wsl.rocm_sees_gpu) { + (false, _) => "missing".to_owned(), + (true, Some(true)) => "ok".to_owned(), + (true, Some(false)) => "no-agents".to_owned(), + (true, None) => "unknown".to_owned(), + }; +} + +/// Whether the distro release clears the WSL floor in [`WSL_MIN_UBUNTU`]. +/// +/// `None` means the release could not be read as a `major.minor` pair. That is +/// deliberately not "supported": an unparseable release is not evidence of a good +/// one, and reporting a perfect host on a release nobody could identify is how a +/// user ends up chasing a GPU fault that is really a glibc floor. +/// +/// Only Ubuntu carries a floor today, because that is the only distro the WSL +/// path documents. Anything else returns `None` rather than a false verdict. +fn distro_clears_wsl_floor(distro_id: &str, distro_version: &str) -> Option { + if !distro_id.eq_ignore_ascii_case("ubuntu") { + return None; + } + let (major, minor) = distro_version.split_once('.')?; + let major: u32 = major.parse().ok()?; + let minor: u32 = minor.parse().ok()?; + Some((major, minor) >= WSL_MIN_UBUNTU) +} + /// Extract the value of `iommu=` from a kernel cmdline string. fn parse_iommu_param(cmdline: &str) -> Option { cmdline.split_whitespace().find_map(|token| { @@ -1450,6 +1834,163 @@ fn probe_msvc_redist_windows(e: &mut Examination) { mod tests { use super::*; + /// Ported from the Python preflight this replaced, whose `wsl.exe` parser + /// was covered by a self-test that CI ran on both lanes. That coverage has to + /// land here, or deleting the script quietly drops it. + #[test] + fn the_distro_list_survives_the_markers_wsl_puts_around_it() { + // `-l -q` prints one bare name per line, which is what the probe asks + // for. + assert_eq!( + parse_wsl_distro_list("Ubuntu\nDebian\n"), + vec!["Ubuntu", "Debian"] + ); + assert_eq!( + parse_wsl_distro_list("Ubuntu-24.04\n"), + vec!["Ubuntu-24.04"] + ); + + // A name may contain spaces: `wsl --import "My Distro"` is legal. + // Splitting on whitespace truncated it to "My", which then neither + // matched what the user asked for nor named a real distribution when + // handed back to `wsl.exe -d`. + assert_eq!( + parse_wsl_distro_list("My Distro\nUbuntu\n"), + vec!["My Distro", "Ubuntu"] + ); + + // The NUL padding of the raw UTF-16 output must not become part of a + // name, and blank lines are not distributions. + assert_eq!( + parse_wsl_distro_list("\0U\0b\0u\0n\0t\0u\0\n\0"), + vec!["Ubuntu"] + ); + assert!(parse_wsl_distro_list("").is_empty()); + assert!(parse_wsl_distro_list("\n\n \n").is_empty()); + + // Tolerance only: `-q` emits no header, but a `-l -v` header must never + // come back as a distribution named "NAME". + assert!(parse_wsl_distro_list(" NAME STATE VERSION\n").is_empty()); + } + + #[test] + fn a_host_that_could_not_be_queried_is_unknown_not_driverless() { + use crate::WslHostDriverProbe; + + // The distinction the catalog acts on. An earlier version flattened this + // to `Option` and defaulted the `None`, so "could not ask the + // host" arrived as `Some("")` -- which reads as "the host has no AMD + // adapter" and reported a missing driver on a machine never looked at. + assert_eq!( + host_driver_fields(WslHostDriverProbe::Unreachable), + (false, None), + "unreachable must stay unknown" + ); + assert_eq!( + host_driver_fields(WslHostDriverProbe::NoAmdDisplay), + (true, Some(String::new())), + "answered with no adapter is evidence, and is not the same thing" + ); + assert_eq!( + host_driver_fields(WslHostDriverProbe::Version("32.0.1".to_owned())), + (true, Some("32.0.1".to_owned())) + ); + } + + #[test] + fn the_distro_list_decodes_as_utf16_whatever_script_it_is_in() { + fn utf16le(text: &str, bom: bool) -> Vec { + let mut bytes = if bom { vec![0xFF, 0xFE] } else { Vec::new() }; + for unit in text.encode_utf16() { + bytes.extend_from_slice(&unit.to_le_bytes()); + } + bytes + } + + assert_eq!(decode_utf16le(&utf16le("Ubuntu\n", true)), "Ubuntu\n"); + assert_eq!(decode_utf16le(&utf16le("Ubuntu\n", false)), "Ubuntu\n"); + assert_eq!(decode_utf16le(b""), ""); + + // The reason this is decoded by declaration rather than sniffed: in a + // Latin or Cyrillic script every UTF-16LE byte is below 0x80, so the + // bytes are *valid UTF-8* and decode without error into mojibake. Read + // as UTF-8 these names come back as "#\u{4}1\u{4}..." rather than + // failing, so no validity or NUL-density test could catch them. + for name in [ + "Ubuntu-24.04\nÉtat\n", + "Ubuntu\nУбунту\n", + "Ubuntu\n日本語\n", + ] { + assert_eq!(decode_utf16le(&utf16le(name, true)), name); + assert_eq!(decode_utf16le(&utf16le(name, false)), name); + } + + // A non-ASCII name must survive all the way through the parser. + assert_eq!( + parse_wsl_distro_list(&decode_utf16le(&utf16le("Ubuntu\nУбунту\n", true))), + vec!["Ubuntu", "Убунту"] + ); + + // An odd trailing byte is dropped rather than panicking. + let mut truncated = utf16le("Ubuntu", false); + truncated.push(0x00); + assert_eq!(decode_utf16le(&truncated), "Ubuntu"); + } + + #[test] + fn command_output_that_is_not_utf8_is_kept_rather_than_dropped() { + // `read_to_string` fails on invalid UTF-8, and the error was discarded -- + // so one stray byte emptied a whole capture, which every caller then read + // as "the command printed nothing". + let (rc, out, _) = run("printf", &["ok\\xffdone"], SHORT); + if rc == 127 { + return; // no `printf` binary on this host + } + assert!( + out.starts_with("ok") && out.ends_with("done"), + "the undecodable byte must not take the rest of the output with it: {out:?}" + ); + } + + /// Ported from the same Python preflight, which owned this rule until the + /// catalog took it over. Its version was well covered and its tests went with + /// it, so the coverage has to live here or the floor becomes an untested + /// constant. + #[test] + fn the_distro_floor_fails_closed_on_anything_it_cannot_read() { + for supported in ["24.04", "24.10", "25.04", "26.04", "28.04"] { + assert_eq!( + distro_clears_wsl_floor("ubuntu", supported), + Some(true), + "ubuntu {supported} clears the floor" + ); + } + // 22.04 ships glibc 2.35, below the 2.38 / GLIBCXX_3.4.32 floor the + // engines are linked against, so it cannot run them at all. + assert_eq!(distro_clears_wsl_floor("ubuntu", "22.04"), Some(false)); + assert_eq!(distro_clears_wsl_floor("ubuntu", "20.04"), Some(false)); + + // Unreadable is `None`, never `Some(true)`. Reporting a release nobody + // could parse as supported is how a user ends up chasing a GPU fault + // that is really a glibc floor -- but claiming it is too old would send + // them to reinstall a perfectly good distro, so neither answer is safe. + for unreadable in ["24.04.1", "unknown", "", "24", "24.x", "..", "24.04.1.2"] { + assert_eq!( + distro_clears_wsl_floor("ubuntu", unreadable), + None, + "{unreadable:?} cannot be read as a release" + ); + } + + // Only Ubuntu carries a documented floor. Anything else abstains rather + // than applying Ubuntu's numbering to a distro that does not share it -- + // Debian 12 is not "below 24.04". + for other in ["debian", "fedora", "arch", ""] { + assert_eq!(distro_clears_wsl_floor(other, "12.0"), None, "{other}"); + } + assert_eq!(distro_clears_wsl_floor("UBUNTU", "24.04"), Some(true)); + } + #[test] fn examination_serializes_expected_keys() { let e = Examination::default(); @@ -1475,10 +2016,14 @@ mod tests { #[test] fn examination_top_level_keys_match_examine_py_contract() { - // The field set examine.py emits, plus the CLI-only `status` addition. - // diagnose.py reads against these names, so this is the frozen wire - // contract — adding/removing/renaming a top-level field is a contract - // change and must be intentional. + // The field set examine.py emits, plus the CLI-only `status` and `wsl` + // additions. diagnose.py reads against these names, so this is the frozen + // wire contract — adding/removing/renaming a top-level field is a + // contract change and must be intentional. + // + // `wsl` is one of the intentional ones: WSL2 has no examine.py analogue, + // and nesting its facts under a single key keeps the rest of the contract + // byte-identical instead of scattering ten flat `wsl_*` fields through it. let expected: std::collections::BTreeSet<&str> = [ "os_family", "os_version", @@ -1487,6 +2032,7 @@ mod tests { "kernel_release", "kernel_cmdline", "is_wsl", + "wsl", "cpu_vendor", "cpu_model", "gpus", diff --git a/crates/rocm-core/src/fix.rs b/crates/rocm-core/src/fix.rs index 3218e920b..82ee6daa5 100644 --- a/crates/rocm-core/src/fix.rs +++ b/crates/rocm-core/src/fix.rs @@ -66,9 +66,17 @@ struct FixRecipe { runner: Option i32>, } +/// Valid on bare-metal Linux, Windows and WSL alike. +/// +/// WSL is named explicitly rather than folded into `linux`: the default for a +/// bare-metal recipe has to be "does not apply on WSL", because the platform has +/// no amdgpu module, no /dev/kfd and no render group. Recipes that survive the +/// move are the ones about wheels, environment variables and PATH. +const LINUX_WINDOWS_AND_WSL: &[&str] = &["linux", "windows", "wsl"]; const LINUX_AND_WINDOWS: &[&str] = &["linux", "windows"]; const LINUX_ONLY: &[&str] = &["linux"]; const WINDOWS_ONLY: &[&str] = &["windows"]; +const WSL_ONLY: &[&str] = &["wsl"]; /// The recipe registry. Mirrors the diagnosis catalog; only the four small, /// safe fixes carry a `runner` and are auto-applicable. @@ -96,7 +104,7 @@ const RECIPES: &[FixRecipe] = &[ "TheRock per-gfx wheels are the recommended fallback when the official pytorch index does not yet cover your gfx (and the only first-party option on Windows AMD).", "HSA_OVERRIDE_GFX_VERSION is NOT the right fix here -- it papers over the mismatch and risks page faults at runtime.", ], - applies_on: LINUX_AND_WINDOWS, + applies_on: LINUX_WINDOWS_AND_WSL, runner: None, }, FixRecipe { @@ -117,7 +125,7 @@ const RECIPES: &[FixRecipe] = &[ needs_relogin: false, verify: "env | grep HSA_OVERRIDE_GFX_VERSION || echo OK_UNSET", notes: &[], - applies_on: LINUX_AND_WINDOWS, + applies_on: LINUX_WINDOWS_AND_WSL, runner: Some(run_unset_override), }, FixRecipe { @@ -190,7 +198,7 @@ const RECIPES: &[FixRecipe] = &[ needs_relogin: false, verify: "rocminfo | head -n 5 && hipcc --version", notes: &[], - applies_on: LINUX_AND_WINDOWS, + applies_on: LINUX_WINDOWS_AND_WSL, runner: Some(run_path_export), }, FixRecipe { @@ -230,7 +238,7 @@ const RECIPES: &[FixRecipe] = &[ needs_relogin: false, verify: "python -c \"import torch; print(torch.__version__, torch.version.hip, torch.cuda.is_available())\"", notes: &[], - applies_on: LINUX_AND_WINDOWS, + applies_on: LINUX_WINDOWS_AND_WSL, runner: None, }, FixRecipe { @@ -429,6 +437,148 @@ const RECIPES: &[FixRecipe] = &[ applies_on: LINUX_ONLY, runner: None, }, + // WSL2 recipes. All print-only: every one of them either installs a package + // with sudo, edits loader configuration, or belongs to the Windows host, and + // none of that meets the bar the four auto-applicable fixes clear (small, + // reversible, user-scoped, verifiable in one line). + FixRecipe { + fix_id: "fix-wsl-1-gpu-not-exposed", + title: "Expose the GPU to the WSL distro (/dev/dxg)", + rationale: "WSL reaches the GPU through /dev/dxg, provided by the Windows host driver via GPU-PV. Without that device nothing else in the ROCm stack can work, so this comes before any package or loader question. In a container the device has to be passed in explicitly; on a host it means the Windows driver or the WSL kernel needs attention.", + auto_applicable: false, + commands: &[ + "# In a container, pass the device and the WSL libraries in:", + "# --device=/dev/dxg -v /usr/lib/wsl:/usr/lib/wsl", + "# On a WSL host, update WSL and the Windows AMD driver, then:", + "# wsl --update", + "# wsl --shutdown", + ], + needs_sudo: false, + needs_reboot: false, + needs_relogin: false, + verify: "ls -l /dev/dxg", + notes: &[ + "A container running on WSL2 reports itself as WSL but sees /dev/dxg only when it was started with the device. Check that before touching the Windows driver.", + ], + applies_on: WSL_ONLY, + runner: None, + }, + FixRecipe { + fix_id: "fix-wsl-2-dxcore-missing", + title: "Restore the WSL DXCore libraries", + rationale: "/usr/lib/wsl/lib holds the DXCore shims the ROCm runtime uses to talk to the Windows host driver. WSL mounts that directory itself, so a distro package manager can neither install nor repair it -- the fix is on the Windows side, plus a loader-path entry inside the distro.", + auto_applicable: false, + commands: &[ + "# From Windows, refresh the WSL runtime that provides these libraries:", + "# wsl --update", + "# wsl --shutdown", + "# Inside the distro, put them on the loader path:", + "echo /usr/lib/wsl/lib | sudo tee /etc/ld.so.conf.d/wsl.conf", + "sudo ldconfig", + ], + needs_sudo: true, + needs_reboot: false, + needs_relogin: false, + verify: "ls -l /usr/lib/wsl/lib/libdxcore.so && ldconfig -p | grep libdxcore", + notes: &["apt cannot repair /usr/lib/wsl: it is a mount supplied by WSL, not a package."], + applies_on: WSL_ONLY, + runner: None, + }, + FixRecipe { + fix_id: "fix-wsl-3-rocdxg-missing", + title: "Install ROCDXG in the WSL distro", + rationale: "ROCDXG (librocdxg) is the ROCm-to-DXCore shim the WSL path runs on. It is a distro-side package, so unlike the driver and DXCore pieces this one is entirely in the user's hands.", + auto_applicable: false, + commands: &[ + "bash scripts/wsl_setup_rocdxg.sh", + "# To verify the download against a digest you trust:", + "# ROCDXG_SHA256=<64-hex-sha256> bash scripts/wsl_setup_rocdxg.sh", + ], + needs_sudo: true, + needs_reboot: false, + needs_relogin: false, + verify: "ldconfig -p | grep librocdxg", + notes: &[ + "Print-only on purpose: this downloads a .deb from a release page and installs it with sudo. rocm-cli does not run that for you, and the script does not bake in a production checksum -- set ROCDXG_SHA256 to one you trust.", + ], + applies_on: WSL_ONLY, + runner: None, + }, + FixRecipe { + fix_id: "fix-wsl-4-rocdxg-not-linked", + title: "Refresh the linker cache so ROCDXG is loadable", + rationale: "librocdxg is installed but absent from the linker cache, so the runtime will not find it at load time. Usually a missed `ldconfig` after a manual install.", + auto_applicable: false, + commands: &["sudo ldconfig"], + needs_sudo: true, + needs_reboot: false, + needs_relogin: false, + verify: "ldconfig -p | grep librocdxg", + notes: &[ + "If ldconfig alone does not do it, the library landed outside the linker's search path: add that directory under /etc/ld.so.conf.d/ and re-run.", + ], + applies_on: WSL_ONLY, + runner: None, + }, + FixRecipe { + fix_id: "fix-wsl-5-distro-too-old", + title: "Move to a distro release the WSL path supports", + rationale: "Ubuntu 22.04 ships glibc 2.35, below the glibc 2.38 / GLIBCXX_3.4.32 floor every published Lemonade embeddable is linked against, so the engine cannot start there at all. This is a hard floor, not a recommendation.", + auto_applicable: false, + commands: &[ + "# From Windows, install a supported distro alongside the current one:", + "# wsl --install -d Ubuntu-24.04", + ], + needs_sudo: false, + needs_reboot: false, + needs_relogin: false, + verify: "grep VERSION_ID /etc/os-release", + notes: &[ + "Distros install side by side, so the current one can stay until the new one is set up.", + ], + applies_on: WSL_ONLY, + runner: None, + }, + FixRecipe { + fix_id: "fix-wsl-6-host-driver-too-old", + title: "Update the AMD driver on the Windows host", + rationale: "Under WSL the GPU kernel-mode driver lives on the Windows host, not in the distro. When the distro-side plumbing is complete and ROCm still sees no GPU, the host driver is the remaining variable.", + auto_applicable: false, + commands: &[ + "# On the Windows host, not in this distro:", + "# install a WSL-capable AMD Adrenalin driver, then `wsl --shutdown`.", + ], + needs_sudo: false, + needs_reboot: false, + needs_relogin: false, + verify: "rocminfo | head -n 20", + notes: &[ + "Nothing inside the distro can carry this out, which is why it prints rather than runs.", + "The ROCm release and the Adrenalin release are paired; check the WSL install guide for the version that matches your ROCm.", + ], + applies_on: WSL_ONLY, + runner: None, + }, + FixRecipe { + fix_id: "fix-wsl-7-wsl1", + title: "Convert the distro from WSL 1 to WSL 2", + rationale: "WSL 1 translates syscalls rather than running a kernel, and exposes no GPU device at all. No driver or package work can give it ROCm support; the distro has to be converted.", + auto_applicable: false, + commands: &[ + "# From Windows PowerShell:", + "# wsl --set-version 2", + "# wsl --set-default-version 2", + ], + needs_sudo: false, + needs_reboot: false, + needs_relogin: false, + verify: "uname -r", + notes: &[ + "Converting rewrites the distro's filesystem and can take a long time on a large install. Back up anything you cannot lose first.", + ], + applies_on: WSL_ONLY, + runner: None, + }, ]; /// Assert that a recipe whose steps span more than one shell says which shell @@ -522,11 +672,19 @@ fn find_recipe(fix_id: &str) -> Option<&'static FixRecipe> { RECIPES.iter().find(|r| r.fix_id == fix_id) } -const fn current_os() -> &'static str { +/// The platform family a recipe's `applies_on` is matched against. +/// +/// WSL2 is its own family rather than `linux`, mirroring `diagnose`. That is what +/// makes `rocm fix fix-4-render-group` on a WSL host refuse with "wrong OS" +/// instead of running `usermod` for a group that governs nothing there — and it +/// is why recipes valid on both platforms have to name `wsl` explicitly. +/// +/// Not `const fn`: unlike the OS, WSL has to be probed at runtime. +fn current_os() -> &'static str { if runtime_is_windows() { "windows" } else if runtime_is_linux() { - "linux" + if crate::is_wsl_host() { "wsl" } else { "linux" } } else { "other" } @@ -1259,7 +1417,8 @@ mod tests { let count = ids.len(); ids.dedup(); assert_eq!(ids.len(), count, "duplicate fix-id in RECIPES"); - assert_eq!(count, 16, "expected 16 catalog entries"); + // 16 bare-metal/Windows entries (including fix-17) plus the 7 WSL ones. + assert_eq!(count, 23, "expected 23 catalog entries"); } #[test] @@ -1269,6 +1428,15 @@ mod tests { assert_engine_shell_boundary_is_labelled(recipe.fix_id, recipe.commands); } + /// Whether a recipe applies on the platform the test is running on. + /// + /// Tests used to gate on `runtime_is_linux()`, which stopped being the same + /// question once WSL became its own family: a WSL host is Linux, but a + /// `LINUX_ONLY` recipe is correctly refused there. + fn recipe_applies_here(fix_id: &str) -> bool { + find_recipe(fix_id).is_some_and(|r| r.applies_on.contains(¤t_os())) + } + #[test] fn auto_applicable_recipes_have_a_runner() { for r in RECIPES { @@ -1326,6 +1494,12 @@ mod tests { // query that identifies the dGPU, so it is a print-only preview and // must return 0 -- not the environment/OS code 3. A dry-run without the // argument must likewise succeed, since the runner never mutates. + // + // fix-9 does not apply on WSL (no per-device topology to collide over), + // where the correct answer is the OS refusal this test exists to rule out. + if !recipe_applies_here("fix-9-igpu-dgpu") { + return; + } for dry_run in [false, true] { let opts = FixOptions { dry_run, @@ -1341,11 +1515,17 @@ mod tests { #[test] fn print_only_fix_returns_zero() { - if !runtime_is_linux() { - return; - } - let code = apply("fix-5-amdgpu-load", &FixOptions::default()); - assert_eq!(code, 0); + // Pick a recipe that applies on THIS platform rather than naming a Linux + // one: the assertion is about print-only recipes succeeding, and hunting + // for an applicable one keeps that meaningful on every lane instead of + // skipping wherever the hardcoded id happens not to apply. + let fix_id = RECIPES + .iter() + .find(|r| !r.auto_applicable && r.applies_on.contains(¤t_os())) + .map(|r| r.fix_id) + .expect("every supported platform has at least one print-only recipe"); + let code = apply(fix_id, &FixOptions::default()); + assert_eq!(code, 0, "{fix_id} is print-only here and must succeed"); } #[test] diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index 44e385c00..1b8594263 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -45,7 +45,9 @@ pub use disk_space::{ mount_for_path, on_same_filesystem, warn_if_low_space, with_margin, }; use examine::extract_rocm_version; -pub use examine::{Examination, FrameworkProbe, WSL_ROUTE_OUT_NOTE, gfx_is_apu_family}; +pub use examine::{ + Examination, FrameworkProbe, WSL_PLATFORM_NOTE, gfx_is_apu_family, probe_wsl_distro_from_host, +}; pub use fix::{FixOptions, apply as apply_fix, list_recipes as list_fix_recipes}; pub use proc_lifecycle::{ IdentityState, KillScope, ProcessIdentity, TerminationOutcome, identity_state, @@ -2431,6 +2433,69 @@ pub fn is_wsl_host() -> bool { ) } +/// Whether the host runs WSL 1 rather than WSL 2. +/// +/// WSL 1 translates syscalls instead of running a real kernel, so it has no +/// `/dev/dxg` and no GPU path at all. Without this the catalog would tell a WSL 1 +/// user to update a Windows driver that could never help them. +/// +/// WSL 1 reports a kernel ending in `-Microsoft`, as in `4.4.0-19041-Microsoft`. +/// WSL 2 builds all carry `microsoft-standard`, with the `-WSL2` suffix added +/// later — `4.19.104-microsoft-standard` was the original and has no `WSL2` in +/// it at all. +/// +/// So the test is the `standard` marker and the trailing position, not the +/// absence of `WSL2`. Keying on `WSL2` alone called every early WSL 2 kernel +/// "WSL 1", which is the asymmetric error [`crate::examine::WslFacts::version`] +/// documents as the one to avoid: it tells the user to convert a distribution +/// that is already converted, at high confidence, while suppressing every other +/// check. Anything unrecognised is read as WSL 2 for the same reason. +#[must_use] +pub(crate) fn is_wsl1_kernel(kernel_release: &str) -> bool { + let kernel = kernel_release.trim().to_ascii_lowercase(); + kernel.ends_with("-microsoft") && !kernel.contains("standard") +} + +/// Whether `relative` exists under any ROCm install on this host. +/// +/// The WSL probe used to hardcode `/opt/rocm`, so a versioned install at +/// `/opt/rocm-7.x` reported ROCDXG missing and the catalog would then blame a +/// package that was in fact installed. Ask the same resolver the rest of the CLI +/// uses, and keep the conventional root as a fallback for the case where +/// discovery finds nothing. +/// The dynamic linker cache, or `None` when `ldconfig` could not be run. +/// +/// `ldconfig` lives in `/sbin`, which is not on a non-root user's `PATH` on +/// Debian and derivatives. Looking it up by bare name there yields nothing, and +/// an empty cache is indistinguishable from a cache that does not list the +/// library — so a correctly installed ROCDXG read as "not registered with the +/// linker" and the catalog told the user to run `ldconfig` on a working install. +/// +/// Search the conventional locations, and report "could not ask" as `None` +/// rather than as an empty answer. +fn ldconfig_cache() -> Option { + for program in ["ldconfig", "/sbin/ldconfig", "/usr/sbin/ldconfig"] { + if let Some(text) = capture_optional_command(program, &["-p"]) { + return Some(text); + } + } + None +} + +/// Whether the linker cache lists ROCDXG, or `None` if it could not be read. +pub(crate) fn ldconfig_lists_librocdxg() -> Option { + ldconfig_cache().map(|text| text.contains("librocdxg.so")) +} + +fn rocm_relative_file_exists(relative: &str) -> bool { + if Path::new("/opt/rocm").join(relative).exists() { + return true; + } + discover_rocm_installs() + .iter() + .any(|install| install.path.join(relative).exists()) +} + /// The predicate itself, separated from reading the machine so the union can be /// tested — including the two cases that used to split the old implementations. fn wsl_signals_indicate_wsl(dxg_device: bool, distro_name_set: bool, proc_version: &str) -> bool { @@ -2450,10 +2515,12 @@ fn detect_wsl_summary() -> Option { let is_wsl = true; let dxcore = Path::new("/usr/lib/wsl/lib/libdxcore.so").exists(); - let librocdxg = Path::new("/opt/rocm/lib/librocdxg.so").exists(); - let rocdxg_dids = Path::new("/opt/rocm/share/rocdxg/dids.conf").exists(); - let ldconfig_text = capture_optional_command("ldconfig", &["-p"]).unwrap_or_default(); - let ldconfig_librocdxg = ldconfig_text.contains("librocdxg.so"); + let librocdxg = rocm_relative_file_exists("lib/librocdxg.so"); + let rocdxg_dids = rocm_relative_file_exists("share/rocdxg/dids.conf"); + let ldconfig_text = ldconfig_cache(); + let ldconfig_librocdxg = ldconfig_text + .as_deref() + .is_some_and(|text| text.contains("librocdxg.so")); let rocminfo = tool_on_path("rocminfo"); let cargo = tool_on_path("cargo"); let mut missing = Vec::new(); @@ -2464,7 +2531,9 @@ fn detect_wsl_summary() -> Option { missing.push("/usr/lib/wsl/lib/libdxcore.so"); } if !librocdxg { - missing.push("/opt/rocm/lib/librocdxg.so"); + // Named without a directory: the file is looked up across every ROCm + // install, so quoting one root would misreport where it was not found. + missing.push("librocdxg.so"); } if !ldconfig_librocdxg { missing.push("ldconfig:librocdxg.so"); @@ -3142,7 +3211,7 @@ pub fn detect_host_gpu_diagnostics() -> String { .as_deref() .unwrap_or("") ); - if is_wsl_environment_fast() { + if is_wsl_host() { let wsl_probe = detect_wsl_windows_display_probe_text().unwrap_or_default(); let _ = writeln!( output, @@ -4334,8 +4403,81 @@ fn detect_wsl_windows_display_name_fast() -> Option { .and_then(parse_windows_display_name) } +/// What the guest was able to learn about the Windows host's AMD display driver. +/// +/// Three states, not two. Reaching the host requires WSL interop, which the user +/// can switch off and which is absent entirely inside a container running on WSL. +/// Collapsing "could not ask" into "no driver found" would make the catalog blame +/// a Windows driver on every locked-down or containerised host, so the two stay +/// distinct and the check abstains on [`Unreachable`](Self::Unreachable). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum WslHostDriverProbe { + /// WSL interop is unavailable, so the host was never asked. + Unreachable, + /// The host answered but reported no AMD display adapter. + NoAmdDisplay, + /// The host's AMD display driver version. + Version(String), +} + +/// The AMD display driver of the machine this is running on. +/// +/// The host-side counterpart to [`detect_wsl_host_driver`]: when `rocm` runs on +/// Windows and inspects a WSL distribution, the driver is a local question and +/// needs no interop to answer. +/// +/// Returns the same tri-state, and for the same reason. An earlier version +/// collapsed it to `Option` and defaulted the `None`, so "this is not +/// Windows" and "the inventory query failed" both arrived as an empty version -- +/// which the catalog reads as "the host has no AMD adapter" and reports as a +/// missing driver on a machine it never managed to look at. +pub(crate) fn detect_local_windows_host_driver() -> WslHostDriverProbe { + if !runtime_is_windows() { + return WslHostDriverProbe::Unreachable; + } + let Some(inventory) = detect_windows_examine_inventory() else { + return WslHostDriverProbe::Unreachable; + }; + inventory + .preferred_amd_display() + .and_then(|display| display.driver_version.as_deref()) + .map(str::trim) + .filter(|version| !version.is_empty()) + .map_or(WslHostDriverProbe::NoAmdDisplay, |version| { + WslHostDriverProbe::Version(version.to_owned()) + }) +} + +/// Ask the Windows host, from inside the distro, which AMD display driver it runs. +pub(crate) fn detect_wsl_host_driver() -> WslHostDriverProbe { + if !is_wsl_host() { + return WslHostDriverProbe::Unreachable; + } + let Some(output) = capture_optional_command_with_timeout( + "powershell.exe", + &[ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + WINDOWS_VIDEO_CONTROLLER_INVENTORY_SCRIPT, + ], + WINDOWS_INVENTORY_QUERY_TIMEOUT, + ) else { + return WslHostDriverProbe::Unreachable; + }; + parse_windows_examine_inventory(&output) + .preferred_amd_display() + .and_then(|display| display.driver_version.as_deref()) + .map(str::trim) + .filter(|version| !version.is_empty()) + .map_or(WslHostDriverProbe::NoAmdDisplay, |version| { + WslHostDriverProbe::Version(version.to_owned()) + }) +} + fn detect_wsl_windows_display_probe_text() -> Option { - if !is_wsl_environment_fast() { + if !is_wsl_host() { return None; } @@ -4359,15 +4501,6 @@ fn detect_wsl_windows_display_probe_text() -> Option { .filter(|output| !output.is_empty()) } -fn is_wsl_environment_fast() -> bool { - if !runtime_is_linux() { - return false; - } - Path::new("/dev/dxg").exists() - || fs::read_to_string("/proc/version") - .is_ok_and(|text| text.to_ascii_lowercase().contains("microsoft")) -} - #[cfg(target_os = "linux")] fn detect_linux_primary_gpu_name() -> Option { if !runtime_is_linux() { @@ -11977,6 +12110,39 @@ last_installed_runtime_id = "therock-release" )); } + #[test] + fn wsl1_is_told_apart_from_wsl2_by_the_kernel_release() { + // The two are the same string family, distinguished only by the WSL2 + // marker. Getting this backwards would send a WSL 1 user chasing a + // Windows driver update that can never give them a GPU, or hide the + // conversion advice from the one platform that needs it. + for wsl1 in [ + "4.4.0-19041-Microsoft", + "4.4.0-18362-MICROSOFT", + "4.4.0-17763-microsoft", + ] { + assert!(is_wsl1_kernel(wsl1), "{wsl1} is a WSL 1 kernel"); + } + for wsl2 in [ + "6.6.87.2-microsoft-standard-WSL2", + "5.15.167.4-microsoft-standard-WSL2", + "6.18.33.2-MICROSOFT-STANDARD-WSL2", + // The `-WSL2` suffix is not the marker. These are the earlier WSL 2 + // kernels, which carry `microsoft-standard` and no `WSL2` at all -- + // testing for the absence of `WSL2` called every one of them WSL 1. + "4.19.104-microsoft-standard", + "4.19.128-microsoft-standard", + "5.10.16.3-microsoft-standard", + ] { + assert!(!is_wsl1_kernel(wsl2), "{wsl2} is a WSL 2 kernel"); + } + // A bare-metal kernel is neither, and must not read as WSL 1 -- the + // caller only asks on a host already known to be WSL, but answering + // "yes" here would be wrong if that ever changed. + assert!(!is_wsl1_kernel("6.8.0-51-generic")); + assert!(!is_wsl1_kernel("")); + } + #[test] fn rocdxg_is_ready_only_when_the_whole_chain_is_present() { let ready = WslSummary { diff --git a/docs/ci-hardware-testing.md b/docs/ci-hardware-testing.md index 4a0a5fca5..08a81e7cc 100644 --- a/docs/ci-hardware-testing.md +++ b/docs/ci-hardware-testing.md @@ -70,9 +70,14 @@ execution boundary, and whatever GPU access WSL exposes on that machine. The GPU preflight is advisory here precisely because GPU-on-WSL is what the lane is proving out: where it is unavailable the capability probe resolves those scenarios to not-applicable and the rest of the suite still runs. Scenarios the -product deliberately routes around on WSL carry `@requires-bare-metal`; the one -scenario whose premise *is* a WSL host carries `@requires-wsl`, and this is the -only lane that runs it. +product deliberately routes around on WSL carry `@requires-bare-metal`; +scenarios whose premise *is* a WSL host carry `@requires-wsl`, and this is the +only lane that runs them. + +`rocm diagnose` is no longer one of the things routed around: it carries a WSL +catalog of its own, so the `@requires-wsl` diagnose scenarios — that a WSL host +is never given a bare-metal cause, and that a WSL remedy is explained rather +than carried out — are proven here and nowhere else. ### What the WSL distro needs diff --git a/docs/testing.md b/docs/testing.md index 11cf50264..657a9302e 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -939,20 +939,29 @@ Reference: [TheRock Windows install tools](https://github.com/ROCm/TheRock/blob/ Read-only WSL/ROCDXG preflight: ```bash -python scripts/wsl_preflight.py --json -python scripts/wsl_preflight.py --require-ready +rocm diagnose --json ``` -`--require-ready` checks WSL, `/dev/dxg`, DXCore, ROCDXG, `python3 -m venv`, -and library registration. Source-build tools such as Windows SDK headers, -CMake, and compilers are optional for runtime acceptance; add -`--require-build-tools` only when validating a WSL source-build environment. +The WSL catalog covers `/dev/dxg`, the DXCore handoff, ROCDXG and its linker +entry, the distro release floor, the Windows host driver, and WSL 1. A clean run +reports no findings; anything it does report carries a `fix-wsl-*` id and a plan. + +From the Windows host, to inspect a distro without installing anything in it: + +```powershell +rocm diagnose --distro # the only distro installed +rocm diagnose --distro Ubuntu # a named one +``` + +Both forms run the same catalog. The host-side one collects its facts over +`wsl.exe` with a POSIX shell, so the target distro needs neither `rocm-cli` nor +Python. Interactive ROCDXG install inside WSL: ```bash bash scripts/wsl_setup_rocdxg.sh -python scripts/wsl_preflight.py --require-ready +rocm diagnose ``` To require checksum verification for the downloaded ROCDXG `.deb`, provide the diff --git a/docs/wsl.md b/docs/wsl.md index 24a536796..6a85be9a4 100644 --- a/docs/wsl.md +++ b/docs/wsl.md @@ -21,20 +21,34 @@ The AMD WSL path is: 3. ROCDXG (`librocdxg`) installed inside WSL. 4. A TheRock runtime installed by `rocm-cli` into a managed Python venv. -Useful read-only preflight: +Useful read-only preflight, from inside the distro: ```bash -python scripts/wsl_preflight.py -python scripts/wsl_preflight.py --json -python scripts/wsl_preflight.py --require-ready +rocm diagnose +rocm diagnose --json ``` -From Windows PowerShell, target a specific distro: +From Windows PowerShell, inspect a distro without installing anything in it: ```powershell -python scripts\wsl_preflight.py --distro Ubuntu +rocm diagnose --distro # the only distro installed +rocm diagnose --distro Ubuntu # a named one ``` +The host-side form collects the facts over `wsl.exe` and runs the same catalog. +It needs no `rocm-cli`, and no Python, inside the target distro — which is the +point, since the distro being checked is usually the one that is not set up yet. + +It sees less than a run from inside, so prefer the in-distro form where you can: + +- It probes the conventional ROCm roots (`/opt/rocm*`, `/usr/local/rocm*`) but + cannot honour a `$ROCM_PATH` pointing elsewhere. `wsl.exe --exec` runs a + non-login, non-interactive shell, so nothing exported from a shell profile is + set. +- For the same reason it collects no environment, so the checks that read one — + `HSA_OVERRIDE_GFX_VERSION`, `PATH`, and the framework/ROCm version pairing — + do not run. It reports on the WSL GPU stack, not on the whole installation. + ## Install ROCDXG In WSL Install build/runtime prerequisites: @@ -57,7 +71,7 @@ From this repo inside WSL, the same supported path is wrapped as: ```bash bash scripts/wsl_setup_rocdxg.sh -python scripts/wsl_preflight.py --require-ready +rocm diagnose ``` To require checksum verification before installing the downloaded `.deb`, set @@ -127,20 +141,66 @@ For `rocm-cli`, the command itself should resolve the managed runtime manifest and apply that environment before launching HIP apps such as Lemonade's bundled `llama.cpp` backend. Users should not have to hand-export these values. -## Examine And Install UX Recommendations +## Diagnosing A WSL Host + +`rocm diagnose` carries a WSL catalog, separate from the bare-metal Linux one. +The bare-metal checks (render group, `/dev/kfd`, `modprobe amdgpu`, `iommu=pt`) +never run here: WSL has no `amdgpu` module and no `/dev/kfd`, so a finding +naming one would send you after a fault that cannot exist on this platform. + +```bash +rocm diagnose +rocm diagnose --json +``` + +The WSL entries, in the order a broken stack usually reveals them: -`rocm examine` should detect WSL cheaply and report: +| Fix id | Reported when | +| --- | --- | +| `fix-wsl-7-wsl1` | The distro runs under WSL 1, which has no GPU path at all | +| `fix-wsl-1-gpu-not-exposed` | `/dev/dxg` is missing. Names which of the three causes applies: a container started without the device, a distro with no WSL GPU support wired in, or the Windows host driver | +| `fix-wsl-2-dxcore-missing` | `libdxcore.so` is absent or off the loader path | +| `fix-wsl-3-rocdxg-missing` | ROCDXG is not installed in the distro | +| `fix-wsl-4-rocdxg-not-linked` | ROCDXG is installed but absent from the linker cache | +| `fix-wsl-5-distro-too-old` | The distro release is below the floor in the prerequisites above | +| `fix-wsl-6-host-driver-too-old` | The distro-side plumbing is complete but the Windows host driver is missing or too old | + +Every WSL remedy is print-only. `rocm fix ` shows the commands and does not +run them: they either install packages with `sudo`, edit loader configuration, or +belong to the Windows host, and none of that meets the bar the four +auto-applicable fixes clear. + +Two deliberate silences, so a report can be trusted: + +- `fix-wsl-6` **abstains** when WSL interop cannot reach the Windows host, rather + than reading "could not ask" as "driver is too old". Inside a container, or + with interop switched off, the host driver is reported as unknown and no + finding blames it. +- `fix-wsl-5` **abstains** when the distro release cannot be parsed. An + unreadable release is not evidence of a supported one, but it is not evidence + of an old one either. + +## What `rocm examine` Reports On WSL + +`rocm examine` detects WSL cheaply and reports: - `wsl: true` -- WSL distro/version +- WSL distro/version, and whether the release clears the supported floor +- WSL major version (1 or 2) - `/dev/dxg` presence - `/usr/lib/wsl/lib/libdxcore.so` presence -- `/opt/rocm/lib/librocdxg.so` presence +- `librocdxg.so` presence, resolved across every ROCm install rather than + assuming `/opt/rocm` - `librocdxg` linker-cache visibility from `ldconfig -p` -- `rocminfo` from the active TheRock runtime after activation -- whether `HSA_ENABLE_DXG_DETECTION` is needed or set +- the Windows host AMD driver version, when WSL interop can reach the host +- whether `HSA_ENABLE_DXG_DETECTION` is set - managed TheRock runtime count and active/default runtime +The driver, device-node and group probes stay skipped, so `has_amd_gpu` and +`gpus` describe the bare-metal view and are not populated here. + +## Install UX Recommendations + `rocm install sdk` inside WSL should: - default to a managed pip venv, same as native Linux and Windows @@ -164,9 +224,9 @@ and apply that environment before launching HIP apps such as Lemonade's bundled Safe tests that do not mutate global WSL state: -- `python scripts/wsl_preflight.py --self-test` -- `python scripts/wsl_preflight.py --json` -- `python scripts/wsl_preflight.py --require-ready` on a prepared WSL machine +- `rocm diagnose --json` +- `rocm diagnose --distro ` from the Windows host +- `cargo test -p rocm-core wsl` for the catalog's own tests - `rocm install sdk --channel release --format wheel --dry-run` inside WSL with isolated `ROCM_CLI_*` directories diff --git a/scripts/wsl_preflight.py b/scripts/wsl_preflight.py deleted file mode 100644 index 042aab4e1..000000000 --- a/scripts/wsl_preflight.py +++ /dev/null @@ -1,483 +0,0 @@ -#!/usr/bin/env python3 -# Copyright © Advanced Micro Devices, Inc., or its affiliates. -# -# SPDX-License-Identifier: MIT - -"""Read-only WSL/ROCDXG preflight for rocm-cli. - -This script intentionally does not install packages or modify global WSL state. -It can run from Windows and inspect a WSL distro, or run directly inside WSL. -""" - -from __future__ import annotations - -import argparse -import json -import platform -import shutil -import subprocess -from dataclasses import dataclass -from typing import Any - -LINUX_COLLECTOR = r""" -import glob -import json -import os -import platform -import shutil -import subprocess -from pathlib import Path - -def read_text(path): - try: - return Path(path).read_text(encoding="utf-8", errors="replace") - except OSError: - return "" - -def parse_os_release(text): - result = {} - for line in text.splitlines(): - if "=" not in line or line.lstrip().startswith("#"): - continue - key, value = line.split("=", 1) - result[key] = value.strip().strip("\"'") - return result - -def command_output(argv, timeout=20): - try: - completed = subprocess.run( - argv, - text=True, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - timeout=timeout, - check=False, - ) - except (OSError, subprocess.TimeoutExpired) as error: - return {"ok": False, "output": str(error)} - return {"ok": completed.returncode == 0, "output": completed.stdout.strip()} - -tools = ["git", "cmake", "gcc", "g++", "make", "python3", "rocminfo", "cargo"] -tool_paths = {tool: shutil.which(tool) for tool in tools} -venv_probe = {"ok": False, "output": "python3 missing"} -if tool_paths.get("python3"): - venv_probe = command_output([tool_paths["python3"], "-m", "venv", "--help"], timeout=20) - -ldconfig = command_output(["ldconfig", "-p"], timeout=20) -ldconfig_text = ldconfig.get("output", "") -proc_version = read_text("/proc/version") -sdk_headers = sorted( - glob.glob("/mnt/c/Program Files (x86)/Windows Kits/10/Include/*/shared/dxcore_interface.h") - + glob.glob("/mnt/c/Program Files (x86)/Windows Kits/10/Include/*/um/dxcore_interface.h") - + glob.glob("/mnt/c/Program Files (x86)/Windows Kits/10/Include/*/um/dxcore.h") -) - -state = { - "collector": "linux", - "kernel": platform.release(), - "machine": platform.machine(), - "proc_version": proc_version.strip(), - "is_wsl": "microsoft" in proc_version.lower() or Path("/dev/dxg").exists(), - "os_release": parse_os_release(read_text("/etc/os-release")), - "paths": { - "/dev/dxg": Path("/dev/dxg").exists(), - "/usr/lib/wsl/lib/libdxcore.so": Path("/usr/lib/wsl/lib/libdxcore.so").exists(), - "/opt/rocm/lib/librocdxg.so": Path("/opt/rocm/lib/librocdxg.so").exists(), - "/opt/rocm/share/rocdxg/dids.conf": Path("/opt/rocm/share/rocdxg/dids.conf").exists(), - }, - "ldconfig": { - "ok": bool(ldconfig.get("ok")), - "libdxcore": "libdxcore.so" in ldconfig_text, - "librocdxg": "librocdxg.so" in ldconfig_text, - "libhsa_runtime64": "libhsa-runtime64.so" in ldconfig_text, - "libamdhip64": "libamdhip64.so" in ldconfig_text, - }, - "tools": tool_paths, - "python_venv": bool(venv_probe.get("ok")), - "windows_sdk_headers": sdk_headers, - "env": { - "HSA_ENABLE_DXG_DETECTION": os.environ.get("HSA_ENABLE_DXG_DETECTION"), - "ROCM_ROOT": os.environ.get("ROCM_ROOT"), - "ROCM_PATH": os.environ.get("ROCM_PATH"), - "HIP_PATH": os.environ.get("HIP_PATH"), - "LD_LIBRARY_PATH": os.environ.get("LD_LIBRARY_PATH"), - }, -} -print(json.dumps(state, sort_keys=True)) -""" - - -@dataclass -class Check: - name: str - ok: bool - detail: str - required: bool = True - - -def clean_output(text: str) -> str: - return text.replace("\x00", "") - - -def run(argv: list[str], timeout: int = 60) -> subprocess.CompletedProcess[str]: - return subprocess.run( - argv, - text=True, - encoding="utf-8", - errors="replace", - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - timeout=timeout, - check=False, - ) - - -def parse_wsl_list(text: str) -> list[str]: - distros: list[str] = [] - for raw in clean_output(text).splitlines(): - line = raw.strip() - if not line or line.upper().startswith("NAME"): - continue - if line.startswith("*"): - line = line[1:].strip() - parts = line.split() - if parts: - distros.append(parts[0]) - return distros - - -def collect_from_windows(distro: str | None) -> dict[str, Any]: - if not shutil.which("wsl.exe"): - return {"collector": "windows", "error": "wsl.exe was not found on PATH"} - - listed = run(["wsl.exe", "-l", "-v"], timeout=30) - if listed.returncode != 0: - return { - "collector": "windows", - "error": "failed to list WSL distributions", - "output": clean_output(listed.stdout).strip(), - } - - distros = parse_wsl_list(listed.stdout) - if distro is None and len(distros) > 1: - return { - "collector": "windows", - "error": "multiple WSL distributions found; pass --distro explicitly", - "distros": distros, - "wsl_list": clean_output(listed.stdout).strip(), - } - selected = distro or (distros[0] if distros else None) - if not selected: - return { - "collector": "windows", - "error": "no WSL distributions were found", - "wsl_list": clean_output(listed.stdout).strip(), - } - - inspected = run( - [ - "wsl.exe", - "-d", - selected, - "--exec", - "/bin/bash", - "-lc", - f"python3 - <<'PY'\n{LINUX_COLLECTOR}\nPY", - ], - timeout=90, - ) - if inspected.returncode != 0: - return { - "collector": "windows", - "distro": selected, - "error": "failed to inspect WSL distribution; python3 may be missing", - "output": clean_output(inspected.stdout).strip(), - } - - try: - state = json.loads(clean_output(inspected.stdout)) - except json.JSONDecodeError as error: - return { - "collector": "windows", - "distro": selected, - "error": f"failed to parse WSL inspection JSON: {error}", - "output": clean_output(inspected.stdout).strip(), - } - state["collector"] = "windows-wsl" - state["distro"] = selected - state["wsl_list"] = clean_output(listed.stdout).strip() - return state - - -def collect_local_linux() -> dict[str, Any]: - completed = run( - ["python3", "-c", LINUX_COLLECTOR], - timeout=60, - ) - if completed.returncode != 0: - return { - "collector": "linux", - "error": "failed to inspect local Linux host", - "output": completed.stdout.strip(), - } - try: - return json.loads(completed.stdout) - except json.JSONDecodeError as error: - return { - "collector": "linux", - "error": f"failed to parse local inspection JSON: {error}", - "output": completed.stdout.strip(), - } - - -def collect_state(distro: str | None) -> dict[str, Any]: - if platform.system() == "Windows": - return collect_from_windows(distro) - if platform.system() == "Linux": - return collect_local_linux() - return { - "collector": platform.system().lower(), - "error": "only Windows and Linux are supported", - } - - -def evaluate( - state: dict[str, Any], - require_rocm_tools: bool, - require_build_tools: bool = False, -) -> list[Check]: - if state.get("error"): - return [Check("inspect", False, str(state["error"]))] - - paths = state.get("paths") if isinstance(state.get("paths"), dict) else {} - tools = state.get("tools") if isinstance(state.get("tools"), dict) else {} - os_release = ( - state.get("os_release") if isinstance(state.get("os_release"), dict) else {} - ) - ldconfig = state.get("ldconfig") if isinstance(state.get("ldconfig"), dict) else {} - sdk_headers = state.get("windows_sdk_headers") or [] - - version_id = str(os_release.get("VERSION_ID") or "") - # 22.04 is deliberately unsupported: its glibc 2.35 is below the glibc 2.38 / - # GLIBCXX_3.4.32 floor every published Lemonade embeddable is linked - # against, so the Lemonade engine cannot start on it. See docs/wsl.md. - version_parts = version_id.split(".") - ubuntu_version = None - if len(version_parts) == 2 and all( - part.isascii() and part.isdecimal() for part in version_parts - ): - ubuntu_version = (int(version_parts[0]), int(version_parts[1])) - supported_ubuntu = ( - os_release.get("ID") == "ubuntu" - and ubuntu_version is not None - and ubuntu_version >= (24, 4) - ) - build_tools = all( - tools.get(name) for name in ["git", "cmake", "gcc", "g++", "make"] - ) - - checks = [ - Check("wsl", bool(state.get("is_wsl")), "WSL marker or /dev/dxg detected"), - Check( - "ubuntu", - supported_ubuntu, - f"Ubuntu 24.04 or newer (VERSION_ID={version_id or ''})", - ), - Check("dxg_device", bool(paths.get("/dev/dxg")), "/dev/dxg"), - Check( - "dxcore", - bool(paths.get("/usr/lib/wsl/lib/libdxcore.so")), - "/usr/lib/wsl/lib/libdxcore.so", - ), - Check( - "windows_sdk", - bool(sdk_headers), - "Windows SDK dxcore headers visible from WSL", - required=require_build_tools, - ), - Check( - "librocdxg", - bool(paths.get("/opt/rocm/lib/librocdxg.so")), - "/opt/rocm/lib/librocdxg.so", - ), - Check( - "rocdxg_dids", - bool(paths.get("/opt/rocm/share/rocdxg/dids.conf")), - "/opt/rocm/share/rocdxg/dids.conf (not shipped by rocdxg-roct 1.2.0)", - required=False, - ), - Check( - "ldconfig_librocdxg", - bool(ldconfig.get("librocdxg")), - "librocdxg visible through ldconfig -p", - ), - Check( - "build_tools", - build_tools, - "git cmake gcc g++ make", - required=require_build_tools, - ), - Check("python_venv", bool(state.get("python_venv")), "python3 -m venv"), - Check( - "rocminfo", - bool(tools.get("rocminfo")), - "rocminfo command after ROCm/TheRock activation", - required=require_rocm_tools, - ), - Check( - "cargo", - bool(tools.get("cargo")), - "cargo for building rocm-cli in WSL", - required=False, - ), - ] - return checks - - -def render_human(state: dict[str, Any], checks: list[Check]) -> str: - lines = ["WSL preflight"] - if state.get("distro"): - lines.append(f" distro: {state['distro']}") - if state.get("kernel"): - lines.append(f" kernel: {state['kernel']}") - os_release = ( - state.get("os_release") if isinstance(state.get("os_release"), dict) else {} - ) - if os_release.get("PRETTY_NAME"): - lines.append(f" os: {os_release['PRETTY_NAME']}") - if state.get("error"): - lines.append(f" error: {state['error']}") - lines.append(" checks:") - for check in checks: - status = ( - "ok" if check.ok else ("missing" if check.required else "optional-missing") - ) - lines.append(f" {check.name}: {status} ({check.detail})") - if any(not check.ok and check.required for check in checks): - lines.append(" next:") - lines.append(" install/verify ROCDXG before running GPU HIP apps in WSL") - lines.append(" run with --json for machine-readable details") - return "\n".join(lines) - - -def self_test() -> None: - distros = parse_wsl_list( - " NAME STATE VERSION\n* Ubuntu Stopped 2\n" - ) - assert distros == ["Ubuntu"], distros - distros = parse_wsl_list( - " NAME STATE VERSION\n* Ubuntu Running 2\n Debian Stopped 2\n" - ) - assert distros == ["Ubuntu", "Debian"], distros - - base_state = { - "is_wsl": True, - "os_release": {"ID": "ubuntu", "VERSION_ID": "24.04"}, - "paths": { - "/dev/dxg": True, - "/usr/lib/wsl/lib/libdxcore.so": True, - "/opt/rocm/lib/librocdxg.so": False, - "/opt/rocm/share/rocdxg/dids.conf": False, - }, - "ldconfig": {"librocdxg": False}, - "tools": { - "git": "/usr/bin/git", - "cmake": "/usr/bin/cmake", - "gcc": "/usr/bin/gcc", - "g++": "/usr/bin/g++", - "make": "/usr/bin/make", - "rocminfo": None, - "cargo": None, - }, - "python_venv": True, - "windows_sdk_headers": ["/mnt/c/sdk/shared/dxcore_interface.h"], - } - checks = evaluate(base_state, require_rocm_tools=False) - assert any(check.name == "librocdxg" and not check.ok for check in checks) - assert not all(check.ok for check in checks if check.required) - - ready_state = json.loads(json.dumps(base_state)) - ready_state["paths"]["/opt/rocm/lib/librocdxg.so"] = True - ready_state["ldconfig"]["librocdxg"] = True - checks = evaluate(ready_state, require_rocm_tools=False) - assert all(check.ok for check in checks if check.required), checks - - for version_id in ["24.04", "24.10", "25.04", "26.04", "28.04"]: - supported_state = json.loads(json.dumps(ready_state)) - supported_state["os_release"]["VERSION_ID"] = version_id - checks = evaluate(supported_state, require_rocm_tools=False) - assert any(check.name == "ubuntu" and check.ok for check in checks), checks - assert all(check.ok for check in checks if check.required), checks - - # 22.04 is below the Lemonade glibc floor, and malformed or unknown - # versions must fail closed rather than report an otherwise perfect host ready. - for version_id in ["22.04", "24.04.1", "unknown", ""]: - unsupported_state = json.loads(json.dumps(ready_state)) - unsupported_state["os_release"]["VERSION_ID"] = version_id - checks = evaluate(unsupported_state, require_rocm_tools=False) - assert any(check.name == "ubuntu" and not check.ok for check in checks), checks - assert not all(check.ok for check in checks if check.required), checks - - missing_version_state = json.loads(json.dumps(ready_state)) - del missing_version_state["os_release"]["VERSION_ID"] - checks = evaluate(missing_version_state, require_rocm_tools=False) - assert any(check.name == "ubuntu" and not check.ok for check in checks), checks - assert not all(check.ok for check in checks if check.required), checks - - -def main() -> int: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument( - "--distro", help="WSL distribution name when running from Windows" - ) - parser.add_argument( - "--json", action="store_true", help="print machine-readable JSON" - ) - parser.add_argument( - "--require-ready", - action="store_true", - help="exit non-zero unless WSL and ROCDXG are ready", - ) - parser.add_argument( - "--require-rocm-tools", action="store_true", help="also require rocminfo" - ) - parser.add_argument( - "--require-build-tools", - action="store_true", - help="also require source-build tools such as the Windows SDK and CMake toolchain", - ) - parser.add_argument( - "--self-test", - action="store_true", - help="run parser/status self-tests without touching WSL", - ) - args = parser.parse_args() - - if args.self_test: - self_test() - print("wsl-preflight self-test ok") - return 0 - - state = collect_state(args.distro) - checks = evaluate( - state, - require_rocm_tools=args.require_rocm_tools, - require_build_tools=args.require_build_tools, - ) - payload = { - "state": state, - "checks": [check.__dict__ for check in checks], - "ready": all(check.ok for check in checks if check.required), - } - if args.json: - print(json.dumps(payload, indent=2, sort_keys=True)) - else: - print(render_human(state, checks)) - - if args.require_ready and not payload["ready"]: - return 2 - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/tests/e2e-cucumber/expectations.toml b/tests/e2e-cucumber/expectations.toml index 3eda8618c..1c8ed09f9 100644 --- a/tests/e2e-cucumber/expectations.toml +++ b/tests/e2e-cucumber/expectations.toml @@ -11,8 +11,9 @@ # can't be satisfied on this host is SKIPPED (not-applicable) — never listed # here. Reach for a tag, not a row, whenever the scenario simply has no # premise on that host: a row here says "known BUG", so using one for -# designed-in behaviour (e.g. diagnose not running its bare-metal catalog on -# WSL2) tells every later reader the opposite of the truth. +# designed-in behaviour (e.g. diagnose not offering a bare-metal cause on +# WSL2, where no amdgpu module exists to blame) tells every later reader the +# opposite of the truth. # 2. Otherwise, the FIRST matching condition below marks it expected-fail # (xfail); a scenario that then PASSES is an XPASS — a stale entry to be # removed, UNLESS the row is `flaky = true`, which tolerates either result diff --git a/tests/e2e-cucumber/features/diagnose.feature b/tests/e2e-cucumber/features/diagnose.feature index 28dfd5102..2f389c2ce 100644 --- a/tests/e2e-cucumber/features/diagnose.feature +++ b/tests/e2e-cucumber/features/diagnose.feature @@ -5,18 +5,16 @@ Feature: Diagnosing failures and listing fixes # remediations. Both are black-box and GPU-independent (no serve, no download, # no mutation), so every scenario here runs on the mock lane / per-PR tier. # - # The catalog is OS-gated (the checkers only run on linux/windows), so these - # scenarios do NOT assert a specific fix-id — the top match is environment- - # dependent. They assert the SHAPE of a diagnosis (a scored match with an id - # and a plan) and the query/refusal contracts. - - # @requires-bare-metal: these two need the catalog to actually produce a match. - # On WSL2 the catalog is deliberately not run at all — that platform uses - # /dev/dxg and the Windows host driver, so bare-metal Linux diagnoses would be - # false positives — which leaves these scenarios with no premise there. That is - # designed behaviour with its own unit test, not a bug, so they are skipped - # rather than xfail'd. `@requires-os:linux` would not do it: WSL2 is linux. - @id:diagnose-matches-known-symptom @requires-bare-metal + # The catalog is platform-gated (linux, windows and wsl each select their own + # entries), so these scenarios do NOT assert a specific fix-id — the top match + # is environment-dependent. They assert the SHAPE of a diagnosis (a scored match + # with an id and a plan) and the query/refusal contracts. + # + # These two used to carry @requires-bare-metal, because WSL2 ran no catalog at + # all and so had no premise for a match. WSL2 has its own entries now, and the + # symptom-keyword checks that were always valid there run too, so both hold on + # every supported platform and the tag is gone. + @id:diagnose-matches-known-symptom Scenario: diagnose-01 - Diagnosing a recognised failure reports a likely cause and a fix Given a user who hit a known ROCm failure When the user asks the CLI to diagnose that symptom @@ -29,7 +27,7 @@ Feature: Diagnosing failures and listing fixes When the user asks the CLI to diagnose that symptom in machine-readable form Then the CLI always points to somewhere the problem can be reported - @id:diagnose-json-has-match-flag @requires-bare-metal + @id:diagnose-json-has-match-flag Scenario: diagnose-03 - A diagnosis is available in machine-readable form for tooling Given a user who hit a known ROCm failure When the user asks the CLI to diagnose that symptom in machine-readable form @@ -72,7 +70,15 @@ Feature: Diagnosing failures and listing fixes # same recipe persists through `setx` into the user environment, which the # suite cannot plant or read back safely. The gate itself is shared code, so # this still guards it — just not the Windows persistence step. - @id:diagnose-fix-requires-agreement-before-changing-anything @requires-os:linux + # + # @requires-bare-metal on top of that: the scenario needs a fix that both + # applies here AND reaches the consent gate, and only fix-9 does that on a host + # with nothing installed. fix-9 does not apply on WSL2 — a single device with + # no topology cannot have an iGPU/dGPU collision — so there the run stops at + # the wrong-platform refusal before the gate is ever reached. That is designed + # behaviour, not a bug, so it is a skip rather than an xfail. The gate is + # shared code and stays covered by the mock and Linux GPU lanes. + @id:diagnose-fix-requires-agreement-before-changing-anything @requires-os:linux @requires-bare-metal Scenario: diagnose-08 - A fix that changes the machine is not applied without agreement Given a user who has chosen a fix that would change the machine When the user asks the CLI to apply it without agreeing to the change @@ -101,12 +107,15 @@ Feature: Diagnosing failures and listing fixes # proves nothing about what gets reported there. # # Be precise about where each half runs, because the halves are not equal. - # There is NO WSL2 lane in CI (every job pins a native runner), so the - # route-out half is proven only by a developer running the suite on WSL2. - # What CI gets is the covered half plus the cross-check against the host - # report — both of which can fail, which is the bar an assertion has to clear - # to be worth writing. An earlier version of this scenario returned early on a - # covered platform and asserted nothing at all on any lane CI runs. + # Every lane CI runs — mock, the GPU lanes, and the WSL2 lane on Strix Halo — + # is a covered platform, so what CI proves is the covered half plus the + # cross-check against the host report. Both of those can fail, which is the bar + # an assertion has to clear to be worth writing. An earlier version of this + # scenario returned early on a covered platform and asserted nothing at all. + # + # The uncovered half no longer means WSL2: that platform has its own entries + # now. It means a host that is neither Linux, Windows nor WSL, which no lane + # runs, so that half is exercised by the unit tests rather than here. @id:diagnose-states-whether-the-platform-is-covered Scenario: diagnose-10 - A platform the catalog does not cover says so and routes onward Given a user who hit a known ROCm failure @@ -210,3 +219,40 @@ Feature: Diagnosing failures and listing fixes When the user is asked interactively to apply it and types no Then the CLI declines on the terminal and explains that it needs agreement And the file the fix would have changed is untouched + + # WSL2 reaches the GPU through /dev/dxg and the Windows host driver, so the + # bare-metal questions — render group, /dev/kfd, modprobe amdgpu — have no + # answer there and any finding naming one would be a false positive. This is + # the guard on the platform split; it is what makes covering WSL2 safe rather + # than merely louder. @requires-os:linux would not express it: WSL2 is linux. + @id:diagnose-wsl-never-reports-bare-metal-causes @requires-wsl + Scenario: diagnose-17 - A WSL machine is never given a bare-metal cause + Given a user who hit a known ROCm failure + When the user asks the CLI to diagnose that symptom in machine-readable form + Then no reported cause is one that only exists on bare-metal Linux + And the result says this platform is covered + + # The remedies for a WSL GPU problem mostly live on the Windows host or install + # packages with sudo, so none of them are ones the CLI carries out. A caller + # that could not tell "explained" from "attempted" would report a changed + # machine when nothing was touched. + @id:diagnose-wsl-fix-is-explained-not-attempted @requires-wsl + Scenario: diagnose-18 - A WSL remedy is explained rather than carried out + Given a user who has chosen a WSL remedy that belongs on the Windows host + When the user asks the CLI to apply that fix + Then the CLI explains the remedy instead of carrying it out + And nothing on the machine is changed + + # `rocm diagnose` can be pointed at another machine — a WSL distribution, from + # the Windows host. The dangerous failure is not an error, it is a SILENT + # fallback: reporting on the local machine when the user asked about a + # different one hands them a verdict about the wrong host, and nothing in the + # output says so. This holds everywhere, because "that machine is not reachable + # from here" is as true on Linux, where there is no wsl.exe at all, as it is on + # a Windows host that has no such distribution. + @id:diagnose-unreachable-machine-is-refused-not-substituted + Scenario: diagnose-19 - Asking about a machine that cannot be reached is refused, not substituted + Given a user who asks to diagnose a machine that does not exist + When the user asks the CLI to diagnose that machine + Then the CLI refuses and explains that it could not reach that machine + And no diagnosis of this machine is reported diff --git a/tests/e2e-cucumber/src/expectation.rs b/tests/e2e-cucumber/src/expectation.rs index d8b796315..d5c7e38e0 100644 --- a/tests/e2e-cucumber/src/expectation.rs +++ b/tests/e2e-cucumber/src/expectation.rs @@ -855,6 +855,44 @@ serve_timeout_secs = 90 } } + #[test] + fn combining_requires_os_and_requires_bare_metal_narrows_to_native_linux() { + // diagnose-08's exact tag set. It needs a Linux host (the assertion reads + // back a shell rc file) that is ALSO not WSL (the only auto-applicable + // fix reaching the consent gate does not apply there). Neither tag says + // that alone, so the pair has to compose — and a `-n` filtered local run + // bypasses this resolution entirely, which makes it worth pinning here + // rather than trusting a hand-run of the suite. + let m = Expectations::default(); + let d = decl(&[ + "id:diagnose-fix-requires-agreement-before-changing-anything", + "requires-os:linux", + "requires-bare-metal", + ]); + for wsl in ["wsl", "wsl2"] { + assert!( + matches!( + resolve(&d, &cap(wsl), &m, false, false, false), + Expectation::Skip { .. } + ), + "{wsl} is linux but not bare metal, so the scenario has no premise" + ); + } + assert!(matches!( + resolve(&d, &cap("strix-windows"), &m, false, false, false), + Expectation::Skip { .. } + )); + // `mock` is deliberately absent: the fixture models it as os_family + // "other", so it cannot stand for the real mock lane here. + for host in ["mi300x", "strix-ubuntu"] { + assert_eq!( + resolve(&d, &cap(host), &m, false, false, false), + Expectation::ExpectPass, + "{host} is native Linux and must still run the scenario" + ); + } + } + /// The exact inverse of the tag above: proving both directions keeps the /// pair from silently collapsing into one predicate. #[test] diff --git a/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs b/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs index 0f4351bc4..7e80ca6a6 100644 --- a/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs @@ -81,6 +81,13 @@ const CATALOG_FIX_IDS: &[&str] = &[ // out-of-memory entry on its own branch. The number is a stable handle, so // the two are kept distinct rather than renamed after the fact. "fix-17-torch-dlpack", + "fix-wsl-1-gpu-not-exposed", + "fix-wsl-2-dxcore-missing", + "fix-wsl-3-rocdxg-missing", + "fix-wsl-4-rocdxg-not-linked", + "fix-wsl-5-distro-too-old", + "fix-wsl-6-host-driver-too-old", + "fix-wsl-7-wsl1", ]; /// The fixes the CLI carries out itself. Every other entry only prints a plan. @@ -93,6 +100,29 @@ const AUTO_APPLICABLE_FIX_IDS: &[&str] = &[ "fix-9-igpu-dgpu", ]; +/// A WSL distribution name no host will have. Deliberately not a plausible one: +/// the scenario must fail for "this machine does not exist", never because the +/// runner happened to have a distro by that name. +const UNREACHABLE_DISTRO: &str = "rocm-cli-e2e-no-such-distro"; + +/// The WSL entry whose remedy is entirely on the Windows host, so the CLI can +/// only ever explain it. Applies on WSL, which is what makes the scenario a test +/// of "explained, not attempted" rather than of the wrong-OS refusal. +const WSL_HOST_SIDE_FIX_ID: &str = "fix-wsl-6-host-driver-too-old"; + +/// Causes that can only exist on bare-metal Linux: they name the amdgpu module, +/// /dev/kfd, the render group, or the distro package manager, none of which +/// govern anything under WSL2. +const BARE_METAL_ONLY_FIX_IDS: &[&str] = &[ + "fix-3-rocm-kernel", + "fix-4-render-group", + "fix-5-amdgpu-load", + "fix-7-stale-repos", + "fix-10-container", + "fix-11-iommu", + "fix-12-installer", +]; + /// A catalog entry that cannot apply on the host running the suite, whichever /// host that is. Both are print-only, so the run stops at the OS gate without /// reaching any recipe that could touch the machine. @@ -214,6 +244,11 @@ async fn user_approved_fix_that_will_fail(world: &mut E2eWorld) { world.model_name = Some(COMMAND_FAILURE_FIX_ID.to_string()); } +#[given("a user who has chosen a WSL remedy that belongs on the Windows host")] +async fn user_chose_wsl_host_remedy(world: &mut E2eWorld) { + world.model_name = Some(WSL_HOST_SIDE_FIX_ID.to_string()); +} + #[given("a user who refers to a cause by its position in the diagnosis")] async fn user_named_diagnosis_position(world: &mut E2eWorld) { // Quoted deliberately: unquoted, the shell treats `#1` as a comment and the @@ -224,6 +259,21 @@ async fn user_named_diagnosis_position(world: &mut E2eWorld) { // ── When ─────────────────────────────────────────────────────────── +#[given("a user who asks to diagnose a machine that does not exist")] +async fn user_named_a_missing_machine(world: &mut E2eWorld) { + world.model_name = Some(UNREACHABLE_DISTRO.to_string()); +} + +#[when("the user asks the CLI to diagnose that machine")] +async fn user_diagnoses_named_machine(world: &mut E2eWorld) { + let distro = world.model_name.clone().expect("no machine named"); + let (stdout, stderr, rc) = crate::run_rocm(world, &["diagnose", "--distro", &distro]); + // The refusal goes to stderr; keep both so the assertions can read whichever + // stream carried it without caring which. + world.cli_output = Some(format!("{stdout}\n{stderr}")); + world.cli_rc = Some(rc); +} + #[when("the user asks the CLI to diagnose that symptom")] async fn user_diagnoses(world: &mut E2eWorld) { let symptom = world.model_name.clone().expect("no symptom set"); @@ -610,9 +660,12 @@ async fn assert_json_states_platform_scope(world: &mut E2eWorld) { // skip_serializing_if, so serde emits it either way and its mere presence // proves nothing. Cross-check the verdict against the one the host report // gives for the same machine — the same trick `examine-both-forms-agree-on-gpu` - // uses, and the only version of this assertion that can fail on a covered - // host. The two are computed by different code paths off the same probe, so + // uses. The two are computed by different code paths off the same probe, so // this is a cross-check rather than a tautology. + // + // This used to read `status == "wsl"` as "uncovered". WSL2 has its own + // catalog entries now, so the two questions came apart: the platforms with no + // entries are the ones that are neither Linux, Windows, nor WSL. let (examine, _, rc) = crate::run_rocm(world, &["examine", "--json"]); assert_eq!(rc, 0, "examine should exit 0 (it is an inspector)"); let host: serde_json::Value = @@ -620,7 +673,7 @@ async fn assert_json_states_platform_scope(world: &mut E2eWorld) { let host_says_uncovered = host .get("status") .and_then(serde_json::Value::as_str) - .is_some_and(|status| status == "wsl"); + .is_some_and(|status| status == "unsupported-os"); let diagnosis_says_uncovered = report.get("out_of_scope").is_some_and(|v| !v.is_null()); assert_eq!( diagnosis_says_uncovered, host_says_uncovered, @@ -630,6 +683,97 @@ async fn assert_json_states_platform_scope(world: &mut E2eWorld) { ); } +#[then("no reported cause is one that only exists on bare-metal Linux")] +async fn assert_no_bare_metal_cause(world: &mut E2eWorld) { + let (report, output) = parsed_diagnosis(world); + let matched = report + .get("matched") + .and_then(|m| m.as_array()) + .expect("diagnose JSON has no 'matched' array"); + for entry in matched { + let id = entry + .get("id") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + assert!( + !BARE_METAL_ONLY_FIX_IDS.contains(&id), + "{id} names something WSL2 does not have (amdgpu module, /dev/kfd, \ + render group), so reporting it here would send the user after a \ + fault that cannot exist on this platform:\n{output}" + ); + } +} + +#[then("the CLI refuses and explains that it could not reach that machine")] +async fn assert_unreachable_machine_refused(world: &mut E2eWorld) { + let output = world.cli_output.clone().unwrap_or_default(); + let rc = world.cli_rc.expect("no exit code recorded"); + assert_ne!( + rc, 0, + "asking about an unreachable machine must fail:\n{output}" + ); + // Not `contains("wsl")`: that matches essentially any message this code path + // can emit, so it would pass on a refusal that never said what went wrong. + // The refusal has to name the machine the user asked about, or say that + // reaching another machine is not possible from here at all. + let lowered = output.to_lowercase(); + assert!( + lowered.contains(&UNREACHABLE_DISTRO.to_lowercase()) + || lowered.contains("wsl.exe was not found"), + "the refusal must name the machine it could not reach, or say why no \ + machine could be reached:\n{output}" + ); +} + +#[then("no diagnosis of this machine is reported")] +async fn assert_no_local_diagnosis_substituted(world: &mut E2eWorld) { + // The failure this guards is a silent substitution: reporting on the local + // machine when the user asked about another one. A diagnosis is recognisable + // by its `id:` line and its `apply with:` call to action, so neither may be + // present. + let output = world.cli_output.clone().unwrap_or_default(); + for marker in ["id: fix-", "apply with:"] { + assert!( + !output.contains(marker), + "a request about another machine must not be answered with this \ + one's diagnosis (found {marker:?}):\n{output}" + ); + } +} + +#[then("the result says this platform is covered")] +async fn assert_platform_is_covered(world: &mut E2eWorld) { + let (report, output) = parsed_diagnosis(world); + let out_of_scope = report.get("out_of_scope"); + assert!( + out_of_scope.is_none_or(serde_json::Value::is_null), + "this platform has catalog entries, so it must not be reported as \ + uncovered:\n{output}" + ); +} + +#[then("the CLI explains the remedy instead of carrying it out")] +async fn assert_remedy_explained_not_applied(world: &mut E2eWorld) { + let fix_id = world + .model_name + .clone() + .expect("scenario did not choose a fix"); + let (output, _, rc) = crate::run_rocm(world, &["fix", &fix_id]); + // 0, not the 3 a wrong-OS refusal gives: this fix does apply here. It is + // print-only because the change belongs to the Windows host, and the two + // outcomes must stay distinguishable to a caller. + assert_eq!( + rc, 0, + "{fix_id} applies on this host and is print-only, so it must succeed \ + without acting:\n{output}" + ); + let lowered = output.to_lowercase(); + assert!( + lowered.contains("print-only"), + "the CLI must say it only printed a plan:\n{output}" + ); +} + #[then("a platform that is not covered is given no diagnosis")] async fn assert_uncovered_platform_gets_no_diagnosis(world: &mut E2eWorld) { let (report, output) = parsed_diagnosis(world); From 3163f1560ebbebfc58bae3744fa84c4666898e32 Mon Sep 17 00:00:00 2001 From: Eugene Volen Date: Fri, 11 Sep 2026 08:15:49 +0000 Subject: [PATCH 2/4] fix(doctor): tighten WSL detection and close review gaps in the WSL catalog Require $WSL_DISTRO_NAME to be corroborated by /proc/version (rather than trusted alone) before treating a host as WSL, since a bare-metal host that merely inherited the variable would otherwise have its whole bare-metal catalog silently disabled. Also, from review: - guard check_wsl_6_host_driver_too_old's second arm on the linker cache, symmetric with fix-wsl-4, so an unlinked librocdxg doesn't also raise a host-driver-too-old finding for the same fault - move a misattached doc comment onto the function it actually describes - add a test that independently recomputes ground truth from real /dev/dxg and /proc/version rather than routing through the same current_os() logic under test - assert synced values (not just key presence) for sync_shared_fields_from_wsl, and prove the assertion depends on the sync call actually running - extract select_wsl_distro out of probe_wsl_distro_from_host so its distro-not-found refusal can be unit tested without wsl.exe - note in docs/wsl.md that rocm diagnose does not carry over the old preflight script's --require-build-tools/venv checks Signed-off-by: Eugene Volen --- crates/rocm-core/src/diagnose.rs | 43 +++++++- crates/rocm-core/src/examine.rs | 166 +++++++++++++++++++++++++++---- crates/rocm-core/src/fix.rs | 43 ++++++++ crates/rocm-core/src/lib.rs | 125 +++++++++++++++++------ docs/wsl.md | 5 + 5 files changed, 327 insertions(+), 55 deletions(-) diff --git a/crates/rocm-core/src/diagnose.rs b/crates/rocm-core/src/diagnose.rs index cc24729d4..51226e856 100644 --- a/crates/rocm-core/src/diagnose.rs +++ b/crates/rocm-core/src/diagnose.rs @@ -1719,8 +1719,18 @@ fn check_wsl_6_host_driver_too_old(e: &Examination, symptom: &str) -> Diagnosis // not, and this check fired on a complete working stack. `Some(false)` // specifically -- `None` means rocminfo was absent so the question went // unasked, which is not evidence of anything. + // + // Guarded on the linker cache like fix-wsl-4: when `ldconfig` positively + // shows librocdxg is not registered, that is the root cause and this + // check must not also fire for the same symptom -- the user would be + // left choosing between "run ldconfig" and "update the host driver" for + // a fault that only the former explains. Some(version) - if w.dxg_device && w.dxcore && w.librocdxg && w.rocm_sees_gpu == Some(false) => + if w.dxg_device + && w.dxcore + && w.librocdxg + && w.ldconfig_librocdxg != Some(false) + && w.rocm_sees_gpu == Some(false) => { score += 40; evidence.push(format!( @@ -2656,6 +2666,37 @@ mod tests { assert_eq!(top.id, "fix-wsl-4-rocdxg-not-linked"); } + #[test] + fn an_unlinked_rocdxg_does_not_also_raise_the_host_driver_finding() { + // Same fault as above, but with `rocm_sees_gpu = Some(false)` added so + // this machine also satisfies every other condition + // check_wsl_6_host_driver_too_old's second arm checks (dxg_device, + // dxcore, librocdxg, rocminfo seeing no GPU). Without the ldconfig + // guard on that arm, it fires alongside fix-wsl-4 for the same + // underlying fault, leaving the user to guess between "run ldconfig" + // and "update the Windows host driver" when only the former is true. + // + // The symptom text also carries a generic "cannot open shared object + // file" keyword that fix-8-wheel-rocm scores on regardless of WSL + // state -- that overlap is real and expected, so this only asserts on + // the two WSL findings that fix #2's guard actually governs. + let mut e = wsl_base(); + let w = e.wsl.as_mut().expect("wsl facts"); + w.ldconfig_librocdxg = Some(false); + w.rocm_sees_gpu = Some(false); + let report = diagnose(&e, "librocdxg.so: cannot open shared object file"); + let ids: Vec<&str> = report.matched.iter().map(|d| d.id.as_str()).collect(); + assert!( + ids.contains(&"fix-wsl-4-rocdxg-not-linked"), + "the unlinked-library finding must still fire: ids: {ids:?}" + ); + assert!( + !ids.contains(&"fix-wsl-6-host-driver-too-old"), + "the host-driver-too-old finding must not overlap with the unlinked-library \ + finding on the same fault: ids: {ids:?}" + ); + } + #[test] fn a_distro_below_the_floor_is_reported() { let mut e = wsl_base(); diff --git a/crates/rocm-core/src/examine.rs b/crates/rocm-core/src/examine.rs index 39ebd1420..5a9daa18c 100644 --- a/crates/rocm-core/src/examine.rs +++ b/crates/rocm-core/src/examine.rs @@ -714,27 +714,7 @@ pub fn probe_wsl_distro_from_host(distro: Option<&str>) -> Result { - if !distros.iter().any(|d| d.eq_ignore_ascii_case(name)) { - return Err(format!( - "no WSL distribution named '{name}'; found: {}", - distros.join(", ") - )); - } - name.to_owned() - } - None => match distros.as_slice() { - [] => return Err("no WSL distributions were found".to_owned()), - [only] => only.clone(), - many => { - return Err(format!( - "several WSL distributions are installed; name one with --distro: {}", - many.join(", ") - )); - } - }, - }; + let selected = select_wsl_distro(distro, &distros)?; let (rc, out, _) = run( "wsl.exe", @@ -791,6 +771,36 @@ pub fn probe_wsl_distro_from_host(distro: Option<&str>) -> Result, distros: &[String]) -> Result { + match distro { + Some(name) => { + if !distros.iter().any(|d| d.eq_ignore_ascii_case(name)) { + return Err(format!( + "no WSL distribution named '{name}'; found: {}", + distros.join(", ") + )); + } + Ok(name.to_owned()) + } + None => match distros { + [] => Err("no WSL distributions were found".to_owned()), + [only] => Ok(only.clone()), + many => Err(format!( + "several WSL distributions are installed; name one with --distro: {}", + many.join(", ") + )), + }, + } +} + /// Whether `rocminfo` enumerates a GPU agent. /// /// Only the yes/no answer is taken. Parsing the agents into `gpus` is the job of @@ -1897,6 +1907,120 @@ mod tests { ); } + #[test] + fn a_named_distro_that_does_not_exist_is_refused() { + // This is the blocking refusal `probe_wsl_distro_from_host` returns + // to the caller of `rocm diagnose --distro ` -- a plain error, + // not a `continue-on-error` note the e2e suite can shrug off. It sat + // behind `wsl.exe` and was untested outside a real Windows+WSL host, + // which is not a lane this crate's unit tests run on. + let distros = vec!["Ubuntu".to_owned(), "Debian".to_owned()]; + let err = select_wsl_distro(Some("Fedora"), &distros).expect_err("must be refused"); + assert!(err.contains("Fedora"), "names the distro asked for: {err}"); + assert!(err.contains("Ubuntu"), "lists what does exist: {err}"); + assert!(err.contains("Debian"), "lists what does exist: {err}"); + } + + #[test] + fn a_named_distro_that_exists_is_selected_case_insensitively() { + // Matched case-insensitively against the installed list, but the name + // handed to `wsl.exe -d` afterwards is what the caller typed, not the + // list's original casing -- pre-existing behavior, preserved as-is by + // this extraction. + let distros = vec!["Ubuntu-24.04".to_owned()]; + assert_eq!( + select_wsl_distro(Some("ubuntu-24.04"), &distros).expect("must be selected"), + "ubuntu-24.04" + ); + } + + #[test] + fn no_distro_named_falls_back_to_the_lone_one_or_refuses_ambiguity() { + assert_eq!( + select_wsl_distro(None, &["Ubuntu".to_owned()]).expect("the only one"), + "Ubuntu" + ); + assert!(select_wsl_distro(None, &[]).is_err(), "nothing installed"); + let many = vec!["Ubuntu".to_owned(), "Debian".to_owned()]; + let err = select_wsl_distro(None, &many).expect_err("ambiguous without --distro"); + assert!( + err.contains("--distro"), + "tells the user how to resolve it: {err}" + ); + } + + #[test] + fn sync_shared_fields_from_wsl_copies_the_actual_rocminfo_values() { + // The fields the cross-platform PATH and wheel/ROCm checks read. + // Asserting only that they are *set* would still pass if the sync + // copied the wrong value or a hardcoded default -- assert the actual + // values reached from each distinct `WslFacts` fixture instead. + let mut missing = Examination { + wsl: Some(WslFacts { + rocminfo: false, + rocm_sees_gpu: None, + ..WslFacts::default() + }), + ..Examination::default() + }; + sync_shared_fields_from_wsl(&mut missing); + assert!(!missing.rocminfo_present); + assert_eq!(missing.rocminfo_status, "missing"); + + let mut healthy = Examination { + wsl: Some(WslFacts { + rocminfo: true, + rocm_sees_gpu: Some(true), + ..WslFacts::default() + }), + ..Examination::default() + }; + sync_shared_fields_from_wsl(&mut healthy); + assert!(healthy.rocminfo_present); + assert_eq!(healthy.rocminfo_status, "ok"); + + let mut no_agents = Examination { + wsl: Some(WslFacts { + rocminfo: true, + rocm_sees_gpu: Some(false), + ..WslFacts::default() + }), + ..Examination::default() + }; + sync_shared_fields_from_wsl(&mut no_agents); + assert!(no_agents.rocminfo_present); + assert_eq!(no_agents.rocminfo_status, "no-agents"); + + let mut unasked = Examination { + wsl: Some(WslFacts { + rocminfo: true, + rocm_sees_gpu: None, + ..WslFacts::default() + }), + ..Examination::default() + }; + sync_shared_fields_from_wsl(&mut unasked); + assert!(unasked.rocminfo_present); + assert_eq!(unasked.rocminfo_status, "unknown"); + } + + #[test] + fn probe_wsl_leaves_rocminfo_status_synced_not_at_its_uninitialized_default() { + // `probe_wsl` never sets `rocminfo_status` itself -- only + // `sync_shared_fields_from_wsl`, called at its very end, does. If that + // call were ever removed, this field would stay at + // `Examination::default()`'s empty string on every host, WSL or not, + // regardless of whether rocminfo happens to be installed here -- so + // this holds without depending on real host state, unlike a test that + // compared against the actual probed rocminfo presence. + let mut e = Examination::default(); + probe_wsl(&mut e); + assert_ne!( + e.rocminfo_status, "", + "sync_shared_fields_from_wsl must have run and set a real status" + ); + } + #[test] fn the_distro_list_decodes_as_utf16_whatever_script_it_is_in() { fn utf16le(text: &str, bom: bool) -> Vec { diff --git a/crates/rocm-core/src/fix.rs b/crates/rocm-core/src/fix.rs index 82ee6daa5..42dbb5008 100644 --- a/crates/rocm-core/src/fix.rs +++ b/crates/rocm-core/src/fix.rs @@ -1428,6 +1428,49 @@ mod tests { assert_engine_shell_boundary_is_labelled(recipe.fix_id, recipe.commands); } + #[test] + #[allow(unsafe_code)] // std::env::set_var is unsafe in edition 2024 + fn current_os_reaches_wsl_only_when_the_distro_env_var_is_corroborated() { + // `current_os()`'s wsl branch is `crate::is_wsl_host()`, which trusts + // `/dev/dxg` outright but requires `$WSL_DISTRO_NAME` to be + // corroborated by a kernel string that actually names Microsoft/WSL + // (see `wsl_signals_indicate_wsl`). `recipe_applies_here` below cannot + // check this independently: it calls `current_os()` to answer its own + // question, so a bug in `current_os()` would agree with itself and no + // test built on that helper would ever notice. This test instead + // forces the one signal that can be forced portably + // ($WSL_DISTRO_NAME) and compares the result against ground truth + // computed straight from this machine's real `/dev/dxg` and + // `/proc/version`, so it holds whether it runs on a bare-metal CI + // runner, Windows, or a real WSL host. + let proc_version = std::fs::read_to_string("/proc/version") + .unwrap_or_default() + .to_ascii_lowercase(); + let corroborated = std::path::Path::new("/dev/dxg").exists() + || proc_version.contains("microsoft") + || proc_version.contains("wsl"); + + let previous = std::env::var_os("WSL_DISTRO_NAME"); + unsafe { + std::env::set_var("WSL_DISTRO_NAME", "Ubuntu"); + } + let os = current_os(); + unsafe { + match previous { + Some(value) => std::env::set_var("WSL_DISTRO_NAME", value), + None => std::env::remove_var("WSL_DISTRO_NAME"), + } + } + + assert_eq!( + os == "wsl", + corroborated, + "current_os() must land on \"wsl\" exactly when the forced \ + $WSL_DISTRO_NAME is corroborated by this machine's real /dev/dxg \ + or /proc/version, not merely because the variable is set" + ); + } + /// Whether a recipe applies on the platform the test is running on. /// /// Tests used to gate on `runtime_is_linux()`, which stopped being the same diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index 1b8594263..34ec87a2b 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -2420,9 +2420,11 @@ fn normalize_cpu_model(value: &str) -> String { /// forms. Worse, the e2e harness derives `is_wsl` for its whole expectation /// matrix by reading one of them. /// -/// This is the union of every signal any of them used: a false positive costs a -/// route-out note, a false negative runs bare-metal driver checks against a -/// platform that has no amdgpu module and reports nonsense. +/// `/dev/dxg` is trusted on its own. `$WSL_DISTRO_NAME` is not — it is an +/// ordinary environment variable that can survive into a shell that merely +/// inherited it — so it is corroborated against `/proc/version` before being +/// believed. See [`wsl_signals_indicate_wsl`] for why a false positive is no +/// longer cheap. #[must_use] pub fn is_wsl_host() -> bool { runtime_is_linux() @@ -2456,13 +2458,6 @@ pub(crate) fn is_wsl1_kernel(kernel_release: &str) -> bool { kernel.ends_with("-microsoft") && !kernel.contains("standard") } -/// Whether `relative` exists under any ROCm install on this host. -/// -/// The WSL probe used to hardcode `/opt/rocm`, so a versioned install at -/// `/opt/rocm-7.x` reported ROCDXG missing and the catalog would then blame a -/// package that was in fact installed. Ask the same resolver the rest of the CLI -/// uses, and keep the conventional root as a fallback for the case where -/// discovery finds nothing. /// The dynamic linker cache, or `None` when `ldconfig` could not be run. /// /// `ldconfig` lives in `/sbin`, which is not on a non-root user's `PATH` on @@ -2487,6 +2482,13 @@ pub(crate) fn ldconfig_lists_librocdxg() -> Option { ldconfig_cache().map(|text| text.contains("librocdxg.so")) } +/// Whether `relative` exists under any ROCm install on this host. +/// +/// The WSL probe used to hardcode `/opt/rocm`, so a versioned install at +/// `/opt/rocm-7.x` reported ROCDXG missing and the catalog would then blame a +/// package that was in fact installed. Ask the same resolver the rest of the CLI +/// uses, and keep the conventional root as a fallback for the case where +/// discovery finds nothing. fn rocm_relative_file_exists(relative: &str) -> bool { if Path::new("/opt/rocm").join(relative).exists() { return true; @@ -2498,12 +2500,22 @@ fn rocm_relative_file_exists(relative: &str) -> bool { /// The predicate itself, separated from reading the machine so the union can be /// tested — including the two cases that used to split the old implementations. +/// +/// `/dev/dxg` alone is trusted outright — nothing but WSLg's GPU passthrough +/// creates that device node. `$WSL_DISTRO_NAME` alone is not: it is an ordinary +/// environment variable that survives into a shell someone launched with it +/// inherited or forwarded (e.g. over `ssh`), so it is corroborated against +/// `/proc/version` before being believed. A false positive here no longer costs +/// only a route-out note — this catalog now runs the WSL diagnosis and fix set +/// directly, so a bare-metal host that merely inherited the variable would have +/// its entire bare-metal catalog silently disabled. fn wsl_signals_indicate_wsl(dxg_device: bool, distro_name_set: bool, proc_version: &str) -> bool { - if dxg_device || distro_name_set { + if dxg_device { return true; } let proc_version = proc_version.to_ascii_lowercase(); - proc_version.contains("microsoft") || proc_version.contains("wsl") + let proc_version_matches = proc_version.contains("microsoft") || proc_version.contains("wsl"); + distro_name_set && proc_version_matches } fn detect_wsl_summary() -> Option { @@ -12058,26 +12070,69 @@ last_installed_runtime_id = "therock-release" } #[test] - fn every_wsl_signal_is_believed_by_the_one_predicate() { - // The union. Each of these was decisive to at least one of the three - // implementations this replaces. + fn dev_dxg_is_believed_on_its_own() { + // Nothing but WSLg's GPU passthrough creates this device node, so it is + // trusted without corroboration. assert!(wsl_signals_indicate_wsl(true, false, ""), "/dev/dxg"); assert!( - wsl_signals_indicate_wsl(false, true, ""), - "$WSL_DISTRO_NAME" + wsl_signals_indicate_wsl(true, false, "Linux version 6.8.0-51-generic"), + "/dev/dxg overrides an otherwise ordinary kernel string" + ); + } + + #[test] + fn distro_name_alone_is_not_enough_any_more() { + // $WSL_DISTRO_NAME is an ordinary environment variable: it can survive + // into a shell that merely inherited it (e.g. over ssh), so on its own + // it no longer counts. A false positive here now silently disables the + // entire bare-metal catalog instead of costing a route-out note, so the + // bar for believing it went up. + assert!( + !wsl_signals_indicate_wsl(false, true, ""), + "$WSL_DISTRO_NAME with no /proc/version corroboration is not enough" ); assert!( - wsl_signals_indicate_wsl( + !wsl_signals_indicate_wsl(false, true, "Linux version 6.8.0-51-generic"), + "$WSL_DISTRO_NAME with an ordinary kernel string is not enough" + ); + } + + #[test] + fn proc_version_alone_is_not_enough_any_more() { + // Symmetric with the distro-name case: a kernel string naming Microsoft + // or WSL no longer suffices without $WSL_DISTRO_NAME alongside it. + assert!( + !wsl_signals_indicate_wsl( false, false, "Linux version 6.6.87.2-microsoft-standard-WSL2" ), - "microsoft in /proc/version" + "microsoft in /proc/version alone is not enough" + ); + assert!( + !wsl_signals_indicate_wsl(false, false, "Linux version 5.15.0 wsl2"), + "wsl in /proc/version alone is not enough" + ); + } + + #[test] + fn distro_name_and_proc_version_together_are_believed() { + assert!( + wsl_signals_indicate_wsl( + false, + true, + "Linux version 6.6.87.2-microsoft-standard-WSL2" + ), + "$WSL_DISTRO_NAME corroborated by microsoft in /proc/version" ); assert!( - wsl_signals_indicate_wsl(false, false, "Linux version 5.15.0 wsl2"), - "wsl in /proc/version" + wsl_signals_indicate_wsl(false, true, "Linux version 5.15.0 wsl2"), + "$WSL_DISTRO_NAME corroborated by wsl in /proc/version" ); + } + + #[test] + fn an_ordinary_kernel_with_no_signals_is_not_wsl() { assert!( !wsl_signals_indicate_wsl(false, false, "Linux version 6.8.0-51-generic"), "an ordinary kernel is not WSL" @@ -12085,27 +12140,31 @@ last_installed_runtime_id = "therock-release" } #[test] - fn the_two_old_predicates_disagreed_and_this_one_does_not() { - // The install summary asked for /dev/dxg or "microsoft"; the JSON probe - // asked for "microsoft"/"wsl" or $WSL_DISTRO_NAME. These are the two - // shapes that split them, and the reason `examine` could contradict - // `examine --json` about the platform it was describing. - let only_the_summary_saw_it = (true, false, "Linux version 6.8.0-generic"); - let only_the_probe_saw_it = (false, true, "Linux version 6.8.0-generic"); - for (dxg, distro, version) in [only_the_summary_saw_it, only_the_probe_saw_it] { + fn real_wsl2_and_wsl1_hosts_still_carry_a_corroborating_signal() { + // Real WSL hosts are never "distro name only": WSL sets + // $WSL_DISTRO_NAME for every session, and its own doc comment above + // records that WSL 1 kernels always end in "-microsoft" and WSL 2 + // kernels always carry "microsoft-standard" -- so /proc/version always + // corroborates it. The tightened predicate still recognises both. + let wsl2 = "Linux version 5.15.167.4-microsoft-standard-WSL2"; + let wsl2_early = "Linux version 4.19.104-microsoft-standard"; + let wsl1 = "Linux version 4.4.0-19041-Microsoft"; + for proc_version in [wsl2, wsl2_early, wsl1] { assert!( - wsl_signals_indicate_wsl(dxg, distro, version), - "one predicate already believed this host was WSL: \ - dxg={dxg} distro_name={distro} {version:?}" + wsl_signals_indicate_wsl(false, true, proc_version), + "a real WSL host with $WSL_DISTRO_NAME set was not recognised: {proc_version:?}" ); } + // WSL 2 with GPU passthrough enabled also has /dev/dxg, which is + // believed regardless of the other two signals. + assert!(wsl_signals_indicate_wsl(true, true, wsl2)); } #[test] fn wsl_case_folding_does_not_depend_on_the_kernel_string_casing() { assert!(wsl_signals_indicate_wsl( false, - false, + true, "MICROSOFT-STANDARD-WSL2" )); } diff --git a/docs/wsl.md b/docs/wsl.md index 6a85be9a4..d21afaab4 100644 --- a/docs/wsl.md +++ b/docs/wsl.md @@ -153,6 +153,11 @@ rocm diagnose rocm diagnose --json ``` +This catalog replaces the standalone preflight script this repo used to ship; +it does not carry over that script's `--require-build-tools` flag or its +Python venv-tooling check, so a distro missing only build tools or `venv` +support reports clean here. + The WSL entries, in the order a broken stack usually reveals them: | Fix id | Reported when | From c1132eb4c6e83ac8b3a306fe1defac6545bb5ec2 Mon Sep 17 00:00:00 2001 From: Eugene Volen Date: Fri, 11 Sep 2026 09:30:09 +0000 Subject: [PATCH 3/4] fix(e2e): tag WSL2-inapplicable fix scenarios bare-metal-only diagnose-15 and diagnose-16 exercise fix-4-render-group and fix-9-igpu-dgpu, neither of which is applicable on WSL2 (applies_on excludes wsl). Both scenarios were only tagged requires-os:linux, so on a WSL2 host the CLI correctly refuses the fix as the wrong platform before ever reaching the behavior under test, and the run fails expecting a pass. diagnose-08 already carries requires-bare-metal for the same reason; extend it to these two so they resolve to Skip on WSL2 instead of a false regression. Signed-off-by: Eugene Volen --- tests/e2e-cucumber/features/diagnose.feature | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/e2e-cucumber/features/diagnose.feature b/tests/e2e-cucumber/features/diagnose.feature index 2f389c2ce..d860d0324 100644 --- a/tests/e2e-cucumber/features/diagnose.feature +++ b/tests/e2e-cucumber/features/diagnose.feature @@ -199,7 +199,12 @@ Feature: Diagnosing failures and listing fixes # regression could move the explanation back to stdout, or off exit code 4, # while every other listed scenario kept passing. Linux-only because the # recipe itself is `applies_on: LINUX_ONLY`. - @id:diagnose-fix-command-failure-reported-on-stderr @requires-os:linux + # + # @requires-bare-metal on top of that, same reasoning as diagnose-08: + # `fix-4-render-group`'s `applies_on` does not include `wsl`, so on a WSL2 + # host the CLI refuses it as the wrong platform before ever invoking the + # (faked) `usermod` — there is no command-failure branch to reach there. + @id:diagnose-fix-command-failure-reported-on-stderr @requires-os:linux @requires-bare-metal Scenario: diagnose-15 - A fix whose helper command fails explains why, on stderr, with exit code 4 Given a user who has approved a fix whose helper command will fail When the user asks the CLI to apply the approved fix @@ -213,7 +218,12 @@ Feature: Diagnosing failures and listing fixes # suite driven through the pseudo-terminal harness instead of piped stdin. # Linux-only for the same reason diagnose-08 is: the recipe under test # (`fix-9-igpu-dgpu`) only appends a shell rc file on Linux. - @id:diagnose-fix-interactive-decline-reported @requires-os:linux + # + # @requires-bare-metal for the same reason as diagnose-08: `fix-9-igpu-dgpu` + # does not apply on WSL2 (no per-device topology to collide over there), so + # the run stops at the wrong-platform refusal before the confirmation prompt + # is ever printed. + @id:diagnose-fix-interactive-decline-reported @requires-os:linux @requires-bare-metal Scenario: diagnose-16 - Declining the confirmation prompt on a real terminal is reported the same way Given a user who has chosen a fix that would change the machine When the user is asked interactively to apply it and types no From 32da863e69ee0863bbf811a0a50f952695aacc85 Mon Sep 17 00:00:00 2001 From: Eugene Volen Date: Fri, 11 Sep 2026 09:37:03 +0000 Subject: [PATCH 4/4] fix(doctor): restore /proc/version as sufficient for WSL detection The tightened WSL predicate from the previous round required both /proc/version naming Microsoft/WSL and $WSL_DISTRO_NAME set, to stop trusting a bare env var. That weakened the wrong signal: it demoted the kernel's own build string, which cannot be inherited or forwarded the way an env var can, to merely corroborating evidence. Detection then hinged on $WSL_DISTRO_NAME in exactly the population this catalog serves -- a real WSL2 host without /dev/dxg passthrough -- so a container on WSL2 (no distro var, host kernel in /proc/version), a systemd unit inside the distro, ssh into the distro, sudo without -E, and cron all misread as bare-metal Linux. /proc/version alone is now sufficient again, same as /dev/dxg; $WSL_DISTRO_NAME no longer participates at all, since an inherited env var with no kernel-string match still correctly reads as Linux. Also drop a fix.rs unit test's dependency on forcing $WSL_DISTRO_NAME, since it no longer has any effect on the result. Signed-off-by: Eugene Volen --- crates/rocm-core/src/fix.rs | 49 ++++---------- crates/rocm-core/src/lib.rs | 126 ++++++++++++++---------------------- 2 files changed, 60 insertions(+), 115 deletions(-) diff --git a/crates/rocm-core/src/fix.rs b/crates/rocm-core/src/fix.rs index 42dbb5008..1f190f267 100644 --- a/crates/rocm-core/src/fix.rs +++ b/crates/rocm-core/src/fix.rs @@ -1429,45 +1429,20 @@ mod tests { } #[test] - #[allow(unsafe_code)] // std::env::set_var is unsafe in edition 2024 - fn current_os_reaches_wsl_only_when_the_distro_env_var_is_corroborated() { - // `current_os()`'s wsl branch is `crate::is_wsl_host()`, which trusts - // `/dev/dxg` outright but requires `$WSL_DISTRO_NAME` to be - // corroborated by a kernel string that actually names Microsoft/WSL - // (see `wsl_signals_indicate_wsl`). `recipe_applies_here` below cannot - // check this independently: it calls `current_os()` to answer its own - // question, so a bug in `current_os()` would agree with itself and no - // test built on that helper would ever notice. This test instead - // forces the one signal that can be forced portably - // ($WSL_DISTRO_NAME) and compares the result against ground truth - // computed straight from this machine's real `/dev/dxg` and - // `/proc/version`, so it holds whether it runs on a bare-metal CI + fn current_os_reports_wsl_exactly_when_is_wsl_host_does() { + // `current_os()`'s wsl branch is `crate::is_wsl_host()`, which is + // `crate::wsl_signals_indicate_wsl()` against the real `/dev/dxg` and + // `/proc/version` -- `$WSL_DISTRO_NAME` plays no part any more, so + // there is nothing left to force portably here. The table-driven + // coverage of the predicate itself lives with + // `wsl_signals_indicate_wsl` in lib.rs; this test only checks that + // `current_os()` reports the same answer `is_wsl_host()` does on + // whatever machine actually runs it, whether that is a bare-metal CI // runner, Windows, or a real WSL host. - let proc_version = std::fs::read_to_string("/proc/version") - .unwrap_or_default() - .to_ascii_lowercase(); - let corroborated = std::path::Path::new("/dev/dxg").exists() - || proc_version.contains("microsoft") - || proc_version.contains("wsl"); - - let previous = std::env::var_os("WSL_DISTRO_NAME"); - unsafe { - std::env::set_var("WSL_DISTRO_NAME", "Ubuntu"); - } - let os = current_os(); - unsafe { - match previous { - Some(value) => std::env::set_var("WSL_DISTRO_NAME", value), - None => std::env::remove_var("WSL_DISTRO_NAME"), - } - } - assert_eq!( - os == "wsl", - corroborated, - "current_os() must land on \"wsl\" exactly when the forced \ - $WSL_DISTRO_NAME is corroborated by this machine's real /dev/dxg \ - or /proc/version, not merely because the variable is set" + current_os() == "wsl", + crate::is_wsl_host(), + "current_os() must agree with is_wsl_host()" ); } diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index 34ec87a2b..92fedbc4a 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -2420,17 +2420,18 @@ fn normalize_cpu_model(value: &str) -> String { /// forms. Worse, the e2e harness derives `is_wsl` for its whole expectation /// matrix by reading one of them. /// -/// `/dev/dxg` is trusted on its own. `$WSL_DISTRO_NAME` is not — it is an -/// ordinary environment variable that can survive into a shell that merely -/// inherited it — so it is corroborated against `/proc/version` before being -/// believed. See [`wsl_signals_indicate_wsl`] for why a false positive is no -/// longer cheap. +/// `/dev/dxg` is trusted on its own, and so is the kernel's own build string in +/// `/proc/version` — nothing else stamps a kernel `-microsoft-standard-WSL2` or +/// `-Microsoft`. `$WSL_DISTRO_NAME` is not trusted at all: it is an ordinary +/// environment variable that can survive into a shell that merely inherited it +/// (over `ssh`, in a `systemd` unit, under `sudo` without `-E`, under `env -i` or +/// cron) without corroborating anything. See [`wsl_signals_indicate_wsl`] for why +/// a false positive is no longer cheap. #[must_use] pub fn is_wsl_host() -> bool { runtime_is_linux() && wsl_signals_indicate_wsl( Path::new("/dev/dxg").exists(), - std::env::var_os("WSL_DISTRO_NAME").is_some(), &fs::read_to_string("/proc/version").unwrap_or_default(), ) } @@ -2502,20 +2503,22 @@ fn rocm_relative_file_exists(relative: &str) -> bool { /// tested — including the two cases that used to split the old implementations. /// /// `/dev/dxg` alone is trusted outright — nothing but WSLg's GPU passthrough -/// creates that device node. `$WSL_DISTRO_NAME` alone is not: it is an ordinary -/// environment variable that survives into a shell someone launched with it -/// inherited or forwarded (e.g. over `ssh`), so it is corroborated against -/// `/proc/version` before being believed. A false positive here no longer costs -/// only a route-out note — this catalog now runs the WSL diagnosis and fix set -/// directly, so a bare-metal host that merely inherited the variable would have -/// its entire bare-metal catalog silently disabled. -fn wsl_signals_indicate_wsl(dxg_device: bool, distro_name_set: bool, proc_version: &str) -> bool { +/// creates that device node. The kernel's own build string in `/proc/version` is +/// also trusted alone: only a WSL kernel is built `-microsoft-standard[-WSL2]` or +/// `-Microsoft`, and that string cannot be inherited, forwarded, or left behind +/// by an unrelated shell the way `$WSL_DISTRO_NAME` can. `$WSL_DISTRO_NAME` plays +/// no part here at all — an ordinary bare-metal host that merely inherited it +/// (over `ssh`, from a parent shell, under `sudo` without `-E`) has no +/// `/proc/version` match to go with it, so it still reads as Linux. A false +/// positive the other way no longer costs only a route-out note — this catalog +/// now runs the WSL diagnosis and fix set directly, so a bare-metal host +/// misread as WSL would have its entire bare-metal catalog silently disabled. +fn wsl_signals_indicate_wsl(dxg_device: bool, proc_version: &str) -> bool { if dxg_device { return true; } let proc_version = proc_version.to_ascii_lowercase(); - let proc_version_matches = proc_version.contains("microsoft") || proc_version.contains("wsl"); - distro_name_set && proc_version_matches + proc_version.contains("microsoft") || proc_version.contains("wsl") } fn detect_wsl_summary() -> Option { @@ -12073,100 +12076,67 @@ last_installed_runtime_id = "therock-release" fn dev_dxg_is_believed_on_its_own() { // Nothing but WSLg's GPU passthrough creates this device node, so it is // trusted without corroboration. - assert!(wsl_signals_indicate_wsl(true, false, ""), "/dev/dxg"); + assert!(wsl_signals_indicate_wsl(true, ""), "/dev/dxg"); assert!( - wsl_signals_indicate_wsl(true, false, "Linux version 6.8.0-51-generic"), + wsl_signals_indicate_wsl(true, "Linux version 6.8.0-51-generic"), "/dev/dxg overrides an otherwise ordinary kernel string" ); } #[test] - fn distro_name_alone_is_not_enough_any_more() { - // $WSL_DISTRO_NAME is an ordinary environment variable: it can survive - // into a shell that merely inherited it (e.g. over ssh), so on its own - // it no longer counts. A false positive here now silently disables the - // entire bare-metal catalog instead of costing a route-out note, so the - // bar for believing it went up. + fn proc_version_alone_is_believed() { + // Only a WSL kernel is built `-microsoft-standard[-WSL2]` or + // `-Microsoft` -- unlike $WSL_DISTRO_NAME, that string cannot be + // inherited or forwarded into an unrelated shell, so it needs no + // corroboration. This is also what makes a WSL2 container correctly + // read as WSL even when it was not started with /dev/dxg passed in: + // it shares the host kernel, so /proc/version still carries the + // marker even though the container has no $WSL_DISTRO_NAME of its own. assert!( - !wsl_signals_indicate_wsl(false, true, ""), - "$WSL_DISTRO_NAME with no /proc/version corroboration is not enough" + wsl_signals_indicate_wsl(false, "Linux version 6.6.87.2-microsoft-standard-WSL2"), + "microsoft in /proc/version is enough on its own" ); assert!( - !wsl_signals_indicate_wsl(false, true, "Linux version 6.8.0-51-generic"), - "$WSL_DISTRO_NAME with an ordinary kernel string is not enough" - ); - } - - #[test] - fn proc_version_alone_is_not_enough_any_more() { - // Symmetric with the distro-name case: a kernel string naming Microsoft - // or WSL no longer suffices without $WSL_DISTRO_NAME alongside it. - assert!( - !wsl_signals_indicate_wsl( - false, - false, - "Linux version 6.6.87.2-microsoft-standard-WSL2" - ), - "microsoft in /proc/version alone is not enough" - ); - assert!( - !wsl_signals_indicate_wsl(false, false, "Linux version 5.15.0 wsl2"), - "wsl in /proc/version alone is not enough" - ); - } - - #[test] - fn distro_name_and_proc_version_together_are_believed() { - assert!( - wsl_signals_indicate_wsl( - false, - true, - "Linux version 6.6.87.2-microsoft-standard-WSL2" - ), - "$WSL_DISTRO_NAME corroborated by microsoft in /proc/version" - ); - assert!( - wsl_signals_indicate_wsl(false, true, "Linux version 5.15.0 wsl2"), - "$WSL_DISTRO_NAME corroborated by wsl in /proc/version" + wsl_signals_indicate_wsl(false, "Linux version 5.15.0 wsl2"), + "wsl in /proc/version is enough on its own" ); } #[test] fn an_ordinary_kernel_with_no_signals_is_not_wsl() { assert!( - !wsl_signals_indicate_wsl(false, false, "Linux version 6.8.0-51-generic"), + !wsl_signals_indicate_wsl(false, "Linux version 6.8.0-51-generic"), "an ordinary kernel is not WSL" ); + assert!( + !wsl_signals_indicate_wsl(false, ""), + "no device and no /proc/version to read is not WSL" + ); } #[test] - fn real_wsl2_and_wsl1_hosts_still_carry_a_corroborating_signal() { - // Real WSL hosts are never "distro name only": WSL sets - // $WSL_DISTRO_NAME for every session, and its own doc comment above - // records that WSL 1 kernels always end in "-microsoft" and WSL 2 - // kernels always carry "microsoft-standard" -- so /proc/version always - // corroborates it. The tightened predicate still recognises both. + fn real_wsl2_and_wsl1_hosts_are_recognised_by_proc_version_alone() { + // Its own doc comment above records that WSL 1 kernels always end in + // "-microsoft" and WSL 2 kernels always carry "microsoft-standard" -- + // so every real WSL host is recognised without needing $WSL_DISTRO_NAME + // or /dev/dxg at all. let wsl2 = "Linux version 5.15.167.4-microsoft-standard-WSL2"; let wsl2_early = "Linux version 4.19.104-microsoft-standard"; let wsl1 = "Linux version 4.4.0-19041-Microsoft"; for proc_version in [wsl2, wsl2_early, wsl1] { assert!( - wsl_signals_indicate_wsl(false, true, proc_version), - "a real WSL host with $WSL_DISTRO_NAME set was not recognised: {proc_version:?}" + wsl_signals_indicate_wsl(false, proc_version), + "a real WSL host was not recognised: {proc_version:?}" ); } // WSL 2 with GPU passthrough enabled also has /dev/dxg, which is - // believed regardless of the other two signals. - assert!(wsl_signals_indicate_wsl(true, true, wsl2)); + // believed regardless of /proc/version. + assert!(wsl_signals_indicate_wsl(true, wsl2)); } #[test] fn wsl_case_folding_does_not_depend_on_the_kernel_string_casing() { - assert!(wsl_signals_indicate_wsl( - false, - true, - "MICROSOFT-STANDARD-WSL2" - )); + assert!(wsl_signals_indicate_wsl(false, "MICROSOFT-STANDARD-WSL2")); } #[test]