Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/cli/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ impl Install {
use_locked_version: true,
latest_versions: true,
before_date: self.get_before_date()?,
offline: false,
},
Comment on lines 233 to 237

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.

P2 Redundant explicit offline: false

offline: false is already the value produced by Default::default(), so this explicit field is unnecessary in install.rs, upgrade.rs, and use.rs. The pattern is consistent across all three callers, which helps with readability, but if ResolveOptions::default() is ever changed to use a different default, these explicit falses would mask the inconsistency — consider leaving them as a documentation signal or adding a comment explaining they are intentionally overriding the default.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Leaving as-is — the surrounding initializers in install.rs, upgrade.rs, and use.rs already list every field of ResolveOptions explicitly rather than using ..Default::default(), so the explicit offline: false matches the local style and serves as the documentation signal you mentioned. Happy to revisit if jdx prefers the spread form.

This comment was generated by an AI coding assistant.

dry_run: self.is_dry_run(),
locked: Settings::get().locked,
Expand Down
2 changes: 1 addition & 1 deletion src/cli/prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub async fn prunable_tools(
}

// Remove versions that are still needed by tracked configs
let needed_versions = get_versions_needed_by_tracked_configs(config, true).await?;
let needed_versions = get_versions_needed_by_tracked_configs(config, true, true).await?;
for key in needed_versions {
to_delete.remove(&key);
}
Expand Down
4 changes: 3 additions & 1 deletion src/cli/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ impl Upgrade {
use_locked_version: false,
latest_versions: true,
before_date,
offline: false,
};
// Filter tools to check before doing expensive version lookups
let filter_tools = if !self.interactive && !self.tool.is_empty() {
Expand Down Expand Up @@ -260,6 +261,7 @@ impl Upgrade {
use_locked_version: false,
latest_versions: true,
before_date,
offline: false,
},
..Default::default()
};
Expand Down Expand Up @@ -338,7 +340,7 @@ impl Upgrade {
// Get versions needed by tracked configs AFTER upgrade
// This ensures we don't uninstall versions still needed by other projects
let versions_needed_by_tracked =
get_versions_needed_by_tracked_configs(config, false).await?;
get_versions_needed_by_tracked_configs(config, false, false).await?;

// Only uninstall old versions of tools that were successfully upgraded
// and are not needed by any tracked config
Expand Down
1 change: 1 addition & 0 deletions src/cli/use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ impl Use {
latest_versions: false,
use_locked_version: true,
before_date: self.get_before_date()?,
offline: false,
};
let versions: Vec<_> = self
.tool
Expand Down
4 changes: 4 additions & 0 deletions src/toolset/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,13 +602,17 @@ impl From<ToolRequestSet> for Toolset {
pub async fn get_versions_needed_by_tracked_configs(
config: &Arc<Config>,
use_locked_version: bool,
offline: bool,
) -> Result<std::collections::HashSet<(String, String)>> {
let mut needed = std::collections::HashSet::new();
// `mise prune` should keep versions pinned by lockfiles. `mise upgrade`
// passes false because it checks what tracked configs resolve to after an
// upgrade, before their lockfiles have been updated.
// Prune also passes offline=true: it only protects installed versions, so
// remote resolution can never affect the outcome and just adds latency.
let opts = ResolveOptions {
use_locked_version,
offline,
..Default::default()
};
for (path, cf) in config.get_tracked_config_files().await? {
Expand Down
34 changes: 33 additions & 1 deletion src/toolset/tool_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ impl ToolVersion {
latest_versions: true,
use_locked_version: false,
before_date: base_opts.before_date,
offline: base_opts.offline,
};
let tv = self.request.resolve(config, &opts).await?;
// map cargo backend specific prefixes to ref
Expand Down Expand Up @@ -339,7 +340,7 @@ impl ToolVersion {
}

let settings = Settings::get();
let is_offline = settings.offline();
let is_offline = settings.offline() || opts.offline;

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.

medium

The is_offline check is correctly implemented here for resolve_version, but it appears that resolve_sub (lines 451-468) was missed. If a tracked config uses a version like sub-1:latest, it will still attempt to hit the network via backend.latest_version at line 461. Consider adding a similar offline check to resolve_sub to ensure mise prune remains fully offline for all version types.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch — fixed in 5171a18. resolve_sub now returns the unresolved request when opts.offline && v == "latest", mirroring the resolve_version latest path.

This comment was generated by an AI coding assistant.


if v == "latest" {
if !opts.latest_versions
Expand All @@ -362,6 +363,13 @@ impl ToolVersion {
return build(v);
}
}
// Prune-style offline (opts.offline) wants a non-erroring no-op
// when nothing is installed — the literal "latest" can't match
// any installed pathname so it's safe. Global MISE_OFFLINE keeps
// the original error to avoid surprising upgrade/outdated callers.
if opts.offline {
return build(v);
}
Comment thread
cursor[bot] marked this conversation as resolved.
return Err(Self::no_versions_found(&backend, opts.before_date));
}
if !opts.latest_versions {
Expand Down Expand Up @@ -449,6 +457,21 @@ impl ToolVersion {
opts: &ResolveOptions,
) -> Result<Self> {
let backend = request.backend()?;
if v == "latest" && opts.offline {
// Use the latest installed version as the basis for the sub
// computation so the resolved concrete version still matches an
// installed pathname (and gets protected from prune). Falling
// straight to the raw "sub-N:latest" string would never match
// anything in `to_delete`.
if !opts.latest_versions
&& let Some(latest) = backend.latest_installed_version(None)?
{
let v = tool_request::version_sub(&latest, sub);
return Box::pin(Self::resolve_version(config, request, &v, opts)).await;
}
let version = request.version();
return Ok(Self::new(request, version));
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
let v = match v {
"latest" => backend
.latest_version(config, None, opts.before_date)
Expand All @@ -472,6 +495,9 @@ impl ToolVersion {
{
return Ok(Self::new(request, v.to_string()));
}
if opts.offline {
return Ok(Self::new(request, prefix.to_string()));
}
let matches = backend
.list_versions_matching_with_opts(config, prefix, opts.before_date)
.await?;
Expand Down Expand Up @@ -553,6 +579,8 @@ pub struct ResolveOptions {
pub use_locked_version: bool,
/// Only consider versions released before this timestamp
pub before_date: Option<Timestamp>,
/// Additive to `Settings::offline()` — either being true skips remote version listing.
pub offline: bool,
}

impl Default for ResolveOptions {
Expand All @@ -561,6 +589,7 @@ impl Default for ResolveOptions {
latest_versions: false,
use_locked_version: true,
before_date: None,
offline: false,
}
}
}
Expand Down Expand Up @@ -598,6 +627,9 @@ impl Display for ResolveOptions {
if let Some(ts) = &self.before_date {
opts.push(format!("before_date={ts}"));
}
if self.offline {
opts.push("offline".to_string());
}
write!(f, "({})", opts.join(", "))
}
}
Expand Down
Loading