Skip to content

fix(cargo): skip binstall for cargo install options#9928

Merged
jdx merged 1 commit into
jdx:mainfrom
risu729:fix/cargo-binstall-tool-options
May 18, 2026
Merged

fix(cargo): skip binstall for cargo install options#9928
jdx merged 1 commit into
jdx:mainfrom
risu729:fix/cargo-binstall-tool-options

Conversation

@risu729

@risu729 risu729 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • fall back to cargo install when cargo tool options require source-build feature selection
  • keep cargo-binstall for compatible options such as bin, crate, and locked
  • document that cargo-binstall owns its own artifact/compile fallback; mise does not retry cargo install after a cargo-binstall failure
  • document cargo-binstall behavior for cargo settings and tool options

Testing

  • cargo fmt --all --check
  • git diff --check
  • MISE_TRUSTED_CONFIG_PATHS=$PWD mise x prettier -- prettier --check docs/dev-tools/backends/cargo.md
  • RUSTC_WRAPPER= cargo test --bin mise backend::cargo::tests

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Cargo backend to more intelligently decide between using cargo-binstall and cargo install based on the provided tool options. It introduces a BinstallStatus enum to manage these states and updates the documentation to reflect how options like features and default-features trigger a fallback to source builds. Feedback from the review suggests including the locked option in installation and lockfile resolution keys to ensure consistent behavior, skipping cargo-binstall when a custom registry is specified due to lack of CLI support, and adding the crate option to the list of settings that require a cargo install fallback to avoid potential argument conflicts.

Comment thread src/backend/cargo.rs
Comment thread src/backend/cargo.rs Outdated
Comment thread src/backend/cargo.rs
Comment thread src/backend/cargo.rs
@greptile-apps

greptile-apps Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the cargo backend to fall back to cargo install (instead of using cargo-binstall) when tool options like features or default-features = false require a source build. It also tracks crate and locked in the lockfile options and documents the cargo-binstall compatibility for each tool option.

  • Introduces a BinstallStatus enum (Enabled, Disabled, Unavailable, UnsupportedOptions) replacing the old boolean is_binstall_enabled helper, with a match-arm dispatch in install_version_ that correctly handles all binstall_only combinations.
  • Extends lockfile_options and install_time_option_keys to include crate and locked, and adds four unit tests covering the new logic.
  • Adds a compatibility table and per-option notes in the docs and settings.toml description explaining which options bypass cargo-binstall and that mise does not retry with cargo install after a binstall failure.

Confidence Score: 4/5

Safe to merge; the core install logic is correct for all documented cases and well-tested.

The only concern is a priority-inversion in binstall_status: unsupported options are detected before the cargo-binstall availability check, so a user with binstall_only=true, no cargo-binstall installed, and features set gets a misleading hint. This affects only an unusual combination of settings with no correctness impact.

src/backend/cargo.rs — specifically the ordering of checks in binstall_status

Important Files Changed

Filename Overview
src/backend/cargo.rs Refactors binstall logic into a BinstallStatus enum with a match arm dispatch; adds cargo_install_required_options helper; extends lockfile_options and install_time_option_keys with crate and locked; adds four unit tests. Logic is correct for all main paths; minor priority inversion in binstall_status (options checked before availability).
docs/dev-tools/backends/cargo.md Adds a compatibility table and per-option notes clarifying which options skip cargo-binstall and which do not; content is accurate relative to the code changes.
settings.toml Expands the cargo.binstall setting description to clarify that build settings don't apply to pre-built binaries and that mise does not retry with cargo install after a binstall failure.

Reviews (4): Last reviewed commit: "fix(cargo): skip binstall for cargo inst..." | Re-trigger Greptile

Comment thread src/backend/cargo.rs
@risu729 risu729 force-pushed the fix/cargo-binstall-tool-options branch 2 times, most recently from 1d58eb9 to fa8dd6c Compare May 16, 2026 20:23
@risu729 risu729 force-pushed the fix/cargo-binstall-tool-options branch from fa8dd6c to aaa19c2 Compare May 18, 2026 10:35
@risu729

This comment was marked as outdated.

@risu729 risu729 marked this pull request as ready for review May 18, 2026 11:06
@jdx jdx merged commit 3e0f9c4 into jdx:main May 18, 2026
33 checks passed
@risu729 risu729 deleted the fix/cargo-binstall-tool-options branch May 18, 2026 16:25
mise-en-dev added a commit that referenced this pull request May 19, 2026
### 🚀 Features

- **(cli)** rename before flag to minimum release age by @risu729 in
[#9768](#9768)
- **(core)** deprecate default package files by @jdx in
[#9970](#9970)
- **(edit)** add --global flag for editing the global config file by
@fru1tworld in [#9953](#9953)

### 🐛 Bug Fixes

- **(aqua)** support cosign public-key bundles by @jdx in
[#9972](#9972)
- **(backend)** pass install_env to postinstall by @risu729 in
[#9930](#9930)
- **(backend)** apply install_env to install commands by @risu729 in
[#9929](#9929)
- **(cargo)** skip binstall for cargo install options by @risu729 in
[#9928](#9928)
- **(config)** restore MISE_ENV_FILE setting by @risu729 in
[#9903](#9903)

### 🚜 Refactor

- **(cli)** use tool wording in version env help by @risu729 in
[#9906](#9906)
- **(conda)** parse tool options locally by @risu729 in
[#9960](#9960)
- **(core)** parse plugin tool options locally by @risu729 in
[#9963](#9963)
- **(go)** parse tool options locally by @risu729 in
[#9961](#9961)
- **(http)** parse tool options locally by @risu729 in
[#9870](#9870)

### 📦️ Dependency Updates

- lock file maintenance by @renovate[bot] in
[#9954](#9954)
- lock file maintenance by @renovate[bot] in
[#9957](#9957)

### 📦 Registry

- use aqua backend for qsv by @risu729 in
[#9910](#9910)

### Ci

- build/publish snap package for arm64 by @jnsgruk in
[#9948](#9948)

### New Contributors

- @jnsgruk made their first contribution in
[#9948](#9948)

## 📦 Aqua Registry Updates

### New Packages (2)

- [`AOMediaCodec/libavif`](https://github.com/AOMediaCodec/libavif)
- [`julian7/redact`](https://github.com/julian7/redact)

### Updated Packages (1)

- [`apache/jena`](https://github.com/apache/jena)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants