fix(install): run tool postinstall against the just-installed version#10415
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesPostinstall hook bin path fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Greptile SummaryThis PR fixes a bug where a tool-level
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to The fix correctly addresses the root cause: No files require special attention. Important Files Changed
Reviews (4): Last reviewed commit: "fix(install): run tool postinstall again..." | Re-trigger Greptile |
82cdc20 to
b6d0bd2
Compare
|
Also addressed the "Comments Outside Diff" note about |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/toolset/tool_version.rs`:
- Around line 838-852: The regression test in the ToolVersion fuzzy version
resolution scenario does not actually set up the problematic case it is supposed
to validate. The test needs to be hardened by creating a stale fuzzy runtime
symlink (such as installs/dummy/3 pointing to a previous version) before
constructing the ToolVersion, and then add explicit assertions to verify the
difference between unlocked and locked behavior: the unlocked runtime_path()
call should return the stale symlink path (demonstrating the bug), while the
with_locked().runtime_path() call should return the correct install_path
(demonstrating the fix). This ensures the test will catch regressions if
with_locked() is broken in the future.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68238779-20eb-4324-b910-39926254e208
📒 Files selected for processing (2)
src/backend/mod.rssrc/toolset/tool_version.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/backend/mod.rs
A tool-level `postinstall` hook resolved its bin PATH from the request's runtime path. For a fuzzy version request (e.g. `[tools.python] version = "3"`) that is the runtime symlink `installs/python/3`, which during `mise up` still points at the previous version because symlinks are only rebuilt after all installs finish. So `postinstall = "pip install ..."` for python 3.14.6 found pip from the old 3.14.5 tree and installed into it -- which mise then uninstalled (jdx#10347). Resolve both the hook's environment (exec_env) and its bin paths against the exact version just installed by locking a clone of the ToolVersion (runtime_path() then returns install_path() instead of the fuzzy runtime symlink). Add ToolVersion::with_locked for this; `locked` is ignored by Eq/Hash so the install-path cache key is unchanged. Addresses discussion jdx#10347. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
b6d0bd2 to
0c3a846
Compare
Problem
A tool-level
postinstallhook can run against a different version of the tool than the one just installed. Reported in #10347:Upgrading to Python 3.14.6 ran the hook''s
pipfrom the previous3.14.5install, so the packages were installed into the3.14.5tree — which mise then uninstalled. The docs promise the tool''s bin path is on PATH during the hook.Root cause
run_postinstall_hookbuilds the hook''s PATH fromlist_bin_paths, whose default resolvestv.runtime_path(). For a fuzzy version request (e.g.version = "3"),runtime_path()returns the runtime symlinkinstalls/python/3. Duringmise upthat symlink still points at the previous version, because runtime symlinks are only rebuilt after all installs finish — while the postinstall hook runs during the install. Sopipresolved from the stale3.14.5tree.Fix
Resolve the postinstall hook''s bin paths against the exact version just installed by locking a clone of the
ToolVersion:runtime_path()returnsinstall_path()(the concrete version) when locked, so the hook''s PATH points at the version just installed. A newToolVersion::with_lockedbuilder sets this;lockedis already ignored byEq/Hash, so the install-path cache key is unchanged. The backendlist_bin_pathsindirection is preserved (aqua etc. still work via their install-path-based dirs).Testing
with_locked(true).runtime_path()resolves toinstall_path()for a fuzzy request.cargo fmt --all -- --checkpasses.Note: the
$MISE_TOOL_INSTALL_PATHenv var mise already sets for the hook remains a deterministic workaround for older versions.Addresses #10347
🤖 Generated with Claude Code
Summary by CodeRabbit