From 6c5918a2dc7926927c0b35c795eafca006652776 Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Sun, 10 Dec 2023 16:53:32 +0800 Subject: [PATCH 1/7] make julialauncher have saner error messages for channels and versions Signed-off-by: Soc Virnyl Estela --- src/bin/julialauncher.rs | 46 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index 1201237f..0290c764 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -159,13 +159,53 @@ fn get_julia_path_from_channel( juliaupconfig_path: &Path, juliaup_channel_source: JuliaupChannelSource, ) -> Result<(PathBuf, Vec)> { + use juliaup::global_paths::get_paths; + use juliaup::versions_file::load_versions_db; + + let paths = get_paths().with_context(|| "Trying to load all global paths.")?; + let versiondb_data = + load_versions_db(&paths).with_context(|| "command failed to load versions db.")?; + // TODO Use versiondb data instead of config_data. + // RATIONALE: + // Using versiondb.available_channels is more useful + // because we can just check if the channel provided by the user + // is also a valid channel of the versiondb.available_channels. + // Because having to error because it's not installed yet + // running `julialauncher` by itself will download and run + // latest release of julia is inconsistent behavior. + // So we should check for available_channels instead of installed + // channels. let channel_info = config_data .installed_channels .get(channel) .ok_or_else(|| match juliaup_channel_source { - JuliaupChannelSource::CmdLine => UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` at command line.", channel).to_string() }.into(), - JuliaupChannelSource::EnvVar => UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in environment variable JULIAUP_CHANNEL.", channel).to_string() }.into(), - JuliaupChannelSource::Override => UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in directory override.", channel).to_string() }.into(), + JuliaupChannelSource::CmdLine => { + if versiondb_data.available_channels.contains_key(channel) { + UserError { msg: format!("`{}` is not installed. Please run `juliaup add {}` to install channel or version", channel, channel) } + } else if versiondb_data.available_versions.contains_key(channel) { + 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 `{}` is not installed. Please run `juliaup list` to check a valid channel or version", channel) } + } + }.into(), + JuliaupChannelSource::EnvVar=> { + if versiondb_data.available_channels.contains_key(channel) { + UserError { msg: format!("`{}` for environment variable JULIAUP_CHANNEL is not installed. Please run `juliaup add {}` to install channel or version", channel, channel) } + } else if versiondb_data.available_versions.contains_key(channel) { + 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 check a valid channel or version", channel) } + } + }.into(), + JuliaupChannelSource::Override=> { + if versiondb_data.available_channels.contains_key(channel) { + UserError { msg: format!("`{}` for directory override is not installed. Please run `juliaup add {}` to install channel or version", channel, channel) } + } else if versiondb_data.available_versions.contains_key(channel) { + 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 check a valid channel or version", channel) } + } + }.into(), JuliaupChannelSource::Default => anyhow!("The Juliaup configuration is in an inconsistent state, the currently configured default channel `{}` is not installed.", channel) })?; From a0bac66c6c52f3cd32086f337c714c35b01a3c4d Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Sun, 10 Dec 2023 16:58:11 +0800 Subject: [PATCH 2/7] remove comment. irrelevant and unrelated now Signed-off-by: Soc Virnyl Estela --- src/bin/julialauncher.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index 0290c764..1744103a 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -165,16 +165,6 @@ fn get_julia_path_from_channel( let paths = get_paths().with_context(|| "Trying to load all global paths.")?; let versiondb_data = load_versions_db(&paths).with_context(|| "command failed to load versions db.")?; - // TODO Use versiondb data instead of config_data. - // RATIONALE: - // Using versiondb.available_channels is more useful - // because we can just check if the channel provided by the user - // is also a valid channel of the versiondb.available_channels. - // Because having to error because it's not installed yet - // running `julialauncher` by itself will download and run - // latest release of julia is inconsistent behavior. - // So we should check for available_channels instead of installed - // channels. let channel_info = config_data .installed_channels .get(channel) From 62650cd4c627a71c2b3e0d7e6a6e6046290d7b79 Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Sun, 10 Dec 2023 16:59:38 +0800 Subject: [PATCH 3/7] remove redundant use Signed-off-by: Soc Virnyl Estela --- src/bin/julialauncher.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index 1744103a..043aff82 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -159,8 +159,6 @@ fn get_julia_path_from_channel( juliaupconfig_path: &Path, juliaup_channel_source: JuliaupChannelSource, ) -> Result<(PathBuf, Vec)> { - use juliaup::global_paths::get_paths; - use juliaup::versions_file::load_versions_db; let paths = get_paths().with_context(|| "Trying to load all global paths.")?; let versiondb_data = From e50633b22cd6b949ee45eb8a16d81d8d6edb287d Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Sun, 10 Dec 2023 17:00:15 +0800 Subject: [PATCH 4/7] run cargo fmt Signed-off-by: Soc Virnyl Estela --- src/bin/julialauncher.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index 043aff82..32c43112 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -159,7 +159,6 @@ fn get_julia_path_from_channel( juliaupconfig_path: &Path, juliaup_channel_source: JuliaupChannelSource, ) -> Result<(PathBuf, Vec)> { - let paths = get_paths().with_context(|| "Trying to load all global paths.")?; let versiondb_data = load_versions_db(&paths).with_context(|| "command failed to load versions db.")?; From ccd1dcbdb6735e0488e6b3fbd0602cf388cd4b64 Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Sun, 10 Dec 2023 17:31:12 +0800 Subject: [PATCH 5/7] fix an oopsie on one of the messages Signed-off-by: Soc Virnyl Estela --- src/bin/julialauncher.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index 32c43112..8487fb4e 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -172,7 +172,7 @@ fn get_julia_path_from_channel( } else if versiondb_data.available_versions.contains_key(channel) { 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 `{}` is not installed. Please run `juliaup list` to check a valid channel or version", channel) } + UserError { msg: format!("ERROR: Invalid Juliaup channel `{}`. Please run `juliaup list` to check a valid channel or version", channel) } } }.into(), JuliaupChannelSource::EnvVar=> { From e692fe981462316ba21360e8945acf7c583dc380 Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Sun, 10 Dec 2023 17:41:59 +0800 Subject: [PATCH 6/7] fix error message on tests channel selection Signed-off-by: Soc Virnyl Estela --- tests/channel_selection.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/channel_selection.rs b/tests/channel_selection.rs index 35034837..1ac9da7b 100644 --- a/tests/channel_selection.rs +++ b/tests/channel_selection.rs @@ -99,7 +99,7 @@ fn channel_selection() { .env("JULIAUP_DEPOT_PATH", depot_dir.path()) .assert() .failure() - .stderr("ERROR: Invalid Juliaup channel `1.8.6` at command line.\n"); + .stderr("ERROR: Invalid Juliaup channel `1.8.6`. Please run `juliaup list` to check a valid channel or version\n"); Command::cargo_bin("julialauncher") .unwrap() From d8233b717001f198033106e2cb68712fa3c2a217 Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Sun, 10 Dec 2023 17:46:27 +0800 Subject: [PATCH 7/7] fix other error messages as well Signed-off-by: Soc Virnyl Estela --- tests/channel_selection.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/channel_selection.rs b/tests/channel_selection.rs index 1ac9da7b..58867da1 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.\n", + "ERROR: Invalid Juliaup channel `1.7.4` in environment variable JULIAUP_CHANNEL. Please run `juliaup list` to check a valid channel or version\n", ); Command::cargo_bin("julialauncher") @@ -124,5 +124,5 @@ fn channel_selection() { .env("JULIAUP_CHANNEL", "1.7.4") .assert() .failure() - .stderr("ERROR: Invalid Juliaup channel `1.8.6` at command line.\n"); + .stderr("ERROR: Invalid Juliaup channel `1.8.6`. Please run `juliaup list` to check a valid channel or version\n"); }