Add cache clean all for tool caches#117
Open
t-kalinowski wants to merge 24 commits into
Open
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27ae70aba9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Finding 1:
driver/cache-clean.R:127-131 only deletes tools::R_user_dir("reticulate", "cache") and the legacy r-reticulate cache, but ir creates Python environments through reticulate:::uv_get_or_create_env() in driver/tooling.R:296-317. When uv is already on PATH, reticulate uses that external uv installation as-is, so its downloads, Python installs, and tool environments live in uv's own cache, not under the reticulate cache this cleaner removes. In that common case, `ir cache clean --all` reports reticulate cache cleanup while leaving the Python cache artifacts created by ir behind. Add public coverage for an external-uv case, then either clear the external uv cache intentionally or narrow the CLI/docs to say `--all` only clears reticulate-managed uv caches.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 833a9f1f13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
<<<FINDING 1 START>>>
driver/cache-clean.R:145 only discovers an external uv with Sys.which("uv"), but reticulate also uses ~/.local/bin/uv when it is usable and not on PATH. In that common Linux/macOS setup, `ir cache clean --all` reports success while leaving the uv cache, uv Python dir, and uv tool dir that `ir` actually used untouched. Mirror reticulate's uv discovery, or call reticulate's non-bootstrapping uv resolver, and add a public CLI regression test with a fake uv at HOME/.local/bin outside PATH.
<<<FINDING 1 END>>>
Finding addressed: src/cache.rs:45-47 deletes the ir cache before validating or launching the R cleaner. If `IR_RSCRIPT`/`IR_R_VERSION` are conflicting, `Rscript` is missing, or the selected R startup fails, `ir cache clean --all` will already have removed the ir cache and then exit nonzero without cleaning the tool caches. Resolve/spawn the R cleaner before deleting the ir cache, or add a regression test that documents this partial-clean behavior as intentional.
Ensure Python cache marker tests that verify warm-cache behavior clear ambient uv resolver configuration inherited from the test runner. In this environment, UV_RUN_RECURSION_DEPTH is set, which correctly disables Python resolver cache markers and made these tests fail for the wrong reason. Addressed finding: <<<FINDING 1 START>>> Reviewer-facing readiness: the local branch I reviewed is three commits ahead of the PR branch (`HEAD` d4c0c7e vs `origin/ir-cache-clean-all` / PR head 833a9f1), so PR #117 currently does not include the external uv cache cleanup or the IR-cache-preservation fix. Push the local commits before another human review; also update the PR body because it omits the external uv cache deletion that is documented in docs/cache.qmd:37 and implemented in driver/cache-clean.R:200. <<<FINDING 1 END>>>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0fcfd2d79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addressed finding:
<<<FINDING 1 START>>>
driver/cache-clean.R:197-203 deletes the reticulate cache before resolving and running the external `uv` binary. If `RETICULATE_UV` or `options(reticulate.uv_binary)` points at a `uv` executable under `tools::R_user_dir("reticulate", "cache")`, line 197 removes that executable first, then the later `uv cache dir` / `uv python dir` / `uv tool dir` calls fail. That leaves `ir cache clean --all` partially cleaned and prevents the Rust-side `ir` cache removal in src/cache.rs:45-47. Resolve the uv directories before deleting the reticulate cache, or treat uv paths under the reticulate cache as managed and skip external uv cleanup.
<<<FINDING 1 END>>>
Finding addressed:
<<<FINDING 1 START>>>
driver/cache-clean.R:205 clears the external uv cache by recursively deleting the directory returned by `uv cache dir`. uv's supported cleanup path is `uv cache clean`, which takes uv's cache lock; direct deletion can race with another uv process and leave a shared cache in a broken or partial state. Keep resolving the path for reporting if needed, but call `system2(uv, c("cache", "clean"))` for the uv cache and add a public CLI test with a fake uv that asserts `cache clean` is invoked.
<<<FINDING 1 END>>>
Use cache-specific missing-Rscript advice for the R cache cleaner, and align
the no-global-renv fallback with renv's legacy root selection.
Finding 1:
<<<FINDING 1 START>>>
src/cache.rs:69-73 reuses `spawn_error()`, whose missing-Rscript advice says to “pass --rscript”, but `ir cache clean` has no `--rscript` option. A missing `Rscript` for the new `ir cache clean --all` path therefore points users at an invalid command. Add cache-specific launch advice or add a supported cache-level `--rscript` option, and cover it with a public CLI test.
<<<FINDING 1 END>>>
Finding 2:
<<<FINDING 2 START>>>
driver/cache-clean.R:77-92 only falls back to `tools::R_user_dir("renv", "cache")/cache` when `RENV_PATHS_CACHE` is unset and `renv` is not installed in the cleaner R session. `ir` can still populate renv’s older default cache root through its private resolver tooling, because renv chooses an existing legacy root before the current `R_user_dir` root. In that setup `ir cache clean --all` leaves the actual renv cache behind. Add a public CLI regression for an existing legacy renv root without `RENV_PATHS_CACHE`, and make the cleaner include the same fallback roots renv uses.
<<<FINDING 2 END>>>
Finding 1:
driver/cache-clean.R:95-101 resolves the pak cache with ir_r_user_cache_dir("pak") when R_PKG_CACHE_DIR is unset. On default Windows installs, that points at tools::R_user_dir("pak", "cache") under %LOCALAPPDATA%/R/cache/R/pak, but pak's private library fallback uses its own user_cache_dir("pak")/lib/... under %LOCALAPPDATA%/R/Cache/pak. As a result, ir cache clean --all can leave pak's private library behind on Windows; add a Windows/default-path public CLI regression and mirror pak's Windows default path for ir_pak_cache_dir().
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04321a17f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addressed finding: <<<FINDING 1 START>>> driver/cache-clean.R:214-293 skips uv cleanup whenever `RETICULATE_UV=managed` or no external uv is found, but `ir` resolves Python via `reticulate:::uv_get_or_create_env()`, which uses `uv tool run --isolated`. For reticulate-managed uv, reticulate sets `UV_CACHE_DIR` and `UV_PYTHON_INSTALL_DIR` under the reticulate cache, but leaves `UV_TOOL_DIR` at uv's default, so the actual tool environments can remain in `uv tool dir` after `ir cache clean --all` reports success. Add public coverage for the managed-uv/no-external-uv path and clear/query that tool dir before deleting the reticulate cache, or make the resolver put `UV_TOOL_DIR` under the reticulate cache too. <<<FINDING 1 END>>>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e38e4b6f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Resolve cache-clean review feedback for pak, renv, reticulate, and uv-owned paths. - Install Unix rig directly from the latest GitHub release asset in the dev setup script. - Add public CLI regressions for the reviewed cache-clean behavior. Finding 1: <<<FINDING 1 START>>> PR checks failed with exit code 1. stdout: Refreshing checks status every 10 seconds. Press Ctrl+C to quit. test (macos-latest) fail 1m46s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252532 test (ubuntu-latest) fail 1m44s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252545 rustfmt + clippy pass 21s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252543 test (windows-latest) pending 0 https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252641 Refreshing checks status every 10 seconds. Press Ctrl+C to quit. test (macos-latest) fail 1m46s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252532 test (ubuntu-latest) fail 1m44s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252545 rustfmt + clippy pass 21s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252543 test (windows-latest) pending 0 https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252641 test (macos-latest) fail 1m46s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252532 test (ubuntu-latest) fail 1m44s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252545 rustfmt + clippy pass 21s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252543 test (windows-latest) pass 11m19s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252641 <<<FINDING 1 END>>> Finding 2: <<<FINDING 2 START>>> Unresolved PR review comments: - src/cache.rs:? by chatgpt-codex-connector **<sub><sub></sub></sub> Honor IR_RSCRIPT when running cache cleaner** When `ir cache clean --all` is used in an environment that relies on `IR_RSCRIPT` or `IR_R_VERSION` because the desired R is not the default `Rscript` on `PATH`, this unconditionally falls back to `rscript_command()` and ignores those selectors. That makes the new command fail with “could not find `Rscript`” or clean paths for a different R setup even though other `ir` commands work with the configured R; resolve the R executable through the same environment-selection path used elsewhere before launching the cleaner. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Avoid deleting the R_PKG_CACHE_DIR parent** When `R_PKG_CACHE_DIR` is set to a shared cache root, the code above already treats `<root>/R/pkgcache` as pkgcache's cache, but this branch returns the parent root as a second “pak cache” and `ir_clear_cache()` will `unlink()` it recursively. That can remove sibling data such as renv caches or other application files under the same root instead of only pak/pkgcache state; use pak's actual cache path/env vars or avoid deleting the parent directory here. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Use renv's actual default cache root** When `RENV_PATHS_CACHE` and `RENV_PATHS_ROOT` are unset but the user already has renv's legacy root (for example `~/.local/share/renv`), renv chooses that existing root for `renv::use()`. This fallback always constructs the newer `tools::R_user_dir("renv", "cache")/cache` path, so `ir cache clean --all` clears or reports the wrong location while leaving the renv cache actually used by `ir` intact; query renv's own cache path or account for its legacy-root selection. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:99 by chatgpt-codex-connector **<sub><sub></sub></sub> Honor pak's configured cache directories** When pak is configured through its documented `PKG_CACHE_DIR` / `pkg.cache_dir` (or related `PKG_PACKAGE_CACHE_DIR` / `pkg.package_cache_dir`) settings, including from an `R_PROFILE_USER` that this cleaner intentionally loads, the resolver's pak subprocess writes downloads/package cache there, but this helper only derives a path from `R_PKG_CACHE_DIR` or `tools::R_user_dir("pak", "cache")`. In that setup `ir cache clean --all` reports/removes the hard-coded default while leaving the actual pak cache intact, so subsequent runs still reuse the supposedly-cleared pak data; query pak/pkgdepends configuration after startup instead of inferring these paths. Pak d... #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Keep cache cleaning compatible with older R** When `IR_RSCRIPT` or `IR_R_VERSION` selects R 3.5/3.6, normal `ir run` paths can still work because the resolver has fallbacks for pre-4.0 cache directories, but this cleaner unconditionally calls `tools::R_user_dir()`, which was introduced in R 4.0. In that environment `ir cache clean --all` removes the Rust cache first and then aborts before clearing the pak/renv/reticulate caches; use the same fallback logic here before calling `tools::R_user_dir()`. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Don't miss renv's legacy cache without global renv** When the user has only ir's private resolver copy of `renv` (the README says users don't need to pre-install it) and an existing legacy renv root such as `~/.local/share/renv`, this fallback still guesses the newer `tools::R_user_dir("renv", "cache")/cache` location and leaves the cache actually populated by ir untouched. Fresh evidence in this version is that the cleaner runs a plain Rscript without adding ir's tooling library to `.libPaths()` (and the ir cache is cleared before this script runs), so `requireNamespace("renv")` is false unless the user also installed renv globally. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Do not delete uv's installed tools directory** In environments where the external `uv` binary is shared with normal user workflows, this removes the directory returned by `uv tool dir`. I checked `uv tool dir --help` and uv's storage docs: this is the persistent tool installation directory used by `uv tool install`, not a disposable cache, and installed tool environments are not removed unless the tool is uninstalled. `ir cache clean --all` can therefore delete unrelated globally installed tools such as `ruff` or `black` even though they are not used by ir's resolver; limit the cleanup to uv's cache/ir-reticulate state instead of unlinking the tool directory. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:321 by chatgpt-codex-connector **<sub><sub></sub></sub> Preserve uv-managed Python installations** When an external `uv` binary is shared with normal user workflows, this treats `uv python dir` as a cache and recursively deletes it. I checked `uv python dir --help`, which describes this as the directory where `uv python` installs Python executables, so `ir cache clean --all` can remove uv-managed interpreters used by other projects instead of just clearing disposable cache state; limit this cleanup to `uv cache clean` or ir-owned reticulate state. Useful? React with 👍 / 👎. #117 (comment) <<<FINDING 2 END>>> Finding 3: <<<FINDING 3 START>>> Cloud review posted new PR feedback. Inspect the PR comments, reviews, and review threads, then address all actionable items. <<<FINDING 3 END>>> Additional PR feedback addressed: **<sub><sub></sub></sub> Treat empty XDG_DATA_HOME as unset** When `XDG_DATA_HOME` is exported as an empty string, `Sys.getenv("XDG_DATA_HOME", "~/.local/share")` returns `""` rather than the default, so the legacy renv root becomes the relative path `renv`. If `~/.cache/R/renv` is absent and `ir cache clean --all` is run from a project that has `renv/cache`, this will delete the project-local directory instead of the global legacy cache; check `nzchar()` before using the environment value. **<sub><sub></sub></sub> Use an absolute Windows fallback when LOCALAPPDATA is unset** When the cleaner runs under an older R without `tools::R_user_dir()` on Windows and `LOCALAPPDATA` is unset, this fallback constructs the relative path `R/cache/R/<package>`. The Rust cache code already supports falling back to `USERPROFILE` in that environment, so `ir cache clean --all` can otherwise delete a project-local `R/cache` directory and still miss the user-profile cache locations; check `LOCALAPPDATA` and fall back before returning this path.
Finding 1: tests/run.rs:2055 runs `ir cache clean --all` in `cache_clean_all_uses_absolute_windows_r_user_dir_fallback` without isolating `HOME` or `XDG_CACHE_HOME`. When `rappdirs` is installed, driver/cache-clean.R:203-206 resolves the legacy reticulate cache through `rappdirs::user_cache_dir()`, which ignores the test's global `.Platform`/`Sys.info` shims and can point at the developer's real cache, e.g. `~/Library/Caches/r-reticulate`, before `ir_clear_cache()` unlinks it. Set `HOME`/`XDG_CACHE_HOME` to temp dirs for this test, or force the no-rappdirs branch in its R profile, so the public CLI test cannot delete user state.
Remove unsupported pkg.* and PKG_* cache selectors from the all-cache cleaner. Current pak/pkgcache derive their cache locations from R_PKG_CACHE_DIR, R_USER_CACHE_DIR, and tools::R_user_dir(), so the cleaner should not treat generic pkg configuration names as deletion targets. Finding 1: driver/cache-clean.R:105-136 treats `options(pkg.cache_dir)`, `options(pkg.package_cache_dir)`, `PKG_CACHE_DIR`, and `PKG_PACKAGE_CACHE_DIR` as pak/pkgcache cache locations and deletes those paths during `ir cache clean --all`. Current pak/pkgcache do not read those settings; they derive cache roots from `R_PKG_CACHE_DIR`, `R_USER_CACHE_DIR`, and `tools::R_user_dir()`. This can delete an unrelated directory just because the user has a generic `PKG_CACHE_DIR` or `pkg.cache_dir` set. Remove those unsupported selectors, or replace them with calls to the real pak/pkgcache APIs and cover that public behavior.
Include pkgdepends' public pak cache configuration when resolving paths for `ir cache clean --all`, so caches selected by R startup files are cleared along with the default pak/pkgcache locations. Finding 1: driver/cache-clean.R:105 derives pak/pkgcache cleanup paths only from R_PKG_CACHE_DIR, R_USER_CACHE_DIR, and defaults, while tests/run.rs:1805 and tests/run.rs:1870 explicitly assert that pkg.cache_dir/pkg.package_cache_dir and PKG_CACHE_DIR/PKG_PACKAGE_CACHE_DIR are preserved. That contradicts the `--all` promise to clear the pak caches used after R startup files run: pak reads `pkg.*` options and `PKG_*` env vars as public config and passes `package_cache_dir` into its package cache during `pak::pkg_deps()`. Users with those cache locations set can keep stale pak resolver state after `ir cache clean --all`.
Addressed finding: <<<FINDING 1 START>>> driver/cache-clean.R:114-133 misses pkgdepends' configured metadata cache. The cleaner handles `pkg.cache_dir`/`PKG_CACHE_DIR` and `pkg.package_cache_dir`/`PKG_PACKAGE_CACHE_DIR`, but pak forwards `pkg.metadata_cache_dir`/`PKG_METADATA_CACHE_DIR` to pkgdepends, where it is used for the cranlike metadata cache. Users with that persistent cache configured will still keep stale pak metadata after `ir cache clean --all`; add this path to the R cleaner and cover it with a public CLI regression test. <<<FINDING 1 END>>>
Use the workflow token when the Unix dev-dependency script queries the latest rig release asset, so CI avoids anonymous GitHub API rate limits. Addressed finding 1: <<<FINDING 1 START>>> PR checks failed with exit code 1. stdout: test (macos-latest) fail 1m46s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252532 test (ubuntu-latest) fail 1m44s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252545 rustfmt + clippy pass 21s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252543 test (windows-latest) pass 11m19s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252641 test (macos-latest) fail 1m46s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252532 test (ubuntu-latest) fail 1m44s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252545 rustfmt + clippy pass 21s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252543 test (windows-latest) pass 11m19s https://github.com/t-kalinowski/ir/actions/runs/28096514779/job/83187252641 <<<FINDING 1 END>>> Addressed finding 2: <<<FINDING 2 START>>> Unresolved PR review comments: - src/cache.rs:? by chatgpt-codex-connector **<sub><sub></sub></sub> Honor IR_RSCRIPT when running cache cleaner** When `ir cache clean --all` is used in an environment that relies on `IR_RSCRIPT` or `IR_R_VERSION` because the desired R is not the default `Rscript` on `PATH`, this unconditionally falls back to `rscript_command()` and ignores those selectors. That makes the new command fail with “could not find `Rscript`” or clean paths for a different R setup even though other `ir` commands work with the configured R; resolve the R executable through the same environment-selection path used elsewhere before launching the cleaner. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Avoid deleting the R_PKG_CACHE_DIR parent** When `R_PKG_CACHE_DIR` is set to a shared cache root, the code above already treats `<root>/R/pkgcache` as pkgcache's cache, but this branch returns the parent root as a second “pak cache” and `ir_clear_cache()` will `unlink()` it recursively. That can remove sibling data such as renv caches or other application files under the same root instead of only pak/pkgcache state; use pak's actual cache path/env vars or avoid deleting the parent directory here. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Use renv's actual default cache root** When `RENV_PATHS_CACHE` and `RENV_PATHS_ROOT` are unset but the user already has renv's legacy root (for example `~/.local/share/renv`), renv chooses that existing root for `renv::use()`. This fallback always constructs the newer `tools::R_user_dir("renv", "cache")/cache` path, so `ir cache clean --all` clears or reports the wrong location while leaving the renv cache actually used by `ir` intact; query renv's own cache path or account for its legacy-root selection. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Honor pak's configured cache directories** When pak is configured through its documented `PKG_CACHE_DIR` / `pkg.cache_dir` (or related `PKG_PACKAGE_CACHE_DIR` / `pkg.package_cache_dir`) settings, including from an `R_PROFILE_USER` that this cleaner intentionally loads, the resolver's pak subprocess writes downloads/package cache there, but this helper only derives a path from `R_PKG_CACHE_DIR` or `tools::R_user_dir("pak", "cache")`. In that setup `ir cache clean --all` reports/removes the hard-coded default while leaving the actual pak cache intact, so subsequent runs still reuse the supposedly-cleared pak data; query pak/pkgdepends configuration after startup instead of inferring these paths. Pak d... #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Keep cache cleaning compatible with older R** When `IR_RSCRIPT` or `IR_R_VERSION` selects R 3.5/3.6, normal `ir run` paths can still work because the resolver has fallbacks for pre-4.0 cache directories, but this cleaner unconditionally calls `tools::R_user_dir()`, which was introduced in R 4.0. In that environment `ir cache clean --all` removes the Rust cache first and then aborts before clearing the pak/renv/reticulate caches; use the same fallback logic here before calling `tools::R_user_dir()`. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Don't miss renv's legacy cache without global renv** When the user has only ir's private resolver copy of `renv` (the README says users don't need to pre-install it) and an existing legacy renv root such as `~/.local/share/renv`, this fallback still guesses the newer `tools::R_user_dir("renv", "cache")/cache` location and leaves the cache actually populated by ir untouched. Fresh evidence in this version is that the cleaner runs a plain Rscript without adding ir's tooling library to `.libPaths()` (and the ir cache is cleared before this script runs), so `requireNamespace("renv")` is false unless the user also installed renv globally. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Do not delete uv's installed tools directory** In environments where the external `uv` binary is shared with normal user workflows, this removes the directory returned by `uv tool dir`. I checked `uv tool dir --help` and uv's storage docs: this is the persistent tool installation directory used by `uv tool install`, not a disposable cache, and installed tool environments are not removed unless the tool is uninstalled. `ir cache clean --all` can therefore delete unrelated globally installed tools such as `ruff` or `black` even though they are not used by ir's resolver; limit the cleanup to uv's cache/ir-reticulate state instead of unlinking the tool directory. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Preserve uv-managed Python installations** When an external `uv` binary is shared with normal user workflows, this treats `uv python dir` as a cache and recursively deletes it. I checked `uv python dir --help`, which describes this as the directory where `uv python` installs Python executables, so `ir cache clean --all` can remove uv-managed interpreters used by other projects instead of just clearing disposable cache state; limit this cleanup to `uv cache clean` or ir-owned reticulate state. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Treat empty XDG_DATA_HOME as unset** When `XDG_DATA_HOME` is exported as an empty string, `Sys.getenv("XDG_DATA_HOME", "~/.local/share")` returns `""` rather than the default, so the legacy renv root becomes the relative path `renv`. If `~/.cache/R/renv` is absent and `ir cache clean --all` is run from a project that has `renv/cache`, this will delete the project-local directory instead of the global legacy cache; check `nzchar()` before using the environment value. Useful? React with 👍 / 👎. #117 (comment) - driver/cache-clean.R:? by chatgpt-codex-connector **<sub><sub></sub></sub> Use an absolute Windows fallback when LOCALAPPDATA is unset** When the cleaner runs under an older R without `tools::R_user_dir()` on Windows and `LOCALAPPDATA` is unset, this fallback constructs the relative path `R/cache/R/<package>`. The Rust cache code already supports falling back to `USERPROFILE` in that environment, so `ir cache clean --all` can otherwise delete a project-local `R/cache` directory and still miss the user-profile cache locations; check `LOCALAPPDATA` and fall back before returning this path. Useful? React with 👍 / 👎. #117 (comment) <<<FINDING 2 END>>>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b00019460
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mask rappdirs in cache-clean tests that create and assert the fallback legacy reticulate cache under R_USER_CACHE_DIR. This keeps those tests on the path they materialize instead of allowing an installed rappdirs package to redirect cleanup to a user cache outside the fixture.
Addressed findings:
<<<FINDING 1 START>>>
tests/run.rs:2448: The new cache-clean tests compute the legacy reticulate cache as `R_USER_CACHE_DIR/r-reticulate`, but driver/cache-clean.R:207 uses `rappdirs::user_cache_dir("r-reticulate", NULL)` whenever `rappdirs` is installed. CI installs `reticulate`, which imports `rappdirs`, before these tests run, so assertions such as tests/run.rs:1648 will leave the temp cache in place and fail; on developer machines they can also delete the real user legacy reticulate cache. Isolate or mask `rappdirs` in these tests, or create/assert the same path the driver will use.
<<<FINDING 1 END>>>
<<<FINDING 2 START>>>
The worktree is one commit ahead of `origin/ir-cache-clean-all` (`cbf773f`, `Use GitHub token for rig release lookup`), while the open PR tracks the remote branch. Push that commit before the next human review round, otherwise reviewers will not see the CI token fix present in the local diff.
<<<FINDING 2 END>>>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 718bae27f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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
1 participant
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
Adds
ir cache clean --allso users can clearir's own cache along with the pak/pkgcache, renv, and reticulate caches used during dependency and Python environment resolution.This is useful when resolver/tooling state needs a full reset. External cache locations are resolved from R at runtime, after R startup files and environment setup have run, instead of hardcoding those paths in Rust.
User-facing changes
ir cache clean --all.ir, pak/pkgcache, renv, reticulate, and legacy reticulate caches.Internal changes
driver/cache-clean.Rfor R-owned cache path resolution and deletion.Rscriptfrom the cache command.ircache only.R_PROFILE_USER.