fix(rust): reinstall incomplete rustup toolchains#9839
Conversation
Greptile SummaryThis PR fixes a class of cache-restore failures where
Confidence Score: 5/5The Rust-side changes are well-scoped to install-decision paths only; hook-env and status paths retain the cheap marker check. The expensive filesystem check is carefully bounded to install decisions, and the e2e test covers the primary scenarios directly. xtasks/render/schema.ts — the prettier invocation has no fallback if prettier is unavailable Important Files Changed
Reviews (2): Last reviewed commit: "fix(schema): keep rendered json formatte..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn rust_component_candidates(component: &str, host: &str) -> Vec<String> { | ||
| let mut candidates = vec![component.to_string(), format!("{component}-{host}")]; | ||
| if !component.ends_with("-preview") { | ||
| let preview = format!("{component}-preview"); | ||
| candidates.push(preview.clone()); | ||
| candidates.push(format!("{preview}-{host}")); | ||
| } | ||
| candidates.sort(); | ||
| candidates.dedup(); | ||
| candidates | ||
| } |
There was a problem hiding this comment.
rust_component_candidates generates preview-variant names like "rust-std-wasm32-unknown-unknown-preview" and "rust-std-wasm32-unknown-unknown-preview-x86_64-unknown-linux-gnu" for any component that doesn't end with -preview. These are not valid rustup component entries and can never appear in a lib/rustlib/components file. The correct match for a target's std library comes from the exact "rust-std-{target}" candidate, which is already included. The preview variants are appropriate for components like "clippy" or "llvm-tools" (where rustup stores them as "clippy-preview-{host}"), but not for "rust-std-{target}" components whose host is encoded in the target triple, not appended as a separate suffix.
| if required_components.is_empty() && required_targets.is_empty() { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
When a tool is configured without profile, components, or targets (e.g., rust = "1.81.0"), required_components and required_targets are both empty, and the function returns true after only verifying that settings.toml, the three proxy bins, and the toolchain directory exist. If a toolchain directory is present but the lib/rustlib/components file is absent or empty — which can happen after a partially-interrupted rustup toolchain install — the completeness check reports the install as complete. The PR's primary motivation is cache-restore scenarios; the toolchain-dir check covers the main case, but a missing components file within an existing directory would be silently ignored.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the Rust installation process by introducing a more comprehensive completeness check. It adds the is_version_installed_for_install method to verify that essential components, targets, and proxy binaries are present, rather than just checking for the existence of the installation directory. A new E2E test is included to validate these changes. Feedback suggests extending the component verification to include other Rust profiles, such as the 'complete' profile, to ensure full installation integrity across different configurations.
| if profile.as_deref() == Some("default") { | ||
| required_components.extend(["rustfmt".to_string(), "clippy".to_string()]); | ||
| } |
There was a problem hiding this comment.
The current implementation only explicitly checks for rustfmt and clippy when the profile is set to default. While this covers the most common use case, it might be worth considering if other profiles (like complete) should also have their expected components verified to ensure a truly complete installation state.
| tools | ||
| } | ||
|
|
||
| pub async fn missing_tools_for_install(&self, config: &Arc<Config>) -> Vec<&ToolRequest> { |
There was a problem hiding this comment.
it's too hacky and this isn't a bug I really care that much to resolve
There was a problem hiding this comment.
I'm sorry, I thought I told Codex to create a draft PR. I agree with you, it doesn't affect performance that much, but it's too much for just Rust.
What about jdx/mise-action#474? It's still hacky as we can't simply exclude a child dir... It might be better to just document it doesn't work well with Rust so cache by yourself or force reinstall Rust.
There was a problem hiding this comment.
not sure, I'm half-tempted to just deprecate rust since I don't use it and don't really think others should. Though I know some do like it.
There was a problem hiding this comment.
That's fair. I use mise to manage Rust because I prefer managing everything with mise, but just because of it.
I think a config like nightly for clippy and stable for other components is not supported in .rust-toolchain.toml and could be a case mise can be used for, but I don't think it's a good use case. I'm not even sure if it actually works.
2 participants
Summary
installs/rust/<version>marker as incomplete when the matching rustup/Cargo state is absentprofile = "default"clippy/rustfmt, explicit components, explicit targets, customMISE_RUSTUP_HOME/MISE_CARGO_HOME, and the hook-env fast pathWhy this shape
This supersedes the action-side cache proposal in jdx/mise-action#467. In that review, @jdx pushed back on making mise-action own a Rust cache model: jdx/mise-action#467 (comment)
This PR avoids that model. mise-action can keep caching mise state the same way it already does; mise itself now notices, only during install decisions, that the restored Rust install marker is not enough if rustup's toolchain/components are missing. The interactive paths still use the cheap check: hook-env, PATH generation, status display, list_current_installed_versions, and normal command lookup do not perform Rust filesystem validation.
The Rust completeness check is filesystem-only and scoped to install. It first requires the existing mise install marker, then verifies the configured
MISE_CARGO_HOME/MISE_RUSTUP_HOMEcontain the proxy bins, requested toolchain directory,profile = "default"rustfmt/clippy, explicitly configured components, and explicitly configured targets. If that state is missing,mise installrerunsrustup toolchain installinstead of reportingall tools are installed.Related action failure: jdx/mise-action#215
Supersedes: jdx/mise-action#467
Test plan
cargo fmt --checkgit diff --check$(rustup which cargo) build -p mise --all-featuresmise run test:e2e e2e/core/test_rust_install_completenessThis PR body was generated by an AI coding assistant.