diff --git a/CHANGELOG.md b/CHANGELOG.md index 532b3f1b..263a4c31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed ### Changed +- Reduce logs verbosity, add docstrings, and use async methods (#384) ### Removed diff --git a/README.md b/README.md index 5142b0ea..b05e58cf 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,7 @@ Options: > **Note** > > #### GitHub API -> During the installation process, several GitHub queries are made, [which are subject to certain limits](https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limiting). Our number of queries should not hit the limits unless you are running `espup install` command numerous times in a short span of time. We recommend setting the [`GITHUB_TOKEN` environment variable](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret) when using `espup` in CI, if you want to use `espup` on CI, recommend using it via the [`xtensa-toolchain` action](https://github.com/esp-rs/xtensa-toolchain/), and making sure `GITHUB_TOKEN` is not set when using it on a host machine. See https://github.com/esp-rs/xtensa-toolchain/issues/15 for more details on this. +> During the installation process, several GitHub queries are made, [which are subject to certain limits](https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limiting). Our number of queries should not hit the limit unless you are running `espup install` command numerous times in a short span of time. We recommend setting the [`GITHUB_TOKEN` environment variable](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret) when using `espup` in CI, if you want to use `espup` on CI, recommend using it via the [`xtensa-toolchain` action](https://github.com/esp-rs/xtensa-toolchain/), and making sure `GITHUB_TOKEN` is not set when using it on a host machine. See https://github.com/esp-rs/xtensa-toolchain/issues/15 for more details on this. ``` Usage: espup install [OPTIONS] diff --git a/src/cli.rs b/src/cli.rs index a24730fe..d6a663d3 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,3 +1,5 @@ +//! Command line interface. + use crate::targets::{parse_targets, Target}; use clap::Parser; use clap_complete::Shell; diff --git a/src/env.rs b/src/env.rs index 9b17d915..1204f1e3 100644 --- a/src/env.rs +++ b/src/env.rs @@ -5,9 +5,8 @@ use directories::BaseDirs; use log::info; #[cfg(windows)] use log::warn; -#[cfg(windows)] -use std::env; use std::{ + env, fs::File, io::Write, path::{Path, PathBuf}, @@ -58,7 +57,7 @@ pub fn get_export_file(export_file: Option) -> Result { if export_file.is_absolute() { Ok(export_file) } else { - let current_dir = std::env::current_dir()?; + let current_dir = env::current_dir()?; Ok(current_dir.join(export_file)) } } else { @@ -113,7 +112,12 @@ pub fn export_environment(export_file: &Path) -> Result<(), Error> { mod tests { use crate::env::{create_export_file, get_export_file, DEFAULT_EXPORT_FILE}; use directories::BaseDirs; - use std::{env::current_dir, path::PathBuf}; + use std::{ + env::current_dir, + fs::{create_dir_all, read_to_string}, + path::PathBuf, + }; + use tempfile::TempDir; #[test] #[allow(unused_variables)] @@ -142,20 +146,20 @@ mod tests { #[test] fn test_create_export_file() { // Creates the export file and writes the correct content to it - let temp_dir = tempfile::TempDir::new().unwrap(); + let temp_dir = TempDir::new().unwrap(); let export_file = temp_dir.path().join("export.sh"); let exports = vec![ "export VAR1=value1".to_string(), "export VAR2=value2".to_string(), ]; create_export_file(&export_file, &exports).unwrap(); - let contents = std::fs::read_to_string(export_file).unwrap(); + let contents = read_to_string(export_file).unwrap(); assert_eq!(contents, "export VAR1=value1\nexport VAR2=value2\n"); // Returns the correct error when it fails to create the export file (it already exists) - let temp_dir = tempfile::TempDir::new().unwrap(); + let temp_dir = TempDir::new().unwrap(); let export_file = temp_dir.path().join("export.sh"); - std::fs::create_dir_all(&export_file).unwrap(); + create_dir_all(&export_file).unwrap(); let exports = vec![ "export VAR1=value1".to_string(), "export VAR2=value2".to_string(), diff --git a/src/main.rs b/src/main.rs index ba5b2618..2c203207 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,17 +3,20 @@ use clap::{CommandFactory, Parser}; use espup::env::set_environment_variable; use espup::{ cli::{CompletionsOpts, InstallOpts, UninstallOpts}, - error::Error, logging::initialize_logger, toolchain::{ - gcc::uninstall_gcc_toolchains, install as toolchain_install, llvm::Llvm, - rust::get_rustup_home, InstallMode, + gcc::uninstall_gcc_toolchains, + install as toolchain_install, + llvm::Llvm, + remove_dir, + rust::{get_rustup_home, XtensaRust}, + InstallMode, }, update::check_for_update, }; use log::info; use miette::Result; -use std::{env, fs::remove_dir_all}; +use std::{env, io::stdout}; #[derive(Parser)] #[command(about, version)] @@ -42,12 +45,7 @@ async fn completions(args: CompletionsOpts) -> Result<()> { info!("Generating completions for {} shell", args.shell); - clap_complete::generate( - args.shell, - &mut Cli::command(), - "espup", - &mut std::io::stdout(), - ); + clap_complete::generate(args.shell, &mut Cli::command(), "espup", &mut stdout()); info!("Completions successfully generated!"); @@ -69,21 +67,20 @@ async fn uninstall(args: UninstallOpts) -> Result<()> { check_for_update(env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")); info!("Uninstalling the Espressif Rust ecosystem"); - let install_path = get_rustup_home().join("toolchains").join(args.name); + let toolchain_dir = get_rustup_home().join("toolchains").join(args.name); - Llvm::uninstall(&install_path)?; + if toolchain_dir.exists() { + Llvm::uninstall(&toolchain_dir).await?; - uninstall_gcc_toolchains(&install_path)?; + uninstall_gcc_toolchains(&toolchain_dir).await?; - info!( - "Deleting the Xtensa Rust toolchain located in '{}'", - &install_path.display() - ); - remove_dir_all(&install_path) - .map_err(|_| Error::RemoveDirectory(install_path.display().to_string()))?; + XtensaRust::uninstall(&toolchain_dir).await?; - #[cfg(windows)] - set_environment_variable("PATH", &env::var("PATH").unwrap())?; + remove_dir(&toolchain_dir).await?; + + #[cfg(windows)] + set_environment_variable("PATH", &env::var("PATH").unwrap())?; + } info!("Uninstallation successfully completed!"); Ok(()) diff --git a/src/toolchain/gcc.rs b/src/toolchain/gcc.rs index 53130c44..38826c6a 100644 --- a/src/toolchain/gcc.rs +++ b/src/toolchain/gcc.rs @@ -8,10 +8,10 @@ use crate::{ use async_trait::async_trait; use log::{debug, info, warn}; use miette::Result; -use std::{ - fs::remove_dir_all, - path::{Path, PathBuf}, -}; +#[cfg(windows)] +use std::env; +use std::path::{Path, PathBuf}; +use tokio::fs::remove_dir_all; const DEFAULT_GCC_REPOSITORY: &str = "https://github.com/espressif/crosstool-NG/releases/download"; const DEFAULT_GCC_RELEASE: &str = "13.2.0_20230928"; @@ -87,9 +87,9 @@ impl Installable for Gcc { "$Env:PATH = \"{};\" + $Env:PATH", &self.get_bin_path() )); - std::env::set_var( + env::set_var( "PATH", - self.get_bin_path().replace('/', "\\") + ";" + &std::env::var("PATH").unwrap(), + self.get_bin_path().replace('/', "\\") + ";" + &env::var("PATH").unwrap(), ); } #[cfg(unix)] @@ -125,7 +125,7 @@ fn get_artifact_extension(host_triple: &HostTriple) -> &str { } /// Checks if the toolchain is pressent, if present uninstalls it. -pub fn uninstall_gcc_toolchains(toolchain_path: &Path) -> Result<(), Error> { +pub async fn uninstall_gcc_toolchains(toolchain_path: &Path) -> Result<(), Error> { info!("Uninstalling GCC"); let gcc_toolchains = vec![XTENSA_GCC, RISCV_GCC]; @@ -141,14 +141,15 @@ pub fn uninstall_gcc_toolchains(toolchain_path: &Path) -> Result<(), Error> { DEFAULT_GCC_RELEASE, toolchain ); - std::env::set_var( + env::set_var( "PATH", - std::env::var("PATH") + env::var("PATH") .unwrap() .replace(&format!("{gcc_path};"), ""), ); } remove_dir_all(&gcc_path) + .await .map_err(|_| Error::RemoveDirectory(gcc_path.display().to_string()))?; } } diff --git a/src/toolchain/llvm.rs b/src/toolchain/llvm.rs index 1a436059..c417d01d 100644 --- a/src/toolchain/llvm.rs +++ b/src/toolchain/llvm.rs @@ -13,13 +13,12 @@ use directories::BaseDirs; use log::{info, warn}; use miette::Result; use regex::Regex; +#[cfg(windows)] +use std::env; +use std::path::{Path, PathBuf}; #[cfg(unix)] use std::{fs::create_dir_all, os::unix::fs::symlink}; -use std::{ - fs::remove_dir_all, - path::{Path, PathBuf}, - u8, -}; +use tokio::fs::remove_dir_all; const DEFAULT_LLVM_REPOSITORY: &str = "https://github.com/espressif/llvm-project/releases/download"; const DEFAULT_LLVM_15_VERSION: &str = "esp-15.0.0-20221201"; @@ -57,7 +56,7 @@ impl Llvm { /// Gets the binary path. fn get_lib_path(&self) -> String { #[cfg(windows)] - let llvm_path = format!("{}/esp-clang/bin", self.path.to_str().unwrap()); + let llvm_path = format!("{}/esp-clang/bin", self.path.to_str().unwrap()).replace('/', "\\"); #[cfg(unix)] let llvm_path = format!("{}/esp-clang/lib", self.path.to_str().unwrap()); llvm_path @@ -66,7 +65,8 @@ impl Llvm { /// Gets the binary path of clang fn get_bin_path(&self) -> String { #[cfg(windows)] - let llvm_path = format!("{}/esp-clang/bin/clang.exe", self.path.to_str().unwrap()); + let llvm_path = + format!("{}/esp-clang/bin/clang.exe", self.path.to_str().unwrap()).replace('/', "\\"); #[cfg(unix)] let llvm_path = format!("{}/esp-clang/bin/clang", self.path.to_str().unwrap()); llvm_path @@ -121,7 +121,7 @@ impl Llvm { } /// Uninstall LLVM toolchain. - pub fn uninstall(toolchain_path: &Path) -> Result<(), Error> { + pub async fn uninstall(toolchain_path: &Path) -> Result<(), Error> { info!("Uninstalling Xtensa LLVM"); let llvm_path = toolchain_path.join(CLANG_NAME); if llvm_path.exists() { @@ -129,7 +129,7 @@ impl Llvm { if cfg!(windows) { delete_environment_variable("LIBCLANG_PATH")?; delete_environment_variable("CLANG_PATH")?; - let updated_path = std::env::var("PATH").unwrap().replace( + let mut updated_path = env::var("PATH").unwrap().replace( &format!( "{}\\{}\\esp-clang\\bin;", llvm_path.display().to_string().replace('/', "\\"), @@ -137,20 +137,29 @@ impl Llvm { ), "", ); + updated_path = updated_path.replace( + &format!( + "{}\\{}\\esp-clang\\bin;", + llvm_path.display().to_string().replace('/', "\\"), + DEFAULT_LLVM_16_VERSION, + ), + "", + ); set_environment_variable("PATH", &updated_path)?; } + remove_dir_all(&llvm_path) + .await + .map_err(|_| Error::RemoveDirectory(llvm_path.display().to_string()))?; #[cfg(unix)] if cfg!(unix) { let espup_dir = BaseDirs::new().unwrap().home_dir().join(".espup"); if espup_dir.exists() { remove_dir_all(espup_dir.display().to_string()) + .await .map_err(|_| Error::RemoveDirectory(espup_dir.display().to_string()))?; } } - let path = toolchain_path.join(CLANG_NAME); - remove_dir_all(&path) - .map_err(|_| Error::RemoveDirectory(path.display().to_string()))?; } Ok(()) } @@ -193,9 +202,9 @@ impl Installable for Llvm { &format!("{}\\libclang.dll", self.get_lib_path().replace('/', "\\")), )?; - std::env::set_var( + env::set_var( "PATH", - self.get_lib_path().replace('/', "\\") + ";" + &std::env::var("PATH").unwrap(), + self.get_lib_path().replace('/', "\\") + ";" + &env::var("PATH").unwrap(), ); } #[cfg(unix)] @@ -210,6 +219,7 @@ impl Installable for Llvm { let llvm_symlink_path = espup_dir.join("esp-clang"); if llvm_symlink_path.exists() { remove_dir_all(&llvm_symlink_path) + .await .map_err(|_| Error::RemoveDirectory(llvm_symlink_path.display().to_string()))?; } info!( diff --git a/src/toolchain/mod.rs b/src/toolchain/mod.rs index efc1f2b6..aa7e6a14 100644 --- a/src/toolchain/mod.rs +++ b/src/toolchain/mod.rs @@ -21,11 +21,11 @@ use retry::{delay::Fixed, retry}; use std::{ env, fs::{create_dir_all, remove_file, File}, - io::Write, + io::{copy, Write}, path::{Path, PathBuf}, }; use tar::Archive; -use tokio::sync::mpsc; +use tokio::{fs::remove_dir_all, sync::mpsc}; use tokio_retry::{strategy::FixedInterval, Retry}; use xz2::read::XzDecoder; use zip::ZipArchive; @@ -63,11 +63,11 @@ pub async fn download_file( ); remove_file(&file_path)?; } else if !Path::new(&output_directory).exists() { - info!("Creating directory: '{}'", output_directory); + debug!("Creating directory: '{}'", output_directory); create_dir_all(output_directory) .map_err(|_| Error::CreateDirectory(output_directory.to_string()))?; } - info!("Downloading file '{}' from '{}'", &file_path, url); + info!("Downloading '{}'", &file_name); let resp = reqwest::get(&url).await?; let bytes = resp.bytes().await?; if uncompress { @@ -93,7 +93,7 @@ pub async fn download_file( } else { create_dir_all(outpath.parent().unwrap())?; let mut outfile = File::create(&outpath)?; - std::io::copy(&mut file, &mut outfile)?; + copy(&mut file, &mut outfile)?; } } } else { @@ -101,7 +101,7 @@ pub async fn download_file( } } "gz" => { - info!("Extracting tar.gz file to '{}'", output_directory); + debug!("Extracting tar.gz file to '{}'", output_directory); let bytes = bytes.to_vec(); let tarfile = GzDecoder::new(bytes.as_slice()); @@ -109,7 +109,7 @@ pub async fn download_file( archive.unpack(output_directory)?; } "xz" => { - info!("Extracting tar.xz file to '{}'", output_directory); + debug!("Extracting tar.xz file to '{}'", output_directory); let bytes = bytes.to_vec(); let tarfile = XzDecoder::new(bytes.as_slice()); let mut archive = Archive::new(tarfile); @@ -120,7 +120,7 @@ pub async fn download_file( } } } else { - info!("Creating file: '{}'", file_path); + debug!("Creating file: '{}'", file_path); let mut out = File::create(&file_path)?; out.write_all(&bytes)?; } @@ -145,9 +145,9 @@ pub async fn install(args: InstallOpts, install_mode: InstallMode) -> Result<()> } else { XtensaRust::get_latest_version().await? }; - let install_path = get_rustup_home().join("toolchains").join(args.name); + let toolchain_dir = get_rustup_home().join("toolchains").join(args.name); let llvm: Llvm = Llvm::new( - &install_path, + &toolchain_dir, &host_triple, args.extended_llvm, &xtensa_rust_version, @@ -160,7 +160,7 @@ pub async fn install(args: InstallOpts, install_mode: InstallMode) -> Result<()> Some(XtensaRust::new( &xtensa_rust_version, &host_triple, - &install_path, + &toolchain_dir, )) } else { None @@ -184,7 +184,7 @@ pub async fn install(args: InstallOpts, install_mode: InstallMode) -> Result<()> xtensa_rust, &args.skip_version_parse, targets, - &install_path, + &toolchain_dir, args.toolchain_version, ); @@ -210,13 +210,13 @@ pub async fn install(args: InstallOpts, install_mode: InstallMode) -> Result<()> .iter() .any(|t| t == &Target::ESP32 || t == &Target::ESP32S2 || t == &Target::ESP32S3) { - let xtensa_gcc = Gcc::new(XTENSA_GCC, &host_triple, &install_path); + let xtensa_gcc = Gcc::new(XTENSA_GCC, &host_triple, &toolchain_dir); to_install.push(Box::new(xtensa_gcc)); } // All RISC-V targets use the same GCC toolchain // ESP32S2 and ESP32S3 also install the RISC-V toolchain for their ULP coprocessor if targets.iter().any(|t| t != &Target::ESP32) { - let riscv_gcc = Gcc::new(RISCV_GCC, &host_triple, &install_path); + let riscv_gcc = Gcc::new(RISCV_GCC, &host_triple, &toolchain_dir); to_install.push(Box::new(riscv_gcc)); } } @@ -257,7 +257,7 @@ pub async fn install(args: InstallOpts, install_mode: InstallMode) -> Result<()> /// Queries the GitHub API and returns the JSON response. pub fn github_query(url: &str) -> Result { - info!("Querying GitHub API: '{}'", url); + debug!("Querying GitHub API: '{}'", url); let mut headers = header::HeaderMap::new(); headers.insert(header::USER_AGENT, "espup".parse().unwrap()); headers.insert( @@ -293,3 +293,17 @@ pub fn github_query(url: &str) -> Result { .unwrap(); Ok(json) } + +/// Checks if the directory exists and deletes it if it does. +pub async fn remove_dir(path: &Path) -> Result<()> { + if path.exists() { + debug!( + "Deleting the Xtensa Rust toolchain located in '{}'", + &path.display() + ); + remove_dir_all(&path) + .await + .map_err(|_| Error::RemoveDirectory(path.display().to_string()))?; + } + Ok(()) +} diff --git a/src/toolchain/rust.rs b/src/toolchain/rust.rs index c556cd61..6d8e6a6d 100644 --- a/src/toolchain/rust.rs +++ b/src/toolchain/rust.rs @@ -21,12 +21,14 @@ use std::fs::create_dir_all; use std::{ env, fmt::Debug, - fs::{read_dir, remove_dir_all, remove_file}, + fs::read_dir, + io, path::{Path, PathBuf}, process::{Command, Stdio}, }; #[cfg(unix)] use tempfile::tempdir_in; +use tokio::fs::{remove_dir_all, remove_file}; /// Xtensa Rust Toolchain repository const DEFAULT_XTENSA_RUST_REPOSITORY: &str = @@ -157,7 +159,7 @@ impl XtensaRust { } /// Removes the Xtensa Rust toolchain. - pub fn uninstall(toolchain_path: &Path) -> Result<(), Error> { + pub async fn uninstall(toolchain_path: &Path) -> Result<(), Error> { info!("Uninstalling Xtensa Rust toolchain"); let dir = read_dir(toolchain_path)?; for entry in dir { @@ -169,10 +171,10 @@ impl XtensaRust { { if entry_path.is_dir() { remove_dir_all(Path::new(&entry_name)) + .await .map_err(|_| Error::RemoveDirectory(entry_name))?; } else { - // If the entry is a file, delete the file - remove_file(&entry_name)?; + remove_file(&entry_name).await?; } } } @@ -208,7 +210,7 @@ impl Installable for XtensaRust { if !rustc_version.status.success() { warn!("Failed to detect version of Xtensa Rust, reinstalling it"); } - Self::uninstall(&self.toolchain_destination)?; + Self::uninstall(&self.toolchain_destination).await?; } } @@ -254,7 +256,7 @@ impl Installable for XtensaRust { .status .success() { - Self::uninstall(&self.toolchain_destination)?; + Self::uninstall(&self.toolchain_destination).await?; return Err(Error::XtensaRust); } @@ -281,7 +283,7 @@ impl Installable for XtensaRust { .status .success() { - Self::uninstall(&self.toolchain_destination)?; + Self::uninstall(&self.toolchain_destination).await?; return Err(Error::XtensaRustSrc); } } @@ -421,7 +423,7 @@ pub async fn check_rust_installation() -> Result<(), Error> { .stdout(Stdio::piped()) .output() { - if let std::io::ErrorKind::NotFound = e.kind() { + if let io::ErrorKind::NotFound = e.kind() { return Err(Error::MissingRust); } else { return Err(Error::RustupDetection(e.to_string())); @@ -438,6 +440,8 @@ mod tests { toolchain::rust::{get_cargo_home, get_rustup_home, XtensaRust}, }; use directories::BaseDirs; + use std::env; + use tempfile::TempDir; #[test] fn test_xtensa_rust_parse_version() { @@ -459,30 +463,30 @@ mod tests { #[test] fn test_get_cargo_home() { // No CARGO_HOME set - std::env::remove_var("CARGO_HOME"); + env::remove_var("CARGO_HOME"); assert_eq!( get_cargo_home(), BaseDirs::new().unwrap().home_dir().join(".cargo") ); // CARGO_HOME set - let temp_dir = tempfile::TempDir::new().unwrap(); + let temp_dir = TempDir::new().unwrap(); let cargo_home = temp_dir.path().to_path_buf(); - std::env::set_var("CARGO_HOME", cargo_home.to_str().unwrap()); + env::set_var("CARGO_HOME", cargo_home.to_str().unwrap()); assert_eq!(get_cargo_home(), cargo_home); } #[test] fn test_get_rustup_home() { // No RUSTUP_HOME set - std::env::remove_var("RUSTUP_HOME"); + env::remove_var("RUSTUP_HOME"); assert_eq!( get_rustup_home(), BaseDirs::new().unwrap().home_dir().join(".rustup") ); // RUSTUP_HOME set - let temp_dir = tempfile::TempDir::new().unwrap(); + let temp_dir = TempDir::new().unwrap(); let rustup_home = temp_dir.path().to_path_buf(); - std::env::set_var("RUSTUP_HOME", rustup_home.to_str().unwrap()); + env::set_var("RUSTUP_HOME", rustup_home.to_str().unwrap()); assert_eq!(get_rustup_home(), rustup_home); } }