From 5c9f63fb38d5e8c6cd356619d4b86346eb077cd3 Mon Sep 17 00:00:00 2001 From: James Elford Date: Sun, 19 Feb 2017 13:44:21 +0000 Subject: [PATCH] Store downloaded files in a persistent directory until installation This PR should go some way to addressing #889 by caching downloaded components in a persistent directory until they have been installed. This way, if the download/install process is interrupted, the file that made it can be re-used. This should help ease the pain of intermittent connections --- src/rustup-dist/src/dist.rs | 89 +++++++++++++++++++++++++++- src/rustup-dist/src/lib.rs | 1 + src/rustup-dist/src/manifestation.rs | 38 ++++-------- src/rustup-dist/src/notifications.rs | 8 ++- src/rustup/config.rs | 3 + src/rustup/toolchain.rs | 5 +- 6 files changed, 113 insertions(+), 31 deletions(-) diff --git a/src/rustup-dist/src/dist.rs b/src/rustup-dist/src/dist.rs index c8f811e5cce..0827715a808 100644 --- a/src/rustup-dist/src/dist.rs +++ b/src/rustup-dist/src/dist.rs @@ -8,9 +8,12 @@ use manifest::Component; use manifest::Manifest as ManifestV2; use manifestation::{Manifestation, UpdateStatus, Changes}; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::ops; +use url::Url; use std::fmt; use std::env; +use std::fs; use regex::Regex; use sha2::{Sha256, Digest}; @@ -482,9 +485,91 @@ pub fn download_and_check<'a>(url_str: &str, pub struct DownloadCfg<'a> { pub dist_root: &'a str, pub temp_cfg: &'a temp::Cfg, + pub download_dir: &'a PathBuf, pub notify_handler: &'a Fn(Notification), } + +pub struct File { + path: PathBuf, +} + +impl ops::Deref for File { + type Target = Path; + + fn deref(&self) -> &Path { + ops::Deref::deref(&self.path) + } +} + +impl<'a> DownloadCfg<'a> { + + fn notify_handler(&self, event: Notification) { + (self.notify_handler)(event); + } + + pub fn download(&self, url: &Url, hash: &str) -> Result { + + try!(utils::ensure_dir_exists("Download Directory", &self.download_dir, &|n| self.notify_handler(n.into()))); + let target_file = self.download_dir.join(Path::new(hash)); + + if target_file.exists() { + let mut hasher = Sha256::new(); + use std::io::Read; + let mut downloaded = try!(fs::File::open(&target_file).chain_err(|| "opening already downloaded file")); + let mut buf = [0; 1024]; + loop { + if let Ok(n) = downloaded.read(&mut buf) { + if n == 0 { break; } + hasher.input(&buf[..n]); + } else { + break; + } + } + let cached_result = hasher.result_str(); + if hash == cached_result { + self.notify_handler(Notification::FileAlreadyDownloaded); + self.notify_handler(Notification::ChecksumValid(&url.to_string())); + return Ok(File { path: target_file, }); + } else { + self.notify_handler(Notification::CachedFileChecksumFailed); + try!(fs::remove_file(&target_file).chain_err(|| "cleaning up previous download")); + } + } + + let mut hasher = Sha256::new(); + + try!(utils::download_file(&url, + &target_file, + Some(&mut hasher), + &|n| self.notify_handler(n.into()))); + + let actual_hash = hasher.result_str(); + + if hash != actual_hash { + // Incorrect hash + return Err(ErrorKind::ChecksumFailed { + url: url.to_string(), + expected: hash.to_string(), + calculated: actual_hash, + }.into()); + } else { + self.notify_handler(Notification::ChecksumValid(&url.to_string())); + return Ok(File { path: target_file, }) + } + } + + pub fn clean(&self, hashes: &Vec) -> Result<()> { + for hash in hashes.iter() { + let used_file = self.download_dir.join(hash); + if self.download_dir.join(&used_file).exists() { + try!(fs::remove_file(used_file).chain_err(|| "cleaning up cached downloads")); + } + } + Ok(()) + } +} + pub fn download_hash(url: &str, cfg: DownloadCfg) -> Result { let hash_url = try!(utils::parse_url(&(url.to_owned() + ".sha256"))); let hash_file = try!(cfg.temp_cfg.new_file()); @@ -551,7 +636,7 @@ pub fn update_from_dist_<'a>(download: DownloadCfg<'a>, Ok(Some((m, hash))) => { return match try!(manifestation.update(&m, changes, - &download.temp_cfg, + &download, download.notify_handler.clone())) { UpdateStatus::Unchanged => Ok(None), UpdateStatus::Changed => Ok(Some(hash)), diff --git a/src/rustup-dist/src/lib.rs b/src/rustup-dist/src/lib.rs index d87cd868aa8..694bd6c8fb4 100644 --- a/src/rustup-dist/src/lib.rs +++ b/src/rustup-dist/src/lib.rs @@ -7,6 +7,7 @@ extern crate walkdir; extern crate toml; extern crate flate2; extern crate tar; +extern crate url; #[macro_use] extern crate rustup_utils; #[macro_use] diff --git a/src/rustup-dist/src/manifestation.rs b/src/rustup-dist/src/manifestation.rs index c40677d33d9..12f45f16477 100644 --- a/src/rustup-dist/src/manifestation.rs +++ b/src/rustup-dist/src/manifestation.rs @@ -3,14 +3,13 @@ use config::Config; use manifest::{Component, Manifest, TargetedPackage}; -use dist::{download_and_check, DownloadCfg, TargetTriple, DEFAULT_DIST_SERVER}; +use dist::{download_and_check, DownloadCfg, TargetTriple, DEFAULT_DIST_SERVER, File}; use component::{Components, Transaction, TarGzPackage, Package}; use temp; use errors::*; use notifications::*; use rustup_utils::utils; use prefix::InstallPrefix; -use sha2::{Sha256, Digest}; use std::path::Path; pub const DIST_MANIFEST: &'static str = "multirust-channel-manifest.toml"; @@ -73,10 +72,11 @@ impl Manifestation { pub fn update(&self, new_manifest: &Manifest, changes: Changes, - temp_cfg: &temp::Cfg, + download_cfg: &DownloadCfg, notify_handler: &Fn(Notification)) -> Result { // Some vars we're going to need a few times + let temp_cfg = download_cfg.temp_cfg; let prefix = self.installation.prefix(); let ref rel_installed_manifest_path = prefix.rel_manifest_file(DIST_MANIFEST); let ref installed_manifest_path = prefix.path().join(rel_installed_manifest_path); @@ -125,7 +125,8 @@ impl Manifestation { let altered = temp_cfg.dist_server != DEFAULT_DIST_SERVER; // Download component packages and validate hashes - let mut things_to_install: Vec<(Component, temp::File)> = Vec::new(); + let mut things_to_install: Vec<(Component, File)> = Vec::new(); + let mut things_downloaded: Vec = Vec::new(); for (component, url, hash) in components_urls_and_hashes { notify_handler(Notification::DownloadingComponent(&component.pkg, @@ -137,32 +138,14 @@ impl Manifestation { url }; - // Download each package to temp file - let temp_file = try!(temp_cfg.new_file()); let url_url = try!(utils::parse_url(&url)); - let mut hasher = Sha256::new(); - try!(utils::download_file(&url_url, - &temp_file, - Some(&mut hasher), - &|n| notify_handler(n.into())).chain_err(|| { + let dowloaded_file = try!(download_cfg.download(&url_url, &hash).chain_err(|| { ErrorKind::ComponentDownloadFailed(component.clone()) })); + things_downloaded.push(hash); - let actual_hash = hasher.result_str(); - - if hash != actual_hash { - // Incorrect hash - return Err(ErrorKind::ChecksumFailed { - url: url, - expected: hash, - calculated: actual_hash, - }.into()); - } else { - notify_handler(Notification::ChecksumValid(&url)); - } - - things_to_install.push((component, temp_file)); + things_to_install.push((component, dowloaded_file)); } // Begin transaction @@ -226,6 +209,8 @@ impl Manifestation { // End transaction tx.commit(); + try!(download_cfg.clean(&things_downloaded)); + Ok(UpdateStatus::Changed) } @@ -315,8 +300,11 @@ impl Manifestation { &self.target_triple, Some(&self.target_triple))); + use std::path::PathBuf; + let dld_dir = PathBuf::from("bogus"); let dlcfg = DownloadCfg { dist_root: "bogus", + download_dir: &dld_dir, temp_cfg: temp_cfg, notify_handler: notify_handler }; diff --git a/src/rustup-dist/src/notifications.rs b/src/rustup-dist/src/notifications.rs index 3f4d9ec4a02..44580597f2e 100644 --- a/src/rustup-dist/src/notifications.rs +++ b/src/rustup-dist/src/notifications.rs @@ -18,6 +18,8 @@ pub enum Notification<'a> { NoUpdateHash(&'a Path), ChecksumValid(&'a str), SignatureValid(&'a str), + FileAlreadyDownloaded, + CachedFileChecksumFailed, RollingBack, ExtensionNotInstalled(&'a Component), NonFatalError(&'a Error), @@ -48,6 +50,7 @@ impl<'a> Notification<'a> { Temp(ref n) => n.level(), Utils(ref n) => n.level(), ChecksumValid(_) | NoUpdateHash(_) | + FileAlreadyDownloaded | DownloadingLegacyManifest => NotificationLevel::Verbose, Extracting(_, _) | SignatureValid(_) | DownloadingComponent(_, _, _) | @@ -56,7 +59,7 @@ impl<'a> Notification<'a> { ManifestChecksumFailedHack | RollingBack | DownloadingManifest(_) => NotificationLevel::Info, CantReadUpdateHash(_) | ExtensionNotInstalled(_) | - MissingInstalledComponent(_) => NotificationLevel::Warn, + MissingInstalledComponent(_) | CachedFileChecksumFailed => NotificationLevel::Warn, NonFatalError(_) => NotificationLevel::Error, } } @@ -80,6 +83,8 @@ impl<'a> Display for Notification<'a> { NoUpdateHash(path) => write!(f, "no update hash at: '{}'", path.display()), ChecksumValid(_) => write!(f, "checksum passed"), SignatureValid(_) => write!(f, "signature valid"), + FileAlreadyDownloaded => write!(f, "reusing previously downloaded file"), + CachedFileChecksumFailed => write!(f, "bad checksum for cached download"), RollingBack => write!(f, "rolling back changes"), ExtensionNotInstalled(c) => { write!(f, "extension '{}' was not installed", c.name()) @@ -106,4 +111,3 @@ impl<'a> Display for Notification<'a> { } } } - diff --git a/src/rustup/config.rs b/src/rustup/config.rs index 15b95ec2dc9..1ccca4fa038 100644 --- a/src/rustup/config.rs +++ b/src/rustup/config.rs @@ -38,6 +38,7 @@ pub struct Cfg { pub settings_file: SettingsFile, pub toolchains_dir: PathBuf, pub update_hash_dir: PathBuf, + pub download_dir: PathBuf, pub temp_cfg: temp::Cfg, pub gpg_key: Cow<'static, str>, pub env_override: Option, @@ -60,6 +61,7 @@ impl Cfg { let toolchains_dir = multirust_dir.join("toolchains"); let update_hash_dir = multirust_dir.join("update-hashes"); + let download_dir = multirust_dir.join("downloads"); // GPG key let gpg_key = if let Some(path) = env::var_os("RUSTUP_GPG_KEY") @@ -103,6 +105,7 @@ impl Cfg { settings_file: settings_file, toolchains_dir: toolchains_dir, update_hash_dir: update_hash_dir, + download_dir: download_dir, temp_cfg: temp_cfg, gpg_key: gpg_key, notify_handler: notify_handler, diff --git a/src/rustup/toolchain.rs b/src/rustup/toolchain.rs index 7486ee16e29..51f25ba4a18 100644 --- a/src/rustup/toolchain.rs +++ b/src/rustup/toolchain.rs @@ -158,6 +158,7 @@ impl<'a> Toolchain<'a> { dist::DownloadCfg { dist_root: &self.cfg.dist_root_url, temp_cfg: &self.cfg.temp_cfg, + download_dir: &self.cfg.download_dir, notify_handler: &*self.dist_handler, } } @@ -582,7 +583,7 @@ impl<'a> Toolchain<'a> { try!(manifestation.update(&manifest, changes, - self.download_cfg().temp_cfg, + &self.download_cfg(), self.download_cfg().notify_handler.clone())); Ok(()) @@ -631,7 +632,7 @@ impl<'a> Toolchain<'a> { try!(manifestation.update(&manifest, changes, - self.download_cfg().temp_cfg, + &self.download_cfg(), self.download_cfg().notify_handler.clone())); Ok(())