Skip to content

refactor(cargo): parse tool options locally#9922

Merged
jdx merged 2 commits into
jdx:mainfrom
risu729:refactor/cargo-typed-options
May 17, 2026
Merged

refactor(cargo): parse tool options locally#9922
jdx merged 2 commits into
jdx:mainfrom
risu729:refactor/cargo-typed-options

Conversation

@risu729

@risu729 risu729 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a CargoOptions wrapper around cargo backend tool options
  • route cargo install flags, env, and binstall feature checks through local accessors
  • keep cargo lockfile option persistence behavior unchanged

Verification

  • cargo fmt --all -- --check
  • git diff --check

This PR was generated by an AI coding assistant.

@greptile-apps

greptile-apps Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR wraps cargo backend option parsing in a CargoOptions<'a> struct that delegates to BackendOptions, routing all option reads through typed accessor methods. As a side-effect of the refactor, resolve_lockfile_options now receives the target parameter and passes it through to bin_for_target, fixing a pre-existing inconsistency where the lockfile fingerprint used a plain bin key lookup instead of the same platform-aware resolution used during actual installation.

  • CargoOptions<'a> centralises all option accessors (bin, locked, features, default_features_disabled, crate_arg, install_env, has_features_options, lockfile_options) and delegates to BackendOptions / ToolVersionOptions APIs.
  • resolve_lockfile_options now uses CargoOptions::lockfile_options(&target) which calls bin_for_target for platform-specific bin resolution, aligning the lockfile fingerprint with the actual install-time value.
  • A unit test (test_lockfile_options_uses_target_platform_bin) is added to verify the platform-specific bin behaviour in lockfile option generation.

Confidence Score: 5/5

Safe to merge — the change is a pure refactor that encapsulates existing option reads and simultaneously corrects a long-standing inconsistency in lockfile fingerprinting for platform-specific bin values.

All option-read semantics are preserved identically; the only behavioural change (platform-aware bin in lockfile) is intentional and verified by the new unit test. No new code paths are introduced that could fail at runtime.

No files require special attention.

Important Files Changed

Filename Overview
src/backend/cargo.rs Refactors cargo option access into a new CargoOptions<'a> wrapper; correctly fixes lockfile_options to use target-platform-aware bin resolution instead of a raw key lookup.

Reviews (2): Last reviewed commit: "fix(cargo): lock target-specific bin opt..." | Re-trigger Greptile

Comment thread src/backend/cargo.rs Outdated

@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 CargoBackend in src/backend/cargo.rs by introducing a CargoOptions struct to encapsulate tool version options, replacing manual map lookups with structured methods. The review feedback suggests making this implementation platform-aware by using platform_string lookups instead of top-level string access, which ensures that platform-specific overrides in configuration files are correctly respected. Additionally, the reviewer recommends passing the PlatformTarget argument into the lockfile resolution logic to ensure the generated lockfile accurately reflects the settings for the specific target platform.

I am having trouble creating individual review comments. Click here to see my feedback.

src/backend/cargo.rs (35-85)

medium

The CargoOptions implementation should leverage platform-aware lookups for all tool options to ensure that platform-specific overrides in mise.toml (e.g., platforms.linux-x64.features = "...") are correctly respected. Currently, methods like locked, features, and default_features_disabled only look at top-level string values, which ignores these overrides.

Additionally, lockfile_options should take a PlatformTarget argument and use platform_string_for_target to ensure the lockfile accurately reflects the settings for the target platform being locked.

impl<'a> CargoOptions<'a> {
    fn new(raw: &'a ToolVersionOptions) -> Self {
        Self {
            values: BackendOptions::new(raw),
        }
    }

    fn bin(&self) -> Option<String> {
        self.values.platform_string("bin")
    }

    fn locked(&self) -> bool {
        self.values
            .platform_string("locked")
            .is_none_or(|v| v.to_lowercase() != "false")
    }

    fn features(&self) -> Option<String> {
        self.values.platform_string("features")
    }

    fn default_features_disabled(&self) -> bool {
        self.values
            .platform_string("default-features")
            .is_some_and(|v| v.to_lowercase() == "false")
    }

    fn crate_arg(&self) -> Option<String> {
        self.values.platform_string("crate")
    }

    fn install_env(&self) -> &'a IndexMap<String, String> {
        &self.values.raw().install_env
    }

    fn has_features_options(&self) -> bool {
        self.values.platform_string("features").is_some()
            || self.values.platform_string("default-features").is_some()
    }

    fn lockfile_options(&self, target: &PlatformTarget) -> BTreeMap<String, String> {
        let mut result = BTreeMap::new();
        for key in ["features", "default-features", "bin"] {
            if let Some(value) = self.values.platform_string_for_target(key, target) {
                result.insert(key.to_string(), value);
            }
        }
        result
    }
}
References
  1. When overriding a method, use the provided parameters instead of re-deriving their values from a more general context. This implementation supports that by allowing the target to be passed in.

src/backend/cargo.rs (232-236)

medium

Pass the target to lockfile_options to ensure that platform-specific overrides are correctly captured in the lockfile. This ensures we use the provided parameter instead of re-deriving it.

    fn resolve_lockfile_options(
        &self,
        request: &ToolRequest,
        target: &PlatformTarget,
    ) -> BTreeMap<String, String> {
        let opts = request.options();
        CargoOptions::new(&opts).lockfile_options(target)
    }
References
  1. When overriding a method, use the provided parameters instead of re-deriving their values from a more general context.

@risu729 risu729 marked this pull request as ready for review May 16, 2026 15:21
@jdx jdx merged commit 7959279 into jdx:main May 17, 2026
32 checks passed
@risu729 risu729 deleted the refactor/cargo-typed-options branch May 17, 2026 14:30
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