From a691f9a7ed0dd9705579493d2e4e31a75b87e237 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Mon, 24 Aug 2026 16:17:44 -0700 Subject: [PATCH 01/14] fix(install): resolve canonical ROCm streams (EAI-8268) Signed-off-by: Michael Roy Signed-off-by: Juho Vainio (cherry picked from commit ef2aaa98f87d4df0e96d26674ef8e6806eec6836) Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 323 +++++++++++++----- .../features/runtime_setup.feature | 5 + tests/e2e-cucumber/tests/e2e/runtime_steps.rs | 33 ++ 3 files changed, 285 insertions(+), 76 deletions(-) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 6d34ba253..647143d3c 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -27,11 +27,11 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; use std::time::Duration; -const THEROCK_NIGHTLY_PIP_INDEX_BASE: &str = "https://rocm.nightlies.amd.com/v2"; -const THEROCK_RELEASE_PIP_INDEX_BASE: &str = "https://repo.amd.com/rocm/whl"; -const THEROCK_RELEASE_PIP_MULTI_ARCH_INDEX_BASE: &str = "https://repo.amd.com/rocm/whl-multi-arch"; +const THEROCK_NIGHTLY_PIP_INDEX_BASE: &str = "https://rocm.nightlies.amd.com/whl-multi-arch"; +const THEROCK_RELEASE_PIP_INDEX_BASE: &str = "https://repo.amd.com/rocm/whl-multi-arch"; const THEROCK_RELEASE_TARBALL_BASE: &str = "https://repo.amd.com/rocm/tarball/"; const THEROCK_NIGHTLY_TARBALL_BASE: &str = "https://rocm.nightlies.amd.com/tarball/"; +const THEROCK_SOURCE_LAYOUT_GENERATION: &str = "multi-arch-v2"; const DEFAULT_MANAGED_PYTHON_VERSION: &str = "3.12"; const STARTUP_UPDATE_CHECK_INTERVAL_MS: u128 = 12 * 60 * 60 * 1_000; const STARTUP_UPDATE_CHECK_TIMEOUT_SECS: u64 = 2; @@ -53,6 +53,13 @@ enum TheRockChannel { Nightly, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +struct CanonicalSource { + wheel_index: &'static str, + tarball_catalog: &'static str, + layout_generation: &'static str, +} + #[derive(Debug, Clone, Eq, PartialEq)] pub(crate) enum RuntimeVersionSelector { Version(String), @@ -112,14 +119,55 @@ impl TheRockChannel { } } - const fn tarball_base_url(self) -> &'static str { - match self { - Self::Release => THEROCK_RELEASE_TARBALL_BASE, - Self::Nightly => THEROCK_NIGHTLY_TARBALL_BASE, - } + const fn canonical_source(self) -> CanonicalSource { + canonical_source(self) } } +const fn canonical_source(channel: TheRockChannel) -> CanonicalSource { + match channel { + TheRockChannel::Release => CanonicalSource { + wheel_index: THEROCK_RELEASE_PIP_INDEX_BASE, + tarball_catalog: THEROCK_RELEASE_TARBALL_BASE, + layout_generation: THEROCK_SOURCE_LAYOUT_GENERATION, + }, + TheRockChannel::Nightly => CanonicalSource { + wheel_index: THEROCK_NIGHTLY_PIP_INDEX_BASE, + tarball_catalog: THEROCK_NIGHTLY_TARBALL_BASE, + layout_generation: THEROCK_SOURCE_LAYOUT_GENERATION, + }, + } +} + +fn render_canonical_provenance( + output: &mut String, + channel: TheRockChannel, + source_url: &str, + layout_generation: &str, + version: &str, +) { + let build_date = runtime_version_build_date(version) + .unwrap_or_else(|| "".to_owned()); + let _ = writeln!(output, " channel: {}", channel.as_str()); + let _ = writeln!(output, " canonical_source: {source_url}"); + let _ = writeln!(output, " selected_rocm_version: {version}"); + let _ = writeln!(output, " build_date: {build_date}"); + let _ = writeln!(output, " source_layout_generation: {layout_generation}"); +} + +fn validate_aggregate_index_layout(html: &str) -> Result<()> { + let required_packages = ["rocm/", "torch/", "torchvision/", "torchaudio/"]; + if required_packages + .iter() + .all(|package| html.contains(package)) + { + return Ok(()); + } + bail!( + "unknown canonical TheRock aggregate index layout: expected package links for rocm, torch, torchvision, and torchaudio" + ) +} + #[derive(Debug, Clone)] struct FamilyResolution { family: String, @@ -849,7 +897,14 @@ fn install_wheel_runtime( output, " summary: rocm-cli will install the ROCm SDK and matching PyTorch packages for this Python and operating system" ); - let _ = writeln!(output, " channel: {}", channel.as_str()); + let source = channel.canonical_source(); + render_canonical_provenance( + &mut output, + channel, + source.wheel_index, + source.layout_generation, + &resolution.latest_version, + ); let _ = writeln!(output, " format: wheel"); if let Some(selector) = version_selector { let _ = writeln!(output, " requested: {}", selector.describe()); @@ -1062,7 +1117,14 @@ fn install_tarball_runtime( let mut output = String::new(); let _ = writeln!(output, "sdk install"); - let _ = writeln!(output, " channel: {}", channel.as_str()); + let source = channel.canonical_source(); + render_canonical_provenance( + &mut output, + channel, + source.tarball_catalog, + source.layout_generation, + &artifact.version, + ); let _ = writeln!(output, " format: tarball"); let _ = writeln!(output, " family: {}", artifact.family); let _ = writeln!(output, " family_source: {}", artifact.family_source); @@ -1152,33 +1214,44 @@ fn resolve_pip_runtime_with_timeout( download_timeout_secs: Option, ) -> Result { let family_resolution = resolve_family(paths, family_override)?; - let index_urls = therock_index_urls(channel, &family_resolution.family); - let mut errors = Vec::new(); - for index_url in index_urls { - match resolve_pip_runtime_from_index( - paths, - channel, - &family_resolution, - &index_url, - wheel_compatibility, - version_selector, - download_timeout_secs, - ) { - Ok(resolution) => return Ok(resolution), - Err(error) => errors.push(format!("{index_url}: {error}")), - } - } - bail!( - "failed to resolve TheRock {} wheel runtime from candidate indexes:\n - {}\n\n{}", - channel.as_str(), - errors.join("\n - "), - family_resolution_hint( - &family_resolution.source, - &family_resolution.family, - channel, - "wheel", + let source = channel.canonical_source(); + let root_url = format!("{}/", source.wheel_index.trim_end_matches('/')); + let root_html = download_text_cached( + paths, + &format!("canonical-wheel-root-{}", channel.as_str()), + &root_url, + download_timeout_secs, + )? + .text; + validate_aggregate_index_layout(&root_html).with_context(|| { + format!( + "failed to resolve TheRock {} wheel runtime from canonical source {}", + channel.as_str(), + source.wheel_index ) + })?; + resolve_pip_runtime_from_index( + paths, + channel, + &family_resolution, + source.wheel_index, + wheel_compatibility, + version_selector, + download_timeout_secs, ) + .with_context(|| { + format!( + "failed to resolve TheRock {} wheel runtime from canonical source {}\n\n{}", + channel.as_str(), + source.wheel_index, + family_resolution_hint( + &family_resolution.source, + &family_resolution.family, + channel, + "wheel", + ) + ) + }) } fn resolve_pip_runtime_from_index( @@ -1262,28 +1335,64 @@ fn resolve_tarball_artifact_with_timeout( download_timeout_secs: Option, ) -> Result { let family_resolution = resolve_family(paths, family_override)?; + let source = channel.canonical_source(); let html = download_text_cached( paths, &format!("tarball-index-{}", channel.as_str()), - channel.tarball_base_url(), + source.tarball_catalog, download_timeout_secs, )? .text; - let files = parse_tarball_index_html(&html)?; - let prefix = format!( - "therock-dist-{}-{}-", - platform_tarball_token(), - family_resolution.family - ); + let files = parse_tarball_index_html(&html).with_context(|| { + format!( + "unknown canonical TheRock tarball catalog layout at {}", + source.tarball_catalog + ) + })?; + let (file, version) = select_tarball_candidate(&files, channel, &family_resolution.family) + .with_context(|| { + format!( + "canonical TheRock {} tarball stream is incomplete for the resolved GPU family\n\n{}", + channel.as_str(), + family_resolution_hint( + &family_resolution.source, + &family_resolution.family, + channel, + "tarball", + ) + ) + })?; + Ok(TarballArtifact { + family: family_resolution.family, + family_source: family_resolution.source, + url: format!( + "{}/{}", + source.tarball_catalog.trim_end_matches('/'), + file.name + ), + file_name: file.name, + version, + }) +} + +fn select_tarball_candidate( + files: &[TarballIndexFile], + channel: TheRockChannel, + family: &str, +) -> Option<(TarballIndexFile, String)> { + let prefix = format!("therock-dist-{}-{family}-", platform_tarball_token()); let mut candidates = files - .into_iter() + .iter() .filter_map(|file| { let version = file .name .strip_prefix(&prefix)? .strip_suffix(".tar.gz")? .to_owned(); - Some((file, version)) + if matches!(channel, TheRockChannel::Release) && !is_stable_runtime_version(&version) { + return None; + } + Some((file.clone(), version)) }) .collect::>(); candidates.sort_by(|left, right| { @@ -1293,28 +1402,7 @@ fn resolve_tarball_artifact_with_timeout( .unwrap_or(Ordering::Equal) .then_with(|| compare_version_strings(&left.1, &right.1)) }); - let (file, version) = candidates.pop().with_context(|| { - format!( - "no matching TheRock tarball artifact was found for the resolved GPU family\n\n{}", - family_resolution_hint( - &family_resolution.source, - &family_resolution.family, - channel, - "tarball", - ) - ) - })?; - Ok(TarballArtifact { - family: family_resolution.family, - family_source: family_resolution.source, - url: format!( - "{}/{}", - channel.tarball_base_url().trim_end_matches('/'), - file.name - ), - file_name: file.name, - version, - }) + candidates.pop() } fn resolve_family(paths: &AppPaths, family_override: Option<&str>) -> Result { @@ -3692,16 +3780,6 @@ fn parse_version(value: &str) -> Option { }) } -fn therock_index_urls(channel: TheRockChannel, family: &str) -> Vec { - match channel { - TheRockChannel::Release => vec![ - format!("{THEROCK_RELEASE_PIP_INDEX_BASE}/{family}"), - format!("{THEROCK_RELEASE_PIP_MULTI_ARCH_INDEX_BASE}/{family}"), - ], - TheRockChannel::Nightly => vec![format!("{THEROCK_NIGHTLY_PIP_INDEX_BASE}/{family}")], - } -} - /// Recovery guidance appended to family/index resolution failures so a clean /// first run can recover without the user having to guess a `--family`. /// @@ -4382,6 +4460,99 @@ mod tests { ); } + #[test] + fn canonical_channels_use_only_their_aggregate_streams() { + let release = canonical_source(TheRockChannel::Release); + assert_eq!( + release.wheel_index, + "https://repo.amd.com/rocm/whl-multi-arch" + ); + assert_eq!( + release.tarball_catalog, + "https://repo.amd.com/rocm/tarball/" + ); + assert_eq!(release.layout_generation, "multi-arch-v2"); + + let nightly = canonical_source(TheRockChannel::Nightly); + assert_eq!( + nightly.wheel_index, + "https://rocm.nightlies.amd.com/whl-multi-arch" + ); + assert_eq!( + nightly.tarball_catalog, + "https://rocm.nightlies.amd.com/tarball/" + ); + assert_eq!(nightly.layout_generation, "multi-arch-v2"); + } + + #[test] + fn nightly_accepts_future_prerelease_major_without_cli_changes() { + let selected = select_matching_pip_package_versions( + TheRockChannel::Nightly, + &["10.1.0a20260822".to_owned()], + &["2.12.0+rocm10.1.0a20260822".to_owned()], + &["0.27.0+rocm10.1.0a20260822".to_owned()], + &["2.12.0+rocm10.1.0a20260822".to_owned()], + None, + ) + .expect("future nightly major should resolve"); + + assert_eq!(selected.rocm, "10.1.0a20260822"); + } + + #[test] + fn tarball_selection_never_crosses_channels() { + let files = vec![ + TarballIndexFile { + name: "therock-dist-linux-gfx120X-all-7.14.0.tar.gz".to_owned(), + mtime: 1.0, + }, + TarballIndexFile { + name: "therock-dist-linux-gfx120X-all-10.1.0a20260822.tar.gz".to_owned(), + mtime: 2.0, + }, + ]; + + assert_eq!( + select_tarball_candidate(&files, TheRockChannel::Release, "gfx120X-all") + .map(|(_, version)| version), + Some("7.14.0".to_owned()) + ); + assert_eq!( + select_tarball_candidate(&files, TheRockChannel::Nightly, "gfx120X-all") + .map(|(_, version)| version), + Some("10.1.0a20260822".to_owned()) + ); + } + + #[test] + fn canonical_provenance_reports_required_dry_run_fields() { + let source = canonical_source(TheRockChannel::Nightly); + let mut output = String::new(); + render_canonical_provenance( + &mut output, + TheRockChannel::Nightly, + source.wheel_index, + source.layout_generation, + "10.1.0a20260822", + ); + + assert!(output.contains("channel: nightly")); + assert!(output.contains("canonical_source: https://rocm.nightlies.amd.com/whl-multi-arch")); + assert!(output.contains("selected_rocm_version: 10.1.0a20260822")); + assert!(output.contains("build_date: 2026-08-22")); + assert!(output.contains("source_layout_generation: multi-arch-v2")); + } + + #[test] + fn unknown_aggregate_layout_is_rejected_clearly() { + let error = + validate_aggregate_index_layout("legacy") + .unwrap_err() + .to_string(); + assert!(error.contains("unknown canonical TheRock aggregate index layout")); + } + #[test] fn pip_runtime_installs_pinned_devel_and_torch_stack_from_therock_index() { let package_versions = TheRockPipPackageVersions { diff --git a/tests/e2e-cucumber/features/runtime_setup.feature b/tests/e2e-cucumber/features/runtime_setup.feature index c926591e5..217c30284 100644 --- a/tests/e2e-cucumber/features/runtime_setup.feature +++ b/tests/e2e-cucumber/features/runtime_setup.feature @@ -131,3 +131,8 @@ Feature: Runtime configuration When the user tries to adopt the existing install Then the adoption is refused And the error explains which install types can be adopted + + @id:runtime-resolve-canonical-nightly + Scenario: 4 - Previewing a nightly SDK install reports canonical provenance + When the user dry-runs a nightly SDK install for a known family + Then the SDK preview reports canonical nightly provenance diff --git a/tests/e2e-cucumber/tests/e2e/runtime_steps.rs b/tests/e2e-cucumber/tests/e2e/runtime_steps.rs index ea88af310..4657814f8 100644 --- a/tests/e2e-cucumber/tests/e2e/runtime_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/runtime_steps.rs @@ -409,6 +409,39 @@ fn assert_engine_ready(world: &mut E2eWorld) { "no engine runtime is ready:\n{stdout}" ); } +#[when("the user dry-runs a nightly SDK install for a known family")] +async fn user_dry_runs_nightly_sdk(world: &mut E2eWorld) { + let stdout = crate::run_rocm_ok( + world, + &[ + "install", + "sdk", + "--channel", + "nightly", + "--family", + "gfx120X-all", + "--dry-run", + ], + ); + world.cli_output = Some(stdout); +} + +#[then("the SDK preview reports canonical nightly provenance")] +async fn assert_canonical_nightly_provenance(world: &mut E2eWorld) { + let output = world.cli_output.as_deref().expect("no SDK preview output"); + for expected in [ + "channel: nightly", + "canonical_source: https://rocm.nightlies.amd.com/whl-multi-arch", + "selected_rocm_version:", + "build_date:", + "source_layout_generation: multi-arch-v2", + ] { + assert!( + output.contains(expected), + "SDK preview omitted `{expected}`:\n{output}" + ); + } +} #[when("the user tries to adopt the existing install")] async fn user_tries_adopt(world: &mut E2eWorld) { From ac6e34b4f3837eec3e1d2c5c8540b3cfb6cc1aaf Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Mon, 24 Aug 2026 17:01:15 -0700 Subject: [PATCH 02/14] test(install): make tarball fixture platform-aware Signed-off-by: Michael Roy (cherry picked from commit db7e7d5f03688cf57260727c1fcb3c548369fdca) Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 647143d3c..ca5dd59e9 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -4502,13 +4502,14 @@ mod tests { #[test] fn tarball_selection_never_crosses_channels() { + let platform = platform_tarball_token(); let files = vec![ TarballIndexFile { - name: "therock-dist-linux-gfx120X-all-7.14.0.tar.gz".to_owned(), + name: format!("therock-dist-{platform}-gfx120X-all-7.14.0.tar.gz"), mtime: 1.0, }, TarballIndexFile { - name: "therock-dist-linux-gfx120X-all-10.1.0a20260822.tar.gz".to_owned(), + name: format!("therock-dist-{platform}-gfx120X-all-10.1.0a20260822.tar.gz"), mtime: 2.0, }, ]; From 7303ef506977ef701a21980c5408d585f304f139 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Tue, 25 Aug 2026 14:36:51 -0700 Subject: [PATCH 03/14] fix(install): address canonical stream review Signed-off-by: Michael Roy (cherry picked from commit 1a1fad733bf252986638c645f23043467fff3250) Signed-off-by: Michael Roy --- MANIFEST.md | 4 +- apps/rocm/src/therock.rs | 88 +++++++++++++------ .../features/runtime_setup.feature | 4 +- 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/MANIFEST.md b/MANIFEST.md index 25f49ee7b..7701e4123 100644 --- a/MANIFEST.md +++ b/MANIFEST.md @@ -646,8 +646,8 @@ ROCm distribution. Two install formats are supported: - **Wheel format** — Python wheel packages (`rocm`, `torch`, `torchvision`, `torchaudio`) are resolved from AMD-hosted PyPI-compatible indexes and installed via `uv` into a managed virtual environment. Release channel wheels - are served from `https://repo.amd.com/rocm/whl//`. Nightly - channel wheels are served from `https://rocm.nightlies.amd.com/v2//`. + are served from `https://repo.amd.com/rocm/whl-multi-arch`. Nightly channel + wheels are served from `https://rocm.nightlies.amd.com/whl-multi-arch`. - **Tarball format** — Prebuilt SDK tarballs are downloaded from AMD-hosted artifact storage. Release channel tarballs are served from `https://repo.amd.com/rocm/tarball/`. Nightly tarballs are served from diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index ca5dd59e9..8456da372 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -147,7 +147,7 @@ fn render_canonical_provenance( version: &str, ) { let build_date = runtime_version_build_date(version) - .unwrap_or_else(|| "".to_owned()); + .unwrap_or_else(|| "not encoded in stable version".to_owned()); let _ = writeln!(output, " channel: {}", channel.as_str()); let _ = writeln!(output, " canonical_source: {source_url}"); let _ = writeln!(output, " selected_rocm_version: {version}"); @@ -879,16 +879,11 @@ fn install_wheel_runtime( version_selector, )?; progress_line(format!( - "Found TheRock package family {} version {} with a matching PyTorch stack.", - resolution.family, resolution.latest_version + "Found canonical TheRock aggregate version {} with a matching PyTorch stack for target family {}.", + resolution.latest_version, resolution.family )); - let runtime_key = runtime_key( - channel, - "wheel", - &resolution.family, - Some(&resolution.latest_version), - ); - let install_root = resolved_install_root(paths, "wheel", &runtime_key, prefix); + let runtime_key = wheel_runtime_key(channel, &resolution.latest_version); + let install_root = prefix.unwrap_or_else(|| managed_runtime_root(paths, "wheel", &runtime_key)); let manifest_path = runtime_manifest_path(paths, &runtime_key); let mut output = String::new(); @@ -909,8 +904,12 @@ fn install_wheel_runtime( if let Some(selector) = version_selector { let _ = writeln!(output, " requested: {}", selector.describe()); } - let _ = writeln!(output, " family: {}", resolution.family); - let _ = writeln!(output, " family_source: {}", resolution.family_source); + let _ = writeln!(output, " target_family: {}", resolution.family); + let _ = writeln!( + output, + " target_family_source: {}", + resolution.family_source + ); let _ = writeln!(output, " index_url: {}", resolution.index_url); let _ = writeln!( output, @@ -1244,12 +1243,7 @@ fn resolve_pip_runtime_with_timeout( "failed to resolve TheRock {} wheel runtime from canonical source {}\n\n{}", channel.as_str(), source.wheel_index, - family_resolution_hint( - &family_resolution.source, - &family_resolution.family, - channel, - "wheel", - ) + canonical_wheel_resolution_hint(channel) ) }) } @@ -3789,6 +3783,26 @@ fn parse_version(value: &str) -> Option { /// so an auto-detected miss points the user at `--family`, while a user-supplied /// miss confirms the family they already named. Both point at the other channel /// and, where valid for the platform, the other install format. +fn canonical_wheel_resolution_hint(channel: TheRockChannel) -> String { + let other_channel = match channel { + TheRockChannel::Release => "nightly", + TheRockChannel::Nightly => "release", + }; + let mut hint = format!( + "No complete compatible package stack was found in the canonical {} aggregate stream. Try `--channel {other_channel}`", + channel.as_str() + ); + if !runtime_is_windows() { + hint.push_str(" or `--format tarball`"); + } + hint.push('.'); + hint +} + +fn wheel_runtime_key(channel: TheRockChannel, version: &str) -> String { + slugify(&format!("{}-wheel-multi-arch-{version}", channel.as_str())) +} + fn family_resolution_hint( source: &str, family: &str, @@ -4878,7 +4892,7 @@ mod tests { #[test] fn managed_uv_cache_sits_under_the_data_dir_for_generated_runtime_folders() { let (_root, paths) = test_paths("managed-uv-cache"); - let runtime_key = "release-wheel-gfx120x-all-7-14-0"; + let runtime_key = "release-wheel-multi-arch-7-14-0"; let install_root = managed_runtime_root(&paths, "wheel", runtime_key); assert!(install_root.starts_with(&paths.data_dir)); // Without --prefix the generated runtime folder is itself under the data dir, so @@ -5139,16 +5153,36 @@ echo Python 3.12.10 } #[test] - fn runtime_key_includes_version_for_side_by_side_installs() { + fn aggregate_wheel_runtime_key_ignores_target_family() { assert_eq!( - runtime_key( - TheRockChannel::Release, - "wheel", - "gfx120X-all", - Some("7.13.0a20260416") - ), - "release-wheel-gfx120x-all-7-13-0a20260416" + wheel_runtime_key(TheRockChannel::Release, "7.13.0a20260416"), + "release-wheel-multi-arch-7-13-0a20260416" + ); + } + + #[test] + fn aggregate_wheel_resolution_hint_does_not_recommend_family_override() { + let hint = canonical_wheel_resolution_hint(TheRockChannel::Release); + assert!(!hint.contains("--family")); + assert!(hint.contains("--channel nightly")); + if !runtime_is_windows() { + assert!(hint.contains("--format tarball")); + } + } + + #[test] + fn stable_provenance_uses_neutral_build_date_wording() { + let source = canonical_source(TheRockChannel::Release); + let mut output = String::new(); + render_canonical_provenance( + &mut output, + TheRockChannel::Release, + source.wheel_index, + source.layout_generation, + "7.14.0", ); + assert!(output.contains("build_date: not encoded in stable version")); + assert!(!output.contains("not published")); } #[test] diff --git a/tests/e2e-cucumber/features/runtime_setup.feature b/tests/e2e-cucumber/features/runtime_setup.feature index 217c30284..b61644042 100644 --- a/tests/e2e-cucumber/features/runtime_setup.feature +++ b/tests/e2e-cucumber/features/runtime_setup.feature @@ -132,7 +132,7 @@ Feature: Runtime configuration Then the adoption is refused And the error explains which install types can be adopted - @id:runtime-resolve-canonical-nightly - Scenario: 4 - Previewing a nightly SDK install reports canonical provenance + @id:runtime-resolve-canonical-nightly @nightly + Scenario: 5 - Previewing a nightly SDK install reports canonical provenance When the user dry-runs a nightly SDK install for a known family Then the SDK preview reports canonical nightly provenance From 2baad4139200d20b0e5bee014df91344e75dc928 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Tue, 25 Aug 2026 18:08:41 -0700 Subject: [PATCH 04/14] fix(install): include aggregate device payloads Signed-off-by: Michael Roy (cherry picked from commit 5df4c8d806829911f49bb7648446bf0f44102513) Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 8456da372..7790745c6 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -946,7 +946,7 @@ fn install_wheel_runtime( ); let _ = writeln!( output, - " package_policy: find the newest TheRock ROCm SDK version that has a matching PyTorch stack in the same index, then install pinned rocm[libraries,devel], torch, torchvision, and torchaudio versions in one uv transaction" + " package_policy: find the newest TheRock ROCm SDK version that has a matching PyTorch stack in the same index, then install pinned rocm[libraries,devel,device-all], torch, torchvision, and torchaudio versions in one uv transaction" ); if dry_run { let env_python = venv_python_path(&install_root); @@ -1077,7 +1077,10 @@ fn install_wheel_runtime( fn therock_pip_package_specs(package_versions: &TheRockPipPackageVersions) -> Vec { vec![ - format!("rocm[libraries,devel]=={}", package_versions.rocm), + format!( + "rocm[libraries,devel,device-all]=={}", + package_versions.rocm + ), format!("torch=={}", package_versions.torch), format!("torchvision=={}", package_versions.torchvision), format!("torchaudio=={}", package_versions.torchaudio), @@ -1301,7 +1304,7 @@ fn resolve_pip_runtime_from_index( .with_context(|| { let requested = version_selector.map_or_else(|| "latest compatible version".to_owned(), RuntimeVersionSelector::describe); format!( - "no mutually compatible TheRock rocm[libraries,devel], torch, torchvision, and torchaudio versions were found for {requested} in {index_url}" + "no mutually compatible TheRock rocm[libraries,devel,device-all], torch, torchvision, and torchaudio versions were found for {requested} in {index_url}" ) })?; let latest_version = package_versions.rocm.clone(); @@ -4582,7 +4585,7 @@ mod tests { assert_eq!( package_specs, vec![ - "rocm[libraries,devel]==7.13.0a20260513".to_owned(), + "rocm[libraries,devel,device-all]==7.13.0a20260513".to_owned(), "torch==2.10.0+rocm7.13.0a20260513".to_owned(), "torchvision==0.25.0+rocm7.13.0a20260513".to_owned(), "torchaudio==2.10.0+rocm7.13.0a20260513".to_owned(), From dd471da3bc10f700c0e0648310512b909d8e4ea7 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Tue, 25 Aug 2026 19:09:30 -0700 Subject: [PATCH 05/14] fix(install): select detected aggregate device Signed-off-by: Michael Roy (cherry picked from commit 9f7f36dbf4e8f25d0386b5169328d59f40104c7d) Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 126 +++++++++++++++++++++++++++++---------- 1 file changed, 93 insertions(+), 33 deletions(-) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 7790745c6..14400ae69 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -4,14 +4,14 @@ use anyhow::{Context, Result, bail}; use rocm_core::{ - AppPaths, ManagedToolConfig, RocmCliConfig, detect_host_gpu_diagnostics, - detect_host_therock_family, detect_managed_therock_family, disk_space, ensure_uv_binary, - known_therock_families, managed_tools_dir, normalize_runtime_path_for_host, - normalize_runtime_path_for_storage, normalize_runtime_path_text_for_host, - normalize_runtime_path_text_for_storage, normalize_therock_family, runtime_is_windows, - runtime_os_name, runtime_path_for_windows_child, runtime_path_list_split, - runtime_python_executable_in_env, unix_time_millis, uv_command_env, uv_pip_install_base, - uv_venv_args, verify_rsa_pkcs1_sha256_signature, + AppPaths, ManagedToolConfig, RocmCliConfig, detect_host_gfx_target, + detect_host_gpu_diagnostics, detect_host_therock_family, detect_managed_therock_family, + disk_space, ensure_uv_binary, known_therock_families, managed_tools_dir, + normalize_runtime_path_for_host, normalize_runtime_path_for_storage, + normalize_runtime_path_text_for_host, normalize_runtime_path_text_for_storage, + normalize_therock_family, runtime_is_windows, runtime_os_name, runtime_path_for_windows_child, + runtime_path_list_split, runtime_python_executable_in_env, unix_time_millis, uv_command_env, + uv_pip_install_base, uv_venv_args, verify_rsa_pkcs1_sha256_signature, }; #[cfg(test)] use rocm_core::{ @@ -878,6 +878,11 @@ fn install_wheel_runtime( &wheel_compatibility, version_selector, )?; + let device_target = aggregate_device_target(&resolution.family); + let rocm_extras = device_target.as_deref().map_or_else( + || "libraries,devel,device-all".to_owned(), + |target| format!("libraries,devel,device-{target}"), + ); progress_line(format!( "Found canonical TheRock aggregate version {} with a matching PyTorch stack for target family {}.", resolution.latest_version, resolution.family @@ -942,11 +947,11 @@ fn install_wheel_runtime( let _ = writeln!( output, " package_specs: {}", - therock_pip_package_specs(&resolution.package_versions).join(" ") + therock_pip_package_specs(&resolution.package_versions, &rocm_extras).join(" ") ); let _ = writeln!( output, - " package_policy: find the newest TheRock ROCm SDK version that has a matching PyTorch stack in the same index, then install pinned rocm[libraries,devel,device-all], torch, torchvision, and torchaudio versions in one uv transaction" + " package_policy: find the newest TheRock ROCm SDK version that has a matching PyTorch stack in the same index, then install pinned target-complete rocm, torch, torchvision, and torchaudio versions in one uv transaction" ); if dry_run { let env_python = venv_python_path(&install_root); @@ -955,7 +960,10 @@ fn install_wheel_runtime( if matches!(channel, TheRockChannel::Nightly) { install_args.extend(["--prerelease".to_owned(), "allow".to_owned()]); } - install_args.extend(therock_pip_package_specs(&resolution.package_versions)); + install_args.extend(therock_pip_package_specs( + &resolution.package_versions, + &rocm_extras, + )); let venv_args = uv_venv_args(&python_launcher.executable, &install_root); let venv_args_display = venv_args .iter() @@ -995,7 +1003,7 @@ fn install_wheel_runtime( progress_line(format!( "Installing {} from {}", - therock_pip_package_specs(&resolution.package_versions).join(" "), + therock_pip_package_specs(&resolution.package_versions, &rocm_extras).join(" "), resolution.index_url )); let mut install_args = uv_pip_install_base(&env_python); @@ -1003,7 +1011,10 @@ fn install_wheel_runtime( if matches!(channel, TheRockChannel::Nightly) { install_args.extend(["--prerelease".to_owned(), "allow".to_owned()]); } - install_args.extend(therock_pip_package_specs(&resolution.package_versions)); + install_args.extend(therock_pip_package_specs( + &resolution.package_versions, + &rocm_extras, + )); run_uv_progress_command( paths, &uv, @@ -1016,7 +1027,7 @@ fn install_wheel_runtime( )?; progress_line("Checking the installed ROCm SDK..."); - let rocm_sdk_probe = probe_rocm_sdk_runtime(&env_python) + let rocm_sdk_probe = probe_rocm_sdk_runtime_for_target(&env_python, device_target.as_deref()) .context("TheRock packages did not expose a usable rocm_sdk runtime")?; validate_rocm_sdk_runtime_probe(&rocm_sdk_probe)?; let installed_version = rocm_sdk_probe @@ -1075,18 +1086,39 @@ fn install_wheel_runtime( Ok(output) } -fn therock_pip_package_specs(package_versions: &TheRockPipPackageVersions) -> Vec { +fn therock_pip_package_specs( + package_versions: &TheRockPipPackageVersions, + rocm_extras: &str, +) -> Vec { vec![ - format!( - "rocm[libraries,devel,device-all]=={}", - package_versions.rocm - ), + format!("rocm[{rocm_extras}]=={}", package_versions.rocm), format!("torch=={}", package_versions.torch), format!("torchvision=={}", package_versions.torchvision), format!("torchaudio=={}", package_versions.torchaudio), ] } +fn aggregate_device_target(family: &str) -> Option { + let detected = detect_host_gfx_target()?; + canonical_aggregate_device_target(&detected, family) +} + +fn canonical_aggregate_device_target(detected: &str, family: &str) -> Option { + if normalize_therock_family(detected).as_deref() != Some(family) { + return None; + } + let target = detected + .split(':') + .next() + .unwrap_or(detected) + .to_ascii_lowercase(); + if target.starts_with("gfx94") { + Some("gfx942".to_owned()) + } else { + Some(target) + } +} + fn quote_display_arg(value: &str) -> String { if value.is_empty() || value @@ -2715,17 +2747,28 @@ fn python_venv_args(install_root: &Path) -> Vec { } pub(crate) fn probe_rocm_sdk_runtime(python_executable: &Path) -> Result { - let text = capture_python_stdout( - python_executable, - ROCM_SDK_PROBE_SCRIPT, - "launch rocm_sdk probe", - ) - .with_context(|| { - format!( - "failed to launch rocm_sdk probe via {}", - python_executable.display() - ) - })?; + probe_rocm_sdk_runtime_for_target(python_executable, None) +} + +fn probe_rocm_sdk_runtime_for_target( + python_executable: &Path, + device_target: Option<&str>, +) -> Result { + let script = device_target.map_or_else( + || ROCM_SDK_PROBE_SCRIPT.to_owned(), + |target| { + format!( + "import os\nos.environ['ROCM_SDK_TARGET_FAMILY'] = {target:?}\n{ROCM_SDK_PROBE_SCRIPT}" + ) + }, + ); + let text = capture_python_stdout(python_executable, &script, "launch rocm_sdk probe") + .with_context(|| { + format!( + "failed to launch rocm_sdk probe via {}", + python_executable.display() + ) + })?; parse_rocm_sdk_probe(&text) } @@ -4562,6 +4605,22 @@ mod tests { assert!(output.contains("source_layout_generation: multi-arch-v2")); } + #[test] + fn aggregate_device_target_maps_mi300_gfx_alias_to_published_package() { + assert_eq!( + canonical_aggregate_device_target("gfx943", "gfx94X-dcgpu").as_deref(), + Some("gfx942") + ); + assert_eq!( + canonical_aggregate_device_target("gfx1151", "gfx1151").as_deref(), + Some("gfx1151") + ); + assert_eq!( + canonical_aggregate_device_target("gfx1151", "gfx120X-all"), + None + ); + } + #[test] fn unknown_aggregate_layout_is_rejected_clearly() { let error = @@ -4572,7 +4631,7 @@ mod tests { } #[test] - fn pip_runtime_installs_pinned_devel_and_torch_stack_from_therock_index() { + fn pip_runtime_installs_pinned_target_complete_stack_from_aggregate_index() { let package_versions = TheRockPipPackageVersions { rocm: "7.13.0a20260513".to_owned(), torch: "2.10.0+rocm7.13.0a20260513".to_owned(), @@ -4580,12 +4639,13 @@ mod tests { torchaudio: "2.10.0+rocm7.13.0a20260513".to_owned(), compatibility_key: "7.13.0a20260513".to_owned(), }; - let package_specs = therock_pip_package_specs(&package_versions); + let package_specs = + therock_pip_package_specs(&package_versions, "libraries,devel,device-gfx942"); assert_eq!( package_specs, vec![ - "rocm[libraries,devel,device-all]==7.13.0a20260513".to_owned(), + "rocm[libraries,devel,device-gfx942]==7.13.0a20260513".to_owned(), "torch==2.10.0+rocm7.13.0a20260513".to_owned(), "torchvision==0.25.0+rocm7.13.0a20260513".to_owned(), "torchaudio==2.10.0+rocm7.13.0a20260513".to_owned(), From 518a432e2b43d5fb30da4cf27d1816970219c5b6 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Tue, 8 Sep 2026 14:01:48 -0700 Subject: [PATCH 06/14] fix(install): require exact canonical device payloads Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 396 ++++++++++++++---- docs/manual-testing.md | 9 +- docs/testing.md | 8 +- scripts/therock_sdk_install_test.py | 11 +- .../features/runtime_setup.feature | 41 +- tests/e2e-cucumber/tests/e2e/examine_steps.rs | 6 +- tests/e2e-cucumber/tests/e2e/runtime_steps.rs | 196 ++++++++- 7 files changed, 576 insertions(+), 91 deletions(-) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 14400ae69..263a1b530 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -6,8 +6,8 @@ use anyhow::{Context, Result, bail}; use rocm_core::{ AppPaths, ManagedToolConfig, RocmCliConfig, detect_host_gfx_target, detect_host_gpu_diagnostics, detect_host_therock_family, detect_managed_therock_family, - disk_space, ensure_uv_binary, known_therock_families, managed_tools_dir, - normalize_runtime_path_for_host, normalize_runtime_path_for_storage, + disk_space, ensure_uv_binary, extract_first_gfx_token, known_therock_families, + managed_tools_dir, normalize_runtime_path_for_host, normalize_runtime_path_for_storage, normalize_runtime_path_text_for_host, normalize_runtime_path_text_for_storage, normalize_therock_family, runtime_is_windows, runtime_os_name, runtime_path_for_windows_child, runtime_path_list_split, runtime_python_executable_in_env, unix_time_millis, uv_command_env, @@ -155,17 +155,156 @@ fn render_canonical_provenance( let _ = writeln!(output, " source_layout_generation: {layout_generation}"); } +/// The package names a canonical aggregate index links to, lowercased. +/// +/// The simple index publishes one `` per package. Matching on +/// parsed names rather than on raw substrings keeps `rocm` from being satisfied +/// by `rocm-sdk-core` and `torch` from being satisfied by +/// `amd-torch-device-gfx1100`. +fn parse_aggregate_package_links(html: &str) -> Vec { + let mut names = Vec::new(); + for tail in html.split("href=\"").skip(1) { + let Some(href) = tail.split('"').next() else { + continue; + }; + let name = href.trim().trim_matches('/').to_ascii_lowercase(); + if name.is_empty() || name.contains('/') { + continue; + } + if !names.contains(&name) { + names.push(name); + } + } + names +} + +/// The exact GFX targets the canonical source publishes a device payload for. +/// +/// This is the authoritative list rather than a table in this file: the +/// aggregate `rocm` distribution declares one `device-` extra per +/// `rocm-sdk-device-` package the index links, and `rocm_sdk` refuses +/// any target outside that set. Reading it from the stream means a target the +/// source adds or withdraws needs no CLI change, and a target it never +/// published cannot be requested by accident. +fn parse_aggregate_device_targets(html: &str) -> Vec { + let mut targets = parse_aggregate_package_links(html) + .iter() + .filter_map(|name| name.strip_prefix("rocm-sdk-device-")) + .map(str::to_owned) + .collect::>(); + targets.sort(); + targets +} + fn validate_aggregate_index_layout(html: &str) -> Result<()> { - let required_packages = ["rocm/", "torch/", "torchvision/", "torchaudio/"]; - if required_packages + let names = parse_aggregate_package_links(html); + let missing = ["rocm", "torch", "torchvision", "torchaudio"] + .into_iter() + .filter(|package| !names.iter().any(|name| name == package)) + .collect::>(); + if !missing.is_empty() { + bail!( + "unknown canonical TheRock aggregate index layout: expected package links for rocm, torch, torchvision, and torchaudio, but {} not published", + missing.join(", ") + ); + } + if !names .iter() - .all(|package| html.contains(package)) + .any(|name| name.starts_with("rocm-sdk-device-")) { - return Ok(()); + bail!( + "unknown canonical TheRock aggregate index layout: no `rocm-sdk-device-*` payload packages are published, so no GPU backend could be selected" + ); + } + Ok(()) +} + +/// Which device payload the canonical aggregate source must supply for this host. +/// +/// The aggregate `rocm` distribution ships no GPU backend unless a `device-*` +/// extra asks for one, so this choice decides whether the installed runtime can +/// launch a kernel at all. Exactly one exact target is ever requested. The +/// blanket `device-all` extra pulls every published payload — measured at 24 +/// wheels and 4451 MiB on an MI300X that needs one of them — and then leaves +/// `rocm_sdk` to guess a target family out of that pile; a family-bucket alias +/// such as `device-gfx120X-all` is not an extra the source declares at all. +/// +/// When no exact target can be pinned the answer is [`Undetermined`] rather +/// than a fallback: a preview still renders and says so, and a real install +/// refuses instead of producing a runtime with no kernels. +/// +/// [`Undetermined`]: AggregateDeviceTarget::Undetermined +#[derive(Debug, Clone, Eq, PartialEq)] +enum AggregateDeviceTarget { + /// The exact detected GFX target, published by the canonical source. + Exact(String), + /// No exact target could be pinned, and why. + Undetermined(String), +} + +#[derive(Debug, Clone, Eq, PartialEq)] +struct ResolvedAggregateWheelSource { + index_url: &'static str, + device_target: AggregateDeviceTarget, +} + +/// Stands in for a real target in a preview, so a plan that cannot be installed +/// yet reads as incomplete rather than as installable. +const UNDETERMINED_DEVICE_TARGET: &str = ""; + +impl AggregateDeviceTarget { + fn resolve(detected: Option<&str>, family: &str, published: &[String]) -> Self { + let Some(detected) = detected else { + return Self::Undetermined("no AMD GPU target was detected on this host".to_owned()); + }; + // KFD reports feature suffixes (`gfx90a:sramecc+:xnack-`); published + // targets never carry them. + let Some(target) = extract_first_gfx_token(detected) else { + return Self::Undetermined(format!( + "detected GPU target `{detected}` is not a recognizable GFX target" + )); + }; + match normalize_therock_family(&target) { + Some(detected_family) if detected_family == family => {} + Some(detected_family) => { + return Self::Undetermined(format!( + "detected GPU target `{target}` belongs to family `{detected_family}`, not the resolved target family `{family}`" + )); + } + None => { + return Self::Undetermined(format!( + "detected GPU target `{target}` belongs to no recognized package family" + )); + } + } + if !published.iter().any(|candidate| candidate == &target) { + return Self::Undetermined(format!( + "the canonical source publishes no `device-{target}` payload for detected GPU target `{target}` (published targets: {})", + published.join(", ") + )); + } + Self::Exact(target) + } + + fn as_str(&self) -> &str { + match self { + Self::Exact(target) => target, + Self::Undetermined(_) => UNDETERMINED_DEVICE_TARGET, + } + } + + /// The `rocm` extras this target implies. Always three extras, so a plan + /// never reads as though the GPU backend were optional. + fn rocm_extras(&self) -> String { + format!("libraries,devel,device-{}", self.as_str()) + } + + fn reason(&self) -> Option<&str> { + match self { + Self::Exact(_) => None, + Self::Undetermined(reason) => Some(reason), + } } - bail!( - "unknown canonical TheRock aggregate index layout: expected package links for rocm, torch, torchvision, and torchaudio" - ) } #[derive(Debug, Clone)] @@ -181,6 +320,9 @@ struct PipRuntimeResolution { index_url: String, latest_version: String, package_versions: TheRockPipPackageVersions, + /// The device payload the canonical source must supply for this host, + /// decided against the targets that source actually publishes. + device_target: AggregateDeviceTarget, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -878,17 +1020,13 @@ fn install_wheel_runtime( &wheel_compatibility, version_selector, )?; - let device_target = aggregate_device_target(&resolution.family); - let rocm_extras = device_target.as_deref().map_or_else( - || "libraries,devel,device-all".to_owned(), - |target| format!("libraries,devel,device-{target}"), - ); + let rocm_extras = resolution.device_target.rocm_extras(); progress_line(format!( "Found canonical TheRock aggregate version {} with a matching PyTorch stack for target family {}.", resolution.latest_version, resolution.family )); let runtime_key = wheel_runtime_key(channel, &resolution.latest_version); - let install_root = prefix.unwrap_or_else(|| managed_runtime_root(paths, "wheel", &runtime_key)); + let install_root = resolved_install_root(paths, "wheel", &runtime_key, prefix); let manifest_path = runtime_manifest_path(paths, &runtime_key); let mut output = String::new(); @@ -915,6 +1053,14 @@ fn install_wheel_runtime( " target_family_source: {}", resolution.family_source ); + let _ = writeln!( + output, + " device_target: {}", + resolution.device_target.as_str() + ); + if let Some(reason) = resolution.device_target.reason() { + let _ = writeln!(output, " device_target_reason: {reason}"); + } let _ = writeln!(output, " index_url: {}", resolution.index_url); let _ = writeln!( output, @@ -988,6 +1134,20 @@ fn install_wheel_runtime( return Ok(output); } + // Past the preview, the plan has to be installable. A runtime composed + // without its exact device payload loads and then faults on the first + // kernel, so an undetermined target is refused here rather than papered + // over with every published payload. + if let Some(reason) = resolution.device_target.reason() { + bail!( + "cannot compose a canonical TheRock {} runtime: {reason}.\n\ + The aggregate `rocm` distribution ships no GPU backend unless an exact `device-` extra requests one, so this install would produce a runtime that cannot run a kernel.\n\ + Re-run `rocm install sdk` on the target host, or preview the plan with `--dry-run`.\n\n{}", + channel.as_str(), + detect_host_gpu_diagnostics() + ); + } + let uv = ensure_uv_binary(paths)?; fs::create_dir_all( install_root @@ -1027,7 +1187,7 @@ fn install_wheel_runtime( )?; progress_line("Checking the installed ROCm SDK..."); - let rocm_sdk_probe = probe_rocm_sdk_runtime_for_target(&env_python, device_target.as_deref()) + let rocm_sdk_probe = probe_rocm_sdk_runtime(&env_python) .context("TheRock packages did not expose a usable rocm_sdk runtime")?; validate_rocm_sdk_runtime_probe(&rocm_sdk_probe)?; let installed_version = rocm_sdk_probe @@ -1098,27 +1258,6 @@ fn therock_pip_package_specs( ] } -fn aggregate_device_target(family: &str) -> Option { - let detected = detect_host_gfx_target()?; - canonical_aggregate_device_target(&detected, family) -} - -fn canonical_aggregate_device_target(detected: &str, family: &str) -> Option { - if normalize_therock_family(detected).as_deref() != Some(family) { - return None; - } - let target = detected - .split(':') - .next() - .unwrap_or(detected) - .to_ascii_lowercase(); - if target.starts_with("gfx94") { - Some("gfx942".to_owned()) - } else { - Some(target) - } -} - fn quote_display_arg(value: &str) -> String { if value.is_empty() || value @@ -1264,11 +1403,19 @@ fn resolve_pip_runtime_with_timeout( source.wheel_index ) })?; + let source = ResolvedAggregateWheelSource { + index_url: source.wheel_index, + device_target: AggregateDeviceTarget::resolve( + detect_host_gfx_target().as_deref(), + &family_resolution.family, + &parse_aggregate_device_targets(&root_html), + ), + }; resolve_pip_runtime_from_index( paths, channel, &family_resolution, - source.wheel_index, + &source, wheel_compatibility, version_selector, download_timeout_secs, @@ -1277,7 +1424,7 @@ fn resolve_pip_runtime_with_timeout( format!( "failed to resolve TheRock {} wheel runtime from canonical source {}\n\n{}", channel.as_str(), - source.wheel_index, + source.index_url, canonical_wheel_resolution_hint(channel) ) }) @@ -1287,11 +1434,12 @@ fn resolve_pip_runtime_from_index( paths: &AppPaths, channel: TheRockChannel, family_resolution: &FamilyResolution, - index_url: &str, + source: &ResolvedAggregateWheelSource, wheel_compatibility: &WheelCompatibility, version_selector: Option<&RuntimeVersionSelector>, download_timeout_secs: Option, ) -> Result { + let index_url = source.index_url; let rocm_versions = load_simple_index_versions(paths, index_url, "rocm", None, download_timeout_secs)?; if matches!(channel, TheRockChannel::Release) @@ -1336,7 +1484,7 @@ fn resolve_pip_runtime_from_index( .with_context(|| { let requested = version_selector.map_or_else(|| "latest compatible version".to_owned(), RuntimeVersionSelector::describe); format!( - "no mutually compatible TheRock rocm[libraries,devel,device-all], torch, torchvision, and torchaudio versions were found for {requested} in {index_url}" + "no mutually compatible TheRock rocm, torch, torchvision, and torchaudio versions were found for {requested} in {index_url}" ) })?; let latest_version = package_versions.rocm.clone(); @@ -1346,6 +1494,7 @@ fn resolve_pip_runtime_from_index( index_url: index_url.to_owned(), latest_version, package_versions, + device_target: source.device_target.clone(), }) } @@ -2746,29 +2895,25 @@ fn python_venv_args(install_root: &Path) -> Vec { ] } +/// What `rocm_sdk` reports about the runtime that was just installed. +/// +/// Deliberately does not set `ROCM_SDK_TARGET_FAMILY`: forcing the target the +/// installer chose would make `resolved_target_family` echo that choice back +/// instead of reporting what the environment actually composed, which is the +/// one signal that catches a runtime whose device payload does not match its +/// host. pub(crate) fn probe_rocm_sdk_runtime(python_executable: &Path) -> Result { - probe_rocm_sdk_runtime_for_target(python_executable, None) -} - -fn probe_rocm_sdk_runtime_for_target( - python_executable: &Path, - device_target: Option<&str>, -) -> Result { - let script = device_target.map_or_else( - || ROCM_SDK_PROBE_SCRIPT.to_owned(), - |target| { - format!( - "import os\nos.environ['ROCM_SDK_TARGET_FAMILY'] = {target:?}\n{ROCM_SDK_PROBE_SCRIPT}" - ) - }, - ); - let text = capture_python_stdout(python_executable, &script, "launch rocm_sdk probe") - .with_context(|| { - format!( - "failed to launch rocm_sdk probe via {}", - python_executable.display() - ) - })?; + let text = capture_python_stdout( + python_executable, + ROCM_SDK_PROBE_SCRIPT, + "launch rocm_sdk probe", + ) + .with_context(|| { + format!( + "failed to launch rocm_sdk probe via {}", + python_executable.display() + ) + })?; parse_rocm_sdk_probe(&text) } @@ -4605,22 +4750,126 @@ mod tests { assert!(output.contains("source_layout_generation: multi-arch-v2")); } + /// A representative slice of what the canonical aggregate index publishes. + /// Notably it has no `gfx943`: MI300 steppings other than `gfx942` have no + /// payload of their own, which is exactly the case a remap used to hide. + const PUBLISHED_DEVICE_TARGETS_HTML: &str = r#" +rocm
+torch
+torchvision
+torchaudio
+rocm-sdk-core
+amd-torch-device-gfx942
+rocm-sdk-device-gfx90a
+rocm-sdk-device-gfx942
+rocm-sdk-device-gfx1151
+rocm-sdk-device-gfx1201
+"#; + + fn published_device_targets() -> Vec { + parse_aggregate_device_targets(PUBLISHED_DEVICE_TARGETS_HTML) + } + #[test] - fn aggregate_device_target_maps_mi300_gfx_alias_to_published_package() { + fn published_device_targets_come_from_the_sdk_payload_packages_only() { + // `amd-torch-device-*` and `rocm-sdk-core` share the page; only the + // `rocm-sdk-device-*` names name a `device-` extra of `rocm`. assert_eq!( - canonical_aggregate_device_target("gfx943", "gfx94X-dcgpu").as_deref(), - Some("gfx942") + published_device_targets(), + vec![ + "gfx1151".to_owned(), + "gfx1201".to_owned(), + "gfx90a".to_owned(), + "gfx942".to_owned(), + ] + ); + } + + #[test] + fn device_extra_is_the_exact_detected_target_the_source_publishes() { + let target = AggregateDeviceTarget::resolve( + Some("gfx1201"), + "gfx120X-all", + &published_device_targets(), ); + + assert_eq!(target, AggregateDeviceTarget::Exact("gfx1201".to_owned())); + assert_eq!(target.rocm_extras(), "libraries,devel,device-gfx1201"); + } + + #[test] + fn device_extra_drops_the_kfd_feature_suffix() { assert_eq!( - canonical_aggregate_device_target("gfx1151", "gfx1151").as_deref(), - Some("gfx1151") + AggregateDeviceTarget::resolve( + Some("gfx90a:sramecc+:xnack-"), + "gfx90a", + &published_device_targets(), + ), + AggregateDeviceTarget::Exact("gfx90a".to_owned()) + ); + } + + #[test] + fn unpublished_detected_target_is_undetermined_rather_than_remapped() { + // gfx943 normalizes to the same family as gfx942, so a family-level + // answer would silently install gfx942 kernels on a chip the source + // never published a payload for. + let target = AggregateDeviceTarget::resolve( + Some("gfx943"), + "gfx94X-dcgpu", + &published_device_targets(), ); + + assert_eq!(target.as_str(), ""); + assert!( + target + .reason() + .is_some_and(|reason| reason.contains("no `device-gfx943` payload")) + ); + } + + #[test] + fn no_detected_gpu_yields_an_undetermined_target_not_a_blanket_payload() { + let target = + AggregateDeviceTarget::resolve(None, "gfx110X-all", &published_device_targets()); + assert_eq!( - canonical_aggregate_device_target("gfx1151", "gfx120X-all"), - None + target.rocm_extras(), + "libraries,devel,device-" + ); + assert!( + target + .reason() + .is_some_and(|reason| reason.contains("no AMD GPU target was detected")) + ); + } + + #[test] + fn a_detected_target_from_another_family_is_undetermined() { + let target = AggregateDeviceTarget::resolve( + Some("gfx1151"), + "gfx120X-all", + &published_device_targets(), + ); + + assert!( + target + .reason() + .is_some_and(|reason| reason.contains("not the resolved target family")) ); } + #[test] + fn aggregate_layout_without_device_payloads_is_rejected() { + let error = validate_aggregate_index_layout( + r#"rocmtorchtorchvisiontorchaudio"#, + ) + .unwrap_err() + .to_string(); + + assert!(error.contains("no `rocm-sdk-device-*` payload packages")); + } + #[test] fn unknown_aggregate_layout_is_rejected_clearly() { let error = @@ -4628,6 +4877,13 @@ mod tests { .unwrap_err() .to_string(); assert!(error.contains("unknown canonical TheRock aggregate index layout")); + // `rocm-sdk-core/` must not satisfy the `rocm` requirement. + let error = validate_aggregate_index_layout( + r#"rocm-sdk-coretorch"#, + ) + .unwrap_err() + .to_string(); + assert!(error.contains("rocm, torchvision, torchaudio not published")); } #[test] diff --git a/docs/manual-testing.md b/docs/manual-testing.md index 4add0a28d..59f7809f7 100644 --- a/docs/manual-testing.md +++ b/docs/manual-testing.md @@ -123,15 +123,18 @@ Omit `--prefix` if you want rocm-cli to choose its standard managed folder. Expected result: - rocm-cli creates or reuses a rocm-cli managed Python venv. -- pip installs pinned `rocm[libraries,devel]`, `torch`, `torchvision`, and - `torchaudio` versions from the TheRock index. +- pip installs a pinned `rocm` with the `libraries` and `devel` extras plus + exactly one `device-` extra, alongside pinned `torch`, + `torchvision`, and `torchaudio` versions from the TheRock index. On a host + with no detectable AMD GPU the preview reports `device_target: undetermined` + and a real install refuses rather than pulling every published device payload. - rocm-cli chooses the newest exact ROCm build suffix common to the SDK package and the PyTorch stack for the current Python/platform wheel tags, then pins all four packages in one pip transaction. - The install does not ask for an external Python venv. - Runtime validation uses TheRock's runtime/devel package roots and `rocm_sdk.find_libraries`; `rocm-sdk path --root` is expected after the - pinned `rocm[libraries,devel]` install succeeds. + pinned `rocm[libraries,devel,device-…]` install succeeds. - `rocm examine` reports the active runtime as ready. Developer-only deterministic override: diff --git a/docs/testing.md b/docs/testing.md index 3dc8256db..0c636d5a5 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -164,8 +164,12 @@ Then it verifies: explicit `--prefix` folders - the installer does not pre-create that pip cache during dry-run or setup; pip creates it inside the ROCm folder when packages are downloaded -- a single TheRock-index pip install plan for pinned `rocm[libraries,devel]`, - `torch`, `torchvision`, and `torchaudio` versions +- a single TheRock-index pip install plan for pinned `rocm` — with the + `libraries` and `devel` extras plus exactly one `device-` + extra — and pinned `torch`, `torchvision`, and `torchaudio` versions +- on a host with no detectable AMD GPU the preview reports `device_target: + undetermined` and renders the device extra as a placeholder; a real install + refuses rather than falling back to every published device payload - package selection uses the newest exact ROCm build suffix common to the SDK package and the PyTorch stack for the current Python/platform wheel tags - `python -m rocm_sdk version` diff --git a/scripts/therock_sdk_install_test.py b/scripts/therock_sdk_install_test.py index 5e0db569b..50e4b8af0 100644 --- a/scripts/therock_sdk_install_test.py +++ b/scripts/therock_sdk_install_test.py @@ -26,7 +26,10 @@ from pathlib import Path from typing import Any -THEROCK_SDK_PACKAGE_SPEC = "rocm[libraries,devel]" +# A prefix, not the whole spec: the canonical index always appends a third extra +# naming the device payload (`rocm[libraries,devel,device-gfx1201]`), so the +# closing bracket is no longer in a fixed place. +THEROCK_SDK_PACKAGE_SPEC = "rocm[libraries,devel" THEROCK_TORCH_PACKAGES = ["torch", "torchvision", "torchaudio"] THEROCK_RUNTIME_PACKAGES = ["rocm", "rocm-sdk-core"] @@ -496,10 +499,14 @@ def main() -> int: assert_contains(install_output, "python_wheel_tag:", "sdk install") assert_contains(install_output, "platform_wheel_tags:", "sdk install") assert_contains(install_output, "package_specs:", "sdk install") - assert_contains(install_output, f"{THEROCK_SDK_PACKAGE_SPEC}==", "sdk install") + assert_contains(install_output, THEROCK_SDK_PACKAGE_SPEC, "sdk install") for package in THEROCK_TORCH_PACKAGES: assert_contains(install_output, f"{package}==", "sdk install") assert_not_contains(install_output, "rocm[devel]", "sdk install") + # The canonical source never installs every published device payload: a host + # whose chip it cannot name fails closed instead. Downloading ~4.3 GiB of + # device wheels to use one of them is the regression this forbids. + assert_not_contains(install_output, "device-all", "sdk install") if args.dry_run: assert_contains(install_output, "mode: dry-run", "sdk dry-run") dry_run_target = install_output_field(install_output, "target") diff --git a/tests/e2e-cucumber/features/runtime_setup.feature b/tests/e2e-cucumber/features/runtime_setup.feature index b61644042..2cf14c54a 100644 --- a/tests/e2e-cucumber/features/runtime_setup.feature +++ b/tests/e2e-cucumber/features/runtime_setup.feature @@ -132,7 +132,46 @@ Feature: Runtime configuration Then the adoption is refused And the error explains which install types can be adopted + # Both channels now resolve from one canonical aggregate index each, so the + # provenance the preview prints is the whole of what the user can check before + # committing to a multi-GiB install. This pins the nightly half: the source that + # was selected, the version that came back from it, and the layout generation + # that source was read as. `--family` is supplied so the scenario needs no GPU, + # and `@nightly` because it resolves the real index over the network — the same + # cost that keeps scenario runtime-06 off the unserialized mock lane. @id:runtime-resolve-canonical-nightly @nightly - Scenario: 5 - Previewing a nightly SDK install reports canonical provenance + Scenario: runtime-08 - Previewing a nightly SDK install reports canonical provenance When the user dry-runs a nightly SDK install for a known family Then the SDK preview reports canonical nightly provenance + + # The release channel is where this went wrong in the field (rocm-cli#271). The + # resolver read a per-family index, `repo.amd.com/rocm/whl/{family}`, which is + # frozen at 7.13.0 and has no device payloads at all, so every release install + # got a stale SDK and a bare `rocm[libraries,devel]` — no GPU backend in it. + # Both halves are fixed by the same canonical model: one flat aggregate index + # for the channel, and the device payload for the chip this host actually has. + # + # The obvious regression test for that history is the one that does not work. + # Asserting the broken per-family URL is *absent* passes with the bug present: + # the old resolver only ever printed a candidate it failed on, and the stale + # per-family index succeeded, so the URL never reached stdout either way. These + # Thens assert the positives instead — which source was selected, which version + # came back from it, which device payload the plan asks for. On the old resolver + # the first reports a per-family URL and the second finds no device payload. + # + # The two Thens are not one claim split in half. A preview can name the right + # source and still ask for the wrong payload — that is exactly what the earlier + # attempt at this fix did on Instinct hosts, resolving the aggregate correctly + # and then requesting every device wheel ROCm publishes. Only the second Then + # separates them. + # + # `@requires-gpu` because a device payload can only be exact about a chip that + # is there; it also keeps this off the 64-way mock lane whose concurrency forced + # runtime-06 to `@nightly`, since GPU lanes run one scenario at a time. + # `--dry-run` keeps it to a plan: no venv, no download. + @id:runtime-resolve-canonical-release @requires-gpu + Scenario: runtime-09 - Previewing a release SDK install resolves the canonical aggregate for this GPU + Given a machine with an AMD GPU + When the user dry-runs a release SDK install for this host + Then the SDK preview reports canonical release provenance + And the SDK preview requests the device payload for this host's GPU diff --git a/tests/e2e-cucumber/tests/e2e/examine_steps.rs b/tests/e2e-cucumber/tests/e2e/examine_steps.rs index 95e1dd2a0..fd5b1164b 100644 --- a/tests/e2e-cucumber/tests/e2e/examine_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/examine_steps.rs @@ -6,7 +6,11 @@ use cucumber::{given, then, when}; use crate::E2eWorld; -fn field_value<'a>(output: &'a str, field: &str) -> Option<&'a str> { +/// The value of a ` : ` line in a `rocm` command's plain output. +/// +/// Shared with `runtime_steps`, which reads the same shape out of the `install +/// sdk` preview. +pub(crate) fn field_value<'a>(output: &'a str, field: &str) -> Option<&'a str> { output.lines().find_map(|line| { let (name, value) = line.trim().split_once(':')?; (name == field).then(|| value.trim()) diff --git a/tests/e2e-cucumber/tests/e2e/runtime_steps.rs b/tests/e2e-cucumber/tests/e2e/runtime_steps.rs index 4657814f8..6cf3ee9ee 100644 --- a/tests/e2e-cucumber/tests/e2e/runtime_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/runtime_steps.rs @@ -426,21 +426,193 @@ async fn user_dry_runs_nightly_sdk(world: &mut E2eWorld) { world.cli_output = Some(stdout); } +/// The value of a ` : ` line in the stored `install sdk` preview. +/// +/// The preview is the whole of what a user can check before committing to a +/// multi-GiB install, so a key that is not there is a failure carrying the +/// output rather than a silent `None`. +fn preview_field<'a>(output: &'a str, key: &str) -> &'a str { + super::examine_steps::field_value(output, key) + .unwrap_or_else(|| panic!("the SDK preview has no `{key}` line:\n{output}")) +} + +/// The `rocm[...]==` requirement from the preview's `package_specs` +/// line, which is always the first of the four pinned packages. +fn preview_rocm_spec(output: &str) -> &str { + preview_field(output, "package_specs") + .split_whitespace() + .next() + .unwrap_or_else(|| panic!("the SDK preview plans to install nothing:\n{output}")) +} + +/// The extras inside a `rocm[...]==` requirement. +fn requested_rocm_extras(rocm_spec: &str) -> &str { + rocm_spec + .split_once('[') + .and_then(|(_, rest)| rest.split_once(']')) + .map_or_else( + || panic!("the SDK preview requests no extras at all: {rocm_spec}"), + |(extras, _)| extras, + ) +} + #[then("the SDK preview reports canonical nightly provenance")] async fn assert_canonical_nightly_provenance(world: &mut E2eWorld) { + const AGGREGATE: &str = "https://rocm.nightlies.amd.com/whl-multi-arch"; let output = world.cli_output.as_deref().expect("no SDK preview output"); - for expected in [ - "channel: nightly", - "canonical_source: https://rocm.nightlies.amd.com/whl-multi-arch", - "selected_rocm_version:", - "build_date:", - "source_layout_generation: multi-arch-v2", - ] { - assert!( - output.contains(expected), - "SDK preview omitted `{expected}`:\n{output}" - ); - } + + assert_eq!( + preview_field(output, "channel"), + "nightly", + "the preview is not previewing the nightly channel:\n{output}" + ); + // `canonical_source` is the source the CLI declares it will read; + // `index_url` is the one it actually read. They are separate lines because + // the resolver is what drifted in rocm-cli#271, and a declaration that + // disagrees with it is worse than either alone. + assert_eq!( + preview_field(output, "canonical_source"), + AGGREGATE, + "the preview declares a source other than the canonical nightly aggregate:\n{output}" + ); + assert_eq!( + preview_field(output, "index_url"), + AGGREGATE, + "the nightly install resolved something other than the canonical aggregate:\n{output}" + ); + assert_eq!( + preview_field(output, "source_layout_generation"), + "multi-arch-v2", + "the preview read the aggregate as some other layout:\n{output}" + ); + // A `selected_rocm_version` line is provenance only if it names the version + // that is about to be installed. On its own the line is present whatever it + // says, which is why it is asserted against the pin rather than for its own + // existence. + let version = preview_field(output, "selected_rocm_version"); + let rocm_spec = preview_rocm_spec(output); + assert!( + rocm_spec.ends_with(&format!("=={version}")), + "the preview reports `selected_rocm_version: {version}` but plans to install \ + `{rocm_spec}`:\n{output}" + ); +} + +#[when("the user dry-runs a release SDK install for this host")] +async fn user_dry_runs_release_sdk(world: &mut E2eWorld) { + // Deliberately no `--family`: the device payload is chosen from the chip the + // CLI detects, so passing a family would override the thing under test. + // `--dry-run` keeps it to a plan — no venv, no download. + let stdout = crate::run_rocm_ok( + world, + &["install", "sdk", "--channel", "release", "--dry-run"], + ); + world.cli_output = Some(stdout); +} + +#[then("the SDK preview reports canonical release provenance")] +async fn assert_canonical_release_provenance(world: &mut E2eWorld) { + const AGGREGATE: &str = "https://repo.amd.com/rocm/whl-multi-arch"; + let output = world.cli_output.as_deref().expect("no SDK preview output"); + + assert_eq!( + preview_field(output, "channel"), + "release", + "the preview is not previewing the release channel:\n{output}" + ); + // rocm-cli#271 was this URL with `/{family}` glued on the end: a per-family + // index frozen at 7.13.0 that publishes no device payloads at all. The + // release channel now reads one flat aggregate, and `index_url` is the line + // that says so — the URL the resolver chose, not the one it advertises. + assert_eq!( + preview_field(output, "canonical_source"), + AGGREGATE, + "the preview declares a source other than the canonical release aggregate:\n{output}" + ); + assert_eq!( + preview_field(output, "index_url"), + AGGREGATE, + "the release install resolved something other than the flat aggregate index:\n{output}" + ); + assert_eq!( + preview_field(output, "source_layout_generation"), + "multi-arch-v2", + "the preview read the aggregate as some other layout:\n{output}" + ); + // The release channel installs stable versions only, and a stable version + // encodes no build date. Reporting that instead of a date is the honest + // form; a date here would mean a nightly reached the stable stream. + assert_eq!( + preview_field(output, "build_date"), + "not encoded in stable version", + "the release preview reported a build date:\n{output}" + ); + // Asserting a version literal would pin whatever is current today. The + // falsifiable claim is that the version the provenance block reports is the + // version the plan pins — a block that reports one and installs another is + // the failure a user has no way to see. + let version = preview_field(output, "selected_rocm_version"); + let rocm_spec = preview_rocm_spec(output); + assert!( + rocm_spec.ends_with(&format!("=={version}")), + "the preview reports `selected_rocm_version: {version}` but plans to install \ + `{rocm_spec}`:\n{output}" + ); +} + +#[then("the SDK preview requests the device payload for this host's GPU")] +async fn assert_release_device_payload(world: &mut E2eWorld) { + // Read the chip from the CLI's own detection surface rather than from this + // harness's host probe. That makes this a claim about two commands agreeing + // on which GPU is present, instead of a restatement of the plan. + let examine = crate::run_rocm_ok(world, &["examine"]); + // `examine` always prints the line, using `` when it found nothing, + // so the placeholder has to be rejected explicitly — otherwise a host that + // reached here without a detectable GPU fails on the comparison below and + // reads as a device-selection bug rather than a scenario running where it + // should not. + let detected = super::examine_steps::field_value(&examine, "detected_gfx_target") + .filter(|target| target.starts_with("gfx")) + .unwrap_or_else(|| panic!("`rocm examine` detected no AMD GPU on this host:\n{examine}")); + // Some detection paths append feature flags to the target + // (`gfx90a:sramecc+:xnack-`); the payload is published under the bare chip. + let detected = detected.split(':').next().unwrap_or(detected); + + let output = world.cli_output.as_deref().expect("no SDK preview output"); + + // Without this the next assertion would only say the payload matches + // whatever family the command line asked for. The scenario passes no + // `--family`, so anything but `host` means something else resolved it and + // the match below is no longer about this machine. + assert_eq!( + preview_field(output, "target_family_source"), + "host", + "the preview did not resolve its target family from the host:\n{output}" + ); + + // The payload is the detected chip verbatim: not the family bucket, not a + // neighbouring stepping. `undetermined` lands here too, and the reason the + // CLI gives for it is in the attached output. + let planned = preview_field(output, "device_target"); + assert_eq!( + planned, detected, + "`rocm examine` detected {detected} but the install plans the {planned} device \ + payload:\n{output}" + ); + assert_eq!( + requested_rocm_extras(preview_rocm_spec(output)), + format!("libraries,devel,device-{detected}"), + "the install does not request exactly this host's device payload:\n{output}" + ); + + // The every-GPU payload was the previous answer whenever a chip could not be + // named: on an Instinct host it fetched 24 device wheels, roughly 4.3 GiB, to + // use one of them, and left the post-install probe reporting the first of the + // 24 as the target family. It is gone from the CLI; this is what keeps it out. + assert!( + !output.contains("device-all"), + "the install plans the every-GPU payload instead of this host's:\n{output}" + ); } #[when("the user tries to adopt the existing install")] From 02afcb396de818964473fa074a353a3d6c9ad6d9 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Tue, 8 Sep 2026 14:11:15 -0700 Subject: [PATCH 07/14] test(e2e): gate resolver preview on detected target Signed-off-by: Michael Roy --- .../features/runtime_setup.feature | 10 +++--- tests/e2e-cucumber/src/expectation.rs | 36 +++++++++++++++++++ tests/e2e-cucumber/tests/e2e/examine_steps.rs | 4 +-- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/tests/e2e-cucumber/features/runtime_setup.feature b/tests/e2e-cucumber/features/runtime_setup.feature index 2cf14c54a..8b801936a 100644 --- a/tests/e2e-cucumber/features/runtime_setup.feature +++ b/tests/e2e-cucumber/features/runtime_setup.feature @@ -165,11 +165,11 @@ Feature: Runtime configuration # and then requesting every device wheel ROCm publishes. Only the second Then # separates them. # - # `@requires-gpu` because a device payload can only be exact about a chip that - # is there; it also keeps this off the 64-way mock lane whose concurrency forced - # runtime-06 to `@nightly`, since GPU lanes run one scenario at a time. - # `--dry-run` keeps it to a plan: no venv, no download. - @id:runtime-resolve-canonical-release @requires-gpu + # `@requires-gfx-target` is narrower than `@requires-gpu`: this preview only + # needs a detected chip name and never opens the device. It therefore runs on + # WSL hosts that can read the Windows-side target before ROCm passthrough is + # ready, while mock hosts with no target skip it. + @id:runtime-resolve-canonical-release @requires-gfx-target Scenario: runtime-09 - Previewing a release SDK install resolves the canonical aggregate for this GPU Given a machine with an AMD GPU When the user dry-runs a release SDK install for this host diff --git a/tests/e2e-cucumber/src/expectation.rs b/tests/e2e-cucumber/src/expectation.rs index 6001e9f55..d8b796315 100644 --- a/tests/e2e-cucumber/src/expectation.rs +++ b/tests/e2e-cucumber/src/expectation.rs @@ -25,6 +25,7 @@ const ID_PREFIX: &str = "id:"; const REQUIRES_ENGINE_PREFIX: &str = "requires-engine:"; const REQUIRES_OS_PREFIX: &str = "requires-os:"; const REQUIRES_GPU_TAG: &str = "requires-gpu"; +const REQUIRES_GFX_TARGET_TAG: &str = "requires-gfx-target"; const REQUIRES_NO_GPU_TAG: &str = "requires-no-gpu"; const REQUIRES_BARE_METAL_TAG: &str = "requires-bare-metal"; const REQUIRES_WSL_TAG: &str = "requires-wsl"; @@ -55,6 +56,10 @@ pub enum Expectation { pub struct ScenarioDecl { pub id: Option, pub requires_gpu: bool, + /// `@requires-gfx-target`: the scenario needs a detected chip name but does + /// not access the GPU. This permits resolver dry-runs on WSL before GPU + /// passthrough is ready without weakening `@requires-gpu` serve scenarios. + pub requires_gfx_target: bool, /// `@requires-no-gpu`: the scenario's premise is a host with NO usable AMD GPU /// (e.g. a GPU-required serve must fail fast). Skipped on any host that has a /// GPU — the inverse of `requires_gpu`. This is how the no-GPU fail-fast path @@ -117,6 +122,7 @@ impl ScenarioDecl { pub fn from_tags>(tags: &[S]) -> Self { let mut id = None; let mut requires_gpu = false; + let mut requires_gfx_target = false; let mut requires_no_gpu = false; let mut requires_bare_metal = false; let mut requires_wsl = false; @@ -141,6 +147,8 @@ impl ScenarioDecl { serve_timeout_secs = rest.parse::().ok(); } else if tag == REQUIRES_GPU_TAG { requires_gpu = true; + } else if tag == REQUIRES_GFX_TARGET_TAG { + requires_gfx_target = true; } else if tag == REQUIRES_NO_GPU_TAG { requires_no_gpu = true; } else if tag == REQUIRES_BARE_METAL_TAG { @@ -158,6 +166,7 @@ impl ScenarioDecl { Self { id, requires_gpu, + requires_gfx_target, requires_no_gpu, requires_bare_metal, requires_wsl, @@ -426,6 +435,11 @@ pub fn resolve( reason: "requires an AMD GPU; none detected on this host".to_owned(), }; } + if decl.requires_gfx_target && cap.gfx_target.is_none() { + return Expectation::Skip { + reason: "requires a detected AMD GFX target; none detected on this host".to_owned(), + }; + } if decl.requires_no_gpu && cap.has_amd_gpu { return Expectation::Skip { reason: "requires a host with no AMD GPU; this host has one".to_owned(), @@ -643,6 +657,28 @@ serve_timeout_secs = 90 assert!(!decl(&["id:x", "requires-gpu"]).requires_bare_metal); } + #[test] + fn detected_target_requirement_does_not_require_gpu_passthrough() { + let matrix = Expectations::default(); + let scenario = decl(&["id:resolver-preview", "requires-gfx-target"]); + + assert_eq!( + resolve( + &scenario, + &cap("wsl-no-passthrough"), + &matrix, + false, + false, + false, + ), + Expectation::ExpectPass + ); + assert!(matches!( + resolve(&scenario, &cap("mock"), &matrix, false, false, false,), + Expectation::Skip { .. } + )); + } + #[test] fn serve_timeout_tag_parses_seconds() { let d = decl(&["id:serve-large-model-inference", "serve-timeout:2400"]); diff --git a/tests/e2e-cucumber/tests/e2e/examine_steps.rs b/tests/e2e-cucumber/tests/e2e/examine_steps.rs index fd5b1164b..3fec3a9b7 100644 --- a/tests/e2e-cucumber/tests/e2e/examine_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/examine_steps.rs @@ -21,8 +21,8 @@ pub(crate) fn field_value<'a>(output: &'a str, field: &str) -> Option<&'a str> { async fn setup_gpu_machine(world: &mut E2eWorld) { let (stdout, _, _) = crate::run_rocm(world, &["examine"]); assert!( - stdout.contains("AMD GPU detected") || stdout.contains("detected_gfx_target"), - "no AMD GPU detected on this machine:\n{stdout}" + field_value(&stdout, "detected_gfx_target").is_some_and(|target| target.starts_with("gfx")), + "no AMD GPU target detected on this machine:\n{stdout}" ); } From e7102788ed37bc8bd1bbf2d117bd39e8529338dc Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Tue, 8 Sep 2026 14:30:08 -0700 Subject: [PATCH 08/14] fix(runtime): key managed wheel runtimes by package composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A managed wheel runtime was identified by channel and version alone, so two installs that differ in what they actually install — above all the `device-` payload that decides whether the runtime can launch a kernel at all — resolved to the same runtime key. A corrected composition overwrote the existing tree in place instead of landing beside it, and `rocm update` reported the stale environment as up to date because its version had not moved. Record the exact install intent (source-layout generation and pinned package specs) in the runtime manifest, fold a fingerprint of it into the wheel runtime key, and compare it during freshness resolution. A same-version runtime whose composition or identity is stale now reports `repair_available` and installs a composition-keyed replacement side-by-side. Manifests written before this field load unchanged and are repaired once. An `ahead_of_index` build is never offered a repair: the index cannot reproduce it, so the only install available would be a rollback. Update apply now selects the installed manifest by the runtime key the plan predicted, not by version: a repair produces a sibling sharing the source's channel, format, family and version, which a version match would have resolved back to the runtime being replaced. The install is pinned to the source runtime's family for the same reason, so the applied key and composition match the predicted ones on a host whose GPU differs from the runtime being updated. An already-installed replacement suppresses further repairs of the manifest it superseded, so a retained legacy entry cannot re-trigger the same install or notification on every run. Each `rocm update` line now carries `target=`, the key an apply from that line would produce. The E2E pre-warm uses it to activate the runtime a reuse actually means — after a repair that is the replacement, not the superseded manifest — before installing the default engine, and the shared pre-warm tree is namespaced by source-layout generation so a branch on a new layout cannot poison the cache. Signed-off-by: Michael Roy --- .github/workflows/e2e-selfhosted.yml | 10 +- .github/workflows/nightly.yml | 10 +- Cargo.lock | 1 + apps/rocm/Cargo.toml | 1 + apps/rocm/src/comfyui.rs | 3 + apps/rocm/src/main.rs | 76 ++- apps/rocm/src/storage.rs | 1 + apps/rocm/src/therock.rs | 602 ++++++++++++++++-- apps/rocmd/src/lib.rs | 79 ++- docs/ci-hardware-testing.md | 34 +- tests/e2e-cucumber/src/capability.rs | 33 + tests/e2e-cucumber/tests/e2e/runtime_steps.rs | 34 +- xtask/src/e2e_prewarm.rs | 268 ++++++-- xtask/src/workflow_contract.rs | 22 + 14 files changed, 1016 insertions(+), 158 deletions(-) diff --git a/.github/workflows/e2e-selfhosted.yml b/.github/workflows/e2e-selfhosted.yml index 473d37a43..9cf95bf19 100644 --- a/.github/workflows/e2e-selfhosted.yml +++ b/.github/workflows/e2e-selfhosted.yml @@ -278,7 +278,7 @@ jobs: # instantly (observed on run 29320025393). Installing in place keeps the # baked paths valid, and each scenario's data/runtimes symlink resolves to # this same real tree. - prewarm="$RUNNER_WORKSPACE/e2e-prewarm" + prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2" export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes" # Build the rocm and rocmd binaries ONCE and reuse them for both the @@ -454,7 +454,7 @@ jobs: # is missing)` and fails (diagnosed on the box 2026-07-15). Installing in # place, directly into the persistent shared dir, keeps the baked paths # valid for all scenarios and for the end-of-run version probe. - prewarm="$RUNNER_WORKSPACE/e2e-prewarm" + prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2" export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes" # Build the rocm and rocmd binaries once; reuse them for pre-warm + suite. @@ -605,7 +605,7 @@ jobs: # every later serve sees status=unusable and fails (diagnosed on the Linux # box 2026-07-15; the Windows scenario-8 cold-download failure is the same # class). Install in place so the baked paths stay valid for all scenarios. - $prewarm = "$env:RUNNER_WORKSPACE\e2e-prewarm" + $prewarm = "$env:RUNNER_WORKSPACE\e2e-prewarm-multi-arch-v2" $env:E2E_SHARED_RUNTIMES_DIR = "$prewarm\data\runtimes" # Build the rocm and rocmd binaries once; reuse them for pre-warm + suite. @@ -794,7 +794,7 @@ jobs: # correctness, not speed: `install sdk` bakes ABSOLUTE paths into the # runtime manifest, so installing into a per-scenario temp dir leaves # every later serve pointing at a deleted install root. - prewarm="$RUNNER_WORKSPACE/e2e-prewarm" + prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2" export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes" # Build the rocm and rocmd binaries once; reuse them for pre-warm + suite. @@ -924,7 +924,7 @@ jobs: # optimization — `install sdk` bakes absolute paths into the runtime # manifest, so installing anywhere temporary breaks every later serve. # See e2e-gpu for the full rationale. - prewarm="$RUNNER_WORKSPACE/e2e-prewarm" + prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2" export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes" # See the e2e-gpu lane for why the e2e-test-hooks feature must match diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 4f704759c..373e8ff41 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -422,7 +422,7 @@ jobs: # The shared dir IS the pre-warm's own data/runtimes; NEVER move it — a # post-install mv invalidates the absolute paths install sdk bakes into # the runtime manifest and every serve fails instantly (run 29320025393). - prewarm="$RUNNER_WORKSPACE/e2e-prewarm" + prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2" export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes" # Build both binaries once; reuse them for pre-warm + suite. @@ -524,7 +524,7 @@ jobs: # Separate PVC mounted at exactly this path by the runner's cluster # overlay; if you change this path, change the overlay that mounts it too. export E2E_SHARED_UV_CACHE_DIR="/var/tmp/rocm-e2e-uv-cache" - prewarm="$RUNNER_WORKSPACE/e2e-prewarm" + prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2" export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes" # See the e2e-gpu lane in e2e-selfhosted.yml for why the @@ -620,7 +620,7 @@ jobs: run: | export CARGO_TARGET_DIR="$RUNNER_WORKSPACE/e2e-target" export E2E_SHARED_CACHE_DIR="$RUNNER_WORKSPACE/e2e-shared" - prewarm="$RUNNER_WORKSPACE/e2e-prewarm" + prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2" export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes" # See the e2e-gpu lane in e2e-selfhosted.yml for why the @@ -716,7 +716,7 @@ jobs: # Install the shared runtime in place because its manifest contains # absolute paths. The persistent runner workspace keeps those paths valid # across scenarios and subsequent runs. - $prewarm = "$env:RUNNER_WORKSPACE\e2e-prewarm" + $prewarm = "$env:RUNNER_WORKSPACE\e2e-prewarm-multi-arch-v2" $env:E2E_SHARED_RUNTIMES_DIR = "$prewarm\data\runtimes" # See the e2e-gpu lane in e2e-selfhosted.yml for why the @@ -870,7 +870,7 @@ jobs: # correctness, not speed: `install sdk` bakes ABSOLUTE paths into the # runtime manifest, so installing into a per-scenario temp dir leaves # every later serve pointing at a deleted install root. - prewarm="$RUNNER_WORKSPACE/e2e-prewarm" + prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2" export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes" # See the e2e-gpu lane in e2e-selfhosted.yml for why the diff --git a/Cargo.lock b/Cargo.lock index 473b8eb7b..f9a338fba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3580,6 +3580,7 @@ dependencies = [ "rpassword", "serde", "serde_json", + "sha2", "tar", "tokio", "tracing", diff --git a/apps/rocm/Cargo.toml b/apps/rocm/Cargo.toml index 944e83e8b..2d7a68c5e 100644 --- a/apps/rocm/Cargo.toml +++ b/apps/rocm/Cargo.toml @@ -38,6 +38,7 @@ rocm-engine-vllm = { path = "../../engines/vllm" } rpassword.workspace = true serde.workspace = true serde_json.workspace = true +sha2.workspace = true tar = "0.4" tracing = "0.1" tracing-appender = "0.2" diff --git a/apps/rocm/src/comfyui.rs b/apps/rocm/src/comfyui.rs index 5ae412265..69fbc82d0 100644 --- a/apps/rocm/src/comfyui.rs +++ b/apps/rocm/src/comfyui.rs @@ -2112,6 +2112,7 @@ mod tests { ..therock::RocmSdkPythonProbe::default() }), sdk_torch: None, + wheel_composition: None, read_only: false, imported_from: None, installed_at_unix_ms: 100, @@ -2178,6 +2179,7 @@ mod tests { ..Default::default() }), sdk_torch: None, + wheel_composition: None, read_only: false, imported_from: None, installed_at_unix_ms: 100, @@ -2275,6 +2277,7 @@ mod tests { ..therock::RocmSdkPythonProbe::default() }), sdk_torch: None, + wheel_composition: None, read_only: false, imported_from: None, installed_at_unix_ms: 100, diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index 08cbf8cf7..910a5860e 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -9194,6 +9194,7 @@ fn adopt_runtime_from_probe( // Adoption does not install torch, so the build is derived from the SDK // version instead. sdk_torch: None, + wheel_composition: None, read_only: true, imported_from: Some(install_root), installed_at_unix_ms: rocm_core::unix_time_millis(), @@ -15821,7 +15822,12 @@ fn apply_runtime_update( ) -> Result { let manifests = therock::load_runtime_manifests(paths)?; let source = select_runtime_update_source(&manifests, config, runtime_selector)?; - let plan = therock::runtime_update_plan(paths, source)?; + let plan = therock::runtime_update_plan(paths, source, &manifests)?; + // The plan resolved the index for the SOURCE runtime's family; letting the + // install re-detect one would compose a different runtime than the plan just + // predicted whenever this host's GPU disagrees with the runtime being + // updated, and the key lookup below would then find nothing. + let family_override = Some(source.family.as_str()); let mut output = String::new(); let _ = writeln!(output, "runtime update"); let _ = writeln!(output, " source_runtime_key: {}", source.runtime_key); @@ -15840,6 +15846,7 @@ fn apply_runtime_update( therock::runtime_version_display(&plan.latest_version) ); let _ = writeln!(output, " status: {}", plan.status); + let _ = writeln!(output, " target_runtime_key: {}", plan.target_runtime_key); let _ = writeln!(output, " activate_after_install: {activate}"); if !plan.update_available { let _ = writeln!(output, " result: no newer runtime found"); @@ -15854,7 +15861,7 @@ fn apply_runtime_update( &source.format, None, None, - None, + family_override, true, )?; let _ = writeln!(output, " install_plan:"); @@ -15870,12 +15877,21 @@ fn apply_runtime_update( &source.format, None, None, - None, + family_override, false, )?; let manifests_after = therock::load_runtime_manifests(paths)?; - let installed = select_installed_update_runtime(&manifests_after, source, &plan.latest_version) - .context("updated runtime install completed but the new runtime manifest was not found")?; + // By exact key, never by version: a same-version repair installs a sibling + // that shares the source's channel, format, family AND version, so a + // version match would just as happily return the stale runtime this update + // was meant to replace, and then activate it. + let installed = select_installed_update_runtime(&manifests_after, &plan.target_runtime_key) + .with_context(|| { + format!( + "runtime install completed but no manifest was written for the planned runtime key `{}`", + plan.target_runtime_key + ) + })?; let _ = writeln!(output, " installed_runtime_key: {}", installed.runtime_key); let _ = writeln!( output, @@ -15940,15 +15956,11 @@ fn select_runtime_update_source<'a>( fn select_installed_update_runtime<'a>( manifests: &'a [therock::InstalledRuntimeManifest], - source: &therock::InstalledRuntimeManifest, - latest_version: &str, + target_runtime_key: &str, ) -> Option<&'a therock::InstalledRuntimeManifest> { - manifests.iter().find(|manifest| { - manifest.channel == source.channel - && manifest.format == source.format - && manifest.family == source.family - && manifest.version == latest_version - }) + manifests + .iter() + .find(|manifest| manifest.runtime_key == target_runtime_key) } pub(crate) fn render_automations_text(paths: &AppPaths, config: &RocmCliConfig) -> Result { @@ -30018,32 +30030,40 @@ ID_LIKE="suse opensuse" } #[test] - fn installed_update_runtime_matches_latest_version_and_family() { - let mut source = test_runtime_manifest_for_update( - "old-gfx120", + fn installed_update_runtime_is_selected_by_exact_target_key() { + // Everything a version match would have keyed on is identical here: + // same channel, format, family and version. Only the composition-keyed + // runtime key tells the freshly installed repair apart from the stale + // runtime it was installed to replace. + let stale = test_runtime_manifest_for_update( + "release-wheel-multi-arch-7-14-0", "therock-release:gfx120X-all", "gfx120X-all", - "7.13.0a20260416", + "7.14.0", ); - source.channel = "release".to_owned(); let wrong_family = test_runtime_manifest_for_update( - "new-gfx110", + "release-wheel-multi-arch-7-14-0-ffffffffffffffff", "therock-release:gfx110X-all", "gfx110X-all", - "7.14.0a20260531", + "7.14.0", ); - let target = test_runtime_manifest_for_update( - "new-gfx120", + let repaired = test_runtime_manifest_for_update( + "release-wheel-multi-arch-7-14-0-0123456789abcdef", "therock-release:gfx120X-all", "gfx120X-all", - "7.14.0a20260531", + "7.14.0", ); - let manifests = vec![wrong_family, target.clone()]; + let manifests = vec![stale.clone(), wrong_family, repaired.clone()]; - let selected = select_installed_update_runtime(&manifests, &source, "7.14.0a20260531") - .expect("matching updated runtime should be selected"); + let selected = select_installed_update_runtime(&manifests, &repaired.runtime_key) + .expect("the side-by-side repair must be selected by its exact key"); + assert_eq!(selected.runtime_key, repaired.runtime_key); - assert_eq!(selected.runtime_key, target.runtime_key); + assert!( + select_installed_update_runtime(&manifests, "release-wheel-multi-arch-7-15-0") + .is_none(), + "an install that wrote no manifest for the planned key must not resolve to a sibling" + ); } fn write_test_pip_runtime( @@ -30117,6 +30137,7 @@ ID_LIKE="suse opensuse" ..therock::RocmSdkPythonProbe::default() }), sdk_torch: None, + wheel_composition: None, read_only: false, imported_from: None, installed_at_unix_ms, @@ -30156,6 +30177,7 @@ ID_LIKE="suse opensuse" pip_cache_dir: None, rocm_sdk: None, sdk_torch: None, + wheel_composition: None, read_only: false, imported_from: None, installed_at_unix_ms: 1, diff --git a/apps/rocm/src/storage.rs b/apps/rocm/src/storage.rs index f8e910dbe..7c42e4fa4 100644 --- a/apps/rocm/src/storage.rs +++ b/apps/rocm/src/storage.rs @@ -969,6 +969,7 @@ mod tests { pip_cache_dir: None, rocm_sdk: None, sdk_torch: None, + wheel_composition: None, read_only: false, imported_from: None, installed_at_unix_ms, diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 263a1b530..693cb1a1a 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -18,6 +18,7 @@ use rocm_core::{ generate_rsa_signing_keypair, managed_uv_cache_dir, sign_rsa_pkcs1_sha256_signature, }; use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; use std::cmp::Ordering; use std::ffi::{OsStr, OsString}; use std::fmt::Write as _; @@ -430,9 +431,56 @@ pub(crate) struct RuntimeUpdatePlan { pub latest_source: String, pub format: String, pub status: String, + /// The runtime key the applied install will produce. Update apply selects + /// the resulting manifest by this key rather than by version, because a + /// same-version repair produces a sibling that version alone cannot name. + pub target_runtime_key: String, + pub repair_required: bool, pub update_available: bool, } +/// Exact canonical wheel install intent last applied successfully to a runtime. +/// +/// Version alone cannot identify a reusable environment: adding a required ROCm +/// extra at the same release version must make an older cache repairable. +#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] +pub(crate) struct WheelRuntimeComposition { + pub source_layout_generation: String, + pub package_specs: Vec, +} + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +enum RuntimeFreshness { + UpToDate, + UpdateAvailable, + RepairAvailable, + AheadOfIndex, +} + +impl RuntimeFreshness { + const fn status(self) -> &'static str { + match self { + Self::UpToDate => "up_to_date", + Self::UpdateAvailable => "update_available", + Self::RepairAvailable => "repair_available", + Self::AheadOfIndex => "ahead_of_index", + } + } + + const fn update_available(self) -> bool { + matches!(self, Self::UpdateAvailable | Self::RepairAvailable) + } +} + +#[derive(Debug, Clone)] +struct ResolvedRuntimeUpdate { + latest_version: String, + latest_source: String, + target_runtime_key: String, + format: String, + wheel_composition: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct InstalledRuntimeManifest { pub runtime_key: String, @@ -464,6 +512,10 @@ pub(crate) struct InstalledRuntimeManifest { /// knows that, and it has to survive repeat installs to be worth anything. #[serde(default)] pub sdk_torch: Option, + /// Missing on manifests written before composition-aware freshness. Such a + /// managed wheel runtime is repaired once and rewritten with this field. + #[serde(default)] + pub wheel_composition: Option, #[serde(default)] pub read_only: bool, #[serde(default)] @@ -725,8 +777,8 @@ pub(crate) fn render_update_report(paths: &AppPaths) -> Result { return Ok(output); } - for manifest in manifests { - let plan = match runtime_update_plan(paths, &manifest) { + for manifest in &manifests { + let plan = match runtime_update_plan(paths, manifest, &manifests) { Ok(plan) => Some(plan), Err(error) => { let _ = writeln!( @@ -741,16 +793,21 @@ pub(crate) fn render_update_report(paths: &AppPaths) -> Result { let Some(plan) = plan else { continue; }; + // `target=` names the runtime key an apply from this line would produce. + // For a superseded legacy manifest that is its already-installed + // replacement, which is how a reader — `xtask e2e-prewarm` above all — + // learns which sibling to activate without re-deriving the composition. let _ = writeln!( output, - " runtime {} format={} channel={} family={} installed={} latest={} status={}", + " runtime {} format={} channel={} family={} installed={} latest={} status={} target={}", manifest.runtime_key, plan.format, manifest.channel, manifest.family, runtime_version_display(&manifest.version), runtime_version_display(&plan.latest_version), - plan.status + plan.status, + plan.target_runtime_key ); let _ = writeln!( output, @@ -759,14 +816,21 @@ pub(crate) fn render_update_report(paths: &AppPaths) -> Result { ); let _ = writeln!(output, " source: {}", plan.latest_source); if plan.update_available { + let next_step = if plan.repair_required { + format!( + "run `rocm update --apply --runtime {}` to install a composition-keyed replacement side-by-side", + manifest.runtime_key + ) + } else { + format!( + "run `rocm update --apply --runtime {}` to install the newer runtime side-by-side", + manifest.runtime_key + ) + }; + let _ = writeln!(output, " next step: {next_step}"); let _ = writeln!( output, - " next step: run `rocm update --apply --runtime {}` to install the newer runtime side-by-side", - manifest.runtime_key - ); - let _ = writeln!( - output, - " activate: add `--activate` to make the newly installed runtime the default after install" + " activate: add `--activate` to make the installed runtime the default after install" ); } } @@ -774,23 +838,108 @@ pub(crate) fn render_update_report(paths: &AppPaths) -> Result { Ok(output) } +/// Whether the composition-keyed replacement for `source` is already installed. +/// +/// A repair installs a sibling and leaves the legacy manifest in place until the +/// retention pass removes it. Without this, that retained manifest keeps +/// reporting `repair_available` forever, so every pre-warm reinstalls a runtime +/// that is already there and every startup check re-notifies. +fn replacement_runtime_is_installed( + manifests: &[InstalledRuntimeManifest], + source: &InstalledRuntimeManifest, + target_runtime_key: &str, + required_composition: Option<&WheelRuntimeComposition>, +) -> bool { + // `RepairAvailable` currently implies a wheel composition, but keep this + // helper total if a future freshness state reaches it without one. + let Some(required_composition) = required_composition else { + return false; + }; + manifests.iter().any(|candidate| { + !candidate.read_only + && candidate.runtime_key == target_runtime_key + && candidate.channel == source.channel + && candidate.format == source.format + && candidate.family == source.family + && candidate.wheel_composition.as_ref() == Some(required_composition) + }) +} + +/// Freshness of one runtime against the index, ignoring its siblings. +/// +/// An installed version newer than the index is [`RuntimeFreshness::AheadOfIndex`] +/// before any composition is considered: that build cannot be reproduced from the +/// index at all, so calling it repairable would promise an install that must +/// either fail or silently roll the runtime back. +fn runtime_freshness( + manifest: &InstalledRuntimeManifest, + latest_version: &str, + required_composition: Option<&WheelRuntimeComposition>, + target_runtime_key: &str, +) -> RuntimeFreshness { + match compare_version_strings(&manifest.version, latest_version) { + Ordering::Less => RuntimeFreshness::UpdateAvailable, + Ordering::Greater => RuntimeFreshness::AheadOfIndex, + Ordering::Equal + if !manifest.read_only + && required_composition.is_some() + && (manifest.wheel_composition.as_ref() != required_composition + || manifest.runtime_key != target_runtime_key) => + { + RuntimeFreshness::RepairAvailable + } + Ordering::Equal => RuntimeFreshness::UpToDate, + } +} + +fn runtime_freshness_with_manifests( + manifests: &[InstalledRuntimeManifest], + manifest: &InstalledRuntimeManifest, + latest_version: &str, + required_composition: Option<&WheelRuntimeComposition>, + target_runtime_key: &str, +) -> RuntimeFreshness { + let freshness = runtime_freshness( + manifest, + latest_version, + required_composition, + target_runtime_key, + ); + if freshness == RuntimeFreshness::RepairAvailable + && replacement_runtime_is_installed( + manifests, + manifest, + target_runtime_key, + required_composition, + ) + { + RuntimeFreshness::UpToDate + } else { + freshness + } +} + pub(crate) fn runtime_update_plan( paths: &AppPaths, manifest: &InstalledRuntimeManifest, + manifests: &[InstalledRuntimeManifest], ) -> Result { - let (latest_version, latest_source, format) = - resolve_latest_for_manifest(paths, manifest, None)?; - let status = match compare_version_strings(&manifest.version, &latest_version) { - Ordering::Less => "update_available", - Ordering::Equal => "up_to_date", - Ordering::Greater => "ahead_of_index", - }; + let latest = resolve_latest_for_manifest(paths, manifest, None)?; + let freshness = runtime_freshness_with_manifests( + manifests, + manifest, + &latest.latest_version, + latest.wheel_composition.as_ref(), + &latest.target_runtime_key, + ); Ok(RuntimeUpdatePlan { - latest_version, - latest_source, - format, - status: status.to_owned(), - update_available: status == "update_available", + latest_version: latest.latest_version, + latest_source: latest.latest_source, + format: latest.format, + status: freshness.status().to_owned(), + target_runtime_key: latest.target_runtime_key, + repair_required: freshness == RuntimeFreshness::RepairAvailable, + update_available: freshness.update_available(), }) } @@ -798,7 +947,7 @@ fn resolve_latest_for_manifest( paths: &AppPaths, manifest: &InstalledRuntimeManifest, download_timeout_secs: Option, -) -> Result<(String, String, String)> { +) -> Result { let channel = TheRockChannel::parse(&manifest.channel)?; match manifest.format.as_str() { "wheel" => { @@ -825,11 +974,40 @@ fn resolve_latest_for_manifest( None, download_timeout_secs, )?; - Ok(( - resolution.latest_version, - resolution.index_url, - "wheel".to_owned(), - )) + // Prefer the device payload this runtime was actually built with over + // a fresh host probe. Planning must predict the key an apply will + // produce, and re-probing would disagree with the installed runtime on + // any host whose GPU is absent, hidden, or simply a second card. + let device_target = + wheel_composition_device_target(manifest.wheel_composition.as_ref()) + .and_then(|target| { + canonical_aggregate_device_target(target, &resolution.family) + }) + .map_or_else( + || resolution.device_target.clone(), + AggregateDeviceTarget::Exact, + ); + // No exact target means no reproducible composition, so freshness + // falls back to the version comparison rather than demanding a repair + // this host could not perform. + let wheel_composition = match &device_target { + AggregateDeviceTarget::Exact(_) => Some(wheel_runtime_composition( + &resolution, + &device_target.rocm_extras(), + )), + AggregateDeviceTarget::Undetermined(_) => None, + }; + let target_runtime_key = wheel_composition.as_ref().map_or_else( + || manifest.runtime_key.clone(), + |composition| wheel_runtime_key(channel, &resolution.latest_version, composition), + ); + Ok(ResolvedRuntimeUpdate { + latest_version: resolution.latest_version, + latest_source: resolution.index_url, + target_runtime_key, + format: "wheel".to_owned(), + wheel_composition, + }) } "tarball" => { let artifact = resolve_tarball_artifact_with_timeout( @@ -838,7 +1016,19 @@ fn resolve_latest_for_manifest( Some(manifest.family.as_str()), download_timeout_secs, )?; - Ok((artifact.version, artifact.url, "tarball".to_owned())) + let target_runtime_key = runtime_key( + channel, + "tarball", + &artifact.family, + Some(&artifact.version), + ); + Ok(ResolvedRuntimeUpdate { + latest_version: artifact.version, + latest_source: artifact.url, + target_runtime_key, + format: "tarball".to_owned(), + wheel_composition: None, + }) } other => bail!("unknown manifest format `{other}`"), } @@ -875,6 +1065,7 @@ fn maybe_refresh_startup_update_check_at( let record = build_startup_update_check_record( paths, manifest, + &manifests, now_unix_ms, Some(STARTUP_UPDATE_CHECK_TIMEOUT_SECS), ); @@ -906,25 +1097,28 @@ fn select_startup_update_manifest<'a>( fn build_startup_update_check_record( paths: &AppPaths, manifest: &InstalledRuntimeManifest, + manifests: &[InstalledRuntimeManifest], now_unix_ms: u128, download_timeout_secs: Option, ) -> StartupUpdateCheckRecord { match resolve_latest_for_manifest(paths, manifest, download_timeout_secs) { - Ok((latest_version, _latest_source, kind)) => { - let status = match compare_version_strings(&manifest.version, &latest_version) { - Ordering::Less => "update_available", - Ordering::Equal => "up_to_date", - Ordering::Greater => "ahead_of_index", - }; + Ok(latest) => { + let freshness = runtime_freshness_with_manifests( + manifests, + manifest, + &latest.latest_version, + latest.wheel_composition.as_ref(), + &latest.target_runtime_key, + ); StartupUpdateCheckRecord { runtime_key: manifest.runtime_key.clone(), runtime_id: manifest.runtime_id.clone(), channel: manifest.channel.clone(), - format: kind, + format: latest.format, family: manifest.family.clone(), installed_version: manifest.version.clone(), - latest_version: Some(latest_version), - status: status.to_owned(), + latest_version: Some(latest.latest_version), + status: freshness.status().to_owned(), message: None, checked_at_unix_ms: now_unix_ms, } @@ -1020,12 +1214,18 @@ fn install_wheel_runtime( &wheel_compatibility, version_selector, )?; - let rocm_extras = resolution.device_target.rocm_extras(); + // The exact device payload, not the version alone, decides what this runtime + // can run, so it is what identifies the runtime. A preview on a host with no + // usable target still composes a key here — from the `` extras + // — which no real install can ever produce, and the refusal below stops it + // from reaching a manifest. + let wheel_composition = + wheel_runtime_composition(&resolution, &resolution.device_target.rocm_extras()); progress_line(format!( "Found canonical TheRock aggregate version {} with a matching PyTorch stack for target family {}.", resolution.latest_version, resolution.family )); - let runtime_key = wheel_runtime_key(channel, &resolution.latest_version); + let runtime_key = wheel_runtime_key(channel, &resolution.latest_version, &wheel_composition); let install_root = resolved_install_root(paths, "wheel", &runtime_key, prefix); let manifest_path = runtime_manifest_path(paths, &runtime_key); @@ -1093,7 +1293,7 @@ fn install_wheel_runtime( let _ = writeln!( output, " package_specs: {}", - therock_pip_package_specs(&resolution.package_versions, &rocm_extras).join(" ") + wheel_composition.package_specs.join(" ") ); let _ = writeln!( output, @@ -1106,10 +1306,7 @@ fn install_wheel_runtime( if matches!(channel, TheRockChannel::Nightly) { install_args.extend(["--prerelease".to_owned(), "allow".to_owned()]); } - install_args.extend(therock_pip_package_specs( - &resolution.package_versions, - &rocm_extras, - )); + install_args.extend(wheel_composition.package_specs.iter().cloned()); let venv_args = uv_venv_args(&python_launcher.executable, &install_root); let venv_args_display = venv_args .iter() @@ -1163,7 +1360,7 @@ fn install_wheel_runtime( progress_line(format!( "Installing {} from {}", - therock_pip_package_specs(&resolution.package_versions, &rocm_extras).join(" "), + wheel_composition.package_specs.join(" "), resolution.index_url )); let mut install_args = uv_pip_install_base(&env_python); @@ -1171,10 +1368,7 @@ fn install_wheel_runtime( if matches!(channel, TheRockChannel::Nightly) { install_args.extend(["--prerelease".to_owned(), "allow".to_owned()]); } - install_args.extend(therock_pip_package_specs( - &resolution.package_versions, - &rocm_extras, - )); + install_args.extend(wheel_composition.package_specs.iter().cloned()); run_uv_progress_command( paths, &uv, @@ -1211,6 +1405,7 @@ fn install_wheel_runtime( pip_cache_dir: None, rocm_sdk: Some(rocm_sdk_probe.clone()), sdk_torch: Some(resolution.package_versions.torch.clone()), + wheel_composition: Some(wheel_composition), read_only: false, imported_from: None, installed_at_unix_ms: unix_time_millis(), @@ -1258,6 +1453,36 @@ fn therock_pip_package_specs( ] } +/// The exact install intent for `resolution` under `rocm_extras`. +/// +/// Kept beside [`therock_pip_package_specs`] so the specs that identify a +/// runtime are, by construction, the specs that get installed. +fn wheel_runtime_composition( + resolution: &PipRuntimeResolution, + rocm_extras: &str, +) -> WheelRuntimeComposition { + WheelRuntimeComposition { + source_layout_generation: THEROCK_SOURCE_LAYOUT_GENERATION.to_owned(), + package_specs: therock_pip_package_specs(&resolution.package_versions, rocm_extras), + } +} + +/// The GFX target a recorded composition installed, read back out of its +/// `rocm[...,device-]` requirement. +/// +/// Update planning needs the target the runtime was built with, not the one this +/// host happens to report now; storing the specs verbatim means that answer +/// survives without a second manifest field to keep in sync. +fn wheel_composition_device_target(composition: Option<&WheelRuntimeComposition>) -> Option<&str> { + composition?.package_specs.iter().find_map(|spec| { + let extras = spec.strip_prefix("rocm[")?.split_once(']')?.0; + extras + .split(',') + .map(str::trim) + .find_map(|extra| extra.strip_prefix("device-")) + }) +} + fn quote_display_arg(value: &str) -> String { if value.is_empty() || value @@ -1350,6 +1575,7 @@ fn install_tarball_runtime( pip_cache_dir: None, rocm_sdk: None, sdk_torch: None, + wheel_composition: None, read_only: false, imported_from: None, installed_at_unix_ms: unix_time_millis(), @@ -3990,8 +4216,35 @@ fn canonical_wheel_resolution_hint(channel: TheRockChannel) -> String { hint } -fn wheel_runtime_key(channel: TheRockChannel, version: &str) -> String { - slugify(&format!("{}-wheel-multi-arch-{version}", channel.as_str())) +/// Identify a wheel runtime by channel, version, AND the exact composition it +/// was installed from. +/// +/// Two installs of the same version that request different device payloads are +/// different runtimes: one can run this host's kernels and the other cannot. A +/// version-only key gave them the same name, so a corrected composition +/// overwrote the old tree in place — the one thing side-by-side installs exist +/// to avoid — and left no way to tell the two apart afterwards. +/// +/// The fingerprint is a truncated SHA-256 over the generation and the specs, +/// length-delimited so no regrouping of the same characters collides. Truncation +/// is safe here: this names sibling directories, it does not authenticate them. +fn wheel_runtime_key( + channel: TheRockChannel, + version: &str, + composition: &WheelRuntimeComposition, +) -> String { + let mut hasher = Sha256::new(); + hasher.update(composition.source_layout_generation.as_bytes()); + for package_spec in &composition.package_specs { + hasher.update([0]); + hasher.update(package_spec.as_bytes()); + } + let fingerprint = format!("{:x}", hasher.finalize()); + slugify(&format!( + "{}-wheel-multi-arch-{version}-{}", + channel.as_str(), + &fingerprint[..16] + )) } fn family_resolution_hint( @@ -4859,6 +5112,194 @@ mod tests { ); } + fn test_wheel_composition(device_target: &str) -> WheelRuntimeComposition { + WheelRuntimeComposition { + source_layout_generation: THEROCK_SOURCE_LAYOUT_GENERATION.to_owned(), + package_specs: vec![ + format!("rocm[libraries,devel,device-{device_target}]==7.14.0"), + "torch==2.11.0+rocm7.14.0".to_owned(), + "torchvision==0.26.0+rocm7.14.0".to_owned(), + "torchaudio==2.11.0+rocm7.14.0".to_owned(), + ], + } + } + + #[test] + fn legacy_manifest_without_a_composition_is_repaired_once_then_settles() { + // Exactly what a runtime installed before composition-aware freshness + // deserializes to: no `wheel_composition`, and a version-only key. + let mut old_cache: InstalledRuntimeManifest = serde_json::from_value(serde_json::json!({ + "runtime_key": "release-wheel-multi-arch-7-14-0", + "runtime_id": "therock-release:gfx94X-dcgpu", + "channel": "release", + "format": "wheel", + "family": "gfx94X-dcgpu", + "family_source": "managed-runtime", + "version": "7.14.0", + "install_root": "/tmp/release-wheel-multi-arch-7-14-0", + "selected_artifact_url": "https://repo.amd.com/rocm/whl-multi-arch", + "index_url": "https://repo.amd.com/rocm/whl-multi-arch", + "tarball_file_name": null, + "python_launcher": "/usr/bin/python3", + "python_executable": "/tmp/release-wheel-multi-arch-7-14-0/bin/python", + "pip_cache_dir": null, + "rocm_sdk": null, + "read_only": false, + "imported_from": null, + "installed_at_unix_ms": 1 + })) + .unwrap(); + assert_eq!( + old_cache.wheel_composition, None, + "a pre-composition manifest must still load" + ); + + let required = test_wheel_composition("gfx942"); + let target_runtime_key = wheel_runtime_key(TheRockChannel::Release, "7.14.0", &required); + + assert_eq!( + runtime_freshness(&old_cache, "7.14.0", Some(&required), &target_runtime_key), + RuntimeFreshness::RepairAvailable + ); + + old_cache.wheel_composition = Some(required.clone()); + assert_eq!( + runtime_freshness(&old_cache, "7.14.0", Some(&required), &target_runtime_key), + RuntimeFreshness::RepairAvailable, + "the right packages under the legacy identity still need side-by-side migration" + ); + + old_cache.runtime_key = target_runtime_key.clone(); + assert_eq!( + runtime_freshness(&old_cache, "7.14.0", Some(&required), &target_runtime_key), + RuntimeFreshness::UpToDate + ); + } + + #[test] + fn a_newer_index_version_outranks_a_composition_repair() { + let mut manifest = test_runtime_manifest( + "release-wheel-multi-arch-7-13-0", + "therock-release:gfx94X-dcgpu", + 1, + ); + manifest.version = "7.13.0".to_owned(); + let required = test_wheel_composition("gfx942"); + + assert_eq!( + runtime_freshness( + &manifest, + "7.14.0", + Some(&required), + &wheel_runtime_key(TheRockChannel::Release, "7.14.0", &required), + ), + RuntimeFreshness::UpdateAvailable + ); + } + + #[test] + fn an_ahead_of_index_runtime_is_never_offered_an_unreproducible_repair() { + // Its version is not in the index, so no install could reproduce it. A + // repair here would have to roll the runtime back to the older index + // build, which is exactly what `ahead_of_index` exists to prevent. + let mut pinned = test_runtime_manifest( + "release-wheel-multi-arch-7-15-0", + "therock-release:gfx94X-dcgpu", + 1, + ); + pinned.version = "7.15.0".to_owned(); + let required = test_wheel_composition("gfx942"); + + assert_eq!( + runtime_freshness( + &pinned, + "7.14.0", + Some(&required), + &wheel_runtime_key(TheRockChannel::Release, "7.14.0", &required), + ), + RuntimeFreshness::AheadOfIndex + ); + } + + #[test] + fn a_read_only_runtime_is_never_repaired() { + // `runtimes import` / `adopt` point at a folder this CLI does not own, + // so a side-by-side "replacement" would be an install the user never + // asked for, against packages they did not choose. + let mut adopted = + test_runtime_manifest("imported-rocm-7-14-0", "therock-release:gfx94X-dcgpu", 1); + adopted.version = "7.14.0".to_owned(); + adopted.read_only = true; + let required = test_wheel_composition("gfx942"); + + assert_eq!( + runtime_freshness( + &adopted, + "7.14.0", + Some(&required), + &wheel_runtime_key(TheRockChannel::Release, "7.14.0", &required), + ), + RuntimeFreshness::UpToDate + ); + } + + #[test] + fn an_installed_replacement_sibling_suppresses_repeat_repair() { + let required = test_wheel_composition("gfx942"); + let target_runtime_key = wheel_runtime_key(TheRockChannel::Release, "7.14.0", &required); + let mut source = test_runtime_manifest( + "release-wheel-multi-arch-7-14-0", + "therock-release:gfx94X-dcgpu", + 1, + ); + source.version = "7.14.0".to_owned(); + let mut replacement = + test_runtime_manifest(&target_runtime_key, "therock-release:gfx94X-dcgpu", 2); + replacement.version = "7.14.0".to_owned(); + replacement.wheel_composition = Some(required.clone()); + + let alone = vec![source.clone()]; + assert_eq!( + runtime_freshness_with_manifests( + &alone, + &source, + "7.14.0", + Some(&required), + &target_runtime_key, + ), + RuntimeFreshness::RepairAvailable, + "the legacy runtime alone still has to be replaced" + ); + + let migrated = vec![source.clone(), replacement.clone()]; + assert_eq!( + runtime_freshness_with_manifests( + &migrated, + &source, + "7.14.0", + Some(&required), + &target_runtime_key, + ), + RuntimeFreshness::UpToDate, + "the retained legacy manifest must not keep re-triggering the same repair" + ); + + // A sibling that merely shares the key without the composition is not the + // replacement: accepting it would strand the tree one repair short. + let mut impostor = replacement; + impostor.wheel_composition = Some(test_wheel_composition("gfx950")); + assert_eq!( + runtime_freshness_with_manifests( + &[source.clone(), impostor], + &source, + "7.14.0", + Some(&required), + &target_runtime_key, + ), + RuntimeFreshness::RepairAvailable + ); + } + #[test] fn aggregate_layout_without_device_payloads_is_rejected() { let error = validate_aggregate_index_layout( @@ -5472,11 +5913,61 @@ echo Python 3.12.10 } #[test] - fn aggregate_wheel_runtime_key_ignores_target_family() { + fn aggregate_wheel_runtime_key_separates_device_payloads_at_one_version() { + let base = WheelRuntimeComposition { + source_layout_generation: "multi-arch-v2".to_owned(), + package_specs: vec!["rocm[libraries,devel,device-gfx942]==7.14.0".to_owned()], + }; + let other_payload = WheelRuntimeComposition { + source_layout_generation: "multi-arch-v2".to_owned(), + package_specs: vec!["rocm[libraries,devel,device-gfx950]==7.14.0".to_owned()], + }; + let other_generation = WheelRuntimeComposition { + source_layout_generation: "multi-arch-v3".to_owned(), + package_specs: base.package_specs.clone(), + }; + + let base_key = wheel_runtime_key(TheRockChannel::Release, "7.14.0", &base); + + // Still names its channel and version: the retention policy and every + // human reading `runtimes list` group on that prefix. + assert!( + base_key.starts_with("release-wheel-multi-arch-7-14-0-"), + "{base_key}" + ); + assert_eq!( + base_key, + wheel_runtime_key(TheRockChannel::Release, "7.14.0", &base), + "the same composition must always name the same runtime" + ); + assert_ne!( + base_key, + wheel_runtime_key(TheRockChannel::Release, "7.14.0", &other_payload), + "a different device payload is a different runtime, not an overwrite" + ); + assert_ne!( + base_key, + wheel_runtime_key(TheRockChannel::Release, "7.14.0", &other_generation), + "a source-layout generation change must not reuse the old tree" + ); + } + + #[test] + fn recorded_composition_names_the_device_payload_it_installed() { + let composition = WheelRuntimeComposition { + source_layout_generation: "multi-arch-v2".to_owned(), + package_specs: vec![ + "rocm[libraries,devel,device-gfx1103]==7.14.1".to_owned(), + "torch==2.11.0+rocm7.14.1".to_owned(), + ], + }; + assert_eq!( - wheel_runtime_key(TheRockChannel::Release, "7.13.0a20260416"), - "release-wheel-multi-arch-7-13-0a20260416" + wheel_composition_device_target(Some(&composition)), + Some("gfx1103"), + "update planning must recover the installed target without probing the current host" ); + assert_eq!(wheel_composition_device_target(None), None); } #[test] @@ -6171,6 +6662,7 @@ echo Python 3.12.10 pip_cache_dir: None, rocm_sdk: None, sdk_torch: None, + wheel_composition: None, read_only: false, imported_from: None, installed_at_unix_ms, diff --git a/apps/rocmd/src/lib.rs b/apps/rocmd/src/lib.rs index 1e6385285..80fd64a1e 100644 --- a/apps/rocmd/src/lib.rs +++ b/apps/rocmd/src/lib.rs @@ -1212,21 +1212,35 @@ fn sandbox_check_updates_value(output: CommandCapture) -> Value { fn update_check_status(output: &CommandCapture) -> &'static str { if output.exit_status != 0 { "error" - } else if update_output_reports_update_available(&output.stdout) { + // A newer version outranks a composition repair: reporting the repair while + // some runtime is a whole release behind would understate the tree. + } else if update_output_reports_status(&output.stdout, "update_available") { "update_available" + } else if update_output_reports_status(&output.stdout, "repair_available") { + "repair_available" } else { "checked" } } +fn update_output_reports_status(stdout: &str, status: &str) -> bool { + let expected = format!("status={status}"); + stdout.split_whitespace().any(|part| part == expected) +} + fn update_output_reports_update_available(stdout: &str) -> bool { - stdout - .split_whitespace() - .any(|part| part == "status=update_available" || part == "update_available=true") + update_output_reports_status(stdout, "update_available") + || update_output_reports_status(stdout, "repair_available") + || stdout + .split_whitespace() + .any(|part| part == "update_available=true") } fn update_check_message(status: &str) -> &'static str { match status { + "repair_available" => { + "ran read-only `rocm update`; a ROCm runtime repair is available because its package composition changed; no updates were applied" + } "update_available" => { "ran read-only `rocm update`; a ROCm runtime update is available; no updates were applied" } @@ -3847,7 +3861,7 @@ where None, )?; if result.update_available { - record_update_available_notification(paths, state)?; + record_update_available_notification(paths, state, result.status)?; } } Err(error) => { @@ -3903,7 +3917,7 @@ fn restricted_check_updates_result(value: &Value) -> Result Result Result<()> { - let message = - "A ROCm runtime update is available. Preview it before applying. No updates were applied."; + let message = if status == "repair_available" { + "A ROCm runtime repair is available because its package composition changed. Preview it before applying. No updates were applied." + } else { + "A ROCm runtime update is available. Preview it before applying. No updates were applied." + }; record_event( paths, state, @@ -7918,7 +7936,7 @@ mod tests { }], }; - record_update_available_notification(&paths, &mut state)?; + record_update_available_notification(&paths, &mut state, "update_available")?; let audit_text = fs::read_to_string(paths.audit_events_path())?; let audit = audit_text @@ -8011,6 +8029,49 @@ mod tests { ); } + #[test] + fn sandbox_check_updates_value_marks_runtime_repair_available() { + let value = sandbox_check_updates_value(CommandCapture { + argv: vec!["rocm".to_owned(), "update".to_owned()], + exit_status: 0, + stdout: "update\n runtime release-wheel-multi-arch-7-14-0 status=repair_available installed=7.14.0 latest=7.14.0\n".to_owned(), + stderr: String::new(), + }); + + assert_eq!( + value.get("status").and_then(Value::as_str), + Some("repair_available") + ); + // The watcher's notify decision reads this flag, so a repair the user + // has to apply by hand must not be reported as nothing to do. + assert_eq!( + value.get("update_available").and_then(Value::as_bool), + Some(true) + ); + assert!( + value + .get("message") + .and_then(Value::as_str) + .is_some_and(|message| message.contains("runtime repair is available") + && message.contains("no updates were applied")) + ); + } + + #[test] + fn a_newer_version_outranks_a_composition_repair_in_the_watcher_report() { + let value = sandbox_check_updates_value(CommandCapture { + argv: vec!["rocm".to_owned(), "update".to_owned()], + exit_status: 0, + stdout: "update\n runtime old status=repair_available installed=7.14.0 latest=7.14.0\n runtime stale status=update_available installed=7.13.0 latest=7.14.0\n".to_owned(), + stderr: String::new(), + }); + + assert_eq!( + value.get("status").and_then(Value::as_str), + Some("update_available") + ); + } + #[test] fn sandbox_driver_plan_value_is_read_only_and_preserves_output() { let value = sandbox_driver_plan_value(CommandCapture { diff --git a/docs/ci-hardware-testing.md b/docs/ci-hardware-testing.md index 8cd225fc2..7942a434f 100644 --- a/docs/ci-hardware-testing.md +++ b/docs/ci-hardware-testing.md @@ -152,7 +152,10 @@ gh workflow run e2e-selfhosted.yml --ref -f platform=app-dev-gpu Nearly every GPU serve scenario points its `data/runtimes` at one shared, pre-warmed managed runtime tree (`E2E_SHARED_RUNTIMES_DIR`), so a multi-GiB `rocm install sdk` happens once per runner instead of once per scenario. The tree -lives on the runner's persistent workspace and survives `git clean`. +lives on the runner's persistent workspace, survives `git clean`, and is namespaced +by source-layout generation (`e2e-prewarm-multi-arch-v2`) so a branch using a new +package layout cannot poison the cache consumed by code that only understands the +previous layout. The tree may hold **more than one** runtime — the pre-warm installs a newer one side by side when the channel index publishes it (below) — so scenarios must not @@ -163,24 +166,39 @@ tree's own `active.json`, which lives inside the shared tree and is therefore visible through the symlink. Without that, a serve fails with `no active ROCm runtime is configured` while the precondition still passes. -It is a **cache with invalidation**, not a one-shot install. Each self-hosted lane calls +It is a **cache with invalidation and repair**, not a one-shot install. Each +self-hosted lane calls: ```bash cargo xtask e2e-prewarm --channel release --prewarm-dir "$prewarm" ``` -before the suite, which asks `rocm update` whether the channel index has published -a newer version and then: +before the suite. `rocm update` compares both the channel version and the wheel +composition recorded in the runtime manifest (source-layout generation and exact +pinned package specs, including the `device-` payload). A deterministic +composition fingerprint is part of each wheel runtime key, so a corrected +composition is installed beside — never over — the old environment. Each report +line also carries `target=`: the runtime key an apply from that line would +produce, which on a superseded manifest is its already-installed replacement. +Pre-warm then: - installs the SDK when nothing is present for that channel; -- installs the newer runtime **side-by-side** and activates it +- installs a newer runtime **side-by-side** and activates it (`rocm update --apply --runtime --activate`) when the index is ahead; +- replaces a same-version runtime side-by-side when its manifest has an older or + missing wheel composition, then activates the composition-keyed replacement; +- treats that repair as complete while the matching replacement remains installed, + so retained legacy manifests do not trigger repeated repairs or notifications; +- activates the runtime a reuse actually means — after a repair that is the + replacement named by `target=`, not the superseded manifest the line belongs to; +- ensures the default engine is installed even when the runtime itself is reused; - reuses the existing tree when it is `up_to_date`, when it is `ahead_of_index` - (a pinned build newer than the index must not be rolled back), or when freshness + (a pinned build newer than the index must not be rolled back and cannot be + reproduced from the index, so it is never offered a repair), or when freshness cannot be established at all — an unreachable index reuses and warns rather than re-downloading gigabytes or failing the lane; -- prunes with `rocm storage remove-old-installs` after any install or update, so - the multi-version cache stays bounded. +- prunes with `rocm storage remove-old-installs` after any install, update, or + repair, so the multi-version cache stays bounded. The runtime is always installed **in place**: `install sdk` bakes absolute paths into the runtime manifest, so a tree that is moved after installation leaves every diff --git a/tests/e2e-cucumber/src/capability.rs b/tests/e2e-cucumber/src/capability.rs index de2197cff..90bc4f2bb 100644 --- a/tests/e2e-cucumber/src/capability.rs +++ b/tests/e2e-cucumber/src/capability.rs @@ -244,6 +244,19 @@ fn active_runtime_install_root( Some((version, root)) } +/// Select the canonical aggregate wheel runtime from `rocm runtimes list` output. +/// +/// The list is newest-first, so the first matching key is the runtime installed +/// by the pre-warm refresh when legacy family-keyed entries coexist with it. +/// Matching on the `-wheel-multi-arch-` infix rather than a whole key is what +/// keeps this working once the key carries a composition fingerprint. +pub fn canonical_wheel_runtime_key(inventory: &str) -> Option<&str> { + inventory.lines().find_map(|line| { + line.split_whitespace() + .find(|field| field.contains("-wheel-multi-arch-")) + }) +} + /// Parse the vLLM version from the `vllm-.dist-info` directory in the /// runtime venv's site-packages (works without importing vllm). /// @@ -544,6 +557,26 @@ fn os_normalized(os_family: &str) -> String { mod tests { use super::*; + #[test] + fn canonical_wheel_runtime_key_ignores_markers_and_legacy_entries() { + let inventory = "registered ROCm runtimes\n active_runtime_key: \n installed:\n release-wheel-gfx94x-dcgpu-7-13-0 runtime_id=therock-release:gfx94X-dcgpu\n * release-wheel-multi-arch-7-14-0-0123456789abcdef runtime_id=therock-release:gfx94X-dcgpu\n"; + assert_eq!( + canonical_wheel_runtime_key(inventory), + Some("release-wheel-multi-arch-7-14-0-0123456789abcdef"), + "the `* ` active marker is a separate field and must not be taken for the key" + ); + } + + #[test] + fn canonical_wheel_runtime_key_returns_none_without_canonical_entry() { + assert_eq!( + canonical_wheel_runtime_key( + " release-wheel-gfx94x-dcgpu-7-13-0 runtime_id=therock-release:gfx94X-dcgpu" + ), + None + ); + } + // Drift guard (decision #1): these pin the re-implemented rule to the // product's known behaviour. When task #16 lands a product probe field, // this same table becomes the consistency check (harness rule == probe). diff --git a/tests/e2e-cucumber/tests/e2e/runtime_steps.rs b/tests/e2e-cucumber/tests/e2e/runtime_steps.rs index 6cf3ee9ee..badc65f72 100644 --- a/tests/e2e-cucumber/tests/e2e/runtime_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/runtime_steps.rs @@ -128,6 +128,8 @@ async fn setup_active_runtime(world: &mut E2eWorld) { let (stdout, _, _) = crate::run_rocm(world, &["runtimes", "list"]); if stdout.contains("installed: none") { crate::run_rocm_ok(world, &["install", "sdk"]); + } else { + activate_shared_runtime_if_unset(world, &stdout); } // Name the runtime rather than leaving the CLI to infer it: the shared tree // grows a second runtime whenever the channel index publishes one, and the @@ -137,7 +139,7 @@ async fn setup_active_runtime(world: &mut E2eWorld) { world.activate_shared_runtime(); let (stdout, _, _) = crate::run_rocm(world, &["runtimes", "list"]); assert!( - !stdout.contains("installed: none"), + !stdout.contains("active_runtime_key: "), "no managed runtime is active:\n{stdout}" ); } @@ -152,6 +154,8 @@ async fn setup_runtime_with_engine(world: &mut E2eWorld) { let (stdout, _, _) = crate::run_rocm(world, &["runtimes", "list"]); if stdout.contains("installed: none") { crate::run_rocm_ok(world, &["install", "sdk"]); + } else { + activate_shared_runtime_if_unset(world, &stdout); } // Same reason as `a managed runtime is active`: pin the runtime explicitly, // or the serve that follows refuses to pick one. Not for `assert_engine_ready` @@ -161,6 +165,26 @@ async fn setup_runtime_with_engine(world: &mut E2eWorld) { assert_engine_ready(world); } +/// Point a pre-warmed shared tree at its canonical runtime when nothing is +/// active yet. +/// +/// The pre-warm activates what it installs, but the marker lives in the shared +/// tree while `active_runtime_key` is read per scenario, and a repaired tree +/// holds a superseded runtime beside its replacement. Left unset, the CLI +/// refuses to auto-select from more than one runtime and every serve behind this +/// precondition fails for a reason that names none of this. +fn activate_shared_runtime_if_unset(world: &mut E2eWorld, runtimes: &str) { + if !runtimes.contains("active_runtime_key: ") { + return; + } + let runtime_key = e2e_cucumber::capability::canonical_wheel_runtime_key(runtimes) + .unwrap_or_else(|| { + panic!("shared runtime tree has no canonical wheel runtime:\n{runtimes}") + }) + .to_owned(); + crate::run_rocm_ok(world, &["runtimes", "activate", &runtime_key]); +} + /// Record the torch-alignment opt-out for this scenario's next `rocm` command. /// /// A behavioural precondition rather than a mechanism the feature file has to @@ -713,7 +737,13 @@ fn active_runtime_key(world: &E2eWorld) -> Option { /// form used when the index cannot be reached. `xtask e2e-prewarm` routes on /// exactly these, so a rename here must break this scenario rather than silently /// turn every pre-warm into a no-op reuse. -const UPDATE_STATUSES: [&str; 4] = ["up_to_date", "update_available", "ahead_of_index", "error"]; +const UPDATE_STATUSES: [&str; 5] = [ + "up_to_date", + "update_available", + "repair_available", + "ahead_of_index", + "error", +]; #[then("the report states the runtime's freshness against the channel index")] async fn assert_update_reports_freshness(world: &mut E2eWorld) { diff --git a/xtask/src/e2e_prewarm.rs b/xtask/src/e2e_prewarm.rs index ccb8a90ce..c527476b2 100644 --- a/xtask/src/e2e_prewarm.rs +++ b/xtask/src/e2e_prewarm.rs @@ -21,16 +21,24 @@ //! shared tree and what a fresh install produces was therefore untested, and //! widened silently. //! -//! This keeps the cache and invalidates it only when the channel index actually -//! publishes something newer, reusing the primitives the CLI already ships -//! rather than reimplementing version resolution in workflow shell: +//! This keeps the cache and invalidates it when either the channel index +//! publishes something newer or the package composition the CLI now requires +//! differs from the one the installed runtime records, reusing the primitives +//! the CLI already ships rather than reimplementing resolution in workflow +//! shell: //! //! * `rocm update` reports, per installed runtime, `status=up_to_date | -//! update_available | ahead_of_index` by comparing against the channel index. -//! * `rocm update --apply --runtime --activate` installs the newer runtime -//! SIDE BY SIDE and makes it the default. Side-by-side matters: `install sdk` -//! bakes ABSOLUTE paths into the runtime manifest, so a runtime must be created -//! in its final location and never moved afterwards. +//! update_available | repair_available | ahead_of_index` by comparing both the +//! channel version and the recorded wheel composition, plus `target=`: +//! the runtime key an apply from that line would produce. +//! * `rocm update --apply --runtime --activate` installs the newer or +//! composition-corrected runtime SIDE BY SIDE and makes it the default. +//! Side-by-side matters: `install sdk` bakes ABSOLUTE paths into the runtime +//! manifest, so a runtime must be created in its final location and never +//! moved afterwards. +//! * `rocm runtimes activate ` points the tree at the runtime a reuse +//! actually meant, which after a repair is not the manifest the report line +//! belongs to. //! * `rocm storage remove-old-installs --keep N` bounds the resulting multi-version //! cache with a per-channel/format/family retention policy. //! @@ -56,9 +64,21 @@ pub enum Decision { /// The channel index has a newer version than the installed one; install it /// alongside and activate it. Update { runtime_key: String }, + /// The version is current, but its recorded package composition is stale or + /// absent. Install a composition-keyed replacement alongside it. + Repair { runtime_key: String }, /// The tree is current, or its freshness could not be established. Serve /// against what is already there. - Reuse { reason: String }, + /// + /// `activate` names the runtime that reuse means, when the report identified + /// one. A tree that has already been repaired holds BOTH the superseded + /// legacy runtime and its replacement, and only the replacement can run this + /// host's kernels — so reuse has to say which, or the lane serves whichever + /// one the active pointer happens to hold. + Reuse { + reason: String, + activate: Option, + }, } /// Decide from a `rocm update` report what the pre-warm should do for `channel`. @@ -101,6 +121,7 @@ pub fn decide(update_report: &str, channel: &str) -> Decision { return Decision::Reuse { reason: "could not establish runtime freshness; leaving the shared tree untouched" .to_owned(), + activate: None, }; } @@ -109,37 +130,66 @@ pub fn decide(update_report: &str, channel: &str) -> Decision { return Decision::Install; } - if let Some(stale) = runtimes + let channel_runtimes = runtimes .iter() .filter(|line| line.channel.as_deref() == Some(channel)) - .find(|line| line.status.as_deref() == Some("update_available")) + .collect::>(); + + // A current composition already in the tree satisfies the lane even while an + // obsolete same-channel entry survives until the retention pass removes it. + // Checked FIRST for exactly that reason: the legacy entry is the one that + // would otherwise be repaired, again, on every single run. + if let Some(current) = channel_runtimes + .iter() + .find(|line| line.status.as_deref() == Some("up_to_date")) { - return Decision::Update { - runtime_key: stale.runtime_key.clone(), + return Decision::Reuse { + reason: "runtime is up to date with the channel index".to_owned(), + // Its own key when it is the current runtime; its replacement's key + // when it is the superseded manifest that the repair left behind. + activate: Some(current.serving_runtime_key().to_owned()), }; } // `ahead_of_index` means the installed runtime is NEWER than anything the - // index offers (a hand-placed or pinned build). Reuse it rather than - // "updating" backwards. - let reason = if runtimes + // index offers (a hand-placed or pinned build). The index cannot reproduce + // it, so neither an update nor a repair could do anything but roll it back. + // Serve it as it stands. + if let Some(pinned) = channel_runtimes .iter() - .filter(|line| line.channel.as_deref() == Some(channel)) - .any(|line| line.status.as_deref() == Some("up_to_date")) + .find(|line| line.status.as_deref() == Some("ahead_of_index")) { - "runtime is up to date with the channel index" - } else if runtimes + return Decision::Reuse { + reason: "installed runtime is ahead of the channel index".to_owned(), + activate: Some(pinned.runtime_key.clone()), + }; + } + + if let Some(stale) = channel_runtimes .iter() - .filter(|line| line.channel.as_deref() == Some(channel)) - .any(|line| line.status.as_deref() == Some("ahead_of_index")) + .find(|line| line.status.as_deref() == Some("update_available")) { - "installed runtime is ahead of the channel index" - } else { - // Only `status=error` (or a shape this does not recognise) remains. - "could not establish runtime freshness; leaving the shared tree untouched" - }; + return Decision::Update { + runtime_key: stale.runtime_key.clone(), + }; + } + + if let Some(stale) = channel_runtimes + .iter() + .find(|line| line.status.as_deref() == Some("repair_available")) + { + return Decision::Repair { + runtime_key: stale.runtime_key.clone(), + }; + } + + // Only `status=error` (or a shape this does not recognise) remains. Naming a + // runtime to activate here would be a guess about a tree whose state could + // not be read at all. Decision::Reuse { - reason: reason.to_owned(), + reason: "could not establish runtime freshness; leaving the shared tree untouched" + .to_owned(), + activate: None, } } @@ -332,17 +382,22 @@ pub fn activation_candidates(runtimes_list_report: &str) -> Vec { .collect() } -/// One ` runtime format=… channel=… … status=…` line from `rocm update`. +/// One ` runtime format=… channel=… … status=… target=…` line from +/// `rocm update`. /// /// Both shapes that renderer emits are handled: the full report line, and the /// degraded `runtime format=… status=error message=…` line. `message=` is -/// free text that may contain spaces, but it is last and only `channel`/`status` -/// are read, so a whitespace split is sufficient — trailing words of the message -/// simply carry no `=` and are ignored. +/// free text that may contain spaces, but it is last and only +/// `channel`/`status`/`target` are read, so a whitespace split is sufficient — +/// trailing words of the message simply carry no `=` and are ignored. struct RuntimeLine { runtime_key: String, channel: Option, status: Option, + /// The runtime key an apply from this line would produce. On a superseded + /// legacy manifest this names its already-installed replacement, which is + /// the sibling the lane must actually serve against. + target_runtime_key: Option, } impl RuntimeLine { @@ -352,10 +407,12 @@ impl RuntimeLine { let runtime_key = fields.next()?.to_owned(); let mut channel = None; let mut status = None; + let mut target_runtime_key = None; for field in fields { match field.split_once('=') { Some(("channel", value)) => channel = Some(value.to_owned()), Some(("status", value)) => status = Some(value.to_owned()), + Some(("target", value)) => target_runtime_key = Some(value.to_owned()), _ => {} } } @@ -363,12 +420,20 @@ impl RuntimeLine { runtime_key, channel, status, + target_runtime_key, }) } + + /// The runtime this line says the lane should be serving against. + fn serving_runtime_key(&self) -> &str { + self.target_runtime_key + .as_deref() + .unwrap_or(&self.runtime_key) + } } -/// Bring the shared pre-warm tree at `prewarm_dir` to the newest runtime the -/// `channel` index offers, keeping `keep` recent installs per channel/format/family. +/// Bring the shared pre-warm tree at `prewarm_dir` to the runtime state the +/// `channel` requires, keeping `keep` recent installs per channel/format/family. pub fn run(channel: &str, keep: usize, prewarm_dir: &Path) -> Result<()> { let rocm = resolve_rocm_binary()?; for sub in ["config", "data", "cache"] { @@ -400,6 +465,7 @@ pub fn run(channel: &str, keep: usize, prewarm_dir: &Path) -> Result<()> { println!("pre-warm: `rocm update` failed ({error:#}); reusing the existing tree"); Decision::Reuse { reason: "update probe failed".to_owned(), + activate: None, } } else { println!("pre-warm: `rocm update` failed ({error:#}); no registry yet, installing"); @@ -426,8 +492,29 @@ pub fn run(channel: &str, keep: usize, prewarm_dir: &Path) -> Result<()> { .args(["update", "--apply", "--runtime", runtime_key, "--activate"]) .status_ok("rocm update --apply")?; } - Decision::Reuse { reason } => { + Decision::Repair { runtime_key } => { + println!( + "pre-warm: {runtime_key} has a stale package composition; installing its replacement alongside it" + ); + rocm_command(&rocm, prewarm_dir) + .args(["update", "--apply", "--runtime", runtime_key, "--activate"]) + .status_ok("rocm update --apply")?; + } + Decision::Reuse { reason, activate } => { println!("pre-warm: reusing the shared {channel} runtime ({reason})"); + // Reuse is not "leave the pointer alone". Once a repair has run, the + // tree holds the superseded runtime AND its replacement, and only + // the replacement can run this host's kernels — so name the one this + // reuse actually meant. `runtimes activate` is idempotent, so the + // ordinary warm case just reasserts what is already active. Before + // the engine check below, because that installs into whichever + // runtime is active. + if let Some(runtime_key) = activate { + println!("pre-warm: activating {runtime_key}"); + rocm_command(&rocm, prewarm_dir) + .args(["runtimes", "activate", runtime_key]) + .status_ok("rocm runtimes activate")?; + } } } @@ -447,7 +534,7 @@ pub fn run(channel: &str, keep: usize, prewarm_dir: &Path) -> Result<()> { return Ok(()); } - // An install/update that exits 0 without leaving a registry behind is the + // An install/update/repair that exits 0 without leaving a registry behind is the // confusing case the lanes used to call out by hand: every scenario then falls // back to installing its own runtime and the job quietly blows its time cap. // Say so loudly, but do not fail — the suite can still run. @@ -459,7 +546,7 @@ pub fn run(channel: &str, keep: usize, prewarm_dir: &Path) -> Result<()> { ); } - // Only reached after an install or update actually added a tree. Housekeeping: + // Only reached after an install, update, or repair added a tree. Housekeeping: // a failure here wastes disk but leaves a correct runtime in place, so it must // not fail the lane. let pruned = rocm_command(&rocm, prewarm_dir) @@ -795,10 +882,20 @@ update "; fn report(status: &str, channel: &str) -> String { + report_with_target( + status, + channel, + &format!("{channel}-wheel-gfx94x-dcgpu-7-13-0"), + ) + } + + /// A report line whose `target=` names a runtime other than its own key — + /// what a superseded legacy manifest renders once its replacement exists. + fn report_with_target(status: &str, channel: &str, target: &str) -> String { format!( "update\n policy: bounded startup check, cached metadata, prompt before mutating state.\n \ runtime {channel}-wheel-gfx94x-dcgpu-7-13-0 format=wheel channel={channel} \ -family=gfx94X-dcgpu installed=7.13.0 latest=7.15.0 status={status}\n \ +family=gfx94X-dcgpu installed=7.13.0 latest=7.15.0 status={status} target={target}\n \ install_root: /w/e2e-prewarm/data/runtimes/wheel/{channel}-wheel-gfx94x-dcgpu-7-13-0\n \ source: index\n" ) @@ -1088,12 +1185,16 @@ Local model engines // skip the engine. Reuse must still install and update NOTHING, which is // what keeps it cheap enough to re-check the engine on every run. assert!(!runtime_changed(&Decision::Reuse { - reason: "up to date".to_owned() + reason: "up to date".to_owned(), + activate: None, })); assert!(runtime_changed(&Decision::Install)); assert!(runtime_changed(&Decision::Update { runtime_key: "release-wheel-gfx94x-dcgpu-7-13-0".to_owned() })); + assert!(runtime_changed(&Decision::Repair { + runtime_key: "release-wheel-gfx94x-dcgpu-7-13-0".to_owned() + })); } #[test] @@ -1112,22 +1213,91 @@ Local model engines } #[test] - fn current_runtime_is_reused() { - let Decision::Reuse { reason } = decide(&report("up_to_date", "release"), "release") else { + fn same_version_runtime_with_a_stale_composition_is_repaired() { + assert_eq!( + decide(&report("repair_available", "release"), "release"), + Decision::Repair { + runtime_key: "release-wheel-gfx94x-dcgpu-7-13-0".to_owned() + } + ); + } + + #[test] + fn an_installed_replacement_stops_the_repair_repeating() { + // The retention pass has not yet dropped the legacy manifest, so the + // report still carries its `repair_available` line beside the current + // one. Repairing again would reinstall a runtime already in the tree, + // every run, forever. + let text = format!( + "{}{}", + report("repair_available", "release"), + report("up_to_date", "release") + ); + + let Decision::Reuse { reason, .. } = decide(&text, "release") else { + panic!("an installed current composition must prevent repeated repair"); + }; + assert!(reason.contains("up to date"), "{reason}"); + } + + #[test] + fn current_runtime_is_reused_and_activated() { + let Decision::Reuse { reason, activate } = + decide(&report("up_to_date", "release"), "release") + else { panic!("an up-to-date runtime must be reused, not reinstalled"); }; assert!(reason.contains("up to date"), "{reason}"); + assert_eq!( + activate.as_deref(), + Some("release-wheel-gfx94x-dcgpu-7-13-0"), + "reuse must name the runtime it means, or the lane serves whatever the pointer holds" + ); + } + + #[test] + fn reuse_after_a_repair_activates_the_replacement_not_the_superseded_runtime() { + // The line belongs to the superseded legacy manifest: its own key is the + // old one, and `target=` names the composition-keyed replacement the + // repair already installed. Activating the line's own key here would + // serve the runtime whose device payload cannot run this host's kernels. + let Decision::Reuse { activate, .. } = decide( + &report_with_target( + "up_to_date", + "release", + "release-wheel-multi-arch-7-13-0-0123456789abcdef", + ), + "release", + ) else { + panic!("a migrated tree must be reused"); + }; + assert_eq!( + activate.as_deref(), + Some("release-wheel-multi-arch-7-13-0-0123456789abcdef") + ); } #[test] fn runtime_ahead_of_the_index_is_not_downgraded() { // A pinned or hand-placed build newer than the index must be left alone — - // "updating" it would move the lane backwards. - let Decision::Reuse { reason } = decide(&report("ahead_of_index", "release"), "release") - else { + // "updating" it would move the lane backwards. It is also the one case + // where `target=` must be ignored: the index cannot reproduce this build, + // so the key an apply would produce names a runtime that is not there. + let Decision::Reuse { reason, activate } = decide( + &report_with_target( + "ahead_of_index", + "release", + "release-wheel-multi-arch-7-15-0-deadbeefdeadbeef", + ), + "release", + ) else { panic!("a runtime ahead of the index must be reused"); }; assert!(reason.contains("ahead of"), "{reason}"); + assert_eq!( + activate.as_deref(), + Some("release-wheel-gfx94x-dcgpu-7-13-0") + ); } #[test] @@ -1137,7 +1307,7 @@ Local model engines // conservative pre-warm decision must reuse rather than download again. let text = "update\n runtime release-wheel-gfx94x-dcgpu-7-13-0 format=wheel \ status=error message=failed to reach https://repo.amd.com/rocm/whl after 3 tries\n"; - let Decision::Reuse { reason } = decide(text, "release") else { + let Decision::Reuse { reason, .. } = decide(text, "release") else { panic!("an unattributable index error must reuse the existing tree"); }; assert!(reason.contains("could not establish"), "{reason}"); @@ -1157,7 +1327,7 @@ status=error message=failed to reach the index\n", fn index_error_on_an_attributable_line_is_reused() { let text = "update\n runtime release-wheel-gfx94x-dcgpu-7-13-0 format=wheel \ channel=release status=error message=failed to reach the index\n"; - let Decision::Reuse { reason } = decide(text, "release") else { + let Decision::Reuse { reason, .. } = decide(text, "release") else { panic!("an unreadable freshness status must reuse the existing tree"); }; assert!(reason.contains("could not establish"), "{reason}"); @@ -1193,7 +1363,7 @@ channel=release status=error message=failed to reach the index\n"; #[test] fn unparseable_report_reuses() { - let Decision::Reuse { reason } = decide( + let Decision::Reuse { reason, .. } = decide( "update\n runtime weird-key format=wheel channel=release\n", "release", ) else { @@ -1210,6 +1380,10 @@ message=connect timed out after 30 s"; assert_eq!(parsed.runtime_key, "k"); assert_eq!(parsed.channel.as_deref(), Some("release")); assert_eq!(parsed.status.as_deref(), Some("error")); + // A degraded line has no plan, so it names no target. Falling back to + // the line's own key keeps the reuse path from activating nothing. + assert_eq!(parsed.target_runtime_key, None); + assert_eq!(parsed.serving_runtime_key(), "k"); } #[test] diff --git a/xtask/src/workflow_contract.rs b/xtask/src/workflow_contract.rs index bf150f421..05b8bc87d 100644 --- a/xtask/src/workflow_contract.rs +++ b/xtask/src/workflow_contract.rs @@ -1076,6 +1076,28 @@ trigger-a-workflow#triggering-a-workflow-from-a-workflow" assert_prebuilt_e2e_lanes_enable_test_hooks("nightly.yml", &workflow); } + #[test] + fn gpu_prewarm_caches_are_namespaced_by_source_layout() { + // The shared tree survives `git clean` and every branch on the runner. + // A branch that composes runtimes under a new source layout would + // otherwise leave keys and manifests in that tree which code on the + // previous layout cannot read, poisoning the lanes it never touched. + for workflow_name in ["e2e-selfhosted.yml", "nightly.yml"] { + let workflow = read_workflow(workflow_name); + assert!( + workflow.contains("e2e-prewarm-multi-arch-v2"), + "{workflow_name} must isolate the canonical multi-arch runtime tree" + ); + assert!( + !workflow.lines().any(|line| { + line.trim_end().ends_with("e2e-prewarm\"") + || line.trim_end().ends_with("e2e-prewarm'") + }), + "{workflow_name} still uses the generation-agnostic pre-warm tree" + ); + } + } + // Extractor guards: prove the helpers actually parse multiline forms, so the // contract tests above can't silently false-pass on a shape they don't handle. #[test] From caccb0f06adc41a4422dc3d65998d87cd30ce6f6 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Tue, 8 Sep 2026 14:39:07 -0700 Subject: [PATCH 09/14] fix(runtime): preserve exact composition on repair Signed-off-by: Michael Roy --- apps/rocm/src/main.rs | 2 +- apps/rocm/src/therock.rs | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index 910a5860e..cf7050717 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -30053,7 +30053,7 @@ ID_LIKE="suse opensuse" "gfx120X-all", "7.14.0", ); - let manifests = vec![stale.clone(), wrong_family, repaired.clone()]; + let manifests = vec![stale, wrong_family, repaired.clone()]; let selected = select_installed_update_runtime(&manifests, &repaired.runtime_key) .expect("the side-by-side repair must be selected by its exact key"); diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 693cb1a1a..aa238ee25 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -247,6 +247,7 @@ enum AggregateDeviceTarget { struct ResolvedAggregateWheelSource { index_url: &'static str, device_target: AggregateDeviceTarget, + published_device_targets: Vec, } /// Stands in for a real target in a preview, so a plan that cannot be installed @@ -324,6 +325,8 @@ struct PipRuntimeResolution { /// The device payload the canonical source must supply for this host, /// decided against the targets that source actually publishes. device_target: AggregateDeviceTarget, + /// Exact device payloads advertised by the canonical aggregate source. + published_device_targets: Vec, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -979,14 +982,16 @@ fn resolve_latest_for_manifest( // produce, and re-probing would disagree with the installed runtime on // any host whose GPU is absent, hidden, or simply a second card. let device_target = - wheel_composition_device_target(manifest.wheel_composition.as_ref()) - .and_then(|target| { - canonical_aggregate_device_target(target, &resolution.family) - }) - .map_or_else( - || resolution.device_target.clone(), - AggregateDeviceTarget::Exact, - ); + wheel_composition_device_target(manifest.wheel_composition.as_ref()).map_or_else( + || resolution.device_target.clone(), + |target| { + AggregateDeviceTarget::resolve( + Some(target), + &resolution.family, + &resolution.published_device_targets, + ) + }, + ); // No exact target means no reproducible composition, so freshness // falls back to the version comparison rather than demanding a repair // this host could not perform. @@ -1629,13 +1634,15 @@ fn resolve_pip_runtime_with_timeout( source.wheel_index ) })?; + let published_device_targets = parse_aggregate_device_targets(&root_html); let source = ResolvedAggregateWheelSource { index_url: source.wheel_index, device_target: AggregateDeviceTarget::resolve( detect_host_gfx_target().as_deref(), &family_resolution.family, - &parse_aggregate_device_targets(&root_html), + &published_device_targets, ), + published_device_targets, }; resolve_pip_runtime_from_index( paths, @@ -1721,6 +1728,7 @@ fn resolve_pip_runtime_from_index( latest_version, package_versions, device_target: source.device_target.clone(), + published_device_targets: source.published_device_targets.clone(), }) } From 7fa847cc32c1d712ec6212ae1422a356f0cb79ef Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Tue, 8 Sep 2026 14:46:57 -0700 Subject: [PATCH 10/14] fix(runtime): apply the planned device composition Signed-off-by: Michael Roy --- apps/rocm/src/main.rs | 19 ++---- apps/rocm/src/therock.rs | 81 +++++++++++++++++++++--- tests/e2e-cucumber/src/shared_runtime.rs | 30 +++++++-- 3 files changed, 102 insertions(+), 28 deletions(-) diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index cf7050717..1626eb3e2 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -15823,11 +15823,6 @@ fn apply_runtime_update( let manifests = therock::load_runtime_manifests(paths)?; let source = select_runtime_update_source(&manifests, config, runtime_selector)?; let plan = therock::runtime_update_plan(paths, source, &manifests)?; - // The plan resolved the index for the SOURCE runtime's family; letting the - // install re-detect one would compose a different runtime than the plan just - // predicted whenever this host's GPU disagrees with the runtime being - // updated, and the key lookup below would then find nothing. - let family_override = Some(source.family.as_str()); let mut output = String::new(); let _ = writeln!(output, "runtime update"); let _ = writeln!(output, " source_runtime_key: {}", source.runtime_key); @@ -15855,13 +15850,12 @@ fn apply_runtime_update( if dry_run { let _ = writeln!(output, " mode: dry-run"); - let install_plan = therock::install_sdk( + let install_plan = therock::install_sdk_for_update( paths, &source.channel, &source.format, - None, - None, - family_override, + &source.family, + plan.device_target.as_deref(), true, )?; let _ = writeln!(output, " install_plan:"); @@ -15871,13 +15865,12 @@ fn apply_runtime_update( return Ok(output); } - let install_output = therock::install_sdk( + let install_output = therock::install_sdk_for_update( paths, &source.channel, &source.format, - None, - None, - family_override, + &source.family, + plan.device_target.as_deref(), false, )?; let manifests_after = therock::load_runtime_manifests(paths)?; diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index aa238ee25..797d39519 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -438,6 +438,8 @@ pub(crate) struct RuntimeUpdatePlan { /// the resulting manifest by this key rather than by version, because a /// same-version repair produces a sibling that version alone cannot name. pub target_runtime_key: String, + /// Exact device payload encoded in the planned wheel composition. + pub device_target: Option, pub repair_required: bool, pub update_available: bool, } @@ -729,6 +731,7 @@ pub(crate) fn install_sdk( channel, prefix, family_override, + None, version_selector.as_ref(), dry_run, ), @@ -742,6 +745,32 @@ pub(crate) fn install_sdk( } } +/// Apply an update using the exact family and device payload resolved by its plan. +pub(crate) fn install_sdk_for_update( + paths: &AppPaths, + channel: &str, + format: &str, + family: &str, + device_target: Option<&str>, + dry_run: bool, +) -> Result { + let channel = TheRockChannel::parse(channel)?; + ensure_install_format_supported(format)?; + match format { + "wheel" => install_wheel_runtime( + paths, + channel, + None, + Some(family), + device_target, + None, + dry_run, + ), + "tarball" => install_tarball_runtime(paths, channel, None, Some(family), dry_run), + other => bail!("unsupported install format: {other}"), + } +} + fn ensure_install_format_supported(format: &str) -> Result<()> { ensure_install_format_supported_for_platform(format, runtime_is_windows()) } @@ -865,6 +894,7 @@ fn replacement_runtime_is_installed( && candidate.format == source.format && candidate.family == source.family && candidate.wheel_composition.as_ref() == Some(required_composition) + && has_nontrivial_directory_contents(&candidate.install_root).unwrap_or(false) }) } @@ -935,12 +965,18 @@ pub(crate) fn runtime_update_plan( latest.wheel_composition.as_ref(), &latest.target_runtime_key, ); + let device_target = latest + .wheel_composition + .as_ref() + .and_then(|composition| wheel_composition_device_target(Some(composition))) + .map(str::to_owned); Ok(RuntimeUpdatePlan { latest_version: latest.latest_version, latest_source: latest.latest_source, format: latest.format, status: freshness.status().to_owned(), target_runtime_key: latest.target_runtime_key, + device_target, repair_required: freshness == RuntimeFreshness::RepairAvailable, update_available: freshness.update_available(), }) @@ -1181,6 +1217,7 @@ fn install_wheel_runtime( channel: TheRockChannel, prefix: Option, family_override: Option<&str>, + device_target_override: Option<&str>, version_selector: Option<&RuntimeVersionSelector>, dry_run: bool, ) -> Result { @@ -1219,13 +1256,22 @@ fn install_wheel_runtime( &wheel_compatibility, version_selector, )?; + let device_target = device_target_override.map_or_else( + || resolution.device_target.clone(), + |target| { + AggregateDeviceTarget::resolve( + Some(target), + &resolution.family, + &resolution.published_device_targets, + ) + }, + ); // The exact device payload, not the version alone, decides what this runtime // can run, so it is what identifies the runtime. A preview on a host with no // usable target still composes a key here — from the `` extras // — which no real install can ever produce, and the refusal below stops it // from reaching a manifest. - let wheel_composition = - wheel_runtime_composition(&resolution, &resolution.device_target.rocm_extras()); + let wheel_composition = wheel_runtime_composition(&resolution, &device_target.rocm_extras()); progress_line(format!( "Found canonical TheRock aggregate version {} with a matching PyTorch stack for target family {}.", resolution.latest_version, resolution.family @@ -1258,12 +1304,8 @@ fn install_wheel_runtime( " target_family_source: {}", resolution.family_source ); - let _ = writeln!( - output, - " device_target: {}", - resolution.device_target.as_str() - ); - if let Some(reason) = resolution.device_target.reason() { + let _ = writeln!(output, " device_target: {}", device_target.as_str()); + if let Some(reason) = device_target.reason() { let _ = writeln!(output, " device_target_reason: {reason}"); } let _ = writeln!(output, " index_url: {}", resolution.index_url); @@ -1340,7 +1382,7 @@ fn install_wheel_runtime( // without its exact device payload loads and then faults on the first // kernel, so an undetermined target is refused here rather than papered // over with every published payload. - if let Some(reason) = resolution.device_target.reason() { + if let Some(reason) = device_target.reason() { bail!( "cannot compose a canonical TheRock {} runtime: {reason}.\n\ The aggregate `rocm` distribution ships no GPU backend unless an exact `device-` extra requests one, so this install would produce a runtime that cannot run a kernel.\n\ @@ -5265,6 +5307,10 @@ mod tests { test_runtime_manifest(&target_runtime_key, "therock-release:gfx94X-dcgpu", 2); replacement.version = "7.14.0".to_owned(); replacement.wheel_composition = Some(required.clone()); + let (root, _) = test_paths("installed-replacement-sibling"); + replacement.install_root = root.join("replacement"); + fs::create_dir_all(&replacement.install_root).unwrap(); + fs::write(replacement.install_root.join("installed.marker"), b"ok").unwrap(); let alone = vec![source.clone()]; assert_eq!( @@ -5294,7 +5340,7 @@ mod tests { // A sibling that merely shares the key without the composition is not the // replacement: accepting it would strand the tree one repair short. - let mut impostor = replacement; + let mut impostor = replacement.clone(); impostor.wheel_composition = Some(test_wheel_composition("gfx950")); assert_eq!( runtime_freshness_with_manifests( @@ -5306,6 +5352,21 @@ mod tests { ), RuntimeFreshness::RepairAvailable ); + + // A correct manifest is not an installed replacement after its runtime + // directory disappears; accepting it would suppress every repair. + fs::remove_dir_all(&replacement.install_root).unwrap(); + assert_eq!( + runtime_freshness_with_manifests( + &[source.clone(), replacement], + &source, + "7.14.0", + Some(&required), + &target_runtime_key, + ), + RuntimeFreshness::RepairAvailable + ); + fs::remove_dir_all(root).ok(); } #[test] diff --git a/tests/e2e-cucumber/src/shared_runtime.rs b/tests/e2e-cucumber/src/shared_runtime.rs index df9ec0a0a..be3ab641b 100644 --- a/tests/e2e-cucumber/src/shared_runtime.rs +++ b/tests/e2e-cucumber/src/shared_runtime.rs @@ -83,10 +83,10 @@ fn active_runtime_key(runtimes_dir: &Path) -> Option { /// fails — which a poisoned entry makes it do. So the suite must expect to meet /// one and step over it rather than name it and fail. /// -/// Judged by the same rule the pre-warm's own repair uses: an install root -/// inside this tree is sound, one outside it is a corpse. Checking existence -/// alone would be wrong — on a runner where a foreign path happens to exist the -/// scenario would serve against a runtime outside the shared tree. +/// Judged by the same rule the pre-warm's own repair uses: an install root must +/// both exist and live inside this tree. Existence alone would be insufficient — +/// a foreign path may exist — while containment alone accepts a stale manifest +/// whose in-tree runtime directory has already disappeared. fn activatable_runtime_keys(runtimes_dir: &Path) -> Vec { let roots = comparable_roots(runtimes_dir); registry_runtime_keys(runtimes_dir) @@ -106,7 +106,7 @@ fn activatable_runtime_keys(runtimes_dir: &Path) -> Vec { return true; }; let root = Path::new(root); - roots.iter().any(|base| root.starts_with(base)) + root.is_dir() && roots.iter().any(|base| root.starts_with(base)) }) .collect() } @@ -225,6 +225,17 @@ mod tests { ); } + fn missing_in_tree(dir: &Path, key: &str) { + write( + &dir.join("registry").join(format!("{key}.json")), + &serde_json::json!({ + "runtime_key": key, + "install_root": dir.join("wheel").join(key), + }) + .to_string(), + ); + } + fn active(dir: &Path, key: &str) { write( &dir.join("active.json"), @@ -412,6 +423,15 @@ mod tests { assert_eq!(runtime_key_to_activate(dir), None); } + #[test] + fn ignores_a_manifest_whose_in_tree_install_root_is_missing() { + let tmp = tempfile::TempDir::with_prefix("shared-runtime-").expect("temp dir"); + let dir = tmp.path(); + missing_in_tree(dir, "release-wheel-multi-arch-7-14-0"); + + assert_eq!(runtime_key_to_activate(dir), None); + } + /// The diagnostic reports the tree as it is: a skipped runtime still has to /// appear, or "no runtime to activate" names an empty tree that isn't empty. #[test] From f603779cfd78b1ef218efcd01dfdda3b061ffd76 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Wed, 9 Sep 2026 05:01:57 -0700 Subject: [PATCH 11/14] fix(install): probe the selected ROCm device payload Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 797d39519..8fb2525c2 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -1428,8 +1428,9 @@ fn install_wheel_runtime( )?; progress_line("Checking the installed ROCm SDK..."); - let rocm_sdk_probe = probe_rocm_sdk_runtime(&env_python) - .context("TheRock packages did not expose a usable rocm_sdk runtime")?; + let rocm_sdk_probe = + probe_rocm_sdk_runtime_for_target(&env_python, Some(device_target.as_str())) + .context("TheRock packages did not expose a usable rocm_sdk runtime")?; validate_rocm_sdk_runtime_probe(&rocm_sdk_probe)?; let installed_version = rocm_sdk_probe .rocm_sdk_version @@ -3171,17 +3172,28 @@ fn python_venv_args(install_root: &Path) -> Vec { ] } -/// What `rocm_sdk` reports about the runtime that was just installed. +/// What `rocm_sdk` reports about an installed runtime. /// -/// Deliberately does not set `ROCM_SDK_TARGET_FAMILY`: forcing the target the -/// installer chose would make `resolved_target_family` echo that choice back -/// instead of reporting what the environment actually composed, which is the -/// one signal that catches a runtime whose device payload does not match its -/// host. +/// A newly composed aggregate runtime is probed with the exact device payload +/// selected from the canonical source. Without that input `rocm_sdk` may choose +/// an unrelated default target even though only one device package was installed, +/// producing library paths and a kernel check for the wrong GPU. Adopted legacy +/// runtimes remain unforced so the probe reports their existing composition. pub(crate) fn probe_rocm_sdk_runtime(python_executable: &Path) -> Result { - let text = capture_python_stdout( + probe_rocm_sdk_runtime_for_target(python_executable, None) +} + +fn probe_rocm_sdk_runtime_for_target( + python_executable: &Path, + device_target: Option<&str>, +) -> Result { + let env = device_target + .map(|target| vec![("ROCM_SDK_TARGET_FAMILY".to_owned(), target.to_owned())]) + .unwrap_or_default(); + let text = capture_python_stdout_with_env( python_executable, ROCM_SDK_PROBE_SCRIPT, + &env, "launch rocm_sdk probe", ) .with_context(|| { From f4ab65acda8b1314d19ed4606cec301b2f7678f2 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Wed, 9 Sep 2026 06:06:20 -0700 Subject: [PATCH 12/14] fix(runtime): migrate target-aware SDK probes Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 41 +++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 8fb2525c2..3545db2ab 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -452,6 +452,9 @@ pub(crate) struct RuntimeUpdatePlan { pub(crate) struct WheelRuntimeComposition { pub source_layout_generation: String, pub package_specs: Vec, + /// Exact target supplied to `rocm_sdk` when resolving runtime libraries. + #[serde(default)] + pub rocm_sdk_target: Option, } #[derive(Debug, Clone, Copy, Eq, PartialEq)] @@ -1032,10 +1035,9 @@ fn resolve_latest_for_manifest( // falls back to the version comparison rather than demanding a repair // this host could not perform. let wheel_composition = match &device_target { - AggregateDeviceTarget::Exact(_) => Some(wheel_runtime_composition( - &resolution, - &device_target.rocm_extras(), - )), + AggregateDeviceTarget::Exact(_) => { + Some(wheel_runtime_composition(&resolution, &device_target)) + } AggregateDeviceTarget::Undetermined(_) => None, }; let target_runtime_key = wheel_composition.as_ref().map_or_else( @@ -1271,7 +1273,7 @@ fn install_wheel_runtime( // usable target still composes a key here — from the `` extras // — which no real install can ever produce, and the refusal below stops it // from reaching a manifest. - let wheel_composition = wheel_runtime_composition(&resolution, &device_target.rocm_extras()); + let wheel_composition = wheel_runtime_composition(&resolution, &device_target); progress_line(format!( "Found canonical TheRock aggregate version {} with a matching PyTorch stack for target family {}.", resolution.latest_version, resolution.family @@ -1507,11 +1509,16 @@ fn therock_pip_package_specs( /// runtime are, by construction, the specs that get installed. fn wheel_runtime_composition( resolution: &PipRuntimeResolution, - rocm_extras: &str, + device_target: &AggregateDeviceTarget, ) -> WheelRuntimeComposition { WheelRuntimeComposition { source_layout_generation: THEROCK_SOURCE_LAYOUT_GENERATION.to_owned(), - package_specs: therock_pip_package_specs(&resolution.package_versions, rocm_extras), + package_specs: therock_pip_package_specs( + &resolution.package_versions, + &device_target.rocm_extras(), + ), + rocm_sdk_target: matches!(device_target, AggregateDeviceTarget::Exact(_)) + .then(|| device_target.as_str().to_owned()), } } @@ -1522,12 +1529,15 @@ fn wheel_runtime_composition( /// host happens to report now; storing the specs verbatim means that answer /// survives without a second manifest field to keep in sync. fn wheel_composition_device_target(composition: Option<&WheelRuntimeComposition>) -> Option<&str> { - composition?.package_specs.iter().find_map(|spec| { - let extras = spec.strip_prefix("rocm[")?.split_once(']')?.0; - extras - .split(',') - .map(str::trim) - .find_map(|extra| extra.strip_prefix("device-")) + let composition = composition?; + composition.rocm_sdk_target.as_deref().or_else(|| { + composition.package_specs.iter().find_map(|spec| { + let extras = spec.strip_prefix("rocm[")?.split_once(']')?.0; + extras + .split(',') + .map(str::trim) + .find_map(|extra| extra.strip_prefix("device-")) + }) }) } @@ -5183,6 +5193,7 @@ mod tests { "torchvision==0.26.0+rocm7.14.0".to_owned(), "torchaudio==2.11.0+rocm7.14.0".to_owned(), ], + rocm_sdk_target: Some(device_target.to_owned()), } } @@ -5998,14 +6009,17 @@ echo Python 3.12.10 let base = WheelRuntimeComposition { source_layout_generation: "multi-arch-v2".to_owned(), package_specs: vec!["rocm[libraries,devel,device-gfx942]==7.14.0".to_owned()], + rocm_sdk_target: Some("gfx942".to_owned()), }; let other_payload = WheelRuntimeComposition { source_layout_generation: "multi-arch-v2".to_owned(), package_specs: vec!["rocm[libraries,devel,device-gfx950]==7.14.0".to_owned()], + rocm_sdk_target: Some("gfx950".to_owned()), }; let other_generation = WheelRuntimeComposition { source_layout_generation: "multi-arch-v3".to_owned(), package_specs: base.package_specs.clone(), + rocm_sdk_target: base.rocm_sdk_target.clone(), }; let base_key = wheel_runtime_key(TheRockChannel::Release, "7.14.0", &base); @@ -6041,6 +6055,7 @@ echo Python 3.12.10 "rocm[libraries,devel,device-gfx1103]==7.14.1".to_owned(), "torch==2.11.0+rocm7.14.1".to_owned(), ], + rocm_sdk_target: Some("gfx1103".to_owned()), }; assert_eq!( From 928cc50fb44cc1d16b856d74f18a93f601e9fb27 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Wed, 9 Sep 2026 06:25:50 -0700 Subject: [PATCH 13/14] fix(runtime): include SDK target in composition key Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index 3545db2ab..ce29998ac 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -4311,6 +4311,12 @@ fn wheel_runtime_key( hasher.update([0]); hasher.update(package_spec.as_bytes()); } + hasher.update([0]); + hasher.update(b"rocm-sdk-target"); + if let Some(target) = &composition.rocm_sdk_target { + hasher.update([0]); + hasher.update(target.as_bytes()); + } let fingerprint = format!("{:x}", hasher.finalize()); slugify(&format!( "{}-wheel-multi-arch-{version}-{}", @@ -6021,6 +6027,8 @@ echo Python 3.12.10 package_specs: base.package_specs.clone(), rocm_sdk_target: base.rocm_sdk_target.clone(), }; + let mut legacy_probe_default = base.clone(); + legacy_probe_default.rocm_sdk_target = None; let base_key = wheel_runtime_key(TheRockChannel::Release, "7.14.0", &base); @@ -6040,6 +6048,11 @@ echo Python 3.12.10 wheel_runtime_key(TheRockChannel::Release, "7.14.0", &other_payload), "a different device payload is a different runtime, not an overwrite" ); + assert_ne!( + base_key, + wheel_runtime_key(TheRockChannel::Release, "7.14.0", &legacy_probe_default,), + "a runtime probed without the exact SDK target must be repaired side by side" + ); assert_ne!( base_key, wheel_runtime_key(TheRockChannel::Release, "7.14.0", &other_generation), From 69cc5fe87b29c19890339c893ef7951566849f29 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Wed, 9 Sep 2026 06:52:59 -0700 Subject: [PATCH 14/14] fix(install): request framework device payloads Signed-off-by: Michael Roy --- apps/rocm/src/therock.rs | 40 ++++++++++++++++++---------------------- docs/manual-testing.md | 6 +++--- docs/testing.md | 6 +++--- 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/apps/rocm/src/therock.rs b/apps/rocm/src/therock.rs index ce29998ac..b763949d3 100644 --- a/apps/rocm/src/therock.rs +++ b/apps/rocm/src/therock.rs @@ -295,12 +295,6 @@ impl AggregateDeviceTarget { } } - /// The `rocm` extras this target implies. Always three extras, so a plan - /// never reads as though the GPU backend were optional. - fn rocm_extras(&self) -> String { - format!("libraries,devel,device-{}", self.as_str()) - } - fn reason(&self) -> Option<&str> { match self { Self::Exact(_) => None, @@ -1493,17 +1487,23 @@ fn install_wheel_runtime( fn therock_pip_package_specs( package_versions: &TheRockPipPackageVersions, - rocm_extras: &str, + device_target: &str, ) -> Vec { + let device_extra = format!("device-{device_target}"); vec![ - format!("rocm[{rocm_extras}]=={}", package_versions.rocm), - format!("torch=={}", package_versions.torch), - format!("torchvision=={}", package_versions.torchvision), + format!( + "rocm[libraries,devel,{device_extra}]=={}", + package_versions.rocm + ), + format!("torch[{device_extra}]=={}", package_versions.torch), + format!( + "torchvision[{device_extra}]=={}", + package_versions.torchvision + ), format!("torchaudio=={}", package_versions.torchaudio), ] } - -/// The exact install intent for `resolution` under `rocm_extras`. +/// The exact install intent for `resolution` and its source-validated device target. /// /// Kept beside [`therock_pip_package_specs`] so the specs that identify a /// runtime are, by construction, the specs that get installed. @@ -1515,7 +1515,7 @@ fn wheel_runtime_composition( source_layout_generation: THEROCK_SOURCE_LAYOUT_GENERATION.to_owned(), package_specs: therock_pip_package_specs( &resolution.package_versions, - &device_target.rocm_extras(), + device_target.as_str(), ), rocm_sdk_target: matches!(device_target, AggregateDeviceTarget::Exact(_)) .then(|| device_target.as_str().to_owned()), @@ -5125,7 +5125,7 @@ mod tests { ); assert_eq!(target, AggregateDeviceTarget::Exact("gfx1201".to_owned())); - assert_eq!(target.rocm_extras(), "libraries,devel,device-gfx1201"); + assert_eq!(target.as_str(), "gfx1201"); } #[test] @@ -5164,10 +5164,7 @@ mod tests { let target = AggregateDeviceTarget::resolve(None, "gfx110X-all", &published_device_targets()); - assert_eq!( - target.rocm_extras(), - "libraries,devel,device-" - ); + assert_eq!(target.as_str(), ""); assert!( target .reason() @@ -5434,15 +5431,14 @@ mod tests { torchaudio: "2.10.0+rocm7.13.0a20260513".to_owned(), compatibility_key: "7.13.0a20260513".to_owned(), }; - let package_specs = - therock_pip_package_specs(&package_versions, "libraries,devel,device-gfx942"); + let package_specs = therock_pip_package_specs(&package_versions, "gfx942"); assert_eq!( package_specs, vec![ "rocm[libraries,devel,device-gfx942]==7.13.0a20260513".to_owned(), - "torch==2.10.0+rocm7.13.0a20260513".to_owned(), - "torchvision==0.25.0+rocm7.13.0a20260513".to_owned(), + "torch[device-gfx942]==2.10.0+rocm7.13.0a20260513".to_owned(), + "torchvision[device-gfx942]==0.25.0+rocm7.13.0a20260513".to_owned(), "torchaudio==2.10.0+rocm7.13.0a20260513".to_owned(), ] ); diff --git a/docs/manual-testing.md b/docs/manual-testing.md index 59f7809f7..bc9f5b5b1 100644 --- a/docs/manual-testing.md +++ b/docs/manual-testing.md @@ -123,9 +123,9 @@ Omit `--prefix` if you want rocm-cli to choose its standard managed folder. Expected result: - rocm-cli creates or reuses a rocm-cli managed Python venv. -- pip installs a pinned `rocm` with the `libraries` and `devel` extras plus - exactly one `device-` extra, alongside pinned `torch`, - `torchvision`, and `torchaudio` versions from the TheRock index. On a host +- pip installs pinned `rocm`, `torch`, and `torchvision` requirements with + exactly one `device-` extra (`rocm` also requests + `libraries,devel`), alongside pinned `torchaudio` from the TheRock index. On a host with no detectable AMD GPU the preview reports `device_target: undetermined` and a real install refuses rather than pulling every published device payload. - rocm-cli chooses the newest exact ROCm build suffix common to the SDK package diff --git a/docs/testing.md b/docs/testing.md index 0c636d5a5..11cf50264 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -164,9 +164,9 @@ Then it verifies: explicit `--prefix` folders - the installer does not pre-create that pip cache during dry-run or setup; pip creates it inside the ROCm folder when packages are downloaded -- a single TheRock-index pip install plan for pinned `rocm` — with the - `libraries` and `devel` extras plus exactly one `device-` - extra — and pinned `torch`, `torchvision`, and `torchaudio` versions +- a single TheRock-index pip install plan for pinned `rocm`, `torch`, and + `torchvision` requirements with exactly one `device-` + extra (`rocm` also requests `libraries,devel`), plus pinned `torchaudio` - on a host with no detectable AMD GPU the preview reports `device_target: undetermined` and renders the device extra as a placeholder; a real install refuses rather than falling back to every published device payload