Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for self-update instructions by introducing mechanisms to detect and display upgrade instructions when mise self-update is not directly available through the built-in updater.
- Introduces environment variables and file-based configuration for self-update instructions
- Updates minimum version validation to support both hard and soft version requirements with custom upgrade messaging
- Refactors self-update availability detection to use centralized configuration
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/env.rs | Adds environment variable definitions for self-update instruction paths and availability detection |
| src/config/mod.rs | Updates version validation to handle hard/soft requirements and display upgrade instructions |
| src/config/config_file/mod.rs | Changes min_version trait method signature to work with new MinVersionSpec type |
| src/config/config_file/mise_toml.rs | Updates deserialization to support the new MinVersionSpec format with hard/soft versions |
| src/config/config_file/min_version.rs | Introduces new MinVersionSpec struct to handle version requirement specifications |
| src/cli/version.rs | Adds upgrade instruction display when self-update is unavailable |
| src/cli/self_update.rs | Refactors self-update availability logic and adds instruction file reading functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| #[derive(Debug, Clone, Default, PartialEq, Eq)] | ||
| pub struct MinVersionSpec { | ||
| hard: Option<Versioning>, | ||
| soft: Option<Versioning>, |
There was a problem hiding this comment.
The struct is missing the upgrade_instructions field that is referenced in line 15 of the new method and other methods like to_owned and merge_with.
| soft: Option<Versioning>, | |
| soft: Option<Versioning>, | |
| upgrade_instructions: Option<String>, |
| @@ -0,0 +1,64 @@ | |||
| #[derive(Debug, Clone, Default, PartialEq, Eq)] | |||
There was a problem hiding this comment.
Missing import statement for Versioning type. The struct uses Versioning but doesn't import it.
| let version = Versioning::new(v) | ||
| .ok_or_else(|| versions::Error::IllegalVersioning(v.to_string())) | ||
| .map_err(E::custom)?; | ||
| Ok(MinVersionSpec::new(Some(version), None)) |
There was a problem hiding this comment.
The MinVersionSpec::new method returns Option<Self> but this code expects it to return Self. Either unwrap the Option or change the method signature.
| } | ||
| } | ||
| } | ||
| Ok(MinVersionSpec::new(hard, soft)) |
There was a problem hiding this comment.
The MinVersionSpec::new method returns Option<Self> but this code expects it to return Self. Either unwrap the Option or change the method signature.
| Ok(MinVersionSpec::new(hard, soft)) | |
| MinVersionSpec::new(hard, soft).ok_or_else(|| de::Error::custom("invalid min_version spec")) |
e238421 to
a5a6711
Compare
…aults for self-update; e2e coverage
- Support min_version {hard,soft} with legacy string
- Soft violations warn and show instructions
- Hard violations error and include instructions
- env.rs discovers instructions/disable defaults
- Show instructions in self-update unavailable and version::show_latest
- Add e2e for soft/hard min_version behavior
…ons.toml; refactor validation helper; surface instructions in doctor
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.9.19 x -- echo |
20.7 ± 0.5 | 19.5 | 22.6 | 1.00 |
mise x -- echo |
21.3 ± 0.6 | 19.5 | 24.6 | 1.03 ± 0.04 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.9.19 env |
19.9 ± 0.5 | 18.7 | 23.1 | 1.00 |
mise env |
20.5 ± 0.8 | 18.7 | 24.6 | 1.03 ± 0.05 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.9.19 hook-env |
19.8 ± 0.4 | 18.7 | 21.3 | 1.00 |
mise hook-env |
20.2 ± 0.5 | 18.8 | 22.0 | 1.02 ± 0.03 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.9.19 ls |
17.9 ± 0.6 | 16.4 | 21.6 | 1.00 |
mise ls |
18.4 ± 0.5 | 17.2 | 20.0 | 1.03 ± 0.04 |
xtasks/test/perf
| Command | mise-2025.9.19 | mise | Variance |
|---|---|---|---|
| install (cached) | 174ms | ✅ 109ms | +59% |
| ls (cached) | 66ms | 66ms | +0% |
| bin-paths (cached) | 73ms | 74ms | -1% |
| task-ls (cached) | 536ms | 512ms | +4% |
✅ Performance improvement: install cached is 59%
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pub fn to_owned(&self) -> Self { | ||
| Self { | ||
| hard: self.hard.clone(), | ||
| soft: self.soft.clone(), | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The method to_owned is redundant since MinVersionSpec already implements Clone. Use the standard clone() method instead or remove this method entirely.
| pub fn to_owned(&self) -> Self { | |
| Self { | |
| hard: self.hard.clone(), | |
| soft: self.soft.clone(), | |
| } | |
| } |
| Ok(MinVersionSpec::new(hard, soft)) | ||
| } |
There was a problem hiding this comment.
The MinVersionSpec::new() method returns Option<MinVersionSpec> but this code assumes it always returns Some. Consider handling the None case or change the method signature if this case is impossible.
| Ok(MinVersionSpec::new(hard, soft)) | |
| } | |
| match MinVersionSpec::new(hard, soft) { | |
| Some(spec) => Ok(Some(spec)), | |
| None => Err(de::Error::custom("invalid min_version map: must specify at least one of 'hard' or 'soft'")), | |
| } |
…instructions.toml across packaging scripts
4a403ea to
864a001
Compare
590ecb1 to
0826f5a
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pub fn hard_violation<'a>(&'a self, current: &Versioning) -> Option<&'a Versioning> { | ||
| self.hard().filter(|required| current < *required) | ||
| } | ||
|
|
||
| pub fn soft_violation<'a>(&'a self, current: &Versioning) -> Option<&'a Versioning> { |
There was a problem hiding this comment.
The explicit lifetime annotations 'a are unnecessary here as they can be elided. The compiler can infer these lifetimes automatically.
| pub fn hard_violation<'a>(&'a self, current: &Versioning) -> Option<&'a Versioning> { | |
| self.hard().filter(|required| current < *required) | |
| } | |
| pub fn soft_violation<'a>(&'a self, current: &Versioning) -> Option<&'a Versioning> { | |
| pub fn hard_violation(&self, current: &Versioning) -> Option<&Versioning> { | |
| self.hard().filter(|required| current < *required) | |
| } | |
| pub fn soft_violation(&self, current: &Versioning) -> Option<&Versioning> { |
| pub fn hard_violation<'a>(&'a self, current: &Versioning) -> Option<&'a Versioning> { | ||
| self.hard().filter(|required| current < *required) | ||
| } | ||
|
|
||
| pub fn soft_violation<'a>(&'a self, current: &Versioning) -> Option<&'a Versioning> { |
There was a problem hiding this comment.
The explicit lifetime annotations 'a are unnecessary here as they can be elided. The compiler can infer these lifetimes automatically.
| pub fn hard_violation<'a>(&'a self, current: &Versioning) -> Option<&'a Versioning> { | |
| self.hard().filter(|required| current < *required) | |
| } | |
| pub fn soft_violation<'a>(&'a self, current: &Versioning) -> Option<&'a Versioning> { | |
| pub fn hard_violation(&self, current: &Versioning) -> Option<&Versioning> { | |
| self.hard().filter(|required| current < *required) | |
| } | |
| pub fn soft_violation(&self, current: &Versioning) -> Option<&Versioning> { |
| cat >mise.toml <<'TOML' | ||
| min_version = { soft = "9999.0.0" } | ||
| TOML | ||
|
|
||
| assert_contains 'MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml MISE_LOG_LEVEL=warn mise ls 2>&1' 'recommended' | ||
| assert_contains 'MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml MISE_LOG_LEVEL=warn mise ls 2>&1' 'brew upgrade mise' | ||
|
|
||
| # hard min_version: should fail and include instructions | ||
| cat >mise.toml <<'TOML' | ||
| min_version = { hard = "9999.0.0" } | ||
| TOML | ||
|
|
||
| assert_fail 'MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml mise ls 2>&1' 'is required' | ||
| assert_fail 'MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml mise ls 2>&1' 'brew upgrade mise' |
There was a problem hiding this comment.
[nitpick] The long command line with multiple environment variables is repeated and hard to read. Consider extracting the common environment setup into a variable for better maintainability.
| cat >mise.toml <<'TOML' | |
| min_version = { soft = "9999.0.0" } | |
| TOML | |
| assert_contains 'MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml MISE_LOG_LEVEL=warn mise ls 2>&1' 'recommended' | |
| assert_contains 'MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml MISE_LOG_LEVEL=warn mise ls 2>&1' 'brew upgrade mise' | |
| # hard min_version: should fail and include instructions | |
| cat >mise.toml <<'TOML' | |
| min_version = { hard = "9999.0.0" } | |
| TOML | |
| assert_fail 'MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml mise ls 2>&1' 'is required' | |
| assert_fail 'MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml mise ls 2>&1' 'brew upgrade mise' | |
| # Common environment variables for assertions | |
| COMMON_ENV="MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml MISE_LOG_LEVEL=warn" | |
| COMMON_ENV_NO_LOG="MISE_SELF_UPDATE_AVAILABLE=false MISE_SELF_UPDATE_INSTRUCTIONS=./instructions.toml" | |
| cat >mise.toml <<'TOML' | |
| min_version = { soft = "9999.0.0" } | |
| TOML | |
| assert_contains "$COMMON_ENV mise ls 2>&1" 'recommended' | |
| assert_contains "$COMMON_ENV mise ls 2>&1" 'brew upgrade mise' | |
| # hard min_version: should fail and include instructions | |
| cat >mise.toml <<'TOML' | |
| min_version = { hard = "9999.0.0" } | |
| TOML | |
| assert_fail "$COMMON_ENV_NO_LOG mise ls 2>&1" 'is required' | |
| assert_fail "$COMMON_ENV_NO_LOG mise ls 2>&1" 'brew upgrade mise' |
06c9750 to
73a644a
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let version = Versioning::new(v) | ||
| .ok_or_else(|| versions::Error::IllegalVersioning(v.to_string())) | ||
| .map_err(E::custom)?; | ||
| Ok(MinVersionSpec::new(Some(version), None)) |
There was a problem hiding this comment.
The MinVersionSpec::new method returns Option<MinVersionSpec> but this code assumes it always returns Some. If both parameters are None, it returns None, but here the first parameter is Some(version), so it should be safe. However, the return type mismatch needs to be handled - this should be Ok(Some(MinVersionSpec::new(Some(version), None).unwrap())) or the function should be updated to handle the None case.
| Ok(MinVersionSpec::new(Some(version), None)) | |
| Ok(Some(MinVersionSpec::new(Some(version), None).unwrap())) |
73a644a to
5f4cee0
Compare
bd24b51 to
1c03167
Compare
1c03167 to
f3a8d11
Compare
f3a8d11 to
36e4c9e
Compare
|
bugbot run |
No description provided.