diff --git a/apps/rocm/src/driver_install.rs b/apps/rocm/src/driver_install.rs index 877aa1571..3a7f64ba5 100644 --- a/apps/rocm/src/driver_install.rs +++ b/apps/rocm/src/driver_install.rs @@ -25,7 +25,7 @@ use rocm_core::{AppPaths, ExamineSummary, shell_command_for_host}; use serde::{Deserialize, Serialize}; use crate::cli_report; -use crate::{empty_as_unknown, parse_os_release_field, read_os_release}; +use crate::{empty_as_unknown, read_os_release}; pub(crate) fn install_driver( paths: &AppPaths, @@ -835,13 +835,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( @@ -1738,6 +1764,119 @@ mod tests { .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::>() + .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: "), + "{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 diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index abdf588b3..c3f8a6bd7 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -6800,19 +6800,6 @@ pub(crate) const fn empty_as_unknown(value: &str) -> &str { if value.is_empty() { "" } else { value } } -pub(crate) fn parse_os_release_field(text: &str, key: &str) -> Option { - 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 -} - pub(crate) fn read_os_release() -> Result { fs::read_to_string("/etc/os-release").context("failed to read /etc/os-release") } @@ -6860,8 +6847,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"); @@ -7035,8 +7022,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); diff --git a/crates/rocm-core/src/examine.rs b/crates/rocm-core/src/examine.rs index 48b78fea9..f45a81b4f 100644 --- a/crates/rocm-core/src/examine.rs +++ b/crates/rocm-core/src/examine.rs @@ -715,18 +715,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; } @@ -741,6 +730,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 @@ -3038,6 +3046,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 diff --git a/crates/rocm-core/src/host_gpu.rs b/crates/rocm-core/src/host_gpu.rs index fb702fab8..ff752c6ab 100644 --- a/crates/rocm-core/src/host_gpu.rs +++ b/crates/rocm-core/src/host_gpu.rs @@ -1895,11 +1895,11 @@ fn detect_distro_name() -> Option { } fn parse_os_release_pretty_name(text: &str) -> Option { - 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". + crate::os_release::field(text, "PRETTY_NAME") + .map(|value| value.trim().to_owned()) + .filter(|value| !value.is_empty()) } fn detect_cpu_model_with_windows_inventory( @@ -4509,6 +4509,25 @@ Class Name: Display ); } + /// The distro name is read through the shared `os_release` parser, so it + /// names the distro `sh` would: the last assignment wins, a trailing + /// comment is not part of the value, and an unreadable file names nothing + /// (the caller then falls back to "Linux") rather than a guess. + #[test] + fn pretty_name_reads_as_sh_reads_it() { + assert_eq!( + parse_os_release_pretty_name( + "PRETTY_NAME=\"Debian GNU/Linux 12\"\nPRETTY_NAME='Ubuntu 24.04 LTS' # LTS\n" + ), + Some("Ubuntu 24.04 LTS".to_owned()) + ); + assert_eq!( + parse_os_release_pretty_name("PRETTY_NAME=\"Ubuntu 24.04 LTS\"\nunset ID\n"), + None + ); + assert_eq!(parse_os_release_pretty_name("PRETTY_NAME=\" \"\n"), None); + } + #[test] fn dev_dxg_is_believed_on_its_own() { // Nothing but WSLg's GPU passthrough creates this device node, so it is diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index a39270271..62cde4845 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -36,6 +36,7 @@ pub mod host_gpu; pub mod managed_runtime; pub mod model_readiness; pub mod openmpi; +pub mod os_release; pub mod proc_lifecycle; pub mod report; pub mod report_delivery; diff --git a/crates/rocm-core/src/openmpi.rs b/crates/rocm-core/src/openmpi.rs index cf835803d..c596a6410 100644 --- a/crates/rocm-core/src/openmpi.rs +++ b/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 { - 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 diff --git a/crates/rocm-core/src/os_release.rs b/crates/rocm-core/src/os_release.rs new file mode 100644 index 000000000..78608a10d --- /dev/null +++ b/crates/rocm-core/src/os_release.rs @@ -0,0 +1,733 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! Reading `/etc/os-release`. +//! +//! The one parser every reader in the workspace uses — the driver plan, the +//! package installs it approves (`ensure_openmpi_for_vllm`, +//! `ensure_torch_runtime_dep`), `rocm examine`, the OpenMPI hint, and the host +//! distro name — so they cannot read the same file as different distros. +//! +//! The reference is a POSIX shell sourcing the file: `os-release(5)` defines the +//! format as shell-compatible assignments, and the WSL probe in +//! [`crate::examine`] literally sources it. The rule this module keeps is +//! +//! > **either the value `sh` would assign, or `None` — never anything else.** +//! +//! It returns `sh`'s value for every file made only of blank lines, `#` +//! comments, and assignments the spec allows: +//! +//! * a value bare, in double quotes, or in single quotes; +//! * inside double quotes, the backslash sequences `\"`, `\\`, `` \` `` and +//! `\$` (any other backslash is literal, as in a shell); +//! * inside single quotes, nothing decoded; +//! * in a bare value, `\x` meaning `x`; +//! * spaces and tabs before the name, and after the value; +//! * a `#` comment after the value, separated from it by a space or tab; +//! * an empty value (`ID=`), which assigns the empty string; +//! * a later assignment replacing an earlier one. +//! +//! **Any other line makes the whole file unreadable** — every field is `None`, +//! not just the one the line names, and [`parse`] reports that line. To `sh` +//! such a line is a command, or the start of one: `export ID=debian` and +//! `X=1; ID=debian` assign `ID`; `unset ID` and `X= unset ID` remove it; +//! `NAME="foo` opens a string that swallows the lines after it, so an `ID=` +//! below it is not an assignment at all; `ID="deb"ian` concatenates; `"a$b"` +//! and `` `cmd` `` expand. A command can change any variable, including ones +//! assigned before it, so no key's value can be vouched for once one appears. +//! The parser reproduces none of this; it fails closed. +//! +//! A line other than a blank or a comment is one of those unless it is a valid +//! shell name, `=`, and a value with none of: an unterminated quote; anything +//! after a closing quote but spaces, tabs and a `#` comment; a space or tab +//! between `=` and the value; an unquoted `$` or `` ` ``, anywhere outside +//! single quotes; in a bare value, any unquoted quote, `;`, `|`, `&`, `<`, `>`, +//! `(`, `)`, or trailing lone `\`, or a `~` at the start or after a `:` (which +//! `sh` expands to a home directory); or any control character. +//! +//! The control-character rule is deliberate rather than an omission, and it is +//! what a CRLF file trips: `sh` reads `ID=ubuntu\r` as `ubuntu\r`, a value that +//! matches no distro and would reach a terminal. So a CRLF file is unreadable. +//! Only spaces and tabs are trimmed or separate words; any other whitespace is +//! an ordinary character, as it is to `sh`. +//! +//! What `None` means downstream: never a plan for a distro the file does not +//! declare. When the whole file is unreadable, the driver plan and `rocm +//! examine` say which line, rather than reporting an unsupported distro. + +use std::collections::HashMap; +use std::fmt; + +/// The first line of an os-release file that is not a blank, a comment or a +/// plain assignment, which makes the whole file unreadable. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UnreadableLine { + /// 1-based, as an editor shows it. + pub number: usize, + /// The line as written, cut to [`UnreadableLine::SHOWN_CHARS`] characters. + pub text: String, +} + +impl UnreadableLine { + /// How much of the offending line is kept: enough to recognise it, not so + /// much that one bad line floods a report. + pub const SHOWN_CHARS: usize = 120; +} + +/// `line N is not a plain assignment: ""` — the text `Debug`-quoted, so +/// a control character in it is shown as a `\u{…}` sequence rather than acted +/// on, and the message stays on one line. +impl fmt::Display for UnreadableLine { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "line {} is not a plain assignment: {:?}", + self.number, self.text + ) + } +} + +/// The value of `key` in os-release `text`, as `sh` would assign it, or `None`. +/// +/// `None` when the key is absent, or when the file is unreadable — see +/// [`parse`] for which line made it so. +#[must_use] +pub fn field(text: &str, key: &str) -> Option { + parse(text).ok()?.remove(key) +} + +/// Every assignment in `text`, the last of each name winning — or the first +/// line that is not a blank, a comment or a plain assignment. +/// +/// One pass over the whole file, because the verdict on any key depends on +/// every line, not only the ones that name it. +/// +/// # Errors +/// +/// [`UnreadableLine`] for the first line `sh` would run as a command or that +/// this parser cannot read exactly as `sh` would; see the module docs. +pub fn parse(text: &str) -> Result, UnreadableLine> { + let mut fields = HashMap::new(); + // Split on `\n` only: `str::lines` would also drop a `\r` before it, which + // `sh` keeps as part of the value. + for (index, raw_line) in text.split('\n').enumerate() { + let unreadable = || UnreadableLine { + number: index + 1, + text: raw_line.chars().take(UnreadableLine::SHOWN_CHARS).collect(), + }; + let line = raw_line.trim_start_matches([' ', '\t']); + if line.is_empty() || line.starts_with('#') { + continue; + } + let (name, raw) = line.split_once('=').ok_or_else(unreadable)?; + if !is_shell_name(name) { + return Err(unreadable()); + } + let value = decode(raw.trim_end_matches([' ', '\t'])).ok_or_else(unreadable)?; + fields.insert(name.to_owned(), value); + } + Ok(fields) +} + +/// Whether `name` is something `sh` accepts on the left of an assignment. +fn is_shell_name(name: &str) -> bool { + let mut chars = name.chars(); + chars + .next() + .is_some_and(|first| first == '_' || first.is_ascii_alphabetic()) + && chars.all(|ch| ch == '_' || ch.is_ascii_alphanumeric()) +} + +/// Whether what follows a value is nothing, or a `#` comment after blanks — +/// the only things `sh` lets end an assignment without starting a command. +fn ends_cleanly(rest: &str) -> bool { + rest.is_empty() + || (rest.starts_with([' ', '\t']) && rest.trim_start_matches([' ', '\t']).starts_with('#')) +} + +/// Decode one assignment's right-hand side, trailing spaces and tabs already +/// gone. `None` if it is malformed. +fn decode(raw: &str) -> Option { + // A tab is a blank to `sh`, like a space, and harmless on a terminal; every + // other control character makes the line unreadable. + if raw.chars().any(|ch| ch.is_control() && ch != '\t') { + return None; + } + let mut chars = raw.chars(); + match chars.next() { + None => Some(String::new()), + Some('"') => { + let mut value = String::new(); + loop { + match chars.next()? { + '"' => break, + // Unquoted to the shell even inside `"…"`: it would expand. + '$' | '`' => return None, + '\\' => match chars.next()? { + literal @ ('"' | '\\' | '`' | '$') => value.push(literal), + other => { + value.push('\\'); + value.push(other); + } + }, + ch => value.push(ch), + } + } + ends_cleanly(chars.as_str()).then_some(value) + } + Some('\'') => { + let rest = chars.as_str(); + let end = rest.find('\'')?; + ends_cleanly(&rest[end + 1..]).then(|| rest[..end].to_owned()) + } + Some(_) => { + let mut value = String::new(); + let mut chars = raw.chars(); + // `sh` expands an unquoted `~` at the start of the value and after + // each unquoted `:`. + let mut tilde_expands = true; + while let Some(ch) = chars.next() { + match ch { + '\\' => { + value.push(chars.next()?); + tilde_expands = false; + continue; + } + // A blank ends the value. Past it, only a comment may follow. + ' ' | '\t' => { + let rest = format!("{ch}{}", chars.as_str()); + return ends_cleanly(&rest).then_some(value); + } + '~' if tilde_expands => return None, + '"' | '\'' | '$' | '`' | ';' | '|' | '&' | '<' | '>' | '(' | ')' => { + return None; + } + ch => value.push(ch), + } + tilde_expands = ch == ':'; + } + Some(value) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn reads_both_quote_styles_and_bare_values_alike() { + for text in [ + "VERSION_CODENAME=noble\n", + "VERSION_CODENAME=\"noble\"\n", + "VERSION_CODENAME='noble'\n", + "VERSION_CODENAME=noble \t\n", + " \tVERSION_CODENAME=noble\n", + "VERSION_CODENAME=noble", + ] { + assert_eq!( + field(text, "VERSION_CODENAME").as_deref(), + Some("noble"), + "{text:?}" + ); + } + } + + #[test] + fn decodes_the_backslash_sequences_a_double_quoted_value_may_carry() { + assert_eq!(field(r#"V="24\"04""#, "V").as_deref(), Some("24\"04")); + assert_eq!(field(r#"V="a\\b""#, "V").as_deref(), Some("a\\b")); + assert_eq!(field(r#"V="a\$b""#, "V").as_deref(), Some("a$b")); + assert_eq!(field(r#"V="a\`b""#, "V").as_deref(), Some("a`b")); + // Any other backslash is literal inside double quotes, as in a shell. + assert_eq!(field(r#"V="a\nb""#, "V").as_deref(), Some("a\\nb")); + // And no backslash sequence means anything inside single quotes. + assert_eq!(field(r"V='a\b'", "V").as_deref(), Some("a\\b")); + } + + #[test] + fn a_malformed_assignment_reads_as_absent() { + for text in [ + "VERSION_ID=\"24.04\n", + "VERSION_ID='24.04\n", + "VERSION_ID=\"24\"04\n", + "VERSION_ID='24'.04\n", + "VERSION_ID=24'04\n", + "VERSION_ID=\"24.04\"\"\n", + "VERSION_ID= 24.04\n", + "VERSION_ID=24 04\n", + "VERSION_ID=\"24$X\"\n", + "VERSION_ID=24;x\n", + "VERSION_ID=24>x\n", + "VERSION_ID=24|x\n", + "VERSION_ID=24&x\n", + "VERSION_ID=(24)\n", + "VERSION_ID=~\n", + "VERSION_ID=a:~\n", + "VERSION_ID=24\\\n", + ] { + assert_eq!(field(text, "VERSION_ID"), None, "{text:?}"); + } + } + + /// A line that is not a blank, a comment or a well-formed `NAME=value` is a + /// command to `sh`, and a command can set or unset *any* variable, including + /// ones assigned before it — so every field reads as `None`. Each of these + /// was read wrongly before: the parser returned a value `sh` does not + /// assign. What `sh` does with each, run through `dash`: + #[test] + fn a_file_that_runs_a_command_reads_as_absent_throughout() { + for (text, what_sh_does) in [ + ( + "ID=ubuntu\nNAME=\"foo\nID=debian\n\"\n", + "ID=debian is inside NAME's string; sh keeps ubuntu", + ), + ( + "ID=ubuntu\nNAME='foo\nID=debian\n'\n", + "the same, single-quoted", + ), + ( + "ID=ubuntu\nNAME=foo\\\nID=debian\n", + "a trailing backslash joins the lines; sh keeps ubuntu", + ), + ("ID=ubuntu\nX=1; ID=debian\n", "sh assigns debian"), + ("ID=ubuntu\nexport ID=debian\n", "sh assigns debian"), + ("ID=ubuntu\nunset ID\n", "sh leaves ID unset"), + ( + "ID=ubuntu\nX= unset ID\n", + "unset runs with a temporary X; sh leaves ID unset", + ), + ("ID=ubuntu\nID =debian\n", "runs a command named ID"), + ] { + assert_eq!(field(text, "ID"), None, "{text:?}: {what_sh_does}"); + } + } + + /// `\r` is not whitespace to `sh`: `ID=ubuntu\r` assigns `ubuntu\r`, which + /// matches no distro and would reach a terminal. A control character in a + /// value therefore makes the file malformed — so a CRLF file reads as + /// `None` throughout, deliberately, rather than being quietly tolerated. + #[test] + fn a_control_character_or_crlf_file_reads_as_absent() { + for text in [ + "ID=ubuntu\r\nVERSION_ID=24.04\r\n", + "ID=\"ub\u{1b}[2Kuntu\"\n", + "ID='ub\u{7}untu'\n", + ] { + assert_eq!(field(text, "ID"), None, "{text:?}"); + } + // A tab is the exception: a blank to `sh`, kept inside quotes as `sh` + // keeps it. + assert_eq!(field("ID='ub\tuntu'\n", "ID").as_deref(), Some("ub\tuntu")); + // Other Unicode whitespace is an ordinary character to `sh`, so it is + // kept, not trimmed. + assert_eq!( + field("ID=ubuntu\u{a0}\n", "ID").as_deref(), + Some("ubuntu\u{a0}") + ); + } + + #[test] + fn the_last_assignment_wins_as_in_a_shell() { + assert_eq!( + field("ID=debian\nID=ubuntu\n", "ID").as_deref(), + Some("ubuntu") + ); + // Including an empty one: `sh` assigns "" here, it does not keep ubuntu. + assert_eq!(field("ID=ubuntu\nID=\n", "ID").as_deref(), Some("")); + } + + #[test] + fn comments_blank_lines_and_near_miss_names() { + let text = "# ID=commented \"\n\n \t\nXID=prefixed\nID_LIKE=suffixed\n"; + assert_eq!(field(text, "ID"), None); + assert_eq!(field(text, "ID_LIKE").as_deref(), Some("suffixed")); + } + + /// `sh` ends an unquoted word at a blank, and a `#` that starts the next + /// word starts a comment — so a comment may follow any value form. + #[test] + fn a_comment_after_the_value_is_not_part_of_it() { + for text in [ + "ID=ubuntu # the distro\n", + "ID=ubuntu\t#\n", + "ID=\"ubuntu\" # quoted\n", + "ID='ubuntu' # single-quoted\n", + ] { + assert_eq!(field(text, "ID").as_deref(), Some("ubuntu"), "{text:?}"); + } + // Not after a blank, `#` is an ordinary character… + assert_eq!(field("ID=ubu#ntu\n", "ID").as_deref(), Some("ubu#ntu")); + // …and after a closing quote with no blank, it is concatenation. + assert_eq!(field("ID=\"ubuntu\"#x\n", "ID"), None); + // A blank followed by anything but a comment is a command. + assert_eq!(field("ID=ubuntu debian\n", "ID"), None); + } + + /// An unreadable file names its first offending line, 1-based, so the + /// reason a caller gives points at something a person can open and fix — + /// and quotes it so a control character in it is shown, not acted on. + #[test] + fn an_unreadable_file_reports_its_first_offending_line() { + let text = "# header\nID=ubuntu\n\nexport ID=debian\nunset ID\n"; + let unreadable = parse(text).expect_err("a command line makes the file unreadable"); + assert_eq!( + unreadable, + UnreadableLine { + number: 4, + text: "export ID=debian".to_owned() + } + ); + assert_eq!( + unreadable.to_string(), + "line 4 is not a plain assignment: \"export ID=debian\"" + ); + + let crlf = parse("ID=ubuntu\r\nVERSION_ID=24.04\r\n").unwrap_err(); + assert_eq!(crlf.number, 1); + assert_eq!( + crlf.to_string(), + "line 1 is not a plain assignment: \"ID=ubuntu\\r\"" + ); + + let long = format!("X={}", "a b".repeat(100)); + let cut = parse(&long).unwrap_err(); + assert_eq!(cut.text.chars().count(), UnreadableLine::SHOWN_CHARS); + } + + /// Characters drawn from to build values: mostly the ones the encodings + /// treat specially, so every backslash and quoting path is exercised. + const ALPHABET: &[char] = &[ + 'a', '1', '.', ' ', '"', '\'', '\\', '$', '`', '#', '=', ':', '~', + ]; + + /// Every string over `alphabet` up to `max_len` characters. Exhaustive + /// rather than sampled: the shapes that break a parser are short, and an + /// enumeration cannot miss one by chance. + fn every_string(alphabet: &[char], max_len: usize) -> Vec { + let mut out = vec![String::new()]; + let mut frontier = vec![String::new()]; + for _ in 0..max_len { + frontier = frontier + .iter() + .flat_map(|prefix| { + alphabet.iter().map(move |ch| { + let mut next = prefix.clone(); + next.push(*ch); + next + }) + }) + .collect(); + out.extend(frontier.iter().cloned()); + } + out + } + + /// Every spec-valid way to write `value`: double-quoted with backslashes + /// always; single-quoted when it holds no `'`; bare, with a backslash before + /// every non-alphanumeric character, when it is non-empty. + fn encodings(value: &str) -> Vec { + let mut out = vec![format!( + "\"{}\"", + value + .chars() + .map(|ch| match ch { + '"' | '\\' | '`' | '$' => format!("\\{ch}"), + ch => ch.to_string(), + }) + .collect::() + )]; + if !value.contains('\'') { + out.push(format!("'{value}'")); + } + // Bare, escaping everything that is not a letter or digit. A trailing + // backslash-space is left out: trailing blanks are trimmed before + // decoding, so `a\ ` reads as `a\` — a dangling backslash, which is + // malformed. Failing closed on it is allowed; it is just not "valid". + if !value.is_empty() && !value.ends_with(' ') { + out.push( + value + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() { + ch.to_string() + } else { + format!("\\{ch}") + } + }) + .collect(), + ); + } + out + } + + /// Every spec-valid encoding of every short value decodes back to it. + #[test] + fn every_valid_encoding_decodes_to_its_value() { + let values = every_string(ALPHABET, 3); + assert!(values.len() > 1_000, "enumeration too small to mean much"); + for value in &values { + for encoded in encodings(value) { + let text = format!("K={encoded}\n"); + assert_eq!( + field(&text, "K").as_deref(), + Some(value.as_str()), + "{encoded} should decode to {value:?}" + ); + } + } + } + + /// The parser checked against `sh` itself. Unix-only: there is no POSIX + /// shell on a Windows runner, and these would be dead code there. + #[cfg(unix)] + mod sh_oracle { + use super::*; + use std::fmt::Write as _; + use std::path::{Path, PathBuf}; + + /// `HOME` for the shell, so a `~` that expands is visible as this rather + /// than as nothing. Unset, `dash` leaves `~` literal and an expansion the + /// parser missed would go unnoticed. + const HOME_SENTINEL: &str = "/rocm-os-release-home"; + + /// A scratch directory removed on drop, so a failing assertion does not + /// leave it behind. + struct ScratchDir(PathBuf); + + impl ScratchDir { + fn new(tag: &str) -> Self { + let dir = std::env::temp_dir().join(format!( + "rocm-os-release-{tag}-{}-{}", + std::process::id(), + crate::unix_time_millis() + )); + std::fs::create_dir_all(&dir).unwrap(); + Self(dir) + } + } + + impl Drop for ScratchDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + /// `sh` sourcing `path` from inside `dir`, with nothing it could run: + /// an empty environment, `PATH` naming a directory that does not exist + /// (an *empty* `PATH` makes `dash` search the current directory), and + /// `/bin/sh` by absolute path. A value like `K= x` or `` `x` `` then + /// names a command that is not found. + /// + /// The script goes in a file rather than `-c`: the all-valid-encodings + /// case reads thousands of keys, past the kernel's argument-size limit. + fn sh_source(dir: &Path, path: &Path, script_tail: &str) -> std::process::Output { + let script = dir.join("probe.sh"); + std::fs::write( + &script, + format!(". '{}' || exit 3\n{script_tail}", path.display()), + ) + .unwrap(); + std::process::Command::new("/bin/sh") + .arg(&script) + .current_dir(dir) + .env_clear() + .env("PATH", "/nonexistent") + .env("HOME", HOME_SENTINEL) + .output() + .expect("sh should run") + } + + /// What `sh` leaves in each of `keys` after sourcing `text`: `None` for + /// unset, and `None` for every key if sourcing failed. Each value is + /// printed NUL-terminated behind a set/unset marker. + fn sh_reads(dir: &ScratchDir, text: &str, keys: &[&str]) -> Vec> { + let path = dir.0.join("os-release"); + std::fs::write(&path, text).unwrap(); + let mut tail = String::new(); + for key in keys { + writeln!( + tail, + "case ${{{key}+x}} in x) printf 'S%s\\0' \"${key}\" ;; *) printf 'U\\0' ;; esac" + ) + .expect("writing to a String cannot fail"); + } + let output = sh_source(&dir.0, &path, &tail); + if !output.status.success() { + return vec![None; keys.len()]; + } + output + .stdout + .split(|byte| *byte == 0) + .take(keys.len()) + .map(|chunk| { + let chunk = String::from_utf8_lossy(chunk); + chunk.strip_prefix('S').map(str::to_owned) + }) + .collect() + } + + /// The module's rule for one file: for every key, the parser reads + /// either nothing or exactly what `sh` left in it. Returns how many keys + /// read as a value. + fn assert_sh_or_none(dir: &ScratchDir, text: &str, keys: &[&str]) -> usize { + let from_shell = sh_reads(dir, text, keys); + let mut read = 0; + for (key, shell) in keys.iter().zip(&from_shell) { + if let Some(value) = field(text, key) { + read += 1; + assert_eq!( + Some(&value), + shell.as_ref(), + "{key} in {text:?}: parser read {value:?}, sh left {shell:?}" + ); + } + } + read + } + + /// Every spec-valid encoding, in one file sourced once, reads exactly as + /// `sh` reads it — never `None`. The encoder and the parser could share a + /// misunderstanding of the format; `sh` cannot. + #[test] + fn a_shell_sourcing_the_file_reads_what_this_parser_reads() { + let mut text = String::new(); + let mut expected = Vec::new(); + for value in every_string(ALPHABET, 3) { + for encoded in encodings(&value) { + let key = format!("K{}", expected.len()); + writeln!(text, "{key}={encoded}").expect("writing to a String cannot fail"); + expected.push((key, value.clone())); + } + } + let dir = ScratchDir::new("valid"); + let keys: Vec<&str> = expected.iter().map(|(key, _)| key.as_str()).collect(); + let from_shell = sh_reads(&dir, &text, &keys); + // `parse` once rather than `field` per key: each `field` call reads + // the whole file, and this one has thousands of keys. + let parsed = parse(&text).expect("every line here is a valid assignment"); + for ((key, value), shell) in expected.iter().zip(&from_shell) { + assert_eq!( + shell.as_deref(), + Some(value.as_str()), + "sh disagrees with the encoder for {key}" + ); + assert_eq!( + parsed.get(key.as_str()).map(String::as_str), + Some(value.as_str()), + "this parser disagrees with sh for {key}" + ); + } + } + + /// The value assigned before each case in the next test, spelled with + /// letters outside [`RHS_ALPHABET`] so no right-hand side can produce it. + const PRIOR: &str = "PRIOR"; + + /// Right-hand sides for the next test, valid and malformed alike — with + /// a newline, so a value can open a string or a continuation that + /// swallows the line after it, and `:` with `~`, which `sh` expands. + /// `>`, `<`, `|`, `&` and `(` are left out because `sh` would act on them + /// (create a file, open a pipe); their refusal is pinned by + /// `a_malformed_assignment_reads_as_absent` instead. + const RHS_ALPHABET: &[char] = &[ + 'a', ' ', '\\', '"', '\'', '$', '`', ';', '#', '~', ':', '\n', + ]; + + /// Right-hand sides that are well-formed and must read as exactly what + /// `sh` assigns — never `None`. Without this list, a parser that + /// returned `None` for everything would satisfy "`sh`'s value or `None`". + const MUST_READ: &[&str] = &[ + "", + "a", + r"a\a", + r"\a", + r"a\ a", + r"a\$", + "a ", + "'a'", + "\"a\"", + "\"\"", + "a:a", + r"\~", + "a~", + "'~'", + // A `#` comment after a blank ends the value, for every value form; + // a `#` that does not follow a blank is part of the value. + "a #", + "a #a", + "a\t# the distro", + "'a' #a", + "\"a\" #", + " #a", + "a#", + "#a", + ]; + + /// The rule the module states, against `sh`, for every right-hand side + /// up to three characters over a hostile alphabet. The file is + /// `L=PRIOR`, `K=PRIOR`, `K=`, then `L=b` — so a right-hand side + /// that opens a string or a continuation can swallow the next line, and + /// both keys are checked: for each, the parser reads `sh`'s value or + /// nothing. In particular it never reads back `PRIOR` where `sh` moved + /// on, nor `b` where `sh` never reached it. Every [`MUST_READ`] case + /// must read as a value. + /// + /// One `sh` per case: a malformed line can abort the source, so a + /// shared file would stop at the first one. + #[test] + fn every_right_hand_side_reads_as_sh_assigns_it_or_not_at_all() { + let dir = ScratchDir::new("any"); + let mut cases = every_string(RHS_ALPHABET, 3); + cases.extend(MUST_READ.iter().map(|case| (*case).to_owned())); + let mut read = 0usize; + for rhs in &cases { + let text = format!("L={PRIOR}\nK={PRIOR}\nK={rhs}\nL=b\n"); + read += assert_sh_or_none(&dir, &text, &["K", "L"]); + if MUST_READ.contains(&rhs.as_str()) { + let shell = sh_reads(&dir, &text, &["K"]).remove(0); + assert!( + shell.is_some(), + "MUST_READ case K={rhs:?} is not valid to sh" + ); + assert_eq!( + field(&text, "K"), + shell, + "K={rhs:?} is well-formed and must read as sh assigns it" + ); + } + } + // Printed so `--nocapture` reports the split, not only that it + // cleared the bar. + println!( + "reach: {read} of {} key reads returned a value; the rest fail closed", + cases.len() * 2 + ); + assert!( + read > cases.len() / 10, + "only {read} key reads returned a value; a parser that refuses \ + nearly everything satisfies the rule vacuously" + ); + } + + /// The files that run a command, checked against `sh` too, for every key + /// they mention. + #[test] + fn a_file_that_runs_a_command_never_reads_as_something_sh_does_not_assign() { + let dir = ScratchDir::new("commands"); + for text in [ + "ID=ubuntu\nNAME=\"foo\nID=debian\n\"\n", + "ID=ubuntu\nNAME='foo\nID=debian\n'\n", + "ID=ubuntu\nNAME=foo\\\nID=debian\n", + "ID=ubuntu\nX=1; ID=debian\n", + "ID=ubuntu\nexport ID=debian\n", + "ID=ubuntu\nVERSION_ID=24.04\nunset VERSION_ID\n", + "ID=ubuntu\nVERSION_ID=24.04\nX= unset VERSION_ID\n", + "ID=ubuntu\nVERSION_ID=a:~\n", + ] { + assert_sh_or_none(&dir, text, &["ID", "NAME", "VERSION_ID", "X"]); + } + } + } +} diff --git a/docs/testing.md b/docs/testing.md index e95dc02b7..1ae3a3751 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1289,6 +1289,23 @@ rocm --bin rocm wsl_rocdxg`). Running it end to end needs a WSL2 host with `/dev/dxg` and dxcore present, since the plan refuses before installing otherwise. +## /etc/os-release + +Every reader of `/etc/os-release` (the driver plan, `rocm examine`, the +OpenMPI/libatomic/libnuma hints and the host distro name) goes through +`rocm_core::os_release`, which returns the value `sh` would assign or nothing. +A line that is not a plain assignment makes the whole file unreadable: the +driver plan then has policy `unreadable_os_release`, plans no commands, and its +reason names the line; `rocm examine` leaves the distro empty and records the +same line in `probe_failures`. + +```bash +cargo test -p rocm-core --lib os_release +cargo test -p rocm --bin rocm os_release +``` + +The parser tests that compare against `/bin/sh` itself are Unix-only. + ## Model Fit Preflight `rocm diagnose --model ` answers whether a curated model will run on this