fix(uv): colocate the uv cache with the ROCm CLI data dir

[rominf]

Summary

Point uv at a cache inside the ROCm CLI data directory, so the cache sits on the same filesystem as the environments it populates and hardlinking keeps working.

Root cause

uv_command_env() set only UV_HTTP_TIMEOUT, never UV_CACHE_DIR, so the cache stayed at $HOME/.cache/uv while managed environments were created under the data directory. When those are on different filesystems, uv cannot hardlink and falls back to copying every file — each environment then carries its own full copy of the ROCm SDK and the torch stack.

This is not an exotic setup. It happens with a split /home, a container overlay, or a user who moved the data directory to a larger disk with ROCM_CLI_DATA_DIR — so the workaround for running out of space multiplied the space used.

Measured with uv 0.9.30 on a host where $HOME/.cache is a separate mount, installing a small package and comparing inodes:

same filesystem  -> inode=1740514 links=2 dev=830   (hardlink into the cache)
different fs     -> inode=1740556 links=1 dev=66    (full copy)

A distinct inode with link count 1 — a copy, not a reflink or symlink. Setting UV_LINK_MODE=hardlink explicitly does not help: the request is downgraded and the command still exits 0.

uv does print a fallback warning, but two of the three install paths discard it — ComfyUI pipes stderr to a log file, and the vLLM install captures output with .output() and drops stderr on success.

Technical decisions

Keyed off data_dir, not cache_dir. A cache would naturally live in the cache directory, but ROCM_CLI_CACHE_DIR can point at a different filesystem from the environments, which would reintroduce the exact bug. Colocation with the environments is the property that matters here.

The escape hatch is ROCM_CLI_UV_CACHE_DIR, matching the sibling knobs in the same module (ROCM_CLI_UV_BINARY, ROCM_CLI_UV_VERSION, ROCM_CLI_UV_TIMEOUT_SECS). A bare UV_CACHE_DIR exported for unrelated Python work would otherwise silently opt a user out of the fix with no signal. An ambient UV_CACHE_DIR is still honored — the e2e harness sets one to share a cache across scenarios — but it ranks below the namespaced variable, and UvCacheSource lets callers tell managed / override / inherited apart.

A one-shot notice, not a migration. When a pre-colocation $HOME/.cache/uv is still on disk, the CLI prints one notice and records that it did, following maybe_migrate_legacy_dashboard_config. Nothing is migrated or deleted: the cache is content-addressed and may be shared with unrelated uv projects, so removing it is the user's call.

Threading &AppPaths through uv_command_env touched the call sites in therock.rs, comfyui.rs, and the vLLM engine; all of them already had paths in scope, so no new AppPaths::discover() calls were introduced.

Non-goal: --prefix installs — #187

This colocates with the data directory only. --prefix relocates install_root for a single SDK install and leaves paths.data_dir — and therefore the uv cache — untouched. A --prefix on a different filesystem from the data directory still copies rather than hardlinks, and that case is not fixed here.

It is deliberately out of scope because the obvious fix does not work. Threading the target root through and using <install_root>/uv-cache breaks every wheel install: uv materializes UV_CACHE_DIR on every invocation including uv venv, and uv venv refuses to create an environment in a directory that already exists. Verified:

$ UV_CACHE_DIR=$P/fresh/uv-cache uv venv --python 3.12 $P/fresh
Creating virtual environment at: /tmp/.../fresh
error: Failed to create virtual environment
  Caused by: A directory already exists at: /tmp/.../fresh

$ ls $P/fresh
uv-cache          <-- uv created its cache there, which then blocked venv creation

Since install_root is the venv directory, the cache cannot live inside it. (This is also why the existing <install-root>/pip-cache convention in docs/testing.md works for pip but not for uv: pip creates its cache during downloads, after the venv exists.) Choosing a different scheme is a design decision for maintainers, so it is tracked in #187 with the evidence and four options.

In the meantime the gap is documented in docs/manual-testing.md, pinned by uv_cache_does_not_follow_a_prefix_install_root, and ROCM_CLI_UV_CACHE_DIR is a manual workaround.

The related pre-existing Downloads/cache: message — which reports <install_root>/pip-cache although nothing sets PIP_CACHE_DIR — is folded into #187, since whatever scheme resolves --prefix decides what that message should say.

Consequences worth noting

• rocm uninstall now covers the uv cache as a side effect, since it removes the data directory. That reclaims more space but makes uninstall slower. The --keep-data / --keep-cache help text has been corrected to say so: --keep-cache does not cover the uv cache.
• MANIFEST.md now discloses the wheel cache and its size, not just the uv binary.
• The cache is no longer shared with other uv projects on the machine. Users who want the old sharing can set ROCM_CLI_UV_CACHE_DIR.
• Existing installs re-download once.

Tests

Cache resolution is a pure function over the two override variables, exercised for unset, empty, whitespace-only, trimmed, inherited, and namespaced-override inputs, plus precedence between the two. The relocated-data-dir test goes through AppPaths::with_managed_root rather than two hand-built AppPaths. uv_cache_does_not_follow_a_prefix_install_root pins the #187 gap so it cannot regress silently in either direction.

They exercise the pure helper rather than mutating the process environment, because the crate denies unsafe_code and std::env::set_var is unsafe in this edition. That also keeps them parallel-safe.

CI note

cargo test --workspace --all-targets --no-fail-fast on this branch shows two failures in proc_lifecycle that are unrelated to it — see #168, fixed by #169. That file is not touched by this PR, and those are the only two failures in the workspace. cargo fmt --all --check and cargo clippy --locked --workspace --all-targets -- -D warnings are clean.

Fixes #160

[rominf]

The windows-build-and-test failure here is the known lifecycle-windows-http-install flake tracked in #173, not this change.

Failure signature on every attempt, unchanged:

rocm-cli installer: failed to download
http://127.0.0.1:<port>/rocm-cli-windows-amd64.zip:
An error occurred while sending the request.
FAIL: 'lifecycle-windows-http-install' was expected to pass on this host but FAILED

That is install.ps1 fetching from the test's local HTTP server, before any rocm code executes. This branch changes only Rust — uv.rs, runtime.rs, therock.rs, comfyui.rs, and the vLLM engine — and touches neither the installer nor packaging, so it cannot reach that code path.

The same scenario failed and then passed on re-run, with no code change, on #166, #169 and #172 today. The per-PR failure count rose through the afternoon (1, then 2, then 3, then 5 here), which looks like a worsening runner or network condition rather than anything branch-specific.

I have re-run this job five times and stopped there rather than keep consuming runner time. Could a maintainer re-run it, or advise if the Windows lane is known to be unhealthy right now? Everything else on this PR is green, and fmt, clippy -D warnings, and the test suite pass locally.

[volen-silo]

Reviewed at e10c649. The core mechanism is right and the execution is careful — root cause identified at the correct layer, measured evidence rather than assertion, every uv spawn site found and threaded (I checked all six independently), and a test seam that works around #![deny(unsafe_code)] making set_var unusable. The PR body also self-discloses its known gaps, which made this much faster to review. Two blocking items, one of them a two-minute edit.

Blocking

1. --prefix installs are not covered — and that's the mainline flow

crates/rocm-core/src/uv.rs:78 derives the cache from paths.data_dir unconditionally. But at apps/rocm/src/therock.rs:812:

let install_root = prefix.unwrap_or_else(|| managed_runtime_root(paths, "wheel", &runtime_key));

With --prefix, install_root is a raw user path and data_dir is untouched. The two are never cross-checked, so the same-filesystem property this PR exists to establish does not hold for prefixed installs.

This isn't an edge case:

• The built-in assistant's system prompt (apps/rocm/src/main.rs:8439) instructs the model to always obtain an install folder and pass --prefix, and forbids proposing an install command without one. Every assistant-driven SDK install goes down this path.
• It cascades to ComfyUI — apps/rocm/src/comfyui.rs:666 roots the app at install_root.join("apps").join(APP_ID), so the uv install at comfyui.rs:1426 inherits the prefixed root. Two of the three uv install paths are affected.
• One combination is a genuine regression. ROCM_CLI_DATA_DIR=/mnt/big plus --prefix $HOME/envs: before this PR the cache was ~/.cache/uv, same filesystem as $HOME/envs, hardlinks worked. After, the cache is /mnt/big/uv-cache and uv falls back to copying. The two knobs are independent and nothing validates one against the other.

There's also an existing convention pointing the other way. docs/testing.md:161-164 specifies a localized cache inside the selected ROCm runtime folder at <install-root>/pip-cache, "including both generated managed folders and explicit --prefix folders" — and docs/manual-testing.md:41-46 says the same in user terms. The documented intent is colocate-with-install-root, which covers --prefix; this PR colocates with data_dir, which doesn't. Either follow that convention or say why it's being changed.

The fix looks cheap, which is why I'm flagging it rather than deferring. The target root is already in scope at four of the five call sites: ensure_uv_venv (therock.rs:2218) takes install_root; run_uv_progress_command's only caller (therock.rs:922) has it as a local; run_uv_logged_command's only caller (comfyui.rs:350) has runtime.python; install_vllm_with_uv (engines/vllm/src/lib.rs:926) takes python: &Path. A uv_command_env_for_target(paths, target_root) is a signature change at four sites. ensure_managed_python (therock.rs:2747, :2760) genuinely doesn't fit — it installs an interpreter into uv's own global store — and should stay keyed off data_dir.

If you'd rather land the improvement now and handle --prefix separately, that's a reasonable call — but then it needs to be an explicit non-goal in the description with a follow-up issue, not silently implied as covered. Which brings me to:

2. The description and commit message state something factually wrong about --prefix

PR body and commit e10c649 both say:

a user who moved the data directory to a larger disk with ROCM_CLI_DATA_DIR or --prefix

--prefix does not move the data directory. It relocates install_root for a single SDK install and leaves paths.data_dir — and therefore the uv cache — exactly where it was. Listing it among the fixed scenarios tells reviewers a case is handled when it isn't, and it's precisely the sentence that would stop the next reader from noticing finding 1. Free to correct in the body; the commit message needs an amend or a follow-up note.

Non-blocking

• apps/rocm/src/main.rs:438-442 — the uninstall flag help is now misleading. --keep-data says "Keep app data such as logs, services, and engines" and --keep-cache says "Keep caches." build_uninstall_plan buckets strictly by paths.data_dir / paths.cache_dir, so after this PR --keep-cache has zero effect on the uv cache while --keep-data silently preserves what will often be the largest directory the product manages. A one-line amendment to --keep-data is close to required. (The broader data-vs-cache taxonomy issue pre-dates this PR — managed_pip_cache_dir was already under the data root — so re-bucketing is a separate concern.)
• crates/rocm-core/src/uv.rs:78-81 — the behavior the PR actually adds is untested. All three new tests call the private uv_command_env_with_inherited_cache with a pre-resolved Option<&OsStr>, so the env read and the .filter(|v| !v.is_empty()) are never exercised in either direction. Avoiding set_var is the right call, but the empty-string boundary is a pure function and could be tested as one.
• crates/rocm-core/src/uv.rs:440-448 — command_env_cache_dir_follows_a_relocated_data_dir is near-tautological. managed_uv_cache_dir is an unbranched root.join("uv-cache"), so building two AppPaths with different data_dirs and asserting the results differ only fails if the implementation ignores paths entirely, which the preceding test already covers. It doesn't touch AppPaths::with_managed_root or ROCM_CLI_DATA_DIR, so the name overstates it.
• Stale $HOME/.cache/uv — you flag this as known. Worth noting there's house precedent for the light version: maybe_migrate_legacy_dashboard_config (main.rs:924-939) does one-shot legacy-location detection and prints a one-line notice. A notice would fit; a full cache migration seems neither needed nor advisable.
• crates/rocm-core/src/uv.rs:65 — the escape hatch piggybacks on a third-party variable, against local convention. Every neighbouring override in this same module is namespaced: ROCM_CLI_UV_BINARY, ROCM_CLI_UV_VERSION, ROCM_CLI_UV_TIMEOUT_SECS. UV_CACHE_DIR is uv's generic variable, so a developer who has it exported for unrelated Python work silently opts out of the fix with no signal — indistinguishable from an accident. A ROCM_CLI_UV_CACHE_DIR (keeping UV_CACHE_DIR inheritance for the e2e harness) would match the convention and let the CLI report that an override is active.
• MANIFEST.md:666-672 — the uv section discloses that the uv binary is cached in the managed data directory but not the multi-GB wheel cache that now lives there. For a disclosure document that materially understates the footprint.

While you're here (pre-existing, but this PR is the natural place)

• apps/rocm/src/main.rs:9411, :9423 render Downloads/cache: <value> from active_runtime_pip_cache_dir / setup_runtime_pip_cache_dir, which fall back to <install_root>/pip-cache. But nothing anywhere sets PIP_CACHE_DIR or --cache-dir, and every manifest constructor writes pip_cache_dir: None. So the CLI tells the user downloads stay in their chosen ROCm folder while they actually go to the uv cache. Already wrong before this PR — but this is the PR that decides where downloads live, and fixing finding 1 by keying off install_root would make the existing message true.
• apps/rocm/src/therock.rs:3409 — managed_uv_cache_defaults_inside_generated_runtime_folder only asserts install_root.starts_with(&paths.data_dir) and never touches managed_uv_cache_dir or uv_command_env. It predates this PR, but its name now claims coverage of exactly what changed here. Repurposing it into a --prefix divergence test would have caught finding 1.

Nit

crates/rocm-core/src/uv.rs:79 — the inherited-value check doesn't trim, while both siblings in the same file do (uv_version at :225, env_secs at :332). UV_CACHE_DIR=" " is treated as a deliberate override.

On the data_dir-over-cache_dir question you raised

Your reasoning holds. ROCM_CLI_CACHE_DIR can point at a different filesystem from the environments, which would reintroduce the bug, and AppPaths::with_managed_root already derives cache_dir from data_dir by default. The right axis to colocate on is the one the environments live on — which is also the argument for install_root in finding 1.

[rominf]

Thanks — this was a genuinely useful review; finding 1 turned out to be more interesting than either of us expected.

Note on what changed since your review at e10c649: I force-pushed. e10c649 was amended (message only, content identical) to fix the --prefix claim, and is now 9298e2a. All new work is in a separate commit 35931cb on top, so the delta since your review is exactly that one commit.

---

1. --prefix installs are not covered — scoped out, with evidence: #187

I tried the cheap fix first, and it does not work — it breaks every wheel install. uv materializes UV_CACHE_DIR on every invocation, including uv venv, and uv venv refuses to create an environment in a directory that already exists. Since install_root is the venv directory (venv_python_path(&install_root)), <install_root>/uv-cache is self-defeating:

$ UV_CACHE_DIR=$P/fresh/uv-cache uv venv --python 3.12 $P/fresh
Using CPython 3.12.3 interpreter at: /usr/bin/python3.12
Creating virtual environment at: /tmp/.../fresh
error: Failed to create virtual environment
  Caused by: A directory already exists at: /tmp/.../fresh

$ ls $P/fresh
uv-cache        <-- uv created its own cache dir, which then blocked venv creation

That also explains why the docs/testing.md convention holds for pip but not uv: pip creates its cache during downloads, i.e. after the venv exists, whereas uv both creates the venv and does the downloading. The documented convention is not transferable to uv as-is.

So this isn't a four-call-site signature change — it needs a cache-location scheme chosen first, and the candidates all have real costs (sibling-of-install-root pollutes the user's directory or $HOME; a split cache that switches location after the venv exists is fragile against reordering; validate-and-warn instead of relocate; or accept the copy). That's a maintainer call, not one to make inside this PR.

Opened #187 with the reproduction, the four options, and your Downloads/cache: point folded in — you're right that whichever scheme lands is what decides whether that message becomes true, so it belongs with the decision rather than here.

In this PR the gap is now explicit rather than implied: a non-goal section in the description, a --prefix caveat in docs/manual-testing.md, ROCM_CLI_UV_CACHE_DIR as a manual workaround, and uv_cache_does_not_follow_a_prefix_install_root pinning it so it can't drift silently in either direction.

I did confirm your regression combo (ROCM_CLI_DATA_DIR=/mnt/big + --prefix $HOME/envs) is real: uv_command_env never sees install_root, so the cache lands on the data-dir filesystem regardless of the prefix.

2. Factually wrong --prefix claim in body and commit — fixed

You were right, and it was exactly the sentence that would have stopped the next reader noticing finding 1. Removed from the PR body and from the commit message (amended: e10c649 → 9298e2a). The commit now states the --prefix limitation explicitly instead of listing it as fixed.

Non-blocking

• --keep-data / --keep-cache help (35931cb) — both corrected. --keep-data now names the uv cache and calls out that it is often the largest directory the CLI manages; --keep-cache now says it does not cover the uv cache and points at --keep-data. Agreed the data-vs-cache re-bucketing is a separate concern; not touched.
• Untested env read / empty-string boundary (35931cb) — resolution is now a pure function over both variables, tested for unset, empty, whitespace-only, trimmed, inherited, and namespaced-override, plus precedence between the two. Still no set_var.
• Near-tautological relocated-data-dir test (35931cb) — replaced. It now goes through AppPaths::with_managed_root, which is the path ROCM_CLI_DATA_DIR actually takes, and asserts the resolved cache lands under the relocated root rather than merely differing.
• Stale $HOME/.cache/uv (35931cb) — took the maybe_migrate_legacy_dashboard_config precedent: one-shot detection, one-line notice, a marker so it doesn't nag, and nothing migrated or deleted. It stays silent when an override is in effect, so the e2e harness never sees it.
• Un-namespaced escape hatch (35931cb) — agreed, and your "indistinguishable from an accident" framing is the right one. Added ROCM_CLI_UV_CACHE_DIR, ranked above an ambient UV_CACHE_DIR (still honored for the harness). A UvCacheSource enum distinguishes managed / override / inherited so the CLI can report that an override is active.
• MANIFEST.md (35931cb) — now discloses the wheel cache, its location, its rough magnitude, that rocm uninstall removes it, and the override.

While you're here

• Downloads/cache: message — folded into uv cache does not follow --prefix, so prefixed installs on another filesystem still copy instead of hardlink #187 rather than fixed here, since it can't be made true without deciding the --prefix scheme.
• managed_uv_cache_defaults_inside_generated_runtime_folder (35931cb) — renamed to say what it checks, and joined by uv_cache_does_not_follow_a_prefix_install_root, which is the divergence test you described. It asserts the current (gap) behavior deliberately, with a comment saying so and pointing at the tracking issue.

Nit

• Trim (35931cb) — fixed, for both variables. OsStr has no trim, so it trims the lossy view and keeps the value when it survives, so a path is never silently rewritten.

---

Verification, all actually run on this branch: cargo fmt --all --check clean; cargo clippy --locked --workspace --all-targets -- -D warnings clean (sources touched first so the cache couldn't hide lints); cargo test --workspace --all-targets --no-fail-fast — the only two failures in the whole workspace are the known proc_lifecycle ones from #168, in a file this PR doesn't touch.

Leaving all threads for you to resolve.

[rominf]

CI status on 35931cb — every hosted check is green: build-and-test, windows-build-and-test, clippy, Test (affected crates), E2E tests, Coverage, CodeQL (actions/python/rust), prek, License header check, Commit signatures + sign-off, Third-party notices current, Lint (PowerShell).

Two self-hosted checks I can't drive from here:

• E2E tests (GPU) — FAILURE, infrastructure, not this branch. The job starts and completes in ~1 second with zero steps executed on github-runner-0 (self-hosted, linux, amd-gpu). It fails identically on every recent ci.yml run in the repo, not just this PR:

  run=31104090115  E2E tests (GPU)  failure  dur=1s  steps=0
  run=31103889746  E2E tests (GPU)  failure  dur=0s  steps=0
  run=31103688037  E2E tests (GPU)  failure  dur=1s  steps=0
  run=31103619164  E2E tests (GPU)  failure  dur=1s  steps=0   <- this PR
  run=31103315258  E2E tests (GPU)  failure  dur=0s  steps=0
  

  gh run rerun --failed is refused while the run is in progress. Could a maintainer re-run it once the GPU runner is healthy?

• E2E tests (Strix Halo, Ubuntu / Windows) — still queued after ~40 minutes, waiting on the shared native runners. They do complete successfully on other runs in the same window, so this looks like queue contention rather than a problem with the branch.

Flagging rather than leaving a bare red X. Happy to re-push or re-run if anything here turns out to be real.

Feedback adressed