From 5e16fe4ce1a4be77d79bdabfa05c9e55e02dfda7 Mon Sep 17 00:00:00 2001 From: Sebastian Ullrich Date: Thu, 13 Jun 2024 18:21:30 +0200 Subject: [PATCH] chore: eager resolution cleanups (#128) --- .github/workflows/ci.yml | 2 +- src/elan-cli/common.rs | 4 ++-- src/elan-cli/elan_mode.rs | 25 +++++++++---------------- src/elan-cli/errors.rs | 4 ---- src/elan-dist/src/dist.rs | 6 ++++++ src/elan/config.rs | 27 ++++++++++++--------------- src/elan/errors.rs | 4 ---- src/elan/toolchain.rs | 4 +--- 8 files changed, 31 insertions(+), 45 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 03445ca..3f520c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -103,9 +103,9 @@ jobs: cd foo lake build - uses: softprops/action-gh-release@v1 + if: startsWith(github.ref, 'refs/tags/') with: files: elan-${{ matrix.release-target-name || matrix.target }}* draft: ${{ !startsWith(github.ref, 'refs/tags/v') }} - if: startsWith(github.ref, 'refs/tags/') env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/src/elan-cli/common.rs b/src/elan-cli/common.rs index 065d318..c67d19a 100644 --- a/src/elan-cli/common.rs +++ b/src/elan-cli/common.rs @@ -255,9 +255,9 @@ pub fn list_toolchains(cfg: &Cfg) -> Result<()> { if toolchains.is_empty() { println!("no installed toolchains"); } else { - if let Ok(Some(def_toolchain)) = cfg.find_default() { + if let Ok(Some(def_toolchain)) = cfg.resolve_default() { for toolchain in toolchains { - let if_default = if def_toolchain.name() == toolchain.to_string() { + let if_default = if def_toolchain == toolchain { " (default)" } else { "" diff --git a/src/elan-cli/elan_mode.rs b/src/elan-cli/elan_mode.rs index 7fddf7b..2c88c32 100644 --- a/src/elan-cli/elan_mode.rs +++ b/src/elan-cli/elan_mode.rs @@ -232,22 +232,10 @@ pub fn cli() -> App<'static, 'static> { fn default_(cfg: &Cfg, m: &ArgMatches) -> Result<()> { let ref name = m.value_of("toolchain").expect(""); - let toolchain = lookup_toolchain_desc(cfg, name)?; - let ref toolchain = cfg.get_toolchain(&toolchain, false)?; - - let status = if !toolchain.exists() || !toolchain.is_custom() { - Some(toolchain.install_from_dist_if_not_installed()?) - } else { - None - }; + // sanity-check + let _ = lookup_toolchain_desc(cfg, name)?; cfg.set_default(name)?; - - if let Some(status) = status { - println!(""); - common::show_channel_update(cfg, &toolchain.desc, Ok(status))?; - } - Ok(()) } @@ -319,8 +307,13 @@ fn show(cfg: &Cfg) -> Result<()> { if show_headers { print_header("installed toolchains") } + let default_tc = cfg.resolve_default()?; for t in installed_toolchains { - println!("{}", t); + if default_tc.as_ref() == Some(&t) { + println!("{} (default)", t); + } else { + println!("{}", t); + } } if show_headers { println!("") @@ -389,7 +382,7 @@ fn explicit_or_dir_toolchain<'a>(cfg: &'a Cfg, m: &ArgMatches) -> Result Result<()> { let ref toolchain = m.value_of("toolchain").expect(""); let ref path = m.value_of("path").expect(""); - let desc = ToolchainDesc::from_resolved_str(toolchain)?; + let desc = ToolchainDesc::Local { name: toolchain.to_string() }; let toolchain = cfg.get_toolchain(&desc, true)?; Ok(toolchain.install_from_dir(Path::new(path), true)?) diff --git a/src/elan-cli/errors.rs b/src/elan-cli/errors.rs index 10d7fbf..1a3f14b 100644 --- a/src/elan-cli/errors.rs +++ b/src/elan-cli/errors.rs @@ -23,10 +23,6 @@ error_chain! { PermissionDenied { description("permission denied") } - ToolchainNotInstalled(t: String) { - description("toolchain is not installed") - display("toolchain '{}' is not installed", t) - } InfiniteRecursion { description("infinite recursion detected") } diff --git a/src/elan-dist/src/dist.rs b/src/elan-dist/src/dist.rs index 4c91c0f..ee21894 100644 --- a/src/elan-dist/src/dist.rs +++ b/src/elan-dist/src/dist.rs @@ -43,6 +43,12 @@ impl ToolchainDesc { Err(ErrorKind::InvalidToolchainName(name.to_string()).into()) } } + + pub fn from_toolchain_dir(dir_name: &str) -> Result { + // de-sanitize toolchain file names (best effort...) + let name = dir_name.replace("---", ":").replace("--", "/"); + Self::from_resolved_str(&name) + } } impl fmt::Display for ToolchainDesc { diff --git a/src/elan/config.rs b/src/elan/config.rs index 04d8616..e1aa631 100644 --- a/src/elan/config.rs +++ b/src/elan/config.rs @@ -120,12 +120,6 @@ impl Cfg { Ok(Toolchain::from(self, name)) } - pub fn verify_toolchain(&self, name: &ToolchainDesc) -> Result { - let toolchain = self.get_toolchain(name, false)?; - toolchain.install_from_dist_if_not_installed()?; - Ok(toolchain) - } - pub fn which_binary(&self, path: &Path, binary: &str) -> Result> { if let Some((toolchain, _)) = self.find_override_toolchain_or_default(path)? { Ok(Some(toolchain.binary_file(binary))) @@ -134,13 +128,13 @@ impl Cfg { } } - pub fn find_default(&self) -> Result> { + pub fn resolve_default(&self) -> Result> { let opt_name = self .settings_file .with(|s| Ok(s.default_toolchain.clone()))?; if let Some(name) = opt_name { - let toolchain = self.verify_toolchain(&lookup_toolchain_desc(&self, &name)?)?; + let toolchain = lookup_toolchain_desc(&self, &name)?; Ok(Some(toolchain)) } else { Ok(None) @@ -273,7 +267,7 @@ impl Cfg { if let Some(last) = d.file_name() { if let Some(last) = last.to_str() { return Ok(Some(( - lookup_toolchain_desc(&self, last)?, + ToolchainDesc::from_toolchain_dir(last)?, OverrideReason::InToolchainDirectory(d.into()), ))); } @@ -292,22 +286,25 @@ impl Cfg { if let Some((toolchain, reason)) = self.find_override(path)? { Some((toolchain, Some(reason))) } else { - self.find_default()?.map(|toolchain| (toolchain, None)) + if let Some(tc) = self.resolve_default()? { + Some((self.get_toolchain(&tc, false)?, None)) + } else { + None + } }, ) } pub fn list_toolchains(&self) -> Result> { - // de-sanitize toolchain file names (best effort...) - fn insane(s: String) -> String { - s.replace("---", ":").replace("--", "/") - } if utils::is_directory(&self.toolchains_dir) { let mut toolchains: Vec<_> = utils::read_dir("toolchains", &self.toolchains_dir)? .filter_map(io::Result::ok) .filter(|e| e.file_type().map(|f| !f.is_file()).unwrap_or(false)) .filter_map(|e| e.file_name().into_string().ok()) - .map(insane) + .map(|n| ToolchainDesc::from_toolchain_dir(&n).map_err(|e| e.into())) + .collect::>>()? + .into_iter() + .map(|tc| tc.to_string()) .collect(); utils::toolchain_sort(&mut toolchains); diff --git a/src/elan/errors.rs b/src/elan/errors.rs index 64b0f54..0588727 100644 --- a/src/elan/errors.rs +++ b/src/elan/errors.rs @@ -24,10 +24,6 @@ error_chain! { description("unknown metadata version") display("unknown metadata version: '{}'", v) } - ToolchainNotInstalled(t: ToolchainDesc) { - description("toolchain is not installed") - display("toolchain '{}' is not installed", t) - } NoDefaultToolchain { description("no default toolchain configured. run `elan default stable` to install & configure the latest Lean 4 stable release.") } diff --git a/src/elan/toolchain.rs b/src/elan/toolchain.rs index 19e638c..1314bd8 100644 --- a/src/elan/toolchain.rs +++ b/src/elan/toolchain.rs @@ -200,9 +200,7 @@ impl<'a> Toolchain<'a> { } pub fn create_command>(&self, binary: T) -> Result { - if !self.exists() { - return Err(ErrorKind::ToolchainNotInstalled(self.desc.clone()).into()); - } + self.install_from_dist_if_not_installed()?; let bin_path = self.binary_file(&binary); let path = if utils::is_file(&bin_path) {