fix(install): preview a ROCm 10 SDK install without fetching uv by jussielo-amd · Pull Request #954 · ROCm/rocm-cli

jussielo-amd · 2026-11-06T21:10:04Z

Closes ROCMAI-896.

Summary

rocm install sdk --dry-run (and the assistant's install_sdk_dry_run tool, which reaches the same code) promises to change nothing. #906 made that true for Python. This finishes the other half: for a ROCm 10 or newer wheel the preview resolved the package versions with uv pip compile, through the same resolver a real install uses, and that resolver acquires the managed uv. On a clean host the preview therefore downloaded uv and created the data folder's tools folder before printing its plan. The canonical layout needs no uv and was not affected.

Before: on a host with no uv, rocm install sdk --version 10.0.0 --family gfx1200 --dry-run printed Downloading uv (latest) from https://github.com/astral-sh/uv/releases/latest/download/... on stderr and put the archive in the cache's tools/uv/latest folder (reproduction below); the lock taken before that download is what creates the data folder's tools folder, which the mutation run below shows. After: it downloads nothing and creates nothing. If a uv is already there (the binary ROCM_CLI_UV_BINARY names, or the managed copy any earlier install left) the preview runs it and prints exactly the complete plan it printed before. If none is there it prints what it can know without one and says what it did not do. A real install and rocm update still acquire uv exactly as before.

I chose option B from the ticket: resolve without fetching, using a uv that is already present, and when none is present skip the resolution and say so. Only uv pip compile produces the torch and torchvision versions (the CLI picks the ROCm version and the torchaudio pin), and the package specs, the runtime key (a hash of the specs), the install folder, the manifest and the printed command are all made of them. So a preview that skips the compile cannot print those lines, and it cannot say whether the published stack is jointly installable either; it must not print a guess for them. Option A (always print the unresolved lines) would have changed the plan for everyone, including the hosts where a uv is present and the complete plan costs nothing; B keeps the complete plan wherever it is free and degrades only where completing it would need the network and a write to the data folder. Option C (keep the fetch and document it as an exception to the tools being read-only) is the smallest change but leaves a model-callable tool in the read-only group that downloads a binary, and every statement that calls the preview read-only would need an asterisk. The cost of B is real: on a clean host a ROCm 10 preview is a partial plan until uv has been fetched once, which any real install (or any command that prepares Python or ComfyUI) does.

What the preview now prints on a host with no uv, in the voice of the existing python_note: line. A progress line: No uv is installed; a real install would fetch it and resolve the torch and torchvision versions with `uv pip compile`. This preview does not. And in the plan, after python_note:: resolution_note: no uv is installed, so a real install would fetch it and run `uv pip compile` to choose the torch and torchvision versions and to check that the published packages install together, which this preview did not do; the package specs, runtime key, install folder, manifest and command depend on those versions and are not resolved here. target, runtime_key, package_specs, command and manifest read not resolved by this preview (target is the folder after --prefix, which does not depend on the versions). version_resolution_specs, torchaudio_selection, device_target, selected_rocm_version and the rest are printed as before. No torch or torchvision version appears anywhere in the output.

What changed where

uv.rs: find_uv_binary is ensure_uv_binary stopped short of its download: the override, or the managed copy when it runs, otherwise None. It takes no lock and creates no folder. The managed copy's location comes from one helper shared with the install, and a test shows the lookup finds the copy the install reuses.
uv_access.rs (new): UvAccess (Acquire for a real install and rocm update, PresentOnly for a preview) and plan_version_resolution, a pure function of the layout, the access and whether a uv is present. Acquire never asks whether one is present and never skips, so what a real install and an update decide and call is what they always did.
wheel.rs: resolve_pip_runtime and resolve_pip_runtime_with_timeout are Acquire, and return a resolved plan or an error. The new preview_pip_runtime is PresentOnly and may return PipRuntimePlan::NeedsUv, holding an UnresolvedPipRuntime: the ROCm version, the torchaudio pin, the requirement lines uv would be given, and no package versions, so nothing made from them can be built from it by accident. resolve_published_pip_package_versions now takes the uv to run instead of acquiring it. The uv-access part was moved out into its own file in a separate commit because the previous one took this file past the module-size limit.
wheel_plan.rs (new, split out of install_wheel_runtime in a pure move first): one writer for the sdk install plan. PlanStack::Resolved or PlanStack::NeedsUv is the only place a plan differs, so a line the plan gains appears in both; a test holds the two key sequences equal.
therock.rs: install_wheel_runtime asks preview_pip_runtime when it is a preview.
Docs: the --dry-run help, the README's ROCm 10 section, testing.md, manual-testing.md, architecture.md and the dash-TUI tool-group comment in tools.rs, which said a ROCm 10 preview could still fetch uv, now say it does not. I grepped the other places that call the preview read-only: the assistant tool descriptions, docs/llm-tool-use.md and the assistant skill make no claim about the preview's uv handling, so they are unchanged. The skill is compiled in with include_str! and was not touched.

Test plan

Reproduction, before the fix, on a clean data folder. Scenario runtime-28 (below) runs rocm install sdk --channel release --format wheel --family gfx1200 --version 10.0.0 --dry-run against a loopback ROCm 10 index with no uv anywhere (no ROCM_CLI_UV_BINARY, empty scenario data folder, its own cache folder). I ran it against a debug build of origin/main with the scenario already in place. It failed at the When step: stderr began Downloading uv (latest) from https://github.com/astral-sh/uv/releases/latest/download/uv-x86_64-unknown-linux-gnu.tar.gz, the download reached the scenario's cache and then failed only because the scenario's PATH is empty (failed to extract uv archive .../scratch-cache/tools/uv/latest/uv-x86_64-unknown-linux-gnu.tar.gz ... failed to launch tar). So the managed-uv fetch was reached, over the real network, from a preview. The same scenario passes on the fixed build. What I did not do: observe a successful full download and extraction (the empty PATH stops it) and any Windows run.

Ran, on the final tree (Linux, one private target dir):

cargo test -p rocm --bin rocm therock: 234 passed, 1 ignored (228 before this change plus 6 new). The leaf-name list of cargo test -p rocm --bin rocm -- --list is 1683 names on origin/main and 1689 here; none disappeared, the six added are the new tests below.
cargo test -p rocm-core --lib uv::: 36 passed, 2 ignored (three of them new here, two Unix-only).
cargo fmt --all --check; cargo clippy --locked -p rocm --all-targets -- -D warnings; cargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings; cargo clippy --locked -p rocm-core --all-targets -- -D warnings; the same for rocm and rocm-core against x86_64-pc-windows-msvc (with stub C compilers for the build scripts, since check and clippy never link): all exit 0. Windows because the new tests carry Unix-only parts.
cargo xtask manifest --check, cargo xtask check-crate-edges, cargo xtask check-architecture-doc (all 0), cargo test -p e2e-cucumber --test feature_naming (4 passed), cargo xtask lychee (0). cargo xtask check-module-size reports only service_launch.rs, which is over on origin/main too.
Scenarios runtime-23 to runtime-28 and therock-next-01 to -05, -08 and -10 to -15 against a debug build with rocm/e2e-test-hooks: 18 scenarios passed, run through the real harness with ROCM_CLI_BINARY set. therock-next-02, -10, -11 and -13 need the network, as they did, to install the managed uv.
Leak scan against origin/main: the only hits are the words private in existing architecture entries and internal error in a new refusal message (the same phrase PlannedPython::into_launcher uses).

Not run: the full workspace test gate, scripts/smoke_local.py, the GPU scenarios (therock-next-07 and -09 fail on origin/main too on a host with no GPU), cargo test -p rocm-dash-tui (comment-only change there; the crate is compiled by the clippy runs above), and any Windows execution of the tests.

State matrix. The decision is plan_version_resolution over layout (canonical, ROCm 10) by access (Acquire, PresentOnly) by a uv present or not, 8 rows, each asserting the decision and how many times a present uv was looked for (the_version_step_is_decided_by_layout_access_and_a_present_uv_alone, runs on every platform). Skip exists for exactly one row (ROCm 10, preview, none present). a_preview_never_acquires_uv_and_a_real_resolution_still_does (Unix, because the stand-in uv is a script) runs the real resolution against a served index and asserts per row the outcome and what was touched (looked, acquired, uv runs, tools folder): ROCm 10 preview with none present stops and renders the plan in the same row, with the unresolved lines, the note, and no version only uv could have chosen; ROCm 10 preview with one present runs it once and is not acquired again; ROCm 10 install or update acquires once, with or without one present, and never looks first; a failed acquire fails with the context it always had; the canonical layout never looks, acquires or runs, for either access. the_compile_step_acquires_uv_only_when_told_to covers the injected acquire. The plan's wording is held by the_plan_a_preview_without_uv_prints_is_exactly_this (the whole text), an_explicit_prefix_is_the_target_of_a_preview_without_uv and a_resolved_and_an_unresolved_plan_print_the_same_keys, which run on every platform. In rocm-core: looking_for_a_managed_uv_on_a_clean_host_finds_none_and_creates_nothing, an_installed_managed_uv_is_found_where_the_install_puts_it and a_managed_uv_that_does_not_run_is_not_reported.

Scenarios, all in the hermetic Linux mock lane (@requires-os:linux, no GPU; the existing therock-next ones keep their lane): runtime-27 (runtime-install-preview-of-rocm-10-uses-the-uv-that-is-present) is a characterisation scenario that passes before and after: with a uv present (the recording stand-in answering uv pip compile with the fixture's four pins) the preview runs it exactly once, for the compile and nothing else, and prints the resolved plan. runtime-28 (runtime-install-preview-of-rocm-10-fetches-no-uv) asserts the message and the state together: the progress line, the resolution_note:, the five unresolved lines, the known lines, no torch or torchvision version, and then no managed tools folder, no tool archive in the scenario's own cache, no saved config. It is red on origin/main and green here. therock-next-02, -10, -11 and -13, which print resolved package specs, gain And a uv is already installed, the precondition they always had when the preview fetched uv for them. The stand-in uv gains an opt-in FAKE_UV_DRAIN_STDIN, because the CLI writes the requirements to uv's stdin and a stand-in that exits without reading can fail that write depending on timing. The decision and the plan text run on the Windows job too; no scenario runs a preview on Windows.

Mutation evidence. Each was applied, the named tests were run and shown red, and the edit was reverted by a targeted reverse edit:

Preview acquires uv (PresentOnly returns Compile(Acquire)): the_version_step_is_decided_by_layout_access_and_a_present_uv_alone and a_preview_never_acquires_uv_and_a_real_resolution_still_does red; runtime-28 red.
Present uv ignored (PresentOnly always skips): the same two unit tests red; runtime-27 red.
A real install no longer acquires (ROCm 10 with Acquire looks for a present uv and skips): the same two unit tests red.
Preview acquires uv but still prints the skipped plan (so it succeeds): runtime-28 red at its Then step with the preview created the managed tools folder .../data/tools.
The progress line removed from the preview: runtime-28 red.
resolution_note line dropped from the plan: the_plan_a_preview_without_uv_prints_is_exactly_this, a_resolved_and_an_unresolved_plan_print_the_same_keys and a_preview_never_acquires_uv_and_a_real_resolution_still_does red.
A guessed package_specs printed instead of not resolved by this preview: the whole-text test and the integration test red.
--prefix ignored in the unresolved plan: an_explicit_prefix_is_the_target_of_a_preview_without_uv red.
find_managed_uv creating the tools folder: looking_for_a_managed_uv_on_a_clean_host_finds_none_and_creates_nothing red; without the usability check: a_managed_uv_that_does_not_run_is_not_reported red; ignoring the version in the location: an_installed_managed_uv_is_found_where_the_install_puts_it and concurrent_bootstraps_share_one_download_and_both_get_a_usable_uv red.

One mutation is not caught: changing the single UvAccess::Acquire that resolve_pip_runtime_with_timeout passes to PresentOnly. No hermetic test can distinguish it without the network, because the only difference is whether an absent uv is downloaded. therock-next-08 and -14 stay green under it (I ran them), since a real install's Python step already fetched uv before the resolver looks. It is held by the types: that entry point returns a resolved plan and refuses the unresolved one with an internal error, so a real install on a clean host with a usable Python would fail loudly rather than silently preview. docs/testing.md says so.

Notes for reviewers

Reading order, by commit: find_uv_binary in rocm-core; the pure move of the plan writer (wheel_plan.rs, no behaviour change); the runtime-27 characterisation scenario; the fix; the move of the access decision out of wheel.rs; the docs; a lint fix-up. Each commit builds and passes its own tests.

Sibling that this does not fix: rocm update --apply --dry-run and update_check resolve the update plan first, with the real access, so they can still fetch uv (and prepare a managed Python); that is the update path the ticket asks to leave alone, and it is the one place a ROCm 10 preview can still reach uv over the network. rocm update --apply --dry-run then prints the complete plan, because that fetch has already happened. The preview still writes its package-index metadata cache under the cache folder, as documented.

Open PRs touching the same code: #951 edits the dash-TUI tool-group comment block, tools.rs, and therock_steps.rs and therock.rs (the update-check sibling of #906), so expect a textual conflict in that comment, which both PRs rewrite; the resolution is to keep both statements. #820 and #819 add scenarios to therock_next_generation.feature and therock_steps.rs and edit therock.rs; #568 edits the dry-run block that this change moves into wheel_plan.rs, so its hunk needs to be reapplied there. No open PR adds runtime-27 or runtime-28.

ensure_uv_binary returns the binary named by ROCM_CLI_UV_BINARY or the managed copy, and otherwise takes a lock under the data folder's tools folder and downloads one. A caller that must leave the machine untouched (an install preview) had no way to ask whether a uv is already there. find_uv_binary answers that question and nothing more: the override, or the managed copy when it runs, and None when ensure_uv_binary would have had to fetch. It creates no folder and takes no lock. The managed copy's location now comes from one helper shared with the install, and a test shows the lookup finds the copy the install reuses. No caller yet; ensure_uv_binary behaves exactly as before. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

install_wheel_runtime wrote the sdk install plan inline, from the provenance lines down to the dry-run command and manifest path. Move those writes into therock/wheel_plan.rs behind a WheelPlan value: write_wheel_plan for the lines a preview and a real install share, write_dry_run_tail for the lines only a preview adds. The lines, their order and their wording are unchanged; the existing tests pass as they were. This lets the next change print a plan whose package versions are not chosen yet through the same writer instead of a second copy of it. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

… present A preview of a ROCm 10 or newer wheel has uv pip compile choose the torch and torchvision versions, and the plan's package specs, runtime key and command are made of them. Add runtime-27: with a uv present (the recording stand-in named by ROCM_CLI_UV_BINARY, answering the compile with the fixture's four pins) the preview runs it once, for the compile and nothing else, prints the resolved plan and reports no skipped step. It passes on the current code and is the half of the contract a later change must keep when a preview stops fetching uv on a host that has none. The stand-in gains an opt-in FAKE_UV_DRAIN_STDIN, because the CLI writes the requirements to uv's stdin before it reads the answer and a stand-in that exits without reading can fail that write. The two host setups that put an empty PATH and a cache folder of their own in the scenario now share one helper. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

On the ROCm 10 and newer wheel layout, rocm install sdk --dry-run (and the install_sdk_dry_run assistant tool) resolved the torch and torchvision versions with uv pip compile through the same resolver a real install uses, which acquires the managed uv: on a clean host the preview downloaded uv and created the data folder's tools folder although it promises to change nothing. The canonical layout needs no uv and was not affected. The resolver now takes a UvAccess. Acquire is what a real install and rocm update pass, and they decide and call exactly what they did before. PresentOnly is the preview's: it runs a uv that is already there (the binary ROCM_CLI_UV_BINARY names, or the managed copy a previous install left) and, with none, stops before the compile and returns an UnresolvedPipRuntime holding what needs no uv (the ROCm version, the torchaudio pin, the requirements uv would be given) and no package versions. The preview prints that plan through the same writer as every other plan, with target, runtime_key, package_specs, command and manifest marked not resolved by this preview, and says why in a resolution_note line and a progress line, in the voice of python_note. It cannot say whether the published stack installs together either, and the note says that. The decision is one pure function, tested over both layouts, both accesses and a uv present or not; a second test runs the real resolution against a served index and a stand-in uv and counts what each row looked for, acquired and ran. Scenario runtime-28 asserts the note together with the state on a host with no uv: no tools folder, no tool archive, no saved config and no torch or torchvision version in the output. The previews in therock-next-02, -10, -11 and -13 that print resolved specs start from an installed uv. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

The previous commit took therock/resolve/wheel.rs past the 1500 production-line limit. Move the part that decides who may fetch uv (UvAccess, UvPolicy, VersionResolution, UvChoice, plan_version_resolution and uv_for_compile) and its state-matrix tests into therock/resolve/uv_access.rs. It builds only on SourceLayout, so wheel.rs keeps acting on the answer and nothing else changes: the code and the tests are moved as they were, with the items widened to pub(super) so wheel.rs can name them. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

The notes written when the preview stopped preparing Python limited the no-uv, no-tools-folder guarantee to the canonical layout, because a ROCm 10 preview could still download the managed uv. It no longer does, so make every place that called the preview read-only or described its uv behaviour say what is now true: the --dry-run help, the README's ROCm 10 section, docs/testing.md and docs/manual-testing.md (including the new scenarios and the one gap no test reaches without a network), and the dash-TUI tool-group comment for install_sdk_dry_run. The assistant tool descriptions, docs/llm-tool-use.md and the assistant skill make no claim about the preview's uv handling, so they are unchanged. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Clippy under -D warnings flags a function that can be const, a test helper that appends a format! to a String, and an import that only the tests use since the decision moved to uv_access.rs. Fix the three; no behaviour changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Copilot

🟡 Changes recommended

The testing documentation recommends the unsupported rocm update --check command, which fails before resolution and should be replaced with the default rocm update check.

1 open issue

rocm update --check is unsupported · New

What changed in this PR

This completes the SDK preview work from #906 by preventing ROCm 10+ dry runs from downloading managed uv. Previews reuse an available binary or clearly report unresolved values, while real installs and update resolution retain their existing behavior.

Changes:

Add lookup-only uv access and distinguish preview resolution from installation.
Share plan rendering for resolved and unresolved package versions.
Add regression coverage and update help, documentation, and tool-side-effect comments.

File	Description
tests/e2e-cucumber/tests/e2e/therock_steps.rs	Add preview setup and output/state assertions.
tests/e2e-cucumber/src/bin/fake-uv.rs	Optionally drain stdin for reliable compile simulation.
tests/e2e-cucumber/features/therock_next_generation.feature	Explicitly prepare `uv` for resolved-plan scenarios.
tests/e2e-cucumber/features/runtime_setup.feature	Cover previews with and without available `uv`.
README.md	Explain partial previews when `uv` is absent.
docs/testing.md	Document coverage and verification limits.
docs/manual-testing.md	Update expected preview behavior.
docs/architecture.md	Document new modules and resolution flow.
crates/rocm-dash-tui/src/agent/tools.rs	Clarify preview side effects in comments only.
crates/rocm-core/src/uv.rs	Add non-acquiring lookup and focused tests.
crates/rocm-core/src/lib.rs	Export `find_uv_binary`.
apps/rocm/src/therock/wheel_plan.rs	Centralize resolved and unresolved plan rendering.
apps/rocm/src/therock/resolve/wheel.rs	Implement preview-aware resolution and regression tests.
apps/rocm/src/therock/resolve/uv_access.rs	Define and test `uv` access decisions.
apps/rocm/src/therock/resolve.rs	Register and expose new resolution components.
apps/rocm/src/therock.rs	Route dry runs through preview resolution.
apps/rocm/src/cli.rs	Describe the no-download behavior in dry-run help.

_{Review effort: Balanced}

docs/testing.md said rocm update --check does not preview in the install-preview sense, but rocm update has no --check flag: the plain rocm update (without --apply) is the check. The advised-commands test rejects the spelling. Name the update_check tool and the plain rocm update check instead. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Copilot

🟢 Approval recommended

No active review issues block approval of the changes preventing ROCm 10+ dry runs from downloading managed uv.

No open issues 🎉

1 resolved since last review

rocm update --check is unsupported

_{Review effort: Balanced}

jussielo-amd added 7 commits November 6, 2026 20:08

jussielo-amd marked this pull request as ready for review November 6, 2026 21:10

jussielo-amd requested a review from a team as a code owner November 6, 2026 21:10

jussielo-amd requested review from volen-silo and a balanced review from Copilot November 6, 2026 21:10

Copilot started reviewing on behalf of jussielo-amd November 6, 2026 21:11

View session

Copilot AI reviewed Nov 6, 2026

View reviewed changes

Comment thread docs/testing.md Resolved

jussielo-amd requested a balanced review from Copilot November 6, 2026 21:23

Copilot started reviewing on behalf of jussielo-amd November 6, 2026 21:24

View session

Copilot AI reviewed Nov 6, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(install): preview a ROCm 10 SDK install without fetching uv - #954

fix(install): preview a ROCm 10 SDK install without fetching uv#954
jussielo-amd wants to merge 8 commits into
mainfrom
fix-sdk-preview-without-fetching-uv

jussielo-amd commented Nov 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jussielo-amd commented Nov 6, 2026

Summary

What changed where

Test plan

Notes for reviewers

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants