fix(rust): resolve relative CARGO_HOME/RUSTUP_HOME to absolute paths#8604
fix(rust): resolve relative CARGO_HOME/RUSTUP_HOME to absolute paths#8604
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where relative Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue where relative paths for CARGO_HOME or RUSTUP_HOME could cause problems. The approach of resolving them to absolute paths is sound. My review includes suggestions to simplify the implementation in both rustup_home and cargo_home functions. These changes will improve code clarity and remove duplication.
Greptile SummaryThis PR fixes a real bug where relative Two minor quality gaps are worth addressing before merge:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["rustup_home() / cargo_home() called"] --> B["Read from Settings / env var"]
B --> C{"Path found?"}
C -- No --> D["Use default: dirs::HOME.join('.rustup|.cargo')"]
C -- Yes --> E["Use configured path"]
D --> F{"path.is_relative()?"}
E --> F
F -- No --> G["Return absolute path as-is"]
F -- Yes --> H["Call std::env::current_dir()"]
H --> I{"current_dir() succeeds?"}
I -- Yes --> J["Return cwd.join(path)"]
I -- No --> K["Silent fallback: return relative path unchanged\n⚠ still broken behaviour"]
Last reviewed commit: 29fcba9 |
de52d75 to
683762d
Compare
When CARGO_HOME or RUSTUP_HOME is set to a relative path (e.g. ".cargo" via [env] in mise.toml), the rust plugin used it as-is, producing relative PATH entries like ".cargo/bin". After mise hook-env persists these into the shell and the user changes directories, the relative paths break — surfacing as "undefined/bin" in PATH. Resolve relative paths to absolute via current_dir() in cargo_home() and rustup_home() before returning. Co-Authored-By: Claude Opus 4.6 <[email protected]>
683762d to
29fcba9
Compare
| if path.is_relative() { | ||
| std::env::current_dir() | ||
| .map(|cwd| cwd.join(&path)) | ||
| .unwrap_or(path) | ||
| } else { | ||
| path | ||
| } |
There was a problem hiding this comment.
Silent fallback to relative path on current_dir() failure
When std::env::current_dir() fails (e.g. the working directory has been deleted, or the process lacks read permission on it), unwrap_or(path) silently falls back to returning the original relative path — which is exactly the broken behaviour this PR is fixing. Users would see the same undefined/bin symptom with no indication of why.
Consider emitting a warn! log so that users (and bug reports) have an actionable signal:
| if path.is_relative() { | |
| std::env::current_dir() | |
| .map(|cwd| cwd.join(&path)) | |
| .unwrap_or(path) | |
| } else { | |
| path | |
| } | |
| if path.is_relative() { | |
| match std::env::current_dir() { | |
| Ok(cwd) => cwd.join(&path), | |
| Err(e) => { | |
| warn!("could not determine current directory to resolve relative path {:?}: {e}", path); | |
| path | |
| } | |
| } | |
| } else { | |
| path | |
| } |
The same applies to cargo_home() at lines 384–390.
### 🐛 Bug Fixes - **(config)** resolve trust hash collision for same-name directories by @tdragon in [#8628](#8628) - **(docs)** fix width of tools table by @himkt in [#8625](#8625) - **(docs)** prevent homepage hero atmosphere overflow by @nygmaaa in [#8642](#8642) - **(doctor)** detect PATH ordering issues when mise is activated by @jdx in [#8585](#8585) - **(git)** use origin as remote name by @bentinata in [#8626](#8626) - **(installer)** normalize current version before comparison by @tak848 in [#8649](#8649) - **(lockfile)** Resolve symlink when updating lockfiles by @chancez in [#8589](#8589) - **(python)** verify checksums for precompiled binary downloads by @malept in [#8593](#8593) - **(rust)** resolve relative CARGO_HOME/RUSTUP_HOME to absolute paths by @simonepri in [#8604](#8604) - **(task)** correctly resolve task name for _default files with extensions by @youta1119 in [#8646](#8646) - **(tasks)** global file tasks not properly marked as such by @roele in [#8618](#8618) - **(tasks)** handle broken pipe in table print and task completion output by @vmaleze in [#8608](#8608) - use dark/light logo variants in README for GitHub dark mode by @jdx in [#8656](#8656) - failing rebuild of runtime symlinks for shared tools by @roele in [#8647](#8647) - add spaces around current element operator in flutter version_expr by @roele in [#8616](#8616) - complete task arguments correctly by @KevSlashNull in [#8601](#8601) ### 📚 Documentation - switch body font to DM Sans and darken dark mode background by @jdx in [6e3ad34](6e3ad34) - use Cormorant Garamond for headers and Roc Grotesk for body text by @jdx in [010812a](010812a) - resolve chaotic heading hierarchy in task-arguments.md by @muzimuzhi in [#8644](#8644) ### 🧪 Testing - fix test_java and mark as slow by @roele in [#8634](#8634) ### 📦️ Dependency Updates - update docker/dockerfile:1 docker digest to 4a43a54 by @renovate[bot] in [#8657](#8657) - update ghcr.io/jdx/mise:alpine docker digest to 2584470 by @renovate[bot] in [#8658](#8658) ### 📦 Registry - add viteplus (npm:vite-plus) by @risu729 in [#8594](#8594) - remove backend.options for podman by @roele in [#8633](#8633) - add pi.dev coding agent by @dector in [#8635](#8635) - add ormolu ([github:tweag/ormolu](https://github.com/tweag/ormolu)) by @3w36zj6 in [#8617](#8617) - use version_expr for dart and sort versions by @roele in [#8631](#8631) ### New Contributors - @bentinata made their first contribution in [#8626](#8626) - @tdragon made their first contribution in [#8628](#8628) - @nygmaaa made their first contribution in [#8642](#8642) - @youta1119 made their first contribution in [#8646](#8646) - @chancez made their first contribution in [#8589](#8589) - @dector made their first contribution in [#8635](#8635) - @tak848 made their first contribution in [#8649](#8649) ## 📦 Aqua Registry Updates #### New Packages (5) - [`acsandmann/rift`](https://github.com/acsandmann/rift) - [`alltuner/mise-completions-sync`](https://github.com/alltuner/mise-completions-sync) - [`berbicanes/apiark`](https://github.com/berbicanes/apiark) - [`gitlab.com/graphviz/graphviz`](https://github.com/gitlab.com/graphviz/graphviz) - [`jorgelbg/pinentry-touchid`](https://github.com/jorgelbg/pinentry-touchid) #### Updated Packages (7) - [`UpCloudLtd/upcloud-cli`](https://github.com/UpCloudLtd/upcloud-cli) - [`aquaproj/registry-tool`](https://github.com/aquaproj/registry-tool) - [`go-swagger/go-swagger`](https://github.com/go-swagger/go-swagger) - [`gopinath-langote/1build`](https://github.com/gopinath-langote/1build) - [`sassman/t-rec-rs`](https://github.com/sassman/t-rec-rs) - [`sharkdp/fd`](https://github.com/sharkdp/fd) - [`temporalio/cli`](https://github.com/temporalio/cli)
Problem
When
CARGO_HOMEorRUSTUP_HOMEis set to a relative path (e.g..cargovia[env]inmise.toml), the rust plugin uses it as-is. This produces relative PATH entries like.cargo/binthat only work from the project directory. Whenmise hook-envpersists these into the shell and the user changes directories, the relative paths break — surfacing asundefined/binin PATH.Solution
cargo_home()andrustup_home()now resolve relative paths to absolute usingcurrent_dir()before returningRepro