From 170d9d5c1c328332eefb4f54c3513ba4196bb6d1 Mon Sep 17 00:00:00 2001 From: Christian Guinard <28689358+christiangnrd@users.noreply.github.com> Date: Mon, 26 Feb 2024 16:10:39 -0400 Subject: [PATCH 1/3] Add is_valid_channel and use it everywhere `version_db.available_channels` is used --- src/bin/julialauncher.rs | 7 ++++--- src/command_default.rs | 3 ++- src/command_link.rs | 3 ++- src/operations.rs | 24 ++++++++++++++++++++++++ tests/channel_selection.rs | 26 ++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index 01975de7..db19dd9d 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -5,6 +5,7 @@ use itertools::Itertools; use juliaup::config_file::{load_config_db, JuliaupConfig, JuliaupConfigChannel}; use juliaup::global_paths::get_paths; use juliaup::jsonstructs_versionsdb::JuliaupVersionDB; +use juliaup::operations::is_valid_channel; use juliaup::versions_file::load_versions_db; #[cfg(not(windows))] use nix::{ @@ -179,21 +180,21 @@ fn get_julia_path_from_channel( .get(channel) .ok_or_else(|| match juliaup_channel_source { JuliaupChannelSource::CmdLine => { - if versions_db.available_channels.contains_key(channel) { + if is_valid_channel(versions_db, &channel.to_string()) { UserError { msg: format!("`{}` is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } } else { UserError { msg: format!("ERROR: Invalid Juliaup channel `{}`. Please run `juliaup list` to get a list of valid channels and versions.", channel) } } }.into(), JuliaupChannelSource::EnvVar=> { - if versions_db.available_channels.contains_key(channel) { + if is_valid_channel(versions_db, &channel.to_string()) { UserError { msg: format!("`{}` for environment variable JULIAUP_CHANNEL is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } } else { UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in environment variable JULIAUP_CHANNEL. Please run `juliaup list` to get a list of valid channels and versions.", channel) } } }.into(), JuliaupChannelSource::Override=> { - if versions_db.available_channels.contains_key(channel) { + if is_valid_channel(versions_db, &channel.to_string()) { UserError { msg: format!("`{}` for directory override is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } } else { UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in directory override. Please run `juliaup list` to get a list of valid channels and versions.", channel) } diff --git a/src/command_default.rs b/src/command_default.rs index 89c3a1d9..20969be3 100644 --- a/src/command_default.rs +++ b/src/command_default.rs @@ -1,3 +1,4 @@ +use crate::operations::is_valid_channel; use crate::versions_file::load_versions_db; use crate::{config_file::*, global_paths::GlobalPaths}; use anyhow::{bail, Context, Result}; @@ -9,7 +10,7 @@ pub fn run_command_default(channel: &str, paths: &GlobalPaths) -> Result<()> { if !config_file.data.installed_channels.contains_key(channel) { let version_db = load_versions_db(paths) .with_context(|| "`default` command failed to load versions db.")?; - if !version_db.available_channels.contains_key(channel) { + if !is_valid_channel(&version_db, &channel.to_string()) { bail!("'{}' is not a valid Julia version.", channel); } else { bail!( diff --git a/src/command_link.rs b/src/command_link.rs index c7fcb2e4..2bceccbb 100644 --- a/src/command_link.rs +++ b/src/command_link.rs @@ -3,6 +3,7 @@ use crate::config_file::{load_mut_config_db, save_config_db}; use crate::global_paths::GlobalPaths; #[cfg(not(windows))] use crate::operations::create_symlink; +use crate::operations::is_valid_channel; use crate::versions_file::load_versions_db; use anyhow::{bail, Context, Result}; use path_absolutize::Absolutize; @@ -24,7 +25,7 @@ pub fn run_command_link( bail!("Channel name `{}` is already used.", channel) } - if versiondb_data.available_channels.contains_key(channel) { + if is_valid_channel(&versiondb_data, &channel.to_string()) { eprintln!("WARNING: The channel name `{}` is also a system channel. By linking your custom binary to this channel you are hiding this system channel.", channel); } diff --git a/src/operations.rs b/src/operations.rs index 255a015a..bde42095 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -432,6 +432,30 @@ pub fn compatible_archs() -> Result> { } } +// which nightly channels are compatible with the current system +fn compatible_nightly_channels() -> Result> { + let archs: Result> = compatible_archs(); + + if archs.is_ok() { + let channels: Vec = std::iter::once("nightly".to_string()) + .chain(archs?.into_iter().map(|arch| format!("nightly~{}", arch))) + .collect(); + Ok(channels) + } else { + archs + } +} + +// considers the nightly channels as system channels +pub fn is_valid_channel(versions_db: &JuliaupVersionDB, channel: &String) -> bool { + let regular = versions_db.available_channels.contains_key(channel); + + let nightly_chans = compatible_nightly_channels(); + + let nightly = nightly_chans.is_ok_and(|nightly_chans| nightly_chans.contains(channel)); + regular || nightly +} + // Identify the unversioned name of a nightly (e.g., `latest-macos-x86_64`) for a channel pub fn channel_to_name(channel: &String) -> Result { let mut parts = channel.splitn(2, '~'); diff --git a/tests/channel_selection.rs b/tests/channel_selection.rs index 7af73a4a..1ad9ac46 100644 --- a/tests/channel_selection.rs +++ b/tests/channel_selection.rs @@ -125,4 +125,30 @@ fn channel_selection() { .assert() .failure() .stderr("ERROR: Invalid Juliaup channel `1.8.6`. Please run `juliaup list` to get a list of valid channels and versions.\n"); + + // https://github.com/JuliaLang/juliaup/issues/766 + Command::cargo_bin("julia") + .unwrap() + .arg("+1.8.2") + .arg("-e") + .arg("print(VERSION)") + .env("JULIA_DEPOT_PATH", depot_dir.path()) + .env("JULIAUP_DEPOT_PATH", depot_dir.path()) + .env("JULIAUP_CHANNEL", "1.7.4") + .assert() + .failure() + .stderr("`1.8.2` is not installed. Please run `juliaup add 1.8.2` to install channel or version.\n"); + + // https://github.com/JuliaLang/juliaup/issues/820 + Command::cargo_bin("julia") + .unwrap() + .arg("+nightly") + .arg("-e") + .arg("print(VERSION)") + .env("JULIA_DEPOT_PATH", depot_dir.path()) + .env("JULIAUP_DEPOT_PATH", depot_dir.path()) + .env("JULIAUP_CHANNEL", "1.7.4") + .assert() + .failure() + .stderr("`nightly` is not installed. Please run `juliaup add nightly` to install channel or version.\n"); } From ce2347c9a2c741394e923f151b725a7da5c683aa Mon Sep 17 00:00:00 2001 From: Christian Guinard <28689358+christiangnrd@users.noreply.github.com> Date: Tue, 16 Jul 2024 15:46:13 -0300 Subject: [PATCH 2/3] Phrasing and comments "from" makes more sense to me as the error message seems to be attempting to convey where `julialauncher` got the missing/invalid channel from. --- src/bin/julialauncher.rs | 8 ++++---- src/operations.rs | 1 + tests/channel_selection.rs | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index db19dd9d..6f413812 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -188,16 +188,16 @@ fn get_julia_path_from_channel( }.into(), JuliaupChannelSource::EnvVar=> { if is_valid_channel(versions_db, &channel.to_string()) { - UserError { msg: format!("`{}` for environment variable JULIAUP_CHANNEL is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } + UserError { msg: format!("`{}` from environment variable JULIAUP_CHANNEL is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } } else { - UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in environment variable JULIAUP_CHANNEL. Please run `juliaup list` to get a list of valid channels and versions.", channel) } + UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` from environment variable JULIAUP_CHANNEL. Please run `juliaup list` to get a list of valid channels and versions.", channel) } } }.into(), JuliaupChannelSource::Override=> { if is_valid_channel(versions_db, &channel.to_string()) { - UserError { msg: format!("`{}` for directory override is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } + UserError { msg: format!("`{}` from directory override is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } } else { - UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in directory override. Please run `juliaup list` to get a list of valid channels and versions.", channel) } + UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` from directory override. Please run `juliaup list` to get a list of valid channels and versions.", channel) } } }.into(), JuliaupChannelSource::Default => anyhow!("The Juliaup configuration is in an inconsistent state, the currently configured default channel `{}` is not installed.", channel) diff --git a/src/operations.rs b/src/operations.rs index bde42095..21441cf9 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -447,6 +447,7 @@ fn compatible_nightly_channels() -> Result> { } // considers the nightly channels as system channels +// XXX: does not account for PR channels pub fn is_valid_channel(versions_db: &JuliaupVersionDB, channel: &String) -> bool { let regular = versions_db.available_channels.contains_key(channel); diff --git a/tests/channel_selection.rs b/tests/channel_selection.rs index 1ad9ac46..3ea5bac3 100644 --- a/tests/channel_selection.rs +++ b/tests/channel_selection.rs @@ -111,7 +111,7 @@ fn channel_selection() { .assert() .failure() .stderr( - "ERROR: Invalid Juliaup channel `1.7.4` in environment variable JULIAUP_CHANNEL. Please run `juliaup list` to get a list of valid channels and versions.\n", + "ERROR: Invalid Juliaup channel `1.7.4` from environment variable JULIAUP_CHANNEL. Please run `juliaup list` to get a list of valid channels and versions.\n", ); Command::cargo_bin("julia") From 89841a659dc8aec75dc6a463b194464651af0eea Mon Sep 17 00:00:00 2001 From: Christian Guinard <28689358+christiangnrd@users.noreply.github.com> Date: Tue, 20 Aug 2024 22:59:34 -0300 Subject: [PATCH 3/3] Address feedback --- src/bin/julialauncher.rs | 7 ++++--- src/command_default.rs | 2 +- src/command_link.rs | 2 +- src/operations.rs | 22 +++++++++------------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index 6f413812..abaf7ba5 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -175,26 +175,27 @@ fn get_julia_path_from_channel( juliaupconfig_path: &Path, juliaup_channel_source: JuliaupChannelSource, ) -> Result<(PathBuf, Vec)> { + let channel_valid = is_valid_channel(versions_db, &channel.to_string())?; let channel_info = config_data .installed_channels .get(channel) .ok_or_else(|| match juliaup_channel_source { JuliaupChannelSource::CmdLine => { - if is_valid_channel(versions_db, &channel.to_string()) { + if channel_valid { UserError { msg: format!("`{}` is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } } else { UserError { msg: format!("ERROR: Invalid Juliaup channel `{}`. Please run `juliaup list` to get a list of valid channels and versions.", channel) } } }.into(), JuliaupChannelSource::EnvVar=> { - if is_valid_channel(versions_db, &channel.to_string()) { + if channel_valid { UserError { msg: format!("`{}` from environment variable JULIAUP_CHANNEL is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } } else { UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` from environment variable JULIAUP_CHANNEL. Please run `juliaup list` to get a list of valid channels and versions.", channel) } } }.into(), JuliaupChannelSource::Override=> { - if is_valid_channel(versions_db, &channel.to_string()) { + if channel_valid { UserError { msg: format!("`{}` from directory override is not installed. Please run `juliaup add {}` to install channel or version.", channel, channel) } } else { UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` from directory override. Please run `juliaup list` to get a list of valid channels and versions.", channel) } diff --git a/src/command_default.rs b/src/command_default.rs index 20969be3..bb8e4e8c 100644 --- a/src/command_default.rs +++ b/src/command_default.rs @@ -10,7 +10,7 @@ pub fn run_command_default(channel: &str, paths: &GlobalPaths) -> Result<()> { if !config_file.data.installed_channels.contains_key(channel) { let version_db = load_versions_db(paths) .with_context(|| "`default` command failed to load versions db.")?; - if !is_valid_channel(&version_db, &channel.to_string()) { + if !is_valid_channel(&version_db, &channel.to_string())? { bail!("'{}' is not a valid Julia version.", channel); } else { bail!( diff --git a/src/command_link.rs b/src/command_link.rs index 2bceccbb..a1a4caa4 100644 --- a/src/command_link.rs +++ b/src/command_link.rs @@ -25,7 +25,7 @@ pub fn run_command_link( bail!("Channel name `{}` is already used.", channel) } - if is_valid_channel(&versiondb_data, &channel.to_string()) { + if is_valid_channel(&versiondb_data, &channel.to_string())? { eprintln!("WARNING: The channel name `{}` is also a system channel. By linking your custom binary to this channel you are hiding this system channel.", channel); } diff --git a/src/operations.rs b/src/operations.rs index 21441cf9..7cacd33f 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -434,27 +434,23 @@ pub fn compatible_archs() -> Result> { // which nightly channels are compatible with the current system fn compatible_nightly_channels() -> Result> { - let archs: Result> = compatible_archs(); + let archs: Vec = compatible_archs()?; - if archs.is_ok() { - let channels: Vec = std::iter::once("nightly".to_string()) - .chain(archs?.into_iter().map(|arch| format!("nightly~{}", arch))) - .collect(); - Ok(channels) - } else { - archs - } + let channels: Vec = std::iter::once("nightly".to_string()) + .chain(archs.into_iter().map(|arch| format!("nightly~{}", arch))) + .collect(); + Ok(channels) } // considers the nightly channels as system channels // XXX: does not account for PR channels -pub fn is_valid_channel(versions_db: &JuliaupVersionDB, channel: &String) -> bool { +pub fn is_valid_channel(versions_db: &JuliaupVersionDB, channel: &String) -> Result { let regular = versions_db.available_channels.contains_key(channel); - let nightly_chans = compatible_nightly_channels(); + let nightly_chans = compatible_nightly_channels()?; - let nightly = nightly_chans.is_ok_and(|nightly_chans| nightly_chans.contains(channel)); - regular || nightly + let nightly = nightly_chans.contains(channel); + Ok(regular || nightly) } // Identify the unversioned name of a nightly (e.g., `latest-macos-x86_64`) for a channel