fix(uninstall): report the caches it does not remove - #175
Open
rominf wants to merge 1 commit into
Open
Conversation
Uninstall presents itself as removing what ROCm CLI put on the machine, but it only removes the config, data, and cache directories. The uv package cache and the downloaded model files live outside all three and are usually the largest things on disk after an install, so a plan that stays silent about them reads as a clean slate it does not deliver. List them under the existing 'Please review:' section with their paths and why each is left alone. Nothing new is deleted. Locations are resolved at run time from UV_CACHE_DIR, HF_HOME, and HUGGINGFACE_HUB_CACHE, falling back to the standard defaults, and any path already inside a directory being removed is skipped — so the notes stay truthful as caches move under the data directory. Closes #163 Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rocm uninstallnow names the caches it leaves behind, with their paths and why. Nothing new is deleted.Root cause
build_uninstall_planremoves binaries plus the config, data, and cache directories. The two largest things an install causes to appear on disk are in neither:uvpackage cache —uv_command_env()never setUV_CACHE_DIR, souvused its own default outside the ROCm CLI directories.HF_HOME, so weights land in the shared default.hf_cache_roots_fromonly reads the environment for discovery.A user who uninstalls to reclaim space can be left with tens of gigabytes still gone, with nothing in the output to suggest it.
Technical decisions
Report, don't delete. The
uvcache is shared with every otheruvproject on the machine and ROCm CLI never claimed it; deleting it would slow down unrelated work. Model files are large, slow to re-fetch, sometimes gated behind a token, and may be the user's own. Naming them is what closes the honesty gap; deleting them is a separate decision.Locations are resolved at run time from
UV_CACHE_DIR,HF_HOME, andHUGGINGFACE_HUB_CACHE, falling back to the standard defaults — rather than assuming a fixed path.Anything already inside a directory being removed is skipped, so a note is never printed for a cache that uninstall does in fact delete. This matters because #170 moves the
uvcache under the data directory: once that lands, theuvnote correctly stops appearing without further changes here, and it keeps working for users who pointUV_CACHE_DIRelsewhere. The two PRs are independent and can land in either order.Missing paths are not mentioned, so the output stays quiet on a machine that never populated them.
Tests
The filtering logic is split into a pure
shared_cache_notes_for, so the three tests do not depend on the caller's real environment:CI note
cargo test --workspaceshows two unrelated failures inproc_lifecycle— see #168, fixed by #169. Everything else passes; fmt andclippy -D warningsare clean.Fixes #163