diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 1e12ffa020..2969f70066 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -11,7 +11,10 @@ use regex::Regex; use serde::{Deserialize, Serialize}; use thiserror::Error as ThisError; -use crate::{currentprocess::Process, errors::RustupError, toolchain::ToolchainName, utils::utils}; +use crate::{ + config::Cfg, currentprocess::Process, errors::RustupError, toolchain::ToolchainName, + utils::utils, +}; pub mod component; pub(crate) mod config; @@ -707,80 +710,58 @@ impl fmt::Display for Profile { } } +#[derive(Clone)] +pub(crate) struct DistOptions<'a> { + pub(crate) cfg: &'a Cfg<'a>, + pub(crate) toolchain: &'a ToolchainDesc, + pub(crate) profile: Profile, + pub(crate) update_hash: Option<&'a Path>, + pub(crate) dl_cfg: DownloadCfg<'a>, + /// --force bool is whether to force an update/install + pub(crate) force: bool, + /// --allow-downgrade + pub(crate) allow_downgrade: bool, + /// toolchain already exists + pub(crate) exists: bool, + /// currently installed date and version + pub(crate) old_date_version: Option<(String, String)>, + /// Extra components to install from dist + pub(crate) components: &'a [&'a str], + /// Extra targets to install from dist + pub(crate) targets: &'a [&'a str], +} + // Installs or updates a toolchain from a dist server. If an initial // install then it will be installed with the default components. If // an upgrade then all the existing components will be upgraded. // // Returns the manifest's hash if anything changed. -#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all, fields(profile=format!("{profile:?}"), prefix=prefix.path().to_string_lossy().to_string())))] +#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all, fields(profile=format!("{:?}", opts.profile), prefix=prefix.path().to_string_lossy().to_string())))] pub(crate) async fn update_from_dist( - download: DownloadCfg<'_>, - update_hash: Option<&Path>, - toolchain: &ToolchainDesc, - profile: Option, prefix: &InstallPrefix, - force_update: bool, - allow_downgrade: bool, - old_date: Option<&str>, - components: &[&str], - targets: &[&str], + opts: &DistOptions<'_>, ) -> Result> { let fresh_install = !prefix.path().exists(); - let hash_exists = update_hash.map(Path::exists).unwrap_or(false); - // fresh_install means the toolchain isn't present, but hash_exists means there is a stray hash file - if fresh_install && hash_exists { - // It's ok to unwrap, because hash have to exist at this point - (download.notify_handler)(Notification::StrayHash(update_hash.unwrap())); - std::fs::remove_file(update_hash.unwrap())?; - } - - let res = update_from_dist_( - download, - update_hash, - toolchain, - profile, - prefix, - force_update, - allow_downgrade, - old_date, - components, - targets, - ) - .await; - - // Don't leave behind an empty / broken installation directory - if res.is_err() && fresh_install { - // FIXME Ignoring cascading errors - let _ = utils::remove_dir("toolchain", prefix.path(), download.notify_handler); + if let Some(hash) = opts.update_hash { + // fresh_install means the toolchain isn't present, but hash_exists means there is a stray hash file + if fresh_install && Path::exists(hash) { + (opts.dl_cfg.notify_handler)(Notification::StrayHash(hash)); + std::fs::remove_file(hash)?; + } } - res -} - -async fn update_from_dist_( - download: DownloadCfg<'_>, - update_hash: Option<&Path>, - toolchain: &ToolchainDesc, - profile: Option, - prefix: &InstallPrefix, - force_update: bool, - allow_downgrade: bool, - old_date: Option<&str>, - components: &[&str], - targets: &[&str], -) -> Result> { - let mut toolchain = toolchain.clone(); let mut fetched = String::new(); let mut first_err = None; - let backtrack = toolchain.channel == "nightly" && toolchain.date.is_none(); + let backtrack = opts.toolchain.channel == "nightly" && opts.toolchain.date.is_none(); // We want to limit backtracking if we do not already have a toolchain - let mut backtrack_limit: Option = if toolchain.date.is_some() { + let mut backtrack_limit: Option = if opts.toolchain.date.is_some() { None } else { // We limit the backtracking to 21 days by default (half a release cycle). // The limit of 21 days is an arbitrary selection, so we let the user override it. const BACKTRACK_LIMIT_DEFAULT: i32 = 21; - let provided = download + let provided = opts + .dl_cfg .process .var("RUSTUP_BACKTRACK_LIMIT") .ok() @@ -799,103 +780,116 @@ async fn update_from_dist_( // We could arguably use the date of the first rustup release here, but that would break a // bunch of the tests, which (inexplicably) use 2015-01-01 as their manifest dates. let first_manifest = date_from_manifest_date("2014-12-20").unwrap(); - let old_manifest = old_date - .and_then(date_from_manifest_date) + let old_manifest = opts + .old_date_version + .as_ref() + .and_then(|(d, _)| date_from_manifest_date(d)) .unwrap_or(first_manifest); - let last_manifest = if allow_downgrade { + let last_manifest = if opts.allow_downgrade { first_manifest } else { old_manifest }; let current_manifest = { - let manifestation = Manifestation::open(prefix.clone(), toolchain.target.clone())?; + let manifestation = Manifestation::open(prefix.clone(), opts.toolchain.target.clone())?; manifestation.load_manifest()? }; - loop { - match try_update_from_dist_( - download, - update_hash, + let mut toolchain = opts.toolchain.clone(); + let res = loop { + let result = try_update_from_dist_( + opts.dl_cfg, + opts.update_hash, &toolchain, - profile, + match opts.exists { + false => Some(opts.profile), + true => None, + }, prefix, - force_update, - components, - targets, + opts.force, + opts.components, + opts.targets, &mut fetched, ) - .await - { - Ok(v) => break Ok(v), - Err(e) => { - if !backtrack { - break Err(e); - } - - let cause = e.downcast_ref::(); - match cause { - Some(DistError::ToolchainComponentsMissing(components, manifest, ..)) => { - (download.notify_handler)(Notification::SkippingNightlyMissingComponent( - &toolchain, - current_manifest.as_ref().unwrap_or(manifest), - components, - )); - - if first_err.is_none() { - first_err = Some(e); - } - // We decrement the backtrack count only on unavailable component errors - // so that the limit only applies to nightlies that were indeed available, - // and ignores missing ones. - backtrack_limit = backtrack_limit.map(|n| n - 1); - } + .await; - Some(DistError::MissingReleaseForToolchain(..)) => { - // no need to even print anything for missing nightlies, - // since we don't really "skip" them - } - _ => { - // All other errors break the loop - break Err(e); - } - }; + let e = match result { + Ok(v) => break Ok(v), + Err(e) if !backtrack => break Err(e), + Err(e) => e, + }; - if let Some(backtrack_limit) = backtrack_limit { - if backtrack_limit < 1 { - // This unwrap is safe because we can only hit this if we've - // had a chance to set first_err - break Err(first_err.unwrap()); - } + let cause = e.downcast_ref::(); + match cause { + Some(DistError::ToolchainComponentsMissing(components, manifest, ..)) => { + (opts.dl_cfg.notify_handler)(Notification::SkippingNightlyMissingComponent( + &toolchain, + current_manifest.as_ref().unwrap_or(manifest), + components, + )); + + if first_err.is_none() { + first_err = Some(e); } + // We decrement the backtrack count only on unavailable component errors + // so that the limit only applies to nightlies that were indeed available, + // and ignores missing ones. + backtrack_limit = backtrack_limit.map(|n| n - 1); + } - // The user asked to update their nightly, but the latest nightly does not have all - // the components that the user currently has installed. Let's try the previous - // nightlies in reverse chronological order until we find a nightly that does, - // starting at one date earlier than the current manifest's date. - let toolchain_date = toolchain.date.as_ref().unwrap_or(&fetched); - let try_next = date_from_manifest_date(toolchain_date) - .unwrap_or_else(|| panic!("Malformed manifest date: {toolchain_date:?}")) - .pred_opt() - .unwrap(); - - if try_next < last_manifest { - // Wouldn't be an update if we go further back than the user's current nightly. - if let Some(e) = first_err { - break Err(e); - } else { - // In this case, all newer nightlies are missing, which means there are no - // updates, so the user is already at the latest nightly. - break Ok(None); - } - } + Some(DistError::MissingReleaseForToolchain(..)) => { + // no need to even print anything for missing nightlies, + // since we don't really "skip" them + } + _ => { + // All other errors break the loop + break Err(e); + } + }; - toolchain.date = Some(try_next.format("%Y-%m-%d").to_string()); + if let Some(backtrack_limit) = backtrack_limit { + if backtrack_limit < 1 { + // This unwrap is safe because we can only hit this if we've + // had a chance to set first_err + break Err(first_err.unwrap()); + } + } + + // The user asked to update their nightly, but the latest nightly does not have all + // the components that the user currently has installed. Let's try the previous + // nightlies in reverse chronological order until we find a nightly that does, + // starting at one date earlier than the current manifest's date. + let toolchain_date = toolchain.date.as_ref().unwrap_or(&fetched); + let try_next = date_from_manifest_date(toolchain_date) + .unwrap_or_else(|| panic!("Malformed manifest date: {toolchain_date:?}")) + .pred_opt() + .unwrap(); + + if try_next < last_manifest { + // Wouldn't be an update if we go further back than the user's current nightly. + if let Some(e) = first_err { + break Err(e); + } else { + // In this case, all newer nightlies are missing, which means there are no + // updates, so the user is already at the latest nightly. + break Ok(None); } } + + toolchain.date = Some(try_next.format("%Y-%m-%d").to_string()); + }; + + // Don't leave behind an empty / broken installation directory + if res.is_err() && fresh_install { + // FIXME Ignoring cascading errors + let _ = utils::remove_dir("toolchain", prefix.path(), opts.dl_cfg.notify_handler); } + + res } +#[allow(clippy::too_many_arguments)] async fn try_update_from_dist_( download: DownloadCfg<'_>, update_hash: Option<&Path>, @@ -1008,24 +1002,14 @@ async fn try_update_from_dist_( }; } Ok(None) => return Ok(None), - Err(any) => { - enum Cases { - DNE, - CF, - Other, - } - let case = match any.downcast_ref::() { - Some(RustupError::ChecksumFailed { .. }) => Cases::CF, - Some(RustupError::DownloadNotExists { .. }) => Cases::DNE, - _ => Cases::Other, - }; - match case { - Cases::CF => return Ok(None), - Cases::DNE => { + Err(err) => { + match err.downcast_ref::() { + Some(RustupError::ChecksumFailed { .. }) => return Ok(None), + Some(RustupError::DownloadNotExists { .. }) => { // Proceed to try v1 as a fallback - (download.notify_handler)(Notification::DownloadingLegacyManifest); + (download.notify_handler)(Notification::DownloadingLegacyManifest) } - Cases::Other => return Err(any), + _ => return Err(err), } } } @@ -1033,35 +1017,24 @@ async fn try_update_from_dist_( // If the v2 manifest is not found then try v1 let manifest = match dl_v1_manifest(download, toolchain).await { Ok(m) => m, - Err(any) => { - enum Cases { - DNE, - CF, - Other, + Err(err) => match err.downcast_ref::() { + Some(RustupError::ChecksumFailed { .. }) => return Err(err), + Some(RustupError::DownloadNotExists { .. }) => { + bail!(DistError::MissingReleaseForToolchain( + toolchain.manifest_name() + )); } - let case = match any.downcast_ref::() { - Some(RustupError::ChecksumFailed { .. }) => Cases::CF, - Some(RustupError::DownloadNotExists { .. }) => Cases::DNE, - _ => Cases::Other, - }; - match case { - Cases::DNE => { - bail!(DistError::MissingReleaseForToolchain( + _ => { + return Err(err).with_context(|| { + format!( + "failed to download manifest for '{}'", toolchain.manifest_name() - )); - } - Cases::CF => return Err(any), - Cases::Other => { - return Err(any).with_context(|| { - format!( - "failed to download manifest for '{}'", - toolchain.manifest_name() - ) - }); - } + ) + }); } - } + }, }; + let result = manifestation .update_v1( &manifest, @@ -1071,23 +1044,17 @@ async fn try_update_from_dist_( download.process, ) .await; + // inspect, determine what context to add, then process afterwards. - let mut download_not_exists = false; - match &result { - Ok(_) => (), - Err(e) => { - if let Some(RustupError::DownloadNotExists { .. }) = e.downcast_ref::() { - download_not_exists = true - } + if let Err(e) = &result { + if let Some(RustupError::DownloadNotExists { .. }) = e.downcast_ref::() { + return result.with_context(|| { + format!("could not download nonexistent rust version `{toolchain_str}`") + }); } } - if download_not_exists { - result.with_context(|| { - format!("could not download nonexistent rust version `{toolchain_str}`") - }) - } else { - result - } + + result } pub(crate) async fn dl_v2_manifest( diff --git a/src/install.rs b/src/install.rs index 7c400f1903..f968b4da37 100644 --- a/src/install.rs +++ b/src/install.rs @@ -6,7 +6,7 @@ use anyhow::Result; use crate::{ config::Cfg, - dist::{self, download::DownloadCfg, prefix::InstallPrefix, Notification}, + dist::{self, prefix::InstallPrefix, DistOptions, Notification}, errors::RustupError, notifications::Notification as RootNotification, toolchain::{CustomToolchainName, LocalToolchainName, Toolchain}, @@ -32,25 +32,7 @@ pub(crate) enum InstallMethod<'a> { dest: &'a CustomToolchainName, cfg: &'a Cfg<'a>, }, - Dist { - cfg: &'a Cfg<'a>, - desc: &'a dist::ToolchainDesc, - profile: dist::Profile, - update_hash: Option<&'a Path>, - dl_cfg: DownloadCfg<'a>, - /// --force bool is whether to force an update/install - force: bool, - /// --allow-downgrade - allow_downgrade: bool, - /// toolchain already exists - exists: bool, - /// currently installed date and version - old_date_version: Option<(String, String)>, - /// Extra components to install from dist - components: &'a [&'a str], - /// Extra targets to install from dist - targets: &'a [&'a str], - }, + Dist(DistOptions<'a>), } impl<'a> InstallMethod<'a> { @@ -61,10 +43,10 @@ impl<'a> InstallMethod<'a> { match self { InstallMethod::Copy { .. } | InstallMethod::Link { .. } - | InstallMethod::Dist { + | InstallMethod::Dist(DistOptions { old_date_version: None, .. - } => (nh)(RootNotification::InstallingToolchain(&self.dest_basename())), + }) => (nh)(RootNotification::InstallingToolchain(&self.dest_basename())), _ => (nh)(RootNotification::UpdatingToolchain(&self.dest_basename())), } @@ -83,10 +65,10 @@ impl<'a> InstallMethod<'a> { true => { (nh)(RootNotification::InstalledToolchain(&self.dest_basename())); match self { - InstallMethod::Dist { + InstallMethod::Dist(DistOptions { old_date_version: Some((_, v)), .. - } => UpdateStatus::Updated(v.clone()), + }) => UpdateStatus::Updated(v.clone()), InstallMethod::Copy { .. } | InstallMethod::Link { .. } | InstallMethod::Dist { .. } => UpdateStatus::Installed, @@ -121,36 +103,12 @@ impl<'a> InstallMethod<'a> { utils::symlink_dir(src, path, notify_handler)?; Ok(true) } - InstallMethod::Dist { - desc, - profile, - update_hash, - dl_cfg, - force: force_update, - allow_downgrade, - exists, - old_date_version, - components, - targets, - .. - } => { + InstallMethod::Dist(opts) => { let prefix = &InstallPrefix::from(path.to_owned()); - let maybe_new_hash = dist::update_from_dist( - *dl_cfg, - update_hash.as_deref(), - desc, - if *exists { None } else { Some(*profile) }, - prefix, - *force_update, - *allow_downgrade, - old_date_version.as_ref().map(|dv| dv.0.as_str()), - components, - targets, - ) - .await?; + let maybe_new_hash = dist::update_from_dist(prefix, opts).await?; if let Some(hash) = maybe_new_hash { - if let Some(hash_file) = update_hash { + if let Some(hash_file) = opts.update_hash { utils::write_file("update hash", hash_file, &hash)?; } @@ -166,7 +124,7 @@ impl<'a> InstallMethod<'a> { match self { InstallMethod::Copy { cfg, .. } => cfg, InstallMethod::Link { cfg, .. } => cfg, - InstallMethod::Dist { cfg, .. } => cfg, + InstallMethod::Dist(DistOptions { cfg, .. }) => cfg, } } @@ -174,7 +132,9 @@ impl<'a> InstallMethod<'a> { match self { InstallMethod::Copy { dest, .. } => (*dest).into(), InstallMethod::Link { dest, .. } => (*dest).into(), - InstallMethod::Dist { desc, .. } => (*desc).into(), + InstallMethod::Dist(DistOptions { + toolchain: desc, .. + }) => (*desc).into(), } } @@ -186,7 +146,11 @@ impl<'a> InstallMethod<'a> { match self { InstallMethod::Copy { cfg, dest, .. } => cfg.toolchain_path(&(*dest).into()), InstallMethod::Link { cfg, dest, .. } => cfg.toolchain_path(&(*dest).into()), - InstallMethod::Dist { cfg, desc, .. } => cfg.toolchain_path(&(*desc).into()), + InstallMethod::Dist(DistOptions { + cfg, + toolchain: desc, + .. + }) => cfg.toolchain_path(&(*desc).into()), } } } diff --git a/src/lib.rs b/src/lib.rs index c28048cc21..ad507a930c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,10 +1,6 @@ #![deny(rust_2018_idioms)] #![allow( - clippy::too_many_arguments, clippy::type_complexity, - clippy::upper_case_acronyms, // see https://github.com/rust-lang/rust-clippy/issues/6974 - clippy::vec_init_then_push, // uses two different styles of initialization - clippy::box_default, // its ugly and outside of inner loops irrelevant clippy::result_large_err, // 288 bytes is our 'large' variant today, which is unlikely to be a performance problem clippy::arc_with_non_send_sync, // will get resolved as we move further into async )] diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 83d871e00c..618bb67b71 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -12,7 +12,7 @@ use crate::{ manifest::{Component, ComponentStatus, Manifest}, manifestation::{Changes, Manifestation}, prefix::InstallPrefix, - PartialToolchainDesc, Profile, ToolchainDesc, + DistOptions, PartialToolchainDesc, Profile, ToolchainDesc, }, install::{InstallMethod, UpdateStatus}, notifications::Notification, @@ -327,18 +327,18 @@ impl<'a> DistributableToolchain<'a> { #[cfg_attr(feature = "otel", tracing::instrument(err, skip_all))] pub(crate) async fn install( cfg: &'a Cfg<'a>, - desc: &ToolchainDesc, + toolchain: &ToolchainDesc, components: &[&str], targets: &[&str], profile: Profile, force: bool, ) -> anyhow::Result<(UpdateStatus, DistributableToolchain<'a>)> { - let hash_path = cfg.get_hash_file(desc, true)?; + let hash_path = cfg.get_hash_file(toolchain, true)?; let update_hash = Some(&hash_path as &Path); - let status = InstallMethod::Dist { + let status = InstallMethod::Dist(DistOptions { cfg, - desc, + toolchain, profile, update_hash, dl_cfg: cfg.download_cfg(&|n| (cfg.notify_handler)(n.into())), @@ -348,10 +348,10 @@ impl<'a> DistributableToolchain<'a> { old_date_version: None, components, targets, - } + }) .install() .await?; - Ok((status, Self::new(cfg, desc.clone())?)) + Ok((status, Self::new(cfg, toolchain.clone())?)) } #[cfg_attr(feature = "otel", tracing::instrument(err, skip_all))] @@ -412,9 +412,9 @@ impl<'a> DistributableToolchain<'a> { let hash_path = cfg.get_hash_file(&self.desc, true)?; let update_hash = Some(&hash_path as &Path); - InstallMethod::Dist { + InstallMethod::Dist(DistOptions { cfg, - desc: &self.desc, + toolchain: &self.desc, profile, update_hash, dl_cfg: cfg.download_cfg(&|n| (cfg.notify_handler)(n.into())), @@ -424,7 +424,7 @@ impl<'a> DistributableToolchain<'a> { old_date_version, components, targets, - } + }) .install() .await }