fix: read /etc/os-release with one parser that matches sh

apps/rocm/src/main.rs

@@ -3940,13 +3940,39 @@ fn build_driver_install_plan(
         return wsl_rocdxg_driver_plan(escalation);
     }
 
-    let os_id = parse_os_release_field(os_release_text, "ID").unwrap_or_default();
-    let version_id = parse_os_release_field(os_release_text, "VERSION_ID").unwrap_or_default();
-    let codename = parse_os_release_field(os_release_text, "VERSION_CODENAME")
-        .or_else(|| parse_os_release_field(os_release_text, "UBUNTU_CODENAME"))
+    // An unreadable file is reported as itself, naming the line, rather than
+    // falling through to "this distro is not supported" — which would send the
+    // user looking for a support matrix when the fix is one line of a file.
+    let os_release = match rocm_core::os_release::parse(os_release_text) {
+        Ok(fields) => fields,
+        Err(unreadable) => {
+            return DriverInstallPlan {
+                supported: false,
+                mutating: false,
+                policy: "unreadable_os_release".to_owned(),
+                os_id: String::new(),
+                version_id: String::new(),
+                codename: String::new(),
+                repo_version,
+                reason: format!(
+                    "/etc/os-release {unreadable}; no driver commands were planned \
+                     because the distro cannot be read."
+                ),
+                preflight_checks: Vec::new(),
+                commands: Vec::new(),
+                checks: vec!["rocm examine".to_owned()],
+                reboot_required: false,
+            };
+        }
+    };
+    let field = |key: &str| os_release.get(key).cloned();
+    let os_id = field("ID").unwrap_or_default();
+    let version_id = field("VERSION_ID").unwrap_or_default();
+    let codename = field("VERSION_CODENAME")
+        .or_else(|| field("UBUNTU_CODENAME"))
         .or_else(|| codename_for_version(&os_id, &version_id).map(str::to_owned))
         .unwrap_or_default();
-    let id_like = parse_os_release_field(os_release_text, "ID_LIKE").unwrap_or_default();
+    let id_like = field("ID_LIKE").unwrap_or_default();
 
     match (os_id.as_str(), version_id.as_str()) {
         ("ubuntu", "22.04" | "24.04") => apt_driver_plan(
@@ -4696,19 +4722,6 @@ const fn empty_as_unknown(value: &str) -> &str {
     if value.is_empty() { "<unknown>" } else { value }
 }
 
-fn parse_os_release_field(text: &str, key: &str) -> Option<String> {
-    for line in text.lines() {
-        let Some((name, raw_value)) = line.split_once('=') else {
-            continue;
-        };
-        if name != key {
-            continue;
-        }
-        return Some(raw_value.trim().trim_matches('"').to_owned());
-    }
-    None
-}
-
 fn codename_for_version(os_id: &str, version_id: &str) -> Option<&'static str> {
     match (os_id, version_id) {
         ("ubuntu", "22.04") => Some("jammy"),
@@ -9555,8 +9568,8 @@ fn ensure_openmpi_for_vllm(approved: bool) -> Result<()> {
     }
 
     let os_release = read_os_release().unwrap_or_default();
-    let os_id = parse_os_release_field(&os_release, "ID").unwrap_or_default();
-    let id_like = parse_os_release_field(&os_release, "ID_LIKE").unwrap_or_default();
+    let os_id = rocm_core::os_release::field(&os_release, "ID").unwrap_or_default();
+    let id_like = rocm_core::os_release::field(&os_release, "ID_LIKE").unwrap_or_default();
     let plan = rocm_core::openmpi::build_openmpi_install_plan(&os_id, &id_like);
 
     println!("openmpi setup");
@@ -9730,8 +9743,8 @@ fn ensure_torch_runtime_dep(approved: bool, dep: &TorchRuntimeDep) {
     }
 
     let os_release = read_os_release().unwrap_or_default();
-    let os_id = parse_os_release_field(&os_release, "ID").unwrap_or_default();
-    let id_like = parse_os_release_field(&os_release, "ID_LIKE").unwrap_or_default();
+    let os_id = rocm_core::os_release::field(&os_release, "ID").unwrap_or_default();
+    let id_like = rocm_core::os_release::field(&os_release, "ID_LIKE").unwrap_or_default();
     let plan = (dep.build_plan)(&os_id, &id_like);
 
     println!("{} setup", dep.name);
@@ -31520,6 +31533,119 @@ install therock";
             .collect()
     }
 
+    /// Rewrite every `KEY=value` / `KEY="value"` line of an os-release fixture
+    /// in single quotes, which `os-release(5)` permits just as it does double.
+    fn single_quoted(os_release: &str) -> String {
+        os_release
+            .lines()
+            .map(|line| match line.split_once('=') {
+                Some((key, value)) => format!("{key}='{}'", value.trim_matches('"')),
+                None => line.to_owned(),
+            })
+            .collect::<Vec<_>>()
+            .join("\n")
+    }
+
+    /// How a distro quotes its `/etc/os-release` must not change the plan.
+    ///
+    /// `os-release(5)` allows single or double quotes, but the parser stripped
+    /// only `"`: `ID='ubuntu'` read as `'ubuntu'` and `VERSION_ID='24.04'` as
+    /// `'24.04'`, so a spec-valid file read as an unsupported distro. Every
+    /// supported distro, rewritten in single quotes, must plan exactly as it
+    /// does in double quotes.
+    #[test]
+    fn a_single_quoted_os_release_builds_the_same_plan() {
+        let _env = ScopedTestEnv::with_amd_overrides_cleared();
+        for (label, os_release) in dkms_planning_os_releases() {
+            let plan = |text: &str| {
+                build_driver_install_plan(
+                    &test_examine("linux", false),
+                    text,
+                    true,
+                    PrivilegeEscalation::Sudo,
+                )
+            };
+            let double = plan(os_release);
+            let single = plan(&single_quoted(os_release));
+            assert!(
+                single.supported,
+                "{label}: unsupported when single-quoted: {}\n{}",
+                single.reason,
+                single_quoted(os_release)
+            );
+            assert_eq!(
+                (single.os_id.as_str(), single.version_id.as_str()),
+                (double.os_id.as_str(), double.version_id.as_str()),
+                "{label}: quote style changed the distro read"
+            );
+            assert_eq!(
+                single.execution_commands(),
+                double.execution_commands(),
+                "{label}: quote style changed the planned commands"
+            );
+        }
+    }
+
+    /// An unreadable os-release is refused as itself: the reason names the
+    /// offending line, and it is asserted together with what it claims — no
+    /// plan, no commands — and against the reason it replaced, which blamed the
+    /// distro.
+    #[test]
+    fn an_unreadable_os_release_names_its_line_and_plans_nothing() {
+        let _env = ScopedTestEnv::with_amd_overrides_cleared();
+        let plan = build_driver_install_plan(
+            &test_examine("linux", false),
+            "ID=ubuntu\nVERSION_ID=\"24.04\"\nVERSION_CODENAME=noble\nunset ID\n",
+            true,
+            PrivilegeEscalation::Sudo,
+        );
+        assert_eq!(
+            plan.reason,
+            "/etc/os-release line 4 is not a plain assignment: \"unset ID\"; no driver \
+             commands were planned because the distro cannot be read."
+        );
+        assert!(!plan.supported);
+        assert!(plan.commands.is_empty(), "{:?}", plan.commands);
+        assert_eq!(plan.policy, "unreadable_os_release");
+
+        // The rendered plan says so on one line, and does not also claim the
+        // distro is unsupported.
+        let rendered = render_driver_install_plan(&plan, false, true);
+        assert!(
+            rendered.contains("reason: /etc/os-release line 4 is not a plain assignment"),
+            "{rendered}"
+        );
+        assert!(!rendered.contains("AMD-documented"), "{rendered}");
+        assert!(
+            rendered.contains("execution_commands: <none>"),
+            "{rendered}"
+        );
+    }
+
+    /// With a duplicated key, the plan is built from the last assignment — the
+    /// one a shell keeps, and the one `rocm examine` reports, since both now
+    /// read through `rocm_core::os_release`. The driver plan used to take the
+    /// first, so on this file it planned for Debian 12 while `rocm examine`
+    /// reported Ubuntu.
+    #[test]
+    fn a_duplicated_os_release_key_is_planned_from_its_last_assignment() {
+        let _env = ScopedTestEnv::with_amd_overrides_cleared();
+        let text = "ID=debian\nVERSION_ID=\"12\"\nID=ubuntu\nVERSION_ID=\"24.04\"\n\
+                    VERSION_CODENAME=noble\n";
+        let plan = build_driver_install_plan(
+            &test_examine("linux", false),
+            text,
+            true,
+            PrivilegeEscalation::Sudo,
+        );
+        assert_eq!(
+            (plan.os_id.as_str(), plan.version_id.as_str()),
+            ("ubuntu", "24.04"),
+            "planned for the first assignment instead of the last"
+        );
+        assert!(plan.supported, "{}", plan.reason);
+    }
+
     #[test]
     fn driver_plan_as_root_never_emits_sudo() {
         // The defect: every command was prefixed `sudo` unconditionally, so on a

crates/rocm-core/src/examine.rs

@@ -608,18 +608,7 @@ fn probe_os(e: &mut Examination) {
         e.os_family = "linux".to_owned();
         e.kernel_release = run("uname", &["-r"], SHORT).1.trim().to_owned();
         e.kernel_cmdline = read_text("/proc/cmdline").trim().to_owned();
-        let osr = read_text("/etc/os-release");
-        for line in osr.lines() {
-            let Some((key, value)) = line.split_once('=') else {
-                continue;
-            };
-            let value = value.trim().trim_matches('"');
-            match key {
-                "ID" => e.distro_id = value.to_owned(),
-                "VERSION_ID" => e.distro_version = value.to_owned(),
-                _ => {}
-            }
-        }
+        record_distro(e, &read_text("/etc/os-release"));
         if let Some(param) = parse_iommu_param(&e.kernel_cmdline) {
             e.iommu_kernel_param = param;
         }
@@ -634,6 +623,25 @@ fn probe_os(e: &mut Examination) {
     }
 }
 
+/// Fill the distro fields from os-release `text`, through the same parser the
+/// driver plan uses, so the two report the same distro.
+///
+/// An unreadable file leaves both fields empty and says why in
+/// `probe_failures`, naming the line — an empty distro with no explanation
+/// would read as "nothing to report" rather than "something to fix". A missing
+/// file reads as empty text, which is not a failure.
+fn record_distro(e: &mut Examination, text: &str) {
+    match crate::os_release::parse(text) {
+        Ok(mut fields) => {
+            e.distro_id = fields.remove("ID").unwrap_or_default();
+            e.distro_version = fields.remove("VERSION_ID").unwrap_or_default();
+        }
+        Err(unreadable) => e.probe_failures.push(format!(
+            "/etc/os-release {unreadable}; the distro is not reported."
+        )),
+    }
+}
+
 /// 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
@@ -2395,6 +2403,41 @@ fn probe_msvc_redist_windows(e: &mut Examination) {
 mod tests {
     use super::*;
 
+    /// The message and the state it describes, asserted together: an
+    /// unreadable os-release names its line in `probe_failures` *and* leaves
+    /// the distro unreported — and a readable one reports the distro with no
+    /// failure recorded.
+    #[test]
+    fn an_unreadable_os_release_names_its_line_instead_of_a_distro() {
+        let mut e = Examination::default();
+        record_distro(
+            &mut e,
+            "ID=ubuntu\nVERSION_ID=\"24.04\"\nexport ID=debian\n",
+        );
+        assert_eq!(
+            e.probe_failures,
+            vec![
+                "/etc/os-release line 3 is not a plain assignment: \"export ID=debian\"; \
+                 the distro is not reported."
+                    .to_owned()
+            ]
+        );
+        assert_eq!((e.distro_id.as_str(), e.distro_version.as_str()), ("", ""));
+
+        let mut e = Examination::default();
+        record_distro(&mut e, "ID='ubuntu'\nVERSION_ID='24.04' # LTS\n");
+        assert!(e.probe_failures.is_empty(), "{:?}", e.probe_failures);
+        assert_eq!(
+            (e.distro_id.as_str(), e.distro_version.as_str()),
+            ("ubuntu", "24.04")
+        );
+
+        // No file at all is not a failure: there is nothing to fix.
+        let mut e = Examination::default();
+        record_distro(&mut e, "");
+        assert!(e.probe_failures.is_empty(), "{:?}", e.probe_failures);
+    }
+
     /// Serialises the tests that read or replace the process-global
     /// `RUNTIME_LIBRARY_PATH_ENV` while they run. Env is shared by every test
     /// thread, so a test that sets it and one that composes a child env from it

crates/rocm-core/src/lib.rs

@@ -32,6 +32,7 @@ pub mod disk_space;
 pub mod examine;
 pub mod fix;
 pub mod openmpi;
+pub mod os_release;
 pub mod proc_lifecycle;
 pub mod runtime;
 #[cfg(test)]
@@ -2419,11 +2420,11 @@ fn detect_distro_name() -> Option<String> {
 }
 
 fn parse_os_release_pretty_name(text: &str) -> Option<String> {
-    text.lines().find_map(|line| {
-        let value = line.strip_prefix("PRETTY_NAME=")?.trim();
-        let value = value.trim_matches('"').trim_matches('\'').trim();
-        (!value.is_empty()).then(|| value.to_owned())
-    })
+    // Through the shared parser (see `os_release`). The `trim` and the empty
+    // check are this caller's own: a blank PRETTY_NAME falls back to "Linux".
+    os_release::field(text, "PRETTY_NAME")
+        .map(|value| value.trim().to_owned())
+        .filter(|value| !value.is_empty())
 }
 
 fn detect_cpu_model_with_windows_inventory(

crates/rocm-core/src/openmpi.rs

@@ -829,7 +829,7 @@ pub fn install_hint() -> String {
     #[cfg(target_os = "linux")]
     {
         let os_release = std::fs::read_to_string("/etc/os-release").unwrap_or_default();
-        let field = |key: &str| parse_os_release_field(&os_release, key).unwrap_or_default();
+        let field = |key: &str| crate::os_release::field(&os_release, key).unwrap_or_default();
         let plan = build_openmpi_install_plan(&field("ID"), &field("ID_LIKE"));
         if plan.supported && !plan.commands.is_empty() {
             let rendered = plan
@@ -854,7 +854,7 @@ pub fn libatomic_install_hint() -> String {
     #[cfg(target_os = "linux")]
     {
         let os_release = std::fs::read_to_string("/etc/os-release").unwrap_or_default();
-        let field = |key: &str| parse_os_release_field(&os_release, key).unwrap_or_default();
+        let field = |key: &str| crate::os_release::field(&os_release, key).unwrap_or_default();
         let plan = build_libatomic_install_plan(&field("ID"), &field("ID_LIKE"));
         if plan.supported && !plan.commands.is_empty() {
             let rendered = plan
@@ -879,7 +879,7 @@ pub fn libnuma_install_hint() -> String {
     #[cfg(target_os = "linux")]
     {
         let os_release = std::fs::read_to_string("/etc/os-release").unwrap_or_default();
-        let field = |key: &str| parse_os_release_field(&os_release, key).unwrap_or_default();
+        let field = |key: &str| crate::os_release::field(&os_release, key).unwrap_or_default();
         let plan = build_libnuma_install_plan(&field("ID"), &field("ID_LIKE"));
         if plan.supported && !plan.commands.is_empty() {
             let rendered = plan
@@ -894,26 +894,6 @@ pub fn libnuma_install_hint() -> String {
     "install your distribution's numactl runtime package (providing libnuma.so.1)".to_owned()
 }
 
-/// Parse a single `KEY=VALUE` field from `/etc/os-release` contents, stripping
-/// optional surrounding quotes. Returns `None` when the key is absent.
-// Only the Linux `install_hint` path calls this at runtime; off Linux it is
-// exercised solely by cross-platform unit tests.
-#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
-pub(crate) fn parse_os_release_field(text: &str, key: &str) -> Option<String> {
-    for line in text.lines() {
-        let line = line.trim();
-        let Some((name, value)) = line.split_once('=') else {
-            continue;
-        };
-        if name.trim() != key {
-            continue;
-        }
-        let value = value.trim().trim_matches('"').trim_matches('\'');
-        return Some(value.to_owned());
-    }
-    None
-}
-
 /// Build a non-mutating apt invocation for a planned `apt-get install` command.
 ///
 /// Returns `None` for other commands. Any `sudo` prefix is dropped along with
@@ -1077,22 +1057,6 @@ mod tests {
         assert!(!runtime_present(None, None));
     }
 
-    #[test]
-    fn parses_os_release_fields_with_and_without_quotes() {
-        let text =
-            "NAME=\"Red Hat Enterprise Linux\"\nID=rhel\nID_LIKE=fedora\nVERSION_ID=\"9.4\"\n";
-        assert_eq!(parse_os_release_field(text, "ID").as_deref(), Some("rhel"));
-        assert_eq!(
-            parse_os_release_field(text, "ID_LIKE").as_deref(),
-            Some("fedora")
-        );
-        assert_eq!(
-            parse_os_release_field(text, "VERSION_ID").as_deref(),
-            Some("9.4")
-        );
-        assert_eq!(parse_os_release_field(text, "MISSING"), None);
-    }
-
     #[test]
     fn install_hint_is_non_empty_and_actionable() {
         // The hint is embedded in the serve preflight error, so it must always