From 6c5918a2dc7926927c0b35c795eafca006652776 Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Sun, 10 Dec 2023 16:53:32 +0800 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 07/10] 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"); } From a6705e955b2625f1a9fe7bebffa77bc5457829b1 Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Thu, 4 Jan 2024 18:47:51 +0800 Subject: [PATCH 08/10] fixup: message should be more clear and generic Signed-off-by: Soc Virnyl Estela --- src/bin/julialauncher.rs | 6 +++--- tests/channel_selection.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index 8487fb4e..f60e3484 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 `{}`. Please run `juliaup list` to check a valid channel or version", channel) } + UserError { msg: format!("ERROR: Invalid Juliaup channel `{}`. Please run `juliaup list` to get a list of valid channels and versions", channel) } } }.into(), JuliaupChannelSource::EnvVar=> { @@ -181,7 +181,7 @@ fn get_julia_path_from_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) } + 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=> { @@ -190,7 +190,7 @@ fn get_julia_path_from_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) } + UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in 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/tests/channel_selection.rs b/tests/channel_selection.rs index 58867da1..491fbe3c 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`. Please run `juliaup list` to check a valid channel or version\n"); + .stderr("ERROR: Invalid Juliaup channel `1.8.6`. Please run `juliaup list` to get a list of valid channels and versions\n"); Command::cargo_bin("julialauncher") .unwrap() @@ -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 check a valid channel or version\n", + "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", ); 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`. Please run `juliaup list` to check a valid channel or version\n"); + .stderr("ERROR: Invalid Juliaup channel `1.8.6`. Please run `juliaup list` to get a list of valid channels and versions\n"); } From dc4500c5692a76dac823cb4c4075fbc18a076409 Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Fri, 12 Jan 2024 11:31:01 +0800 Subject: [PATCH 09/10] refactor: remove redundant versiondb_data. also remove versiondb_data.available_versions since users cant access those Signed-off-by: Soc Virnyl Estela --- src/bin/julialauncher.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index f60e3484..cc10c3e7 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -159,35 +159,26 @@ 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.")?; let channel_info = config_data .installed_channels .get(channel) .ok_or_else(|| match juliaup_channel_source { 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) { + if versions_db.available_channels.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 `{}`. Please run `juliaup list` to get a list of valid channels and versions", 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) { + if versions_db.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 { 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 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) { + if versions_db.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 { UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in directory override. Please run `juliaup list` to get a list of valid channels and versions", channel) } From 2e32b91f0fb7c422345a37e7b888cb2cf922a61f Mon Sep 17 00:00:00 2001 From: Soc Virnyl Estela Date: Fri, 12 Jan 2024 18:26:33 +0800 Subject: [PATCH 10/10] cleanup: add a period for each info/error messages. Signed-off-by: Soc Virnyl Estela --- src/bin/julialauncher.rs | 12 ++++++------ tests/channel_selection.rs | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/bin/julialauncher.rs b/src/bin/julialauncher.rs index cc10c3e7..955e9e9a 100644 --- a/src/bin/julialauncher.rs +++ b/src/bin/julialauncher.rs @@ -165,23 +165,23 @@ fn get_julia_path_from_channel( .ok_or_else(|| match juliaup_channel_source { JuliaupChannelSource::CmdLine => { if versions_db.available_channels.contains_key(channel) { - UserError { msg: format!("`{}` is not installed. Please run `juliaup add {}` to install channel or version", channel, 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 `{}`. Please run `juliaup list` to get a list of valid channels and versions", channel) } + 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) { - 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!("`{}` 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) } + 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) { - UserError { msg: format!("`{}` for directory override is not installed. Please run `juliaup add {}` to install channel or version", channel, 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 get a list of valid channels and versions", channel) } + UserError { msg: format!("ERROR: Invalid Juliaup channel `{}` in 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/tests/channel_selection.rs b/tests/channel_selection.rs index 491fbe3c..aabe1def 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`. Please run `juliaup list` to get a list of valid channels and versions\n"); + .stderr("ERROR: Invalid Juliaup channel `1.8.6`. Please run `juliaup list` to get a list of valid channels and versions.\n"); Command::cargo_bin("julialauncher") .unwrap() @@ -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` in environment variable JULIAUP_CHANNEL. Please run `juliaup list` to get a list of valid channels and versions.\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`. Please run `juliaup list` to get a list of valid channels and versions\n"); + .stderr("ERROR: Invalid Juliaup channel `1.8.6`. Please run `juliaup list` to get a list of valid channels and versions.\n"); }