Skip to content

Commit

Permalink
feat(up)!: add --upgrade parameter (#723)
Browse files Browse the repository at this point in the history
`omni up` now takes a new `--upgrade` parameter which forces checking
new available versions of tools matching any requested tool and version
constraint, and installing them if it is not already the case.

This affects any tool which can be upgraded, e.g. this affects the
`homebrew` operation, the `github release` operation, and any
`asdf`-backed operation. For each of those, a new `upgrade` boolean
parameter has been added and can be set to `true` to force the
`--upgrade` behavior in all cases. When using `latest` as the version,
this will only match with any version of the latest `major`.

A new `up_command`/`upgrade` option is also available in the global/repo
configuration and can be set to `true` to force upgrading all tools even
without the `--upgrade` parameter.

BREAKING CHANGE: This changes the default behavior which would now be
the equivalent of setting `up_command`/`upgrade` to `true`. The default
behavior was changed to speed up the operations.

Closes #705
  • Loading branch information
XaF authored Oct 14, 2024
1 parent d56829d commit 2d5d264
Show file tree
Hide file tree
Showing 34 changed files with 740 additions and 151 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:
- target: aarch64-apple-darwin
os: macos-latest
suffix: arm64-darwin
run_tests: false
run_tests: true
- target: x86_64-unknown-linux-musl
os: ubuntu-latest
suffix: x86_64-linux
Expand All @@ -65,7 +65,7 @@ jobs:
- target: x86_64-apple-darwin
os: macos-latest
suffix: x86_64-darwin
run_tests: true
run_tests: false

uses: ./.github/workflows/build-and-test-target.yaml
with:
Expand Down
37 changes: 30 additions & 7 deletions src/internal/commands/builtin/up.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,15 @@ use crate::omni_warning;
#[derive(Debug, Clone)]
struct UpCommandArgs {
cache_enabled: bool,
fail_on_upgrade: bool,
clone_suggested: UpCommandArgsCloneSuggestedOptions,
trust: UpCommandArgsTrustOptions,
update_repository: bool,
update_user_config: UpCommandArgsUpdateUserConfigOptions,
fail_on_upgrade: bool,
prompt: bool,
prompt_all: bool,
prompt_ids: HashSet<String>,
trust: UpCommandArgsTrustOptions,
update_repository: bool,
update_user_config: UpCommandArgsUpdateUserConfigOptions,
upgrade: bool,
}

impl UpCommandArgs {
Expand Down Expand Up @@ -141,6 +142,11 @@ impl UpCommandArgs {
"yes", "ask", "no",
])),
)
.arg(
clap::Arg::new("upgrade")
.long("upgrade")
.action(clap::ArgAction::SetTrue),
)
.try_get_matches_from(&parse_argv);

let matches = match matches {
Expand Down Expand Up @@ -227,16 +233,17 @@ impl UpCommandArgs {

Self {
cache_enabled: !*matches.get_one::<bool>("no-cache").unwrap_or(&false),
fail_on_upgrade: *matches.get_one::<bool>("fail-on-upgrade").unwrap_or(&false),
clone_suggested,
trust,
fail_on_upgrade: *matches.get_one::<bool>("fail-on-upgrade").unwrap_or(&false),
prompt,
prompt_all,
prompt_ids,
trust,
update_repository: *matches
.get_one::<bool>("update-repository")
.unwrap_or(&false),
update_user_config,
upgrade: *matches.get_one::<bool>("upgrade").unwrap_or(&false),
}
}
}
Expand Down Expand Up @@ -1375,6 +1382,20 @@ impl BuiltinCommand for UpCommand {
),
required: false,
},
SyntaxOptArg {
name: "--upgrade".to_string(),
desc: Some(
concat!(
"Whether we should upgrade the resources when the currently-installed ",
"version already matches version constraints. If false, this also means ",
"that if an already installed version for another repository matches ",
"version contraints, we will avoid downloading and building a more ",
"recent version \x1B[90m(default: false)\x1B[0m",
)
.to_string(),
),
required: false,
},
],
})
}
Expand Down Expand Up @@ -1626,7 +1647,8 @@ impl BuiltinCommand for UpCommand {
let options = options
.clone()
.cache(self.cli_args().cache_enabled)
.fail_on_upgrade(self.cli_args().fail_on_upgrade);
.fail_on_upgrade(self.cli_args().fail_on_upgrade)
.upgrade(self.cli_args().upgrade);
if let Err(err) = up_config.up(&options) {
self.handle_sync_operation(
SyncUpdateOperation::OmniError(format!(
Expand Down Expand Up @@ -1686,6 +1708,7 @@ impl BuiltinCommand for UpCommand {
println!("--trust");
println!("--update-repository");
println!("--update-user-config");
println!("--upgrade");

Ok(())
}
Expand Down
6 changes: 2 additions & 4 deletions src/internal/commands/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,8 @@ where

let absolute_path = if path.is_absolute() {
path.to_path_buf()
} else if path.starts_with("~") {
let home_dir = std::env::var("HOME").expect("Failed to determine user's home directory");
let path = path.strip_prefix("~").expect("Failed to strip prefix");
PathBuf::from(home_dir).join(path)
} else if let Ok(path) = path.strip_prefix("~") {
PathBuf::from(user_home()).join(path)
} else {
match frompath {
Some(frompath) => frompath.as_ref().join(path),
Expand Down
32 changes: 32 additions & 0 deletions src/internal/config/config_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,31 @@ impl ConfigValue {
None
}

pub fn as_bool_forced(&self) -> Option<bool> {
if let Some(ConfigData::Value(value)) = self.value.as_ref().map(|data| data.as_ref()) {
match value {
serde_yaml::Value::Null => return None,
serde_yaml::Value::Bool(value) => return Some(*value),
serde_yaml::Value::String(value) => match value.to_lowercase().as_str() {
"true" | "yes" | "y" | "on" | "1" => return Some(true),
"false" | "no" | "n" | "off" | "0" => return Some(false),
_ => return None,
},
serde_yaml::Value::Number(value) => match value.as_i64() {
Some(value) => return Some(value != 0),
None => match value.as_f64() {
Some(value) => return Some(value != 0.0),
None => return None,
},
},
serde_yaml::Value::Sequence(_) => return None,
serde_yaml::Value::Mapping(_) => return None,
serde_yaml::Value::Tagged(_) => return None,
}
}
None
}

#[allow(dead_code)]
pub fn is_float(&self) -> bool {
self.as_float().is_some()
Expand Down Expand Up @@ -599,6 +624,13 @@ impl ConfigValue {
None
}

pub fn get_as_bool_forced(&self, key: &str) -> Option<bool> {
if let Some(value) = self.get(key) {
return value.as_bool_forced();
}
None
}

pub fn get_as_float(&self, key: &str) -> Option<f64> {
if let Some(value) = self.get(key) {
return value.as_float();
Expand Down
6 changes: 3 additions & 3 deletions src/internal/config/parser/askpass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ impl AskPassConfig {

Self {
enabled: config_value
.get_as_bool("enabled")
.get_as_bool_forced("enabled")
.unwrap_or(Self::DEFAULT_ENABLED),
enable_gui: config_value
.get_as_bool("enable_gui")
.get_as_bool_forced("enable_gui")
.unwrap_or(Self::DEFAULT_ENABLE_GUI),
prefer_gui: config_value
.get_as_bool("prefer_gui")
.get_as_bool_forced("prefer_gui")
.unwrap_or(Self::DEFAULT_PREFER_GUI),
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/internal/config/parser/cd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl CdConfig {

Self {
fast_search: config_value
.get_as_bool("fast_search")
.get_as_bool_forced("fast_search")
.unwrap_or(Self::DEFAULT_FAST_SEARCH),
path_match_min_score: config_value
.get_as_float("path_match_min_score")
Expand Down
2 changes: 1 addition & 1 deletion src/internal/config/parser/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl CloneConfig {

Self {
auto_up: config_value
.get_as_bool("auto_up")
.get_as_bool_forced("auto_up")
.unwrap_or(Self::DEFAULT_AUTO_UP),
ls_remote_timeout,
}
Expand Down
2 changes: 1 addition & 1 deletion src/internal/config/parser/org.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl OrgConfig {

Self {
handle: config_value.get_as_str("handle").unwrap().to_string(),
trusted: config_value.get_as_bool("trusted").unwrap_or(false),
trusted: config_value.get_as_bool_forced("trusted").unwrap_or(false),
worktree: config_value.get_as_str("worktree"),
repo_path_format: config_value.get_as_str("repo_path_format"),
}
Expand Down
6 changes: 3 additions & 3 deletions src/internal/config/parser/path_repo_updates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,16 @@ impl PathRepoUpdatesConfig {

Self {
enabled: config_value
.get_as_bool("enabled")
.get_as_bool_forced("enabled")
.unwrap_or(Self::DEFAULT_ENABLED),
self_update,
on_command_not_found,
pre_auth: config_value
.get_as_bool("pre_auth")
.get_as_bool_forced("pre_auth")
.unwrap_or(Self::DEFAULT_PRE_AUTH),
pre_auth_timeout,
background_updates: config_value
.get_as_bool("background_updates")
.get_as_bool_forced("background_updates")
.unwrap_or(Self::DEFAULT_BACKGROUND_UPDATES),
background_updates_timeout,
interval,
Expand Down
33 changes: 21 additions & 12 deletions src/internal/config/parser/up_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub struct UpCommandConfig {
pub notify_workdir_config_updated: bool,
pub notify_workdir_config_available: bool,
pub preferred_tools: Vec<String>,
pub upgrade: bool,
}

impl Default for UpCommandConfig {
Expand All @@ -19,6 +20,7 @@ impl Default for UpCommandConfig {
notify_workdir_config_updated: Self::DEFAULT_NOTIFY_WORKDIR_CONFIG_UPDATED,
notify_workdir_config_available: Self::DEFAULT_NOTIFY_WORKDIR_CONFIG_AVAILABLE,
preferred_tools: Vec::new(),
upgrade: Self::DEFAULT_UPGRADE,
}
}
}
Expand All @@ -27,41 +29,48 @@ impl UpCommandConfig {
const DEFAULT_AUTO_BOOTSTRAP: bool = true;
const DEFAULT_NOTIFY_WORKDIR_CONFIG_UPDATED: bool = true;
const DEFAULT_NOTIFY_WORKDIR_CONFIG_AVAILABLE: bool = true;
const DEFAULT_UPGRADE: bool = false;

pub(super) fn from_config_value(config_value: Option<ConfigValue>) -> Self {
let config_value = match config_value {
Some(config_value) => config_value,
None => return Self::default(),
};

let config_value = match config_value.reject_scope(&ConfigScope::Workdir) {
Some(config_value) => config_value,
None => return Self::default(),
};
// For the values that we don't support overriding in the workdir
let config_value_global = config_value
.reject_scope(&ConfigScope::Workdir)
.unwrap_or_default();

let preferred_tools =
if let Some(preferred_tools) = config_value.get_as_array("preferred_tools") {
if let Some(preferred_tools) = config_value_global.get_as_array("preferred_tools") {
preferred_tools
.iter()
.filter_map(|value| value.as_str().map(|value| value.to_string()))
.collect()
} else if let Some(preferred_tool) = config_value.get_as_str_forced("preferred_tools") {
} else if let Some(preferred_tool) =
config_value_global.get_as_str_forced("preferred_tools")
{
vec![preferred_tool]
} else {
Vec::new()
};

Self {
auto_bootstrap: config_value
.get_as_bool("auto_bootstrap")
auto_bootstrap: config_value_global
.get_as_bool_forced("auto_bootstrap")
.unwrap_or(Self::DEFAULT_AUTO_BOOTSTRAP),
notify_workdir_config_updated: config_value
.get_as_bool("notify_workdir_config_updated")
notify_workdir_config_updated: config_value_global
.get_as_bool_forced("notify_workdir_config_updated")
.unwrap_or(Self::DEFAULT_NOTIFY_WORKDIR_CONFIG_UPDATED),
notify_workdir_config_available: config_value
.get_as_bool("notify_workdir_config_available")
notify_workdir_config_available: config_value_global
.get_as_bool_forced("notify_workdir_config_available")
.unwrap_or(Self::DEFAULT_NOTIFY_WORKDIR_CONFIG_AVAILABLE),
preferred_tools,
// The upgrade option is fine to handle as a workdir option too
upgrade: config_value
.get_as_bool_forced("upgrade")
.unwrap_or(Self::DEFAULT_UPGRADE),
}
}
}
Loading

0 comments on commit 2d5d264

Please sign in to comment.