From 276ba1ed549d2833d0399981aca6c00699208c1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20D=2E=20Rodas?= Date: Wed, 21 Feb 2024 19:01:47 -0300 Subject: [PATCH] Do not download self update if it is the same version (#576) * Do not download self update if it is the same version Fixes #555 * cargo fmt * Update tests and fixed fmt issues --- Cargo.lock | 9 ++-- Cargo.toml | 1 + src/commands/fuelup.rs | 11 +++-- src/file.rs | 32 ++++++++++++-- src/fuelup_cli.rs | 2 +- src/ops/fuelup_check.rs | 76 ++++++-------------------------- src/ops/fuelup_component/list.rs | 19 ++------ src/ops/fuelup_self.rs | 15 +++++-- src/ops/fuelup_show.rs | 57 ++++++------------------ src/path.rs | 11 +++++ src/toolchain.rs | 13 +----- tests/self.rs | 16 +++++-- tests/testcfg/mod.rs | 16 ++++--- 13 files changed, 119 insertions(+), 159 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d22850884..f908564fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -515,6 +515,7 @@ dependencies = [ "strip-ansi-escapes", "tar", "tempfile", + "thiserror", "time", "toml_edit 0.13.4", "tracing", @@ -1110,18 +1111,18 @@ checksum = "222a222a5bfe1bba4a77b45ec488a741b3cb8872e5e499451fd7d0129c9c7c3d" [[package]] name = "thiserror" -version = "1.0.56" +version = "1.0.57" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d54378c645627613241d077a3a79db965db602882668f9136ac42af9ecb730ad" +checksum = "1e45bcbe8ed29775f228095caf2cd67af7a4ccf756ebff23a306bf3e8b47b24b" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.56" +version = "1.0.57" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa0faa943b50f3db30a20aa7e265dbc66076993efed8463e8de414e5d06d3471" +checksum = "a953cb265bef375dae3de6663da4d3804eee9682ea80d8e2542529b73c531c81" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index ce1d887c1..90a530e6a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ serde_json = "1.0" sha2 = "0.10" tar = "0.4" tempfile = "3" +thiserror = "1.0.57" time = { version = "0.3", features = ["macros", "parsing", "formatting", "serde-well-known"] } toml_edit = { version = "0.13", features = ["serde", "easy"] } tracing = "0.1" diff --git a/src/commands/fuelup.rs b/src/commands/fuelup.rs index f334df929..a94bc4497 100644 --- a/src/commands/fuelup.rs +++ b/src/commands/fuelup.rs @@ -6,14 +6,17 @@ use crate::ops::fuelup_self::self_update; #[derive(Debug, Parser)] pub enum FuelupCommand { /// Updates fuelup - Update, + Update(UpdateCommand), } #[derive(Debug, Parser)] -struct UpdateCommand {} +pub struct UpdateCommand { + #[clap(long, short)] + pub force: bool, +} -pub fn exec() -> Result<()> { - if let Err(e) = self_update() { +pub fn exec(force: bool) -> Result<()> { + if let Err(e) = self_update(force) { bail!("fuelup failed to update itself: {}", e) }; diff --git a/src/file.rs b/src/file.rs index 39a6579f2..99eee25d4 100644 --- a/src/file.rs +++ b/src/file.rs @@ -1,14 +1,38 @@ use anyhow::{Context, Result}; -use std::fs; -use std::io; -use std::os::unix::fs::PermissionsExt; -use std::path::Path; +use semver::Version; +use std::{fs, io, os::unix::fs::PermissionsExt, path::Path}; #[cfg(unix)] pub(crate) fn is_executable(file: &Path) -> bool { file.is_file() && file.metadata().unwrap().permissions().mode() & 0o111 != 0 } +#[derive(Debug, thiserror::Error)] +pub(crate) enum BinError { + #[error("not found")] + NotFound, + + #[error("Could not parse version ({0})")] + SemVer(#[from] semver::Error), + + #[error("{0}")] + Io(#[from] io::Error), +} + +pub(crate) fn get_bin_version(exec_path: &Path) -> Result { + if !exec_path.is_file() { + return Err(BinError::NotFound); + } + let output = std::process::Command::new(exec_path) + .arg("--version") + .output()?; + + let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + Ok(Version::parse( + stdout.split_whitespace().last().unwrap_or_default(), + )?) +} + pub(crate) fn hardlink(original: &Path, link: &Path) -> io::Result<()> { let _ = fs::remove_file(link); fs::hard_link(original, link) diff --git a/src/fuelup_cli.rs b/src/fuelup_cli.rs index efa512698..63b7298a6 100644 --- a/src/fuelup_cli.rs +++ b/src/fuelup_cli.rs @@ -51,7 +51,7 @@ pub fn fuelup_cli() -> Result<()> { Commands::Component(command) => component::exec(command), Commands::Default_(command) => default::exec(command), Commands::Fuelup(command) => match command { - FuelupCommand::Update => fuelup::exec(), + FuelupCommand::Update(update) => fuelup::exec(update.force), }, Commands::Show(_command) => show::exec(), Commands::Toolchain(command) => toolchain::exec(command), diff --git a/src/ops/fuelup_check.rs b/src/ops/fuelup_check.rs index 856478375..00854602c 100644 --- a/src/ops/fuelup_check.rs +++ b/src/ops/fuelup_check.rs @@ -3,6 +3,7 @@ use crate::{ commands::check::CheckCommand, config::Config, download::DownloadCfg, + file::get_bin_version, fmt::{bold, colored_bold}, target_triple::TargetTriple, toolchain::{DistToolchainDescription, Toolchain}, @@ -11,12 +12,12 @@ use ansiterm::Color; use anyhow::Result; use component::{self, Components}; use semver::Version; +use std::collections::HashMap; use std::str::FromStr; use std::{ cmp::Ordering::{Equal, Greater, Less}, path::Path, }; -use std::{collections::HashMap, process::Command}; use tracing::{error, info}; fn collect_package_versions(channel: Channel) -> HashMap { @@ -55,38 +56,12 @@ fn format_version_comparison(current_version: &Version, latest_version: &Version } } -fn check_plugin(plugin_executable: &Path, plugin: &str, latest_version: &Version) -> Result<()> { - match std::process::Command::new(plugin_executable) - .arg("--version") - .output() - { - Ok(o) => { - let output = String::from_utf8_lossy(&o.stdout).into_owned(); - match output.split_whitespace().last() { - Some(v) => { - let version = Version::parse(v)?; - info!( - "{:>4}- {} - {}", - "", - bold(plugin), - format_version_comparison(&version, latest_version) - ); - } - None => { - info!("{:>4}- {} - Error getting version string", "", plugin); - } - }; - } - Err(e) => { - let error_text = if plugin_executable.exists() { - format!("execution error - {e}") - } else { - "not found".into() - }; - info!("{:>4}- {} - {}", "", bold(plugin), error_text); - } - } - Ok(()) +fn check_plugin(plugin_executable: &Path, plugin: &str, latest_version: &Version) { + let version_or_err = match get_bin_version(plugin_executable) { + Ok(version) => format_version_comparison(&version, latest_version), + Err(err) => err.to_string(), + }; + info!("{:>4}- {} - {}", "", plugin, version_or_err); } fn check_fuelup() -> Result<()> { @@ -121,36 +96,11 @@ fn check_toolchain(toolchain: &str, verbose: bool) -> Result<()> { for component in Components::collect_exclude_plugins()? { if let Some(latest_version) = latest_package_versions.get(&component.name) { let component_executable = toolchain.bin_path.join(&component.name); - match Command::new(component_executable).arg("--version").output() { - Ok(o) => { - let output = String::from_utf8_lossy(&o.stdout).into_owned(); - - match output.split_whitespace().last() { - Some(v) => { - let version = Version::parse(v)?; - info!( - "{:>2}{} - {}", - "", - bold(&component.name), - format_version_comparison(&version, latest_version) - ); - } - None => { - error!( - "{:>2}{} - Error getting version string", - "", - bold(&component.name) - ); - } - } - } - Err(_) => error!( - "{:>2}{} - Error getting version string", - "", - bold(&component.name) - ), + let version_text = match get_bin_version(&component_executable) { + Ok(version) => format_version_comparison(&version, latest_version), + Err(err) => err.to_string(), }; - + info!("{:>2}{} - {}", "", bold(&component.name), version_text); if verbose && component.name == component::FORC { for plugin in component::Components::collect_plugins()? { if !plugin.is_main_executable() { @@ -173,7 +123,7 @@ fn check_toolchain(toolchain: &str, verbose: bool) -> Result<()> { ); if let Some(latest_version) = maybe_latest_version { - check_plugin(&plugin_executable, plugin_name, latest_version)?; + check_plugin(&plugin_executable, plugin_name, latest_version); } } } diff --git a/src/ops/fuelup_component/list.rs b/src/ops/fuelup_component/list.rs index fe32c9bfb..1e35dfea2 100644 --- a/src/ops/fuelup_component/list.rs +++ b/src/ops/fuelup_component/list.rs @@ -1,9 +1,9 @@ use crate::{ - commands::component::ListCommand, download::get_latest_version, fmt::bold, toolchain::Toolchain, + commands::component::ListCommand, download::get_latest_version, file::get_bin_version, + fmt::bold, toolchain::Toolchain, }; use anyhow::Result; use component::Components; -use semver::Version; use std::fmt::Write; use tracing::info; @@ -46,20 +46,7 @@ pub fn list(_command: ListCommand) -> Result<()> { ); if toolchain.has_component(&component.name) { let exec_path = toolchain.bin_path.join(&component.name); - - let current_version = if let Ok(o) = std::process::Command::new(exec_path) - .arg("--version") - .output() - { - let output = String::from_utf8_lossy(&o.stdout).into_owned(); - output.split_whitespace().last().map_or_else( - || None, - |v| Version::parse(v).map_or_else(|_| None, |v| Some(v.to_string())), - ) - } else { - None - }; - + let current_version = get_bin_version(&exec_path).map(|v| v.to_string()).ok(); let version_info = match Some(&latest_version) == current_version.as_ref() { true => "up-to-date".to_string(), false => format!("latest: {}", &latest_version), diff --git a/src/ops/fuelup_self.rs b/src/ops/fuelup_self.rs index a4004771e..415621bee 100644 --- a/src/ops/fuelup_self.rs +++ b/src/ops/fuelup_self.rs @@ -6,7 +6,7 @@ use tracing::{error, info}; use crate::{ download::{download_file_and_unpack, unpack_bins, DownloadCfg}, - file::hard_or_symlink_file, + file::{get_bin_version, hard_or_symlink_file}, path::{fuelup_bin, fuelup_bin_dir}, target_triple::TargetTriple, }; @@ -18,7 +18,7 @@ pub fn attempt_install_self(download_cfg: DownloadCfg, dst: &Path) -> Result<()> Ok(()) } -pub fn self_update() -> Result<()> { +pub fn self_update(force: bool) -> Result<()> { let download_cfg = DownloadCfg::new( component::FUELUP, TargetTriple::from_component(component::FUELUP)?, @@ -27,6 +27,14 @@ pub fn self_update() -> Result<()> { let fuelup_bin = fuelup_bin(); + if !force && get_bin_version(&fuelup_bin).ok() == Some(download_cfg.version.clone()) { + info!( + "Already up to date (fuelup v{})", + download_cfg.version.to_string() + ); + return Ok(()); + } + let fuelup_new_dir = tempfile::tempdir()?; let fuelup_new_dir_path = fuelup_new_dir.path(); let fuelup_backup = fuelup_new_dir_path.join("fuelup-backup"); @@ -67,7 +75,8 @@ pub fn self_update() -> Result<()> { "Failed to replace old fuelup with new fuelup: {}. Attempting to restore backup fuelup.", e); // If we have failed to replace the old fuelup for whatever reason, we want the backup. - // Although unlikely, should this last step fail, we will recommend to re-install fuelup using fuelup-init. + // Although unlikely, should this last step fail, we will recommend to re-install fuelup + // using fuelup-init. if let Err(e) = fs::copy(&fuelup_backup, &fuelup_bin) { bail!( "Could not restore backup fuelup. {} diff --git a/src/ops/fuelup_show.rs b/src/ops/fuelup_show.rs index 9e032edbd..08741b68a 100644 --- a/src/ops/fuelup_show.rs +++ b/src/ops/fuelup_show.rs @@ -1,11 +1,4 @@ -use anyhow::{bail, Result}; -use component::{self, Components}; -use semver::Version; -use std::collections::HashMap; -use std::path::Path; -use std::str::FromStr; -use tracing::info; - +use crate::file::get_bin_version; use crate::fmt::bold; use crate::store::Store; use crate::{ @@ -16,33 +9,12 @@ use crate::{ toolchain::{DistToolchainDescription, Toolchain}, toolchain_override::ToolchainOverride, }; - -fn exec_version(component_executable: &Path) -> Result { - match std::process::Command::new(component_executable) - .arg("--version") - .output() - { - Ok(o) => { - let output = String::from_utf8_lossy(&o.stdout).into_owned(); - match output.split_whitespace().last() { - Some(v) => { - let version = Version::parse(v)?; - Ok(version) - } - None => { - bail!("Error getting version string"); - } - } - } - Err(e) => { - if component_executable.exists() { - bail!("execution error - {}", e); - } else { - bail!("not found"); - } - } - } -} +use anyhow::Result; +use component::{self, Components}; +use semver::Version; +use std::collections::HashMap; +use std::str::FromStr; +use tracing::info; pub fn show() -> Result<()> { info!("{}: {}", bold("Default host"), TargetTriple::from_host()?); @@ -102,12 +74,12 @@ pub fn show() -> Result<()> { let mut version_map: HashMap = HashMap::new(); for component in Components::collect_exclude_plugins()? { let component_executable = active_toolchain.bin_path.join(&component.name); - let version_text: String = match exec_version(component_executable.as_path()) { + let version_text: String = match get_bin_version(component_executable.as_path()) { Ok(version) => { version_map.insert(component.name.clone(), version.clone()); format!("{}", version) } - Err(e) => format!("{}", e), + Err(e) => e.to_string(), }; info!("{:>2}{} : {}", "", bold(&component.name), version_text); @@ -119,26 +91,23 @@ pub fn show() -> Result<()> { for executable in plugin.executables.iter() { let plugin_executable = active_toolchain.bin_path.join(executable); - let version_text = match exec_version(plugin_executable.as_path()) { + let version_text = match get_bin_version(plugin_executable.as_path()) { Ok(version) => { version_map.insert(executable.clone(), version.clone()); - format!("{}", version) } - Err(e) => { - format!("{}", e) - } + Err(e) => e.to_string(), }; info!("{:>6}- {} : {}", "", bold(executable), version_text); } } else { let plugin_executable = active_toolchain.bin_path.join(&plugin.name); - let version_text = match exec_version(plugin_executable.as_path()) { + let version_text = match get_bin_version(plugin_executable.as_path()) { Ok(version) => { version_map.insert(plugin.name.clone(), version.clone()); format!("{}", version) } - Err(e) => format!("{}", e), + Err(e) => e.to_string(), }; info!("{:>4}- {} : {}", "", bold(&plugin.name), version_text); } diff --git a/src/path.rs b/src/path.rs index cf6302d2e..69330618f 100644 --- a/src/path.rs +++ b/src/path.rs @@ -19,6 +19,17 @@ pub fn fuelup_bin_dir() -> PathBuf { fuelup_dir().join("bin") } +/// Similar to fuelup_bin() but it never fails. If the fuelup binary is not found in $PATH, it +/// returns the current executable binary path +pub fn fuelup_bin_or_current_bin() -> PathBuf { + let fuelup_bin = fuelup_bin(); + if fuelup_bin.exists() { + fuelup_bin + } else { + env::current_exe().unwrap() + } +} + pub fn fuelup_bin() -> PathBuf { fuelup_bin_dir().join("fuelup") } diff --git a/src/toolchain.rs b/src/toolchain.rs index ab7dc182e..50f0ea432 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -2,9 +2,8 @@ use crate::channel::{self, Channel}; use crate::constants::DATE_FORMAT; use crate::download::DownloadCfg; use crate::file::{hard_or_symlink_file, is_executable}; -use crate::ops::fuelup_self::self_update; use crate::path::{ - ensure_dir_exists, fuelup_bin, fuelup_bin_dir, fuelup_tmp_dir, settings_file, + ensure_dir_exists, fuelup_bin_dir, fuelup_bin_or_current_bin, fuelup_tmp_dir, settings_file, toolchain_bin_dir, toolchain_dir, }; use crate::settings::SettingsFile; @@ -305,15 +304,7 @@ impl Toolchain { let fuelup_bin_dir = fuelup_bin_dir(); ensure_dir_exists(&fuelup_bin_dir)?; - let fuelup_bin = fuelup_bin(); - if !fuelup_bin.is_file() { - info!("fuelup not found - attempting to self update"); - match self_update() { - Ok(()) => info!("fuelup installed."), - Err(e) => bail!("Could not install fuelup: {}", e), - }; - } - + let fuelup_bin = fuelup_bin_or_current_bin(); let store = Store::from_env()?; if !store.has_component(&download_cfg.name, &download_cfg.version) diff --git a/tests/self.rs b/tests/self.rs index e7a92c8b4..d0a49f7d4 100644 --- a/tests/self.rs +++ b/tests/self.rs @@ -5,12 +5,22 @@ use testcfg::FuelupState; #[test] fn fuelup_self_update() -> Result<()> { - testcfg::setup(FuelupState::LatestToolchainInstalled, &|cfg| { - let output = cfg.fuelup(&["self", "update"]); - + testcfg::setup(FuelupState::NightlyDateInstalled, &|cfg| { + let output = cfg.fuelup(&["self", "update", "--force"]); let expected_stdout_starts_with = "Fetching binary from"; assert!(output.stdout.starts_with(expected_stdout_starts_with)); })?; Ok(()) } + +#[test] +fn fuelup_self_update_latest() -> Result<()> { + testcfg::setup(FuelupState::LatestToolchainInstalled, &|cfg| { + let output = cfg.fuelup(&["self", "update"]); + let expected_stdout_starts_with = "Already up to date"; + assert!(output.stdout.contains(expected_stdout_starts_with)); + })?; + + Ok(()) +} diff --git a/tests/testcfg/mod.rs b/tests/testcfg/mod.rs index 81ef187f7..e69367fad 100644 --- a/tests/testcfg/mod.rs +++ b/tests/testcfg/mod.rs @@ -36,19 +36,23 @@ pub enum FuelupState { NightlyAndNightlyDateInstalled, /// Inits a state with only the `beta-1` toolchain. Beta1Installed, - /// Inits a state with the `latest` toolchain, with `beta-1` declared within fuel-toolchain.toml. + /// Inits a state with the `latest` toolchain, with `beta-1` declared within + /// fuel-toolchain.toml. LatestWithBetaOverride, } #[derive(Debug)] pub struct TestCfg { - /// The path to the test environment's fuelup executable. This should usually be /.fuelup/bin/fuelup. - /// This should be used to execute fuelup in the test environment. + /// The path to the test environment's fuelup executable. This should usually be + /// /.fuelup/bin/fuelup. This should be used to execute fuelup in the test + /// environment. pub fuelup_path: PathBuf, - /// The path to the test environment's fuelup/bin directory. This should usually be /.fuelup/bin/. - /// This should be used to execute other binaries (eg. forc) in the test environment. + /// The path to the test environment's fuelup/bin directory. This should usually be + /// /.fuelup/bin/. This should be used to execute other binaries (eg. forc) in the + /// test environment. pub fuelup_bin_dirpath: PathBuf, - /// The path to the test environment's home. This should usually be a created tempfile::tempdir::TempDir. + /// The path to the test environment's home. This should usually be a created + /// tempfile::tempdir::TempDir. pub home: PathBuf, }