Repository navigation
fix: clean upm project-local state between benchmark runs - #193
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 minutes. View limit details
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Remove transient package-manager artifacts suite/scripts/clean-helpers.sh |
clean_package_manager_files now removes .upm alongside other transient package-manager artifacts. |
Priority:
Estimated code review effort: 1 (Trivial) | ~2 minutes
Change: Bug fix
Suggested reviewers: ejkorol
Merge Risk: 
Minimal · previous review a4b6c
-
The change safely clears project-local upm cache state between benchmark runs, with no established merge-blocking risk.
Pre-merge checks: 
5
-
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
The upm cleanup was incomplete in two ways: 1. clean_upm_cache() only wiped ~/.upm/, but upm uses XDG base directories on Linux: the store and registry metadata go to $XDG_DATA_HOME/upm (default ~/.local/share/upm), and the compile cache goes to $XDG_CACHE_HOME/upm (default ~/.cache/upm). upm falls back to ~/.upm only when ~/.upm/store already exists. On a fresh CI runner, ~/.upm/store never exists, so the store lands in ~/.local/share/upm/ and the cache in ~/.cache/upm/ — neither of which was cleaned. 2. clean_package_manager_files() cleaned .aube (aube's project-local state) but not .upm or .upm-store (upm's project-level artifacts), so upm's local state could persist across runs. Now clean_upm_cache() wipes all three locations (~/.upm, the XDG data dir, and the XDG cache dir), and clean_package_manager_files() removes .upm and .upm-store alongside the other PMs' project artifacts. Co-authored-by: Evert Pot <me@evertpot.com>
a4b6c25 to
625f336
Compare
Problem
upm's cache cleanup between benchmark runs was incomplete, giving upm an unfair advantage:
1. Global cache: XDG directories not cleaned
clean_upm_cache() only wiped ~/.upm/, but upm uses XDG base directories on Linux:
upm only falls back to ~/.upm when ~/.upm/store already exists. On a fresh CI runner (or after a clean that wipes ~/.upm), the store lands in ~/.local/share/upm/ and the cache in ~/.cache/upm/ — neither of which was being cleaned.
2. Project-local state not cleaned
clean_package_manager_files() removed .aube (aube's project-local state) but not .upm or .upm-store (upm's project-level artifacts).
Fix
Verification
Cache path logic verified against upm source (src/util.ts upmHome function).
Co-authored-by: Evert Pot me@evertpot.com