fix: Rust channel updates installing twice#7565
Conversation
rjvkn
changed the title
roele
left a comment
roele
left a comment
There was a problem hiding this comment.
Great contribution! This topic came up a couple times already.
Only improvement/nitpick i see is that conditional use of the CmdLineRunner could be simplified a bit.
Great suggestion! This eliminates the duplication nicely. Will update. |
|
Bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| runner = runner.arg("toolchain").arg("install"); | ||
| } | ||
|
|
||
| runner |
There was a problem hiding this comment.
rustup update called with unsupported command-line flags
When a channel (nightly/stable/beta) is already installed, the code now uses rustup update instead of rustup toolchain install. However, the command still appends --profile, --component, and --target arguments which are only valid for rustup toolchain install. If users have a rust-toolchain.toml with profile, components, or targets configured, the rustup update command will fail with unknown argument errors. The conditional should also exclude these arguments when using the update subcommand.
| .arg("install") | ||
| let mut runner = CmdLineRunner::new(RUSTUP_BIN).with_pr(ctx.pr.as_ref()); | ||
|
|
||
| if self.is_channel(&tv) && tv.install_path().exists() { |
There was a problem hiding this comment.
Install path check incorrect due to pre-created directory
The condition tv.install_path().exists() always evaluates to true for channel installs because the base install_version method calls create_install_dirs before invoking install_version_, which creates an empty directory at the install path. This means the code uses rustup update instead of rustup toolchain install even for fresh channel installations or force reinstalls where the toolchain was just uninstalled. The check was intended to detect if a channel is already installed in rustup, but it actually detects whether the mise install directory exists (which is always true at this point in the flow).
|
@jdx @roele I updated the scope of this PR to keep it strictly focused on fixing the redundant uninstall for channel-based upgrades when the resolved version doesn’t change. I moved the broader “use update commands instead of uninstall/reinstall” idea out of this PR and will revisit it separately with a cleaner, more general approach. |
|
bugbot run |
3 participants
Summary
This PR fixes an issue in the upgrade flow where channel-based versions were being uninstalled twice during upgrade when the resolved version did not change.
The change is intentionally minimal and only removes the final unnecessary uninstall step.
Current behavior
For channel-based versions (i.e. versions that resolve to the same string on upgrade), mise currently performs:
The last uninstall is redundant and can remove the freshly installed tool.
Fix
Skip the final uninstall step when the currently installed version string is identical to the resolved latest version string.
Scope
current == latestNote
While working on this, I initially explored using tool-specific update commands for channel-based upgrades instead of the uninstall/reinstall flow, using
rusttool as a pilot.Even with relatively small changes, that approach increased complexity and made the code harder to reason about. To keep this PR focused, readable, and easy to review, I decided to keep this minimal fix separate and revisit a cleaner, more general approach in a follow-up.
Follow-up
A future PR could explore a cleaner and more generalized upgrade strategy for channel-based versions, but that work is intentionally out of scope for this fix.
Note
Prevents double-uninstall during upgrades of channel-based versions that resolve in-place.
to_removecomputation inupgrade.rsto skip uninstall wheno.currentequalso.latest(e.g.,nightly,stable,beta)Written by Cursor Bugbot for commit 41ac56d. This will update automatically on new commits. Configure here.