From e5e3a151b002c46dbf5e0d9c8cfe8e41f90fa8c5 Mon Sep 17 00:00:00 2001 From: James Elford Date: Mon, 20 Mar 2017 09:44:20 +0000 Subject: [PATCH 1/8] Support resumption of partial downloads * Adds support only for the Curl backend, which is the default anyway. * In order to distinguish between a file that has been fully downloaded (but not used yet) and should therefore be hash-checked vs. those that require more data, store partials with a .partial extension. * Adds a simple http-server to rustup-mock, to allow the download module to be properly tested. It's not clear how to easily emulate a server that stops half-way without that. The tests for the overall download-resumption functionality should be fairly re-usable if we migrate to another download solution in the future (e.g. in #993) * Don't bother with resumption for meta-data files, since they're likely to go out of date anyway. --- Cargo.lock | 38 +++ src/download/Cargo.toml | 4 + src/download/src/errors.rs | 6 +- src/download/src/lib.rs | 103 ++++-- src/download/tests/download-curl-resume.rs | 87 +++++ src/rustup-dist/src/dist.rs | 19 +- src/rustup-dist/src/errors.rs | 3 +- src/rustup-mock/Cargo.toml | 2 +- src/rustup-mock/src/http_server.rs | 150 +++++++++ src/rustup-mock/src/lib.rs | 3 +- src/rustup-utils/src/utils.rs | 372 +++++++++++---------- 11 files changed, 567 insertions(+), 220 deletions(-) create mode 100644 src/download/tests/download-curl-resume.rs create mode 100644 src/rustup-mock/src/http_server.rs diff --git a/Cargo.lock b/Cargo.lock index e3c102e4e9..aedb935adb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -182,6 +182,8 @@ dependencies = [ "native-tls 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "openssl-probe 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustls 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "rustup-mock 1.0.0", + "tempdir 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -265,6 +267,25 @@ dependencies = [ "url 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "hyper" +version = "0.10.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "httparse 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "language-tags 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", + "mime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "num_cpus 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", + "rustc_version 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", + "time 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)", + "traitobject 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "typeable 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "url 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "idna" version = "0.1.0" @@ -401,6 +422,14 @@ dependencies = [ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "num_cpus" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "ole32-sys" version = "0.2.0" @@ -583,6 +612,7 @@ name = "rustup-mock" version = "1.0.0" dependencies = [ "flate2 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)", + "hyper 0.10.5 (registry+https://github.com/rust-lang/crates.io-index)", "itertools 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", "rustup-utils 1.0.0", @@ -811,6 +841,11 @@ name = "traitobject" version = "0.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "traitobject" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "typeable" version = "0.1.2" @@ -982,6 +1017,7 @@ dependencies = [ "checksum gdi32-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "0912515a8ff24ba900422ecda800b52f4016a56251922d397c576bf92c690518" "checksum hpack 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3d2da7d3a34cf6406d9d700111b8eafafe9a251de41ae71d8052748259343b58" "checksum httparse 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "46534074dbb80b070d60a5cb8ecadd8963a00a438ae1a95268850a7ef73b67ae" +"checksum hyper 0.10.5 (registry+https://github.com/rust-lang/crates.io-index)" = "43a15e3273b2133aaac0150478ab443fb89f15c3de41d8d93d8f3bb14bf560f6" "checksum hyper 0.9.10 (registry+https://github.com/rust-lang/crates.io-index)" = "eb27e8a3e8f17ac43ffa41bbda9cf5ad3f9f13ef66fa4873409d4902310275f7" "checksum idna 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1053236e00ce4f668aeca4a769a09b3bf5a682d802abd6f3cb39374f6b162c11" "checksum itertools 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)" = "c4a9b56eb56058f43dc66e58f40a214b2ccbc9f3df51861b63d51dec7b65bc3f" @@ -1001,6 +1037,7 @@ dependencies = [ "checksum miniz-sys 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "9d1f4d337a01c32e1f2122510fed46393d53ca35a7f429cb0450abaedfa3ed54" "checksum native-tls 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "aa4e52995154bb6f0b41e4379a279482c9387c1632e3798ba4e511ef8c54ee09" "checksum num_cpus 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)" = "cee7e88156f3f9e19bdd598f8d6c9db7bf4078f99f8381f43a55b09648d1a6e3" +"checksum num_cpus 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "a18c392466409c50b87369414a2680c93e739aedeb498eb2bff7d7eb569744e2" "checksum ole32-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5d2c49021782e5233cd243168edfa8037574afed4eba4bbaf538b3d8d1789d8c" "checksum openssl 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c368aec980860439ea434b98dd622b1e09f720c6762308854ef4b9e2e82771ad" "checksum openssl-probe 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "756d49c8424483a3df3b5d735112b4da22109ced9a8294f1f5cdf80fb3810919" @@ -1038,6 +1075,7 @@ dependencies = [ "checksum time 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)" = "3c7ec6d62a20df54e07ab3b78b9a3932972f4b7981de295563686849eb3989af" "checksum toml 0.1.30 (registry+https://github.com/rust-lang/crates.io-index)" = "0590d72182e50e879c4da3b11c6488dae18fccb1ae0c7a3eda18e16795844796" "checksum traitobject 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "07eaeb7689bb7fca7ce15628319635758eda769fed481ecfe6686ddef2600616" +"checksum traitobject 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "efd1f82c56340fdf16f2a953d7bda4f8fdffba13d93b00844c25572110b26079" "checksum typeable 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1410f6f91f21d1612654e7cc69193b0334f909dcf2c790c4826254fbb86f8887" "checksum unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "13a5906ca2b98c799f4b1ab4557b76367ebd6ae5ef14930ec841c74aed5f3764" "checksum unicode-bidi 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c1f7ceb96afdfeedee42bade65a0d585a6a0106f681b6749c8ff4daa8df30b3f" diff --git a/src/download/Cargo.toml b/src/download/Cargo.toml index 264c764a22..0e5b98873b 100644 --- a/src/download/Cargo.toml +++ b/src/download/Cargo.toml @@ -21,6 +21,10 @@ curl = { version = "0.4", optional = true } lazy_static = { version = "0.2", optional = true } native-tls = { version = "0.1", optional = true } +[dev-dependencies] +rustup-mock = { path = "../rustup-mock", version = "1.0.0" } +tempdir = "0.3.4" + [dependencies.hyper] version = "0.9.8" default-features = false diff --git a/src/download/src/errors.rs b/src/download/src/errors.rs index 45b5d0cc27..aa34577fb1 100644 --- a/src/download/src/errors.rs +++ b/src/download/src/errors.rs @@ -1,7 +1,11 @@ +use std::io; + error_chain! { links { } - foreign_links { } + foreign_links { + Io(::std::io::Error) #[cfg(unix)]; + } errors { HttpStatus(e: u32) { diff --git a/src/download/src/lib.rs b/src/download/src/lib.rs index 821ce5fc17..9771e5c82e 100644 --- a/src/download/src/lib.rs +++ b/src/download/src/lib.rs @@ -33,47 +33,86 @@ const BACKENDS: &'static [Backend] = &[ Backend::Rustls ]; -pub fn download(url: &Url, - callback: &Fn(Event) -> Result<()>) - -> Result<()> { - for &backend in BACKENDS { - match download_with_backend(backend, url, callback) { - Err(Error(ErrorKind::BackendUnavailable(_), _)) => (), - Err(e) => return Err(e), - Ok(()) => return Ok(()), - } - } - Err("no working backends".into()) -} - -pub fn download_with_backend(backend: Backend, +fn download_with_backend(backend: Backend, url: &Url, + resume_from: u64, callback: &Fn(Event) -> Result<()>) -> Result<()> { match backend { - Backend::Curl => curl::download(url, callback), + Backend::Curl => curl::download(url, resume_from, callback), Backend::Hyper => hyper::download(url, callback), Backend::Rustls => rustls::download(url, callback), } } +fn supports_partial_download(backend: &Backend) -> bool { + match backend { + &Backend::Curl => true, + _ => false + } +} + pub fn download_to_path_with_backend( backend: Backend, url: &Url, path: &Path, + resume_from_partial: bool, callback: Option<&Fn(Event) -> Result<()>>) -> Result<()> { use std::cell::RefCell; - use std::fs::{self, File}; - use std::io::Write; + use std::fs::{self, File, OpenOptions}; + use std::io::{Read, Write, Seek, SeekFrom}; || -> Result<()> { - let file = RefCell::new(try!(File::create(&path).chain_err( - || "error creating file for download"))); + let (file, resume_from) = if resume_from_partial && supports_partial_download(&backend) { + let mut possible_partial = OpenOptions::new() + .read(true) + .open(&path); + + let downloaded_so_far = if let Ok(mut partial) = possible_partial { + if let Some(cb) = callback { + println!("Reading file in {}", path.display()); + let mut buf = vec![0; 1024*1024*10]; + let mut downloaded_so_far = 0; + let mut number_of_reads = 0; + loop { + let n = try!(partial.read(&mut buf)); + downloaded_so_far += n as u64; + number_of_reads += 1; + if n == 0 { + println!("nothing read after {} reads (accumulated {})", number_of_reads, downloaded_so_far); + break; + } + try!(cb(Event::DownloadDataReceived(&buf[..n]))); + } + downloaded_so_far + } else { + use std::fs::Metadata; + let file_info = try!(partial.metadata()); + file_info.len() + } + } else { + 0 + }; + + let mut possible_partial = try!(OpenOptions::new().write(true).create(true).open(&path).chain_err(|| "error opening file for download")); + try!(possible_partial.seek(SeekFrom::End(0))); + + (possible_partial, downloaded_so_far) + } else { + println!("Download resume not supported"); + (try!(OpenOptions::new() + .write(true) + .create(true) + .open(&path) + .chain_err(|| "error creating file for download")), 0) + }; - try!(download_with_backend(backend, url, &|event| { + let file = RefCell::new(file); + + try!(download_with_backend(backend, url, resume_from, &|event| { if let Event::DownloadDataReceived(data) = event { try!(file.borrow_mut().write_all(data) .chain_err(|| "unable to write download to disk")); @@ -89,11 +128,8 @@ pub fn download_to_path_with_backend( Ok(()) }().map_err(|e| { - if path.is_file() { - // FIXME ignoring compound errors - let _ = fs::remove_file(path); - } + // TODO is there any point clearing up here? What kind of errors will leave us with an unusable partial? e }) } @@ -114,6 +150,7 @@ pub mod curl { use super::Event; pub fn download(url: &Url, + resume_from: u64, callback: &Fn(Event) -> Result<()> ) -> Result<()> { // Fetch either a cached libcurl handle (which will preserve open @@ -128,6 +165,12 @@ pub mod curl { try!(handle.url(&url.to_string()).chain_err(|| "failed to set url")); try!(handle.follow_location(true).chain_err(|| "failed to set follow redirects")); + if resume_from > 0 { + try!(handle.range(&(resume_from.to_string() + "-")).chain_err(|| "setting the range-header for download resumption")); + } else { + try!(handle.range("").chain_err(|| "clearing range header")); + } + // Take at most 30s to connect try!(handle.connect_timeout(Duration::new(30, 0)).chain_err(|| "failed to set connect timeout")); @@ -154,8 +197,8 @@ pub mod curl { if let Ok(data) = str::from_utf8(header) { let prefix = "Content-Length: "; if data.starts_with(prefix) { - if let Ok(s) = data[prefix.len()..].trim().parse() { - let msg = Event::DownloadContentLengthReceived(s); + if let Ok(s) = data[prefix.len()..].trim().parse::() { + let msg = Event::DownloadContentLengthReceived(s + resume_from); match callback(msg) { Ok(()) => (), Err(e) => { @@ -188,10 +231,11 @@ pub mod curl { })); } - // If we didn't get a 200 or 0 ("OK" for files) then return an error + // If we didn't get a 20x or 0 ("OK" for files) then return an error let code = try!(handle.response_code().chain_err(|| "failed to get response code")); - if code != 200 && code != 0 { - return Err(ErrorKind::HttpStatus(code).into()); + match code { + 0 | 200...299 => { println!("status code: {}", code)} + _ => { return Err(ErrorKind::HttpStatus(code).into()); } } Ok(()) @@ -639,6 +683,7 @@ pub mod curl { use super::Event; pub fn download(_url: &Url, + _resume_from: u64, _callback: &Fn(Event) -> Result<()> ) -> Result<()> { Err(ErrorKind::BackendUnavailable("curl").into()) diff --git a/src/download/tests/download-curl-resume.rs b/src/download/tests/download-curl-resume.rs new file mode 100644 index 0000000000..4e75ba1c9b --- /dev/null +++ b/src/download/tests/download-curl-resume.rs @@ -0,0 +1,87 @@ +#![cfg(feature = "curl-backend")] + +extern crate download; +extern crate rustup_mock; +extern crate tempdir; + +use std::fs::{self, File}; +use std::io::{Read}; +use std::path::Path; + +use tempdir::TempDir; + +use rustup_mock::http_server; + +use download::*; + +fn setup(test: &Fn(TempDir, http_server::Server) -> ()) { + let tmp = TempDir::new("rustup-download-test-").expect("creating tempdir for test"); + let served_dir = &tmp.path().join("test-files"); + fs::DirBuilder::new().create(served_dir).expect("setting up a folder to server files from"); + let server = http_server::Server::serve_from(served_dir).expect("setting up http server for test"); + test(tmp, server); +} + +fn file_contents(path: &Path) -> String { + let mut result = String::new(); + File::open(&path).unwrap().read_to_string(&mut result).expect("reading test result file"); + result +} + +#[test] +fn when_download_is_interrupted_partial_file_is_left_on_disk() { + setup(&|tmpdir, mut server| { + let target_path = tmpdir.path().join("downloaded"); + + server.put_file_from_bytes("test-file", b"12345"); + + server.stop_after_bytes(3); + download_to_path_with_backend( + Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None) + .expect("Test download failed"); + + assert_eq!(file_contents(&target_path), "123"); + }); +} + +#[test] +fn download_interrupted_and_resumed() { + setup(&|tmpdir, mut server| { + let target_path = tmpdir.path().join("downloaded"); + + server.put_file_from_bytes("test-file", b"12345"); + + server.stop_after_bytes(3); + download_to_path_with_backend( + Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None) + .expect("Test download failed"); + + server.stop_after_bytes(2); + download_to_path_with_backend( + Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None) + .expect("Test download failed"); + + assert_eq!(file_contents(&target_path), "12345"); + }); +} + +#[test] +fn resuming_download_with_callback_that_needs_to_read_contents() { + setup(&|tmpdir, mut server| { + let target_path = tmpdir.path().join("downloaded"); + + server.put_file_from_bytes("test-file", b"12345"); + + server.stop_after_bytes(3); + download_to_path_with_backend( + Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, Some(&|_| {Ok(())})) + .expect("Test download failed"); + + server.stop_after_bytes(2); + download_to_path_with_backend( + Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, Some(&|_| {Ok(())})) + .expect("Test download failed"); + + assert_eq!(file_contents(&target_path), "12345"); + }); +} diff --git a/src/rustup-dist/src/dist.rs b/src/rustup-dist/src/dist.rs index f16499ac11..48236bf582 100644 --- a/src/rustup-dist/src/dist.rs +++ b/src/rustup-dist/src/dist.rs @@ -533,11 +533,21 @@ impl<'a> DownloadCfg<'a> { } } - let mut hasher = Sha256::new(); - try!(utils::download_file(&url, - &target_file, + let partial_file_path = + target_file.with_file_name( + target_file.file_name().map(|s| { + s.to_str().unwrap_or("_")}) + .unwrap_or("_") + .to_owned() + + ".partial"); + + let mut hasher = Sha256::new(); + + try!(utils::download_file_with_resume(&url, + &partial_file_path, Some(&mut hasher), + true, &|n| notify_handler(n.into()))); let actual_hash = hasher.result_str(); @@ -551,7 +561,8 @@ impl<'a> DownloadCfg<'a> { }.into()); } else { notify_handler(Notification::ChecksumValid(&url.to_string())); - return Ok(File { path: target_file, }) + try!(fs::rename(&partial_file_path, &target_file)); + return Ok(File { path: target_file }); } } diff --git a/src/rustup-dist/src/errors.rs b/src/rustup-dist/src/errors.rs index 5d34a6719b..5dc4e9cfaf 100644 --- a/src/rustup-dist/src/errors.rs +++ b/src/rustup-dist/src/errors.rs @@ -1,5 +1,5 @@ use std::path::PathBuf; -use std::io::Write; +use std::io::{self, Write}; use temp; use toml; use rustup_utils; @@ -12,6 +12,7 @@ error_chain! { foreign_links { Temp(temp::Error); + Io(io::Error); } errors { diff --git a/src/rustup-mock/Cargo.toml b/src/rustup-mock/Cargo.toml index 568c8bcbed..eef2ece98d 100644 --- a/src/rustup-mock/Cargo.toml +++ b/src/rustup-mock/Cargo.toml @@ -22,8 +22,8 @@ toml = "0.1.27" rustup-utils = { path = "../rustup-utils", version = "1.0.0" } sha2 = "0.1.2" wait-timeout = "0.1.3" +hyper = "0.10.5" [target."cfg(windows)".dependencies] winapi = "0.2.8" winreg = "0.3.2" - diff --git a/src/rustup-mock/src/http_server.rs b/src/rustup-mock/src/http_server.rs new file mode 100644 index 0000000000..ffe64aa5ce --- /dev/null +++ b/src/rustup-mock/src/http_server.rs @@ -0,0 +1,150 @@ + + +use std::net::ToSocketAddrs; +use std::path::{Path, PathBuf}; +use std::io::{self, Read, Write, Seek, SeekFrom}; +use std::net::SocketAddr; +use std::fs::File; +use std::sync::{Arc, Mutex, MutexGuard}; + +use url; +use hyper; +use hyper::header::{Range, ByteRangeSpec}; +use hyper::server::Handler; +use hyper::server::request::*; +use hyper::uri::RequestUri::*; +use hyper::server::response::*; +use hyper::net::Fresh; +use hyper::status::StatusCode; + +struct ServerImp { + base_path: PathBuf, + max_bytes: Option, +} + +pub struct Server { + server_addr: SocketAddr, + hserver: hyper::server::Listening, + server_imp: Arc>, +} + +impl ServerImp { + fn put_file_from_bytes>(&self, rel_path: T, bytes: &[u8]) { + File::create(&self.base_path.join(rel_path)) + .expect("creating file for sample data") + .write_all(bytes) + .expect("writing sample data"); + } + + fn stop_after_bytes(&mut self, bytes_to_serve: u64) { + self.max_bytes = if bytes_to_serve > 0 { + Some(bytes_to_serve) + } else { + None + }; + } + + fn write_file(&self, path: &Path, mut to: T, requested_range: Option<&Range>) -> Result<(), io::Error> + where T: Write { + + let mut to_send = try!(File::open(path)); + + // Don't need to support every case here, just trying to test the resume-from case + if let Some(requested_range) = requested_range { + match requested_range { + &Range::Bytes(ref byte_ranges) => { + match byte_ranges[0] { + ByteRangeSpec::AllFrom(n) => { + to_send.seek(SeekFrom::Start(n)).expect("Seeking to start of requested byte range"); + }, + _ => { unimplemented!() } + } + } + _ => unimplemented!() + } + + } + + let mut to_send : Box = if let Some(bytes_to_serve) = self.max_bytes { + Box::new(to_send.take(bytes_to_serve)) + } else { + Box::new(to_send) + }; + + io::copy(&mut to_send, &mut to).expect("Copying from file to response"); + Ok(()) + } + + fn write_file_to_response(&self, path: &Path, mut response: Response, mut request: Request) { + if !path.exists() { + *response.status_mut() = StatusCode::NotFound; + return; + } + let mut response = response.start().unwrap(); + let byte_ranges = request.headers.get::(); + self.write_file(path, &mut response, byte_ranges); + } + + fn serve_file(&self, base_path: &Path, request: Request, mut response: Response) { + match request.uri.clone() { + AbsolutePath(s) => { + let p = base_path.join(&s[1..]); + self.write_file_to_response(&p, response, request); + } + AbsoluteUri(u) => { + let urlpath = u.path(); + let p = base_path.join(&urlpath[1..]); + self.write_file_to_response(&p, response, request); + } + _ => *response.status_mut() = StatusCode::MethodNotAllowed, + } + } +} + +impl Server { + fn imp(&self) -> MutexGuard { + self.server_imp.lock().expect("Lock has been poisoned by failure in another thread - aborting test") + } + + pub fn serve_from(path: &Path) -> Result { + let hserver = try!(hyper::server::Server::http("127.0.0.1:0")); + let base_path = path.to_owned(); + + let server_imp = Arc::new(Mutex::new(ServerImp { + base_path: base_path.clone(), + max_bytes: None, + })); + + let server_clone = server_imp.clone(); + let listening = try!(hserver.handle(move |req: Request, resp: Response| { + server_clone.lock().unwrap().serve_file(&base_path, req, resp); + })); + + Ok(Server { + server_addr: listening.socket.clone(), + hserver: listening, + server_imp: server_imp, + }) + } + + pub fn put_file_from_bytes>(&self, rel_path: T, bytes: &[u8]) { + self.imp().put_file_from_bytes(rel_path, bytes) + } + + + pub fn address(&self) -> url::Url { + let url_string = format!("http://127.0.0.1:{}", self.server_addr.port()); + url::Url::parse(&url_string).expect("Mistake in url for test server") + } + + pub fn stop_after_bytes(&mut self, bytes_to_serve: u64) { + self.imp().stop_after_bytes(bytes_to_serve) + } + +} + +impl Drop for Server { + fn drop(&mut self) { + self.hserver.close().expect("Unable to shut down server"); + } +} diff --git a/src/rustup-mock/src/lib.rs b/src/rustup-mock/src/lib.rs index cf2fe3914b..b8afb0c477 100644 --- a/src/rustup-mock/src/lib.rs +++ b/src/rustup-mock/src/lib.rs @@ -13,6 +13,7 @@ extern crate toml; extern crate rustup_utils; extern crate sha2; extern crate wait_timeout; +extern crate hyper; #[cfg(windows)] extern crate winapi; @@ -21,6 +22,7 @@ extern crate winreg; pub mod dist; pub mod clitools; +pub mod http_server; use std::fs::{self, OpenOptions, File}; use std::path::Path; @@ -124,4 +126,3 @@ pub fn get_path() -> Option { None } #[cfg(unix)] pub fn restore_path(_: &Option) { } - diff --git a/src/rustup-utils/src/utils.rs b/src/rustup-utils/src/utils.rs index b505b72689..c592f5947b 100644 --- a/src/rustup-utils/src/utils.rs +++ b/src/rustup-utils/src/utils.rs @@ -6,7 +6,7 @@ use std::process::Command; use std::ffi::OsString; use std::env; use sha2::Sha256; -use notifications::{Notification}; +use notifications::Notification; use raw; #[cfg(windows)] use winapi::DWORD; @@ -15,8 +15,8 @@ use winreg; use std::cmp::Ord; use url::Url; -pub use raw::{is_directory, is_file, path_exists, if_not_empty, random_string, prefix_arg, - has_cmd, find_cmd}; +pub use raw::{is_directory, is_file, path_exists, if_not_empty, random_string, prefix_arg, has_cmd, + find_cmd}; pub fn ensure_dir_exists(name: &'static str, path: &Path, @@ -24,57 +24,57 @@ pub fn ensure_dir_exists(name: &'static str, -> Result { raw::ensure_dir_exists(path, |p| notify_handler(Notification::CreatingDirectory(name, p))) - .chain_err(|| { - ErrorKind::CreatingDirectory { - name: name, - path: PathBuf::from(path), - } - }) + .chain_err(|| { + ErrorKind::CreatingDirectory { + name: name, + path: PathBuf::from(path), + } + }) } pub fn read_file(name: &'static str, path: &Path) -> Result { raw::read_file(path).chain_err(|| { - ErrorKind::ReadingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::ReadingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn write_file(name: &'static str, path: &Path, contents: &str) -> Result<()> { raw::write_file(path, contents).chain_err(|| { - ErrorKind::WritingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::WritingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn append_file(name: &'static str, path: &Path, line: &str) -> Result<()> { raw::append_file(path, line).chain_err(|| { - ErrorKind::WritingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::WritingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn write_line(name: &'static str, file: &mut File, path: &Path, line: &str) -> Result<()> { writeln!(file, "{}", line).chain_err(|| { - ErrorKind::WritingFile { - name: name, - path: path.to_path_buf(), - } - }) + ErrorKind::WritingFile { + name: name, + path: path.to_path_buf(), + } + }) } pub fn write_str(name: &'static str, file: &mut File, path: &Path, s: &str) -> Result<()> { write!(file, "{}", s).chain_err(|| { - ErrorKind::WritingFile { - name: name, - path: path.to_path_buf(), - } - }) + ErrorKind::WritingFile { + name: name, + path: path.to_path_buf(), + } + }) } pub fn rename_file(name: &'static str, src: &Path, dest: &Path) -> Result<()> { @@ -116,27 +116,27 @@ pub fn match_file Option>(name: &'static str, f: F) -> Result> { raw::match_file(src, f).chain_err(|| { - ErrorKind::ReadingFile { - name: name, - path: PathBuf::from(src), - } - }) + ErrorKind::ReadingFile { + name: name, + path: PathBuf::from(src), + } + }) } pub fn canonicalize_path(path: &Path, notify_handler: &Fn(Notification)) -> PathBuf { fs::canonicalize(path).unwrap_or_else(|_| { - notify_handler(Notification::NoCanonicalPath(path)); - PathBuf::from(path) - }) + notify_handler(Notification::NoCanonicalPath(path)); + PathBuf::from(path) + }) } pub fn tee_file(name: &'static str, path: &Path, w: &mut W) -> Result<()> { raw::tee_file(path, w).chain_err(|| { - ErrorKind::ReadingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::ReadingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn download_file(url: &Url, @@ -144,26 +144,35 @@ pub fn download_file(url: &Url, hasher: Option<&mut Sha256>, notify_handler: &Fn(Notification)) -> Result<()> { + download_file_with_resume(&url, &path, hasher, false, ¬ify_handler) +} + +pub fn download_file_with_resume(url: &Url, + path: &Path, + hasher: Option<&mut Sha256>, + resume_from_partial: bool, + notify_handler: &Fn(Notification)) + -> Result<()> { use download::ErrorKind as DEK; - match download_file_(url, path, hasher, notify_handler) { + match download_file_(url, path, hasher, resume_from_partial, notify_handler) { Ok(_) => Ok(()), Err(e) => { let is_client_error = match e.kind() { - &ErrorKind::Download(DEK::HttpStatus(400 ... 499)) => true, + &ErrorKind::Download(DEK::HttpStatus(400...499)) => true, &ErrorKind::Download(DEK::FileNotFound) => true, - _ => false + _ => false, }; Err(e).chain_err(|| if is_client_error { - ErrorKind::DownloadNotExists { - url: url.clone(), - path: path.to_path_buf(), - } - } else { - ErrorKind::DownloadingFile { - url: url.clone(), - path: path.to_path_buf(), - } - }) + ErrorKind::DownloadNotExists { + url: url.clone(), + path: path.to_path_buf(), + } + } else { + ErrorKind::DownloadingFile { + url: url.clone(), + path: path.to_path_buf(), + } + }) } } } @@ -171,6 +180,7 @@ pub fn download_file(url: &Url, fn download_file_(url: &Url, path: &Path, hasher: Option<&mut Sha256>, + resume_from_partial: bool, notify_handler: &Fn(Notification)) -> Result<()> { @@ -192,7 +202,7 @@ fn download_file_(url: &Url, h.input(data); } } - _ => () + _ => (), } match msg { @@ -218,7 +228,7 @@ fn download_file_(url: &Url, (Backend::Curl, Notification::UsingCurl) }; notify_handler(notification); - try!(download_to_path_with_backend(backend, url, path, Some(callback))); + try!(download_to_path_with_backend(backend, url, path, resume_from_partial, Some(callback))); notify_handler(Notification::DownloadFinished); @@ -230,11 +240,7 @@ pub fn parse_url(url: &str) -> Result { } pub fn cmd_status(name: &'static str, cmd: &mut Command) -> Result<()> { - raw::cmd_status(cmd).chain_err(|| { - ErrorKind::RunningCommand { - name: OsString::from(name), - } - }) + raw::cmd_status(cmd).chain_err(|| ErrorKind::RunningCommand { name: OsString::from(name) }) } pub fn assert_is_file(path: &Path) -> Result<()> { @@ -256,105 +262,108 @@ pub fn assert_is_directory(path: &Path) -> Result<()> { pub fn symlink_dir(src: &Path, dest: &Path, notify_handler: &Fn(Notification)) -> Result<()> { notify_handler(Notification::LinkingDirectory(src, dest)); raw::symlink_dir(src, dest).chain_err(|| { - ErrorKind::LinkingDirectory { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::LinkingDirectory { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) } pub fn hardlink_file(src: &Path, dest: &Path) -> Result<()> { raw::hardlink(src, dest).chain_err(|| { - ErrorKind::LinkingFile { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::LinkingFile { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) } #[cfg(unix)] pub fn symlink_file(src: &Path, dest: &Path) -> Result<()> { ::std::os::unix::fs::symlink(src, dest).chain_err(|| { - ErrorKind::LinkingFile { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::LinkingFile { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) } #[cfg(windows)] pub fn symlink_file(src: &Path, dest: &Path) -> Result<()> { // we are supposed to not use symlink on windows Err(ErrorKind::LinkingFile { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - }.into() - ) + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + .into()) } pub fn copy_dir(src: &Path, dest: &Path, notify_handler: &Fn(Notification)) -> Result<()> { notify_handler(Notification::CopyingDirectory(src, dest)); raw::copy_dir(src, dest).chain_err(|| { - ErrorKind::CopyingDirectory { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::CopyingDirectory { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) } pub fn copy_file(src: &Path, dest: &Path) -> Result<()> { fs::copy(src, dest) .chain_err(|| { - ErrorKind::CopyingFile { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::CopyingFile { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) .map(|_| ()) } -pub fn remove_dir(name: &'static str, path: &Path, notify_handler: &Fn(Notification)) -> Result<()> { +pub fn remove_dir(name: &'static str, + path: &Path, + notify_handler: &Fn(Notification)) + -> Result<()> { notify_handler(Notification::RemovingDirectory(name, path)); raw::remove_dir(path).chain_err(|| { - ErrorKind::RemovingDirectory { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::RemovingDirectory { + name: name, + path: PathBuf::from(path), + } + }) } pub fn remove_file(name: &'static str, path: &Path) -> Result<()> { fs::remove_file(path).chain_err(|| { - ErrorKind::RemovingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::RemovingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn read_dir(name: &'static str, path: &Path) -> Result { fs::read_dir(path).chain_err(|| { - ErrorKind::ReadingDirectory { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::ReadingDirectory { + name: name, + path: PathBuf::from(path), + } + }) } pub fn open_browser(path: &Path) -> Result<()> { match raw::open_browser(path) { Ok(true) => Ok(()), Ok(false) => Err("no browser installed".into()), - Err(e) => Err(e).chain_err(|| "could not open browser") + Err(e) => Err(e).chain_err(|| "could not open browser"), } } pub fn set_permissions(path: &Path, perms: fs::Permissions) -> Result<()> { fs::set_permissions(path, perms).chain_err(|| { - ErrorKind::SettingPermissions { - path: PathBuf::from(path), - } - }) + ErrorKind::SettingPermissions { + path: PathBuf::from(path), + } + }) } pub fn make_executable(path: &Path) -> Result<()> { @@ -367,10 +376,10 @@ pub fn make_executable(path: &Path) -> Result<()> { use std::os::unix::fs::PermissionsExt; let metadata = try!(fs::metadata(path).chain_err(|| { - ErrorKind::SettingPermissions { - path: PathBuf::from(path), - } - })); + ErrorKind::SettingPermissions { + path: PathBuf::from(path), + } + })); let mut perms = metadata.permissions(); let new_mode = (perms.mode() & !0o777) | 0o755; perms.set_mode(new_mode); @@ -391,9 +400,9 @@ pub fn current_exe() -> Result { pub fn to_absolute>(path: P) -> Result { current_dir().map(|mut v| { - v.push(path); - v - }) + v.push(path); + v + }) } // On windows, unlike std and cargo, multirust does *not* consider the @@ -413,7 +422,7 @@ pub fn home_dir() -> Option { let me = GetCurrentProcess(); let mut token = ptr::null_mut(); if OpenProcessToken(me, TOKEN_READ, &mut token) == 0 { - return None + return None; } let _g = scopeguard::guard(token, |h| { let _ = CloseHandle(*h); }); fill_utf16_buf(|buf, mut sz| { @@ -422,7 +431,9 @@ pub fn home_dir() -> Option { 0 => sz, _ => sz - 1, // sz includes the null terminator } - }, os2path).ok() + }, + os2path) + .ok() }) } @@ -438,7 +449,7 @@ fn fill_utf16_buf(mut f1: F1, f2: F2) -> io::Result F2: FnOnce(&[u16]) -> T { use kernel32::{GetLastError, SetLastError}; - use winapi::{ERROR_INSUFFICIENT_BUFFER}; + use winapi::ERROR_INSUFFICIENT_BUFFER; // Start off with a stack buf but then spill over to the heap if we end up // needing more space. @@ -476,7 +487,7 @@ fn fill_utf16_buf(mut f1: F1, f2: F2) -> io::Result } else if k >= n { n = k; } else { - return Ok(f2(&buf[..k])) + return Ok(f2(&buf[..k])); } } } @@ -497,22 +508,19 @@ pub fn cargo_home() -> Result { // install to the wrong place. This check is to make the // multirust-rs to rustup upgrade seamless. let env_var = if let Some(v) = env_var { - let vv = v.to_string_lossy().to_string(); - if vv.contains(".multirust/cargo") || - vv.contains(r".multirust\cargo") || - vv.trim().is_empty() { - None - } else { - Some(v) - } + let vv = v.to_string_lossy().to_string(); + if vv.contains(".multirust/cargo") || vv.contains(r".multirust\cargo") || + vv.trim().is_empty() { + None + } else { + Some(v) + } } else { None }; let cwd = try!(env::current_dir().chain_err(|| ErrorKind::CargoHome)); - let cargo_home = env_var.clone().map(|home| { - cwd.join(home) - }); + let cargo_home = env_var.clone().map(|home| cwd.join(home)); let user_home = home_dir().map(|p| p.join(".cargo")); cargo_home.or(user_home).ok_or(ErrorKind::CargoHome.into()) } @@ -550,15 +558,12 @@ pub fn do_rustup_home_upgrade() -> bool { } fn rustup_old_version_exists() -> bool { - rustup_dir() - .map(|p| p.join("rustup-version").exists()) - .unwrap_or(false) + rustup_dir().map(|p| p.join("rustup-version").exists()).unwrap_or(false) } fn delete_rustup_dir() -> Result<()> { if let Some(dir) = rustup_dir() { - raw::remove_dir(&dir) - .chain_err(|| "unable to delete rustup dir")?; + raw::remove_dir(&dir).chain_err(|| "unable to delete rustup dir")?; } Ok(()) @@ -567,8 +572,7 @@ pub fn do_rustup_home_upgrade() -> bool { fn rename_rustup_dir_to_rustup_sh() -> Result<()> { let dirs = (rustup_dir(), rustup_sh_dir()); if let (Some(rustup), Some(rustup_sh)) = dirs { - fs::rename(&rustup, &rustup_sh) - .chain_err(|| "unable to rename rustup dir")?; + fs::rename(&rustup, &rustup_sh).chain_err(|| "unable to rename rustup dir")?; } Ok(()) @@ -577,8 +581,7 @@ pub fn do_rustup_home_upgrade() -> bool { fn rename_multirust_dir_to_rustup() -> Result<()> { let dirs = (multirust_dir(), rustup_dir()); if let (Some(rustup), Some(rustup_sh)) = dirs { - fs::rename(&rustup, &rustup_sh) - .chain_err(|| "unable to rename multirust dir")?; + fs::rename(&rustup, &rustup_sh).chain_err(|| "unable to rename multirust dir")?; } Ok(()) @@ -586,7 +589,9 @@ pub fn do_rustup_home_upgrade() -> bool { // If RUSTUP_HOME is set then its default path doesn't matter, so we're // not going to risk doing any I/O work and making a mess. - if rustup_home_is_set() { return true } + if rustup_home_is_set() { + return true; + } // Now we are just trying to get a bogus, rustup.sh-created ~/.rustup out // of the way in the manner that is least likely to take time and generate @@ -615,7 +620,8 @@ pub fn do_rustup_home_upgrade() -> bool { }; // Now we're trying to move ~/.multirust to ~/.rustup - old_rustup_dir_removed && if multirust_dir_exists() { + old_rustup_dir_removed && + if multirust_dir_exists() { if rustup_dir_exists() { // There appears to be both a ~/.multirust dir and a valid ~/.rustup // dir. Most likely because one is a symlink to the other, as configured @@ -643,11 +649,12 @@ pub fn create_rustup_home() -> Result<()> { // If RUSTUP_HOME is set then don't make any assumptions about where it's // ok to put ~/.multirust - if env::var_os("RUSTUP_HOME").is_some() { return Ok(()) } + if env::var_os("RUSTUP_HOME").is_some() { + return Ok(()); + } let home = rustup_home_in_user_dir()?; - fs::create_dir_all(&home) - .chain_err(|| "unable to create ~/.rustup")?; + fs::create_dir_all(&home).chain_err(|| "unable to create ~/.rustup")?; // This is a temporary compatibility symlink create_legacy_multirust_symlink()?; @@ -665,9 +672,11 @@ fn create_legacy_multirust_symlink() -> Result<()> { return Ok(()); } - raw::symlink_dir(&newhome, &oldhome) - .chain_err(|| format!("unable to symlink {} from {}", - newhome.display(), oldhome.display()))?; + raw::symlink_dir(&newhome, &oldhome).chain_err(|| { + format!("unable to symlink {} from {}", + newhome.display(), + oldhome.display()) + })?; Ok(()) } @@ -676,8 +685,8 @@ pub fn delete_legacy_multirust_symlink() -> Result<()> { let oldhome = legacy_multirust_home()?; if oldhome.exists() { - let meta = fs::symlink_metadata(&oldhome) - .chain_err(|| "unable to get metadata for ~/.multirust")?; + let meta = + fs::symlink_metadata(&oldhome).chain_err(|| "unable to get metadata for ~/.multirust")?; if meta.file_type().is_symlink() { // remove_dir handles unlinking symlinks raw::remove_dir(&oldhome) @@ -704,9 +713,7 @@ pub fn multirust_home() -> Result { let use_rustup_dir = do_rustup_home_upgrade(); let cwd = try!(env::current_dir().chain_err(|| ErrorKind::MultirustHome)); - let multirust_home = env::var_os("RUSTUP_HOME").map(|home| { - cwd.join(home) - }); + let multirust_home = env::var_os("RUSTUP_HOME").map(|home| cwd.join(home)); let user_home = if use_rustup_dir { dot_dir(".rustup") } else { @@ -743,7 +750,8 @@ pub fn string_from_winreg_value(val: &winreg::RegValue) -> Option { use std::slice; match val.vtype { - RegType::REG_SZ | RegType::REG_EXPAND_SZ => { + RegType::REG_SZ | + RegType::REG_EXPAND_SZ => { // Copied from winreg let words = unsafe { slice::from_raw_parts(val.bytes.as_ptr() as *const u16, val.bytes.len() / 2) @@ -753,10 +761,12 @@ pub fn string_from_winreg_value(val: &winreg::RegValue) -> Option { } else { return None; }; - while s.ends_with('\u{0}') {s.pop();} + while s.ends_with('\u{0}') { + s.pop(); + } Some(s) } - _ => None + _ => None, } } @@ -801,25 +811,21 @@ mod tests { #[test] fn test_toochain_sort() { - let expected = vec![ - "stable-x86_64-unknown-linux-gnu", - "beta-x86_64-unknown-linux-gnu", - "nightly-x86_64-unknown-linux-gnu", - "1.0.0-x86_64-unknown-linux-gnu", - "1.2.0-x86_64-unknown-linux-gnu", - "1.8.0-x86_64-unknown-linux-gnu", - "1.10.0-x86_64-unknown-linux-gnu", - ]; - - let mut v = vec![ - "1.8.0-x86_64-unknown-linux-gnu", - "1.0.0-x86_64-unknown-linux-gnu", - "nightly-x86_64-unknown-linux-gnu", - "stable-x86_64-unknown-linux-gnu", - "1.10.0-x86_64-unknown-linux-gnu", - "beta-x86_64-unknown-linux-gnu", - "1.2.0-x86_64-unknown-linux-gnu", - ]; + let expected = vec!["stable-x86_64-unknown-linux-gnu", + "beta-x86_64-unknown-linux-gnu", + "nightly-x86_64-unknown-linux-gnu", + "1.0.0-x86_64-unknown-linux-gnu", + "1.2.0-x86_64-unknown-linux-gnu", + "1.8.0-x86_64-unknown-linux-gnu", + "1.10.0-x86_64-unknown-linux-gnu"]; + + let mut v = vec!["1.8.0-x86_64-unknown-linux-gnu", + "1.0.0-x86_64-unknown-linux-gnu", + "nightly-x86_64-unknown-linux-gnu", + "stable-x86_64-unknown-linux-gnu", + "1.10.0-x86_64-unknown-linux-gnu", + "beta-x86_64-unknown-linux-gnu", + "1.2.0-x86_64-unknown-linux-gnu"]; toolchain_sort(&mut v); From 146d3b9e0a294f4c0be0732af00750d7e847174f Mon Sep 17 00:00:00 2001 From: James Elford Date: Mon, 20 Mar 2017 13:48:39 +0000 Subject: [PATCH 2/8] Add a (verbose-level) notification that partial download has resumed. --- src/download/src/errors.rs | 2 +- src/download/src/lib.rs | 18 ++++++++---------- src/rustup-utils/src/notifications.rs | 4 +++- src/rustup-utils/src/utils.rs | 3 +++ 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/download/src/errors.rs b/src/download/src/errors.rs index aa34577fb1..a2431b2f2d 100644 --- a/src/download/src/errors.rs +++ b/src/download/src/errors.rs @@ -1,4 +1,4 @@ -use std::io; + error_chain! { links { } diff --git a/src/download/src/lib.rs b/src/download/src/lib.rs index 9771e5c82e..f974a8b927 100644 --- a/src/download/src/lib.rs +++ b/src/download/src/lib.rs @@ -21,6 +21,7 @@ pub enum Backend { Curl, Hyper, Rustls } #[derive(Debug, Copy, Clone)] pub enum Event<'a> { + ResumingPartialDownload, /// Received the Content-Length of the to-be downloaded data. DownloadContentLengthReceived(u64), /// Received some data. @@ -62,34 +63,32 @@ pub fn download_to_path_with_backend( -> Result<()> { use std::cell::RefCell; - use std::fs::{self, File, OpenOptions}; + use std::fs::{OpenOptions}; use std::io::{Read, Write, Seek, SeekFrom}; || -> Result<()> { let (file, resume_from) = if resume_from_partial && supports_partial_download(&backend) { - let mut possible_partial = OpenOptions::new() + let possible_partial = OpenOptions::new() .read(true) .open(&path); let downloaded_so_far = if let Ok(mut partial) = possible_partial { if let Some(cb) = callback { - println!("Reading file in {}", path.display()); + try!(cb(Event::ResumingPartialDownload)); + let mut buf = vec![0; 1024*1024*10]; let mut downloaded_so_far = 0; - let mut number_of_reads = 0; loop { let n = try!(partial.read(&mut buf)); downloaded_so_far += n as u64; - number_of_reads += 1; if n == 0 { - println!("nothing read after {} reads (accumulated {})", number_of_reads, downloaded_so_far); break; } try!(cb(Event::DownloadDataReceived(&buf[..n]))); } + downloaded_so_far } else { - use std::fs::Metadata; let file_info = try!(partial.metadata()); file_info.len() } @@ -102,7 +101,6 @@ pub fn download_to_path_with_backend( (possible_partial, downloaded_so_far) } else { - println!("Download resume not supported"); (try!(OpenOptions::new() .write(true) .create(true) @@ -234,9 +232,9 @@ pub mod curl { // If we didn't get a 20x or 0 ("OK" for files) then return an error let code = try!(handle.response_code().chain_err(|| "failed to get response code")); match code { - 0 | 200...299 => { println!("status code: {}", code)} + 0 | 200 ... 299 => {}, _ => { return Err(ErrorKind::HttpStatus(code).into()); } - } + }; Ok(()) }) diff --git a/src/rustup-utils/src/notifications.rs b/src/rustup-utils/src/notifications.rs index ec1d3820a5..abaa9bf14c 100644 --- a/src/rustup-utils/src/notifications.rs +++ b/src/rustup-utils/src/notifications.rs @@ -19,6 +19,7 @@ pub enum Notification<'a> { /// Download has finished. DownloadFinished, NoCanonicalPath(&'a Path), + ResumingPartialDownload, UsingCurl, UsingHyper, UsingRustls, @@ -35,6 +36,7 @@ impl<'a> Notification<'a> { DownloadContentLengthReceived(_) | DownloadDataReceived(_) | DownloadFinished | + ResumingPartialDownload | UsingCurl | UsingHyper | UsingRustls => NotificationLevel::Verbose, NoCanonicalPath(_) => NotificationLevel::Warn, } @@ -58,10 +60,10 @@ impl<'a> Display for Notification<'a> { DownloadDataReceived(data) => write!(f, "received some data of size {}", data.len()), DownloadFinished => write!(f, "download finished"), NoCanonicalPath(path) => write!(f, "could not canonicalize path: '{}'", path.display()), + ResumingPartialDownload => write!(f, "resuming partial download"), UsingCurl => write!(f, "downloading with curl"), UsingHyper => write!(f, "downloading with hyper + native_tls"), UsingRustls => write!(f, "downloading with hyper + rustls"), } } } - diff --git a/src/rustup-utils/src/utils.rs b/src/rustup-utils/src/utils.rs index c592f5947b..68d14a639b 100644 --- a/src/rustup-utils/src/utils.rs +++ b/src/rustup-utils/src/utils.rs @@ -212,6 +212,9 @@ fn download_file_(url: &Url, Event::DownloadDataReceived(data) => { notify_handler(Notification::DownloadDataReceived(data)); } + Event::ResumingPartialDownload => { + notify_handler(Notification::ResumingPartialDownload); + } } Ok(()) From 71fe60ccfc0bca2ad26df6f16f3c5a052b61dccb Mon Sep 17 00:00:00 2001 From: James Elford Date: Mon, 20 Mar 2017 13:58:06 +0000 Subject: [PATCH 3/8] Clean up unused muts etc. thanks to rustc lints --- src/rustup-mock/src/http_server.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/rustup-mock/src/http_server.rs b/src/rustup-mock/src/http_server.rs index ffe64aa5ce..6875bd2d83 100644 --- a/src/rustup-mock/src/http_server.rs +++ b/src/rustup-mock/src/http_server.rs @@ -1,6 +1,4 @@ - -use std::net::ToSocketAddrs; use std::path::{Path, PathBuf}; use std::io::{self, Read, Write, Seek, SeekFrom}; use std::net::SocketAddr; @@ -10,7 +8,6 @@ use std::sync::{Arc, Mutex, MutexGuard}; use url; use hyper; use hyper::header::{Range, ByteRangeSpec}; -use hyper::server::Handler; use hyper::server::request::*; use hyper::uri::RequestUri::*; use hyper::server::response::*; @@ -75,29 +72,32 @@ impl ServerImp { Ok(()) } - fn write_file_to_response(&self, path: &Path, mut response: Response, mut request: Request) { + fn write_file_to_response(&self, path: &Path, mut response: Response, request: Request) -> Result<(), io::Error> { if !path.exists() { *response.status_mut() = StatusCode::NotFound; - return; + return Ok(()); } let mut response = response.start().unwrap(); let byte_ranges = request.headers.get::(); - self.write_file(path, &mut response, byte_ranges); + try!(self.write_file(path, &mut response, byte_ranges)); + Ok(()) } - fn serve_file(&self, base_path: &Path, request: Request, mut response: Response) { + fn serve_file(&self, base_path: &Path, request: Request, mut response: Response) -> Result<(), io::Error> { match request.uri.clone() { AbsolutePath(s) => { let p = base_path.join(&s[1..]); - self.write_file_to_response(&p, response, request); + try!(self.write_file_to_response(&p, response, request)); } AbsoluteUri(u) => { let urlpath = u.path(); let p = base_path.join(&urlpath[1..]); - self.write_file_to_response(&p, response, request); + try!(self.write_file_to_response(&p, response, request)) } - _ => *response.status_mut() = StatusCode::MethodNotAllowed, + _ => { *response.status_mut() = StatusCode::MethodNotAllowed; } } + + Ok(()) } } @@ -117,7 +117,7 @@ impl Server { let server_clone = server_imp.clone(); let listening = try!(hserver.handle(move |req: Request, resp: Response| { - server_clone.lock().unwrap().serve_file(&base_path, req, resp); + server_clone.lock().unwrap().serve_file(&base_path, req, resp).expect("Rendering response"); })); Ok(Server { From 7aa8b5787502d388de1dd8802e9b38d267785451 Mon Sep 17 00:00:00 2001 From: James Elford Date: Mon, 20 Mar 2017 14:02:28 +0000 Subject: [PATCH 4/8] Include download crate's tests in CI script --- ci/run.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/run.sh b/ci/run.sh index 465551f4df..516d9317c7 100644 --- a/ci/run.sh +++ b/ci/run.sh @@ -10,6 +10,7 @@ cargo -vV cargo build --release --target $TARGET if [ -z "$SKIP_TESTS" ]; then + cargo test --release -p download --target $TARGET cargo test --release -p rustup-dist --target $TARGET cargo test --release --target $TARGET fi From 1286686a46344602be52ff47bee5fd6b85a20fbe Mon Sep 17 00:00:00 2001 From: James Elford Date: Wed, 29 Mar 2017 23:55:50 +0100 Subject: [PATCH 5/8] Use libcurl's support for download resumption directly. Allows the removal of http test server, and simplifies test cases as we can just read/write files on disk, like for existing dist tests --- Cargo.lock | 37 ----- src/download/Cargo.toml | 1 - src/download/src/lib.rs | 15 ++- src/download/tests/download-curl-resume.rs | 111 ++++++++------- src/rustup-mock/Cargo.toml | 1 - src/rustup-mock/src/http_server.rs | 150 --------------------- src/rustup-mock/src/lib.rs | 2 - 7 files changed, 73 insertions(+), 244 deletions(-) delete mode 100644 src/rustup-mock/src/http_server.rs diff --git a/Cargo.lock b/Cargo.lock index aedb935adb..c1e45bc0f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -182,7 +182,6 @@ dependencies = [ "native-tls 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "openssl-probe 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustls 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "rustup-mock 1.0.0", "tempdir 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -267,25 +266,6 @@ dependencies = [ "url 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", ] -[[package]] -name = "hyper" -version = "0.10.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "httparse 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "language-tags 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", - "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", - "mime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", - "num_cpus 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", - "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "rustc_version 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", - "time 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)", - "traitobject 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", - "typeable 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "url 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", -] - [[package]] name = "idna" version = "0.1.0" @@ -422,14 +402,6 @@ dependencies = [ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)", ] -[[package]] -name = "num_cpus" -version = "1.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)", -] - [[package]] name = "ole32-sys" version = "0.2.0" @@ -612,7 +584,6 @@ name = "rustup-mock" version = "1.0.0" dependencies = [ "flate2 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)", - "hyper 0.10.5 (registry+https://github.com/rust-lang/crates.io-index)", "itertools 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", "rustup-utils 1.0.0", @@ -841,11 +812,6 @@ name = "traitobject" version = "0.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -[[package]] -name = "traitobject" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" - [[package]] name = "typeable" version = "0.1.2" @@ -1017,7 +983,6 @@ dependencies = [ "checksum gdi32-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "0912515a8ff24ba900422ecda800b52f4016a56251922d397c576bf92c690518" "checksum hpack 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3d2da7d3a34cf6406d9d700111b8eafafe9a251de41ae71d8052748259343b58" "checksum httparse 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "46534074dbb80b070d60a5cb8ecadd8963a00a438ae1a95268850a7ef73b67ae" -"checksum hyper 0.10.5 (registry+https://github.com/rust-lang/crates.io-index)" = "43a15e3273b2133aaac0150478ab443fb89f15c3de41d8d93d8f3bb14bf560f6" "checksum hyper 0.9.10 (registry+https://github.com/rust-lang/crates.io-index)" = "eb27e8a3e8f17ac43ffa41bbda9cf5ad3f9f13ef66fa4873409d4902310275f7" "checksum idna 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1053236e00ce4f668aeca4a769a09b3bf5a682d802abd6f3cb39374f6b162c11" "checksum itertools 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)" = "c4a9b56eb56058f43dc66e58f40a214b2ccbc9f3df51861b63d51dec7b65bc3f" @@ -1037,7 +1002,6 @@ dependencies = [ "checksum miniz-sys 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "9d1f4d337a01c32e1f2122510fed46393d53ca35a7f429cb0450abaedfa3ed54" "checksum native-tls 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "aa4e52995154bb6f0b41e4379a279482c9387c1632e3798ba4e511ef8c54ee09" "checksum num_cpus 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)" = "cee7e88156f3f9e19bdd598f8d6c9db7bf4078f99f8381f43a55b09648d1a6e3" -"checksum num_cpus 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "a18c392466409c50b87369414a2680c93e739aedeb498eb2bff7d7eb569744e2" "checksum ole32-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5d2c49021782e5233cd243168edfa8037574afed4eba4bbaf538b3d8d1789d8c" "checksum openssl 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c368aec980860439ea434b98dd622b1e09f720c6762308854ef4b9e2e82771ad" "checksum openssl-probe 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "756d49c8424483a3df3b5d735112b4da22109ced9a8294f1f5cdf80fb3810919" @@ -1075,7 +1039,6 @@ dependencies = [ "checksum time 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)" = "3c7ec6d62a20df54e07ab3b78b9a3932972f4b7981de295563686849eb3989af" "checksum toml 0.1.30 (registry+https://github.com/rust-lang/crates.io-index)" = "0590d72182e50e879c4da3b11c6488dae18fccb1ae0c7a3eda18e16795844796" "checksum traitobject 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "07eaeb7689bb7fca7ce15628319635758eda769fed481ecfe6686ddef2600616" -"checksum traitobject 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "efd1f82c56340fdf16f2a953d7bda4f8fdffba13d93b00844c25572110b26079" "checksum typeable 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1410f6f91f21d1612654e7cc69193b0334f909dcf2c790c4826254fbb86f8887" "checksum unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "13a5906ca2b98c799f4b1ab4557b76367ebd6ae5ef14930ec841c74aed5f3764" "checksum unicode-bidi 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c1f7ceb96afdfeedee42bade65a0d585a6a0106f681b6749c8ff4daa8df30b3f" diff --git a/src/download/Cargo.toml b/src/download/Cargo.toml index 0e5b98873b..dbccf1c596 100644 --- a/src/download/Cargo.toml +++ b/src/download/Cargo.toml @@ -22,7 +22,6 @@ lazy_static = { version = "0.2", optional = true } native-tls = { version = "0.1", optional = true } [dev-dependencies] -rustup-mock = { path = "../rustup-mock", version = "1.0.0" } tempdir = "0.3.4" [dependencies.hyper] diff --git a/src/download/src/lib.rs b/src/download/src/lib.rs index f974a8b927..25a27ac2eb 100644 --- a/src/download/src/lib.rs +++ b/src/download/src/lib.rs @@ -96,7 +96,13 @@ pub fn download_to_path_with_backend( 0 }; - let mut possible_partial = try!(OpenOptions::new().write(true).create(true).open(&path).chain_err(|| "error opening file for download")); + let mut possible_partial = + try!(OpenOptions::new() + .write(true) + .create(true) + .open(&path) + .chain_err(|| "error opening file for download")); + try!(possible_partial.seek(SeekFrom::End(0))); (possible_partial, downloaded_so_far) @@ -164,9 +170,12 @@ pub mod curl { try!(handle.follow_location(true).chain_err(|| "failed to set follow redirects")); if resume_from > 0 { - try!(handle.range(&(resume_from.to_string() + "-")).chain_err(|| "setting the range-header for download resumption")); + try!(handle.resume_from(resume_from) + .chain_err(|| "setting the range header for download resumption")); } else { - try!(handle.range("").chain_err(|| "clearing range header")); + // an error here indicates that the range header isn't supported by underlying curl, + // so there's nothing to "clear" - safe to ignore this error. + let _ = handle.resume_from(0); } // Take at most 30s to connect diff --git a/src/download/tests/download-curl-resume.rs b/src/download/tests/download-curl-resume.rs index 4e75ba1c9b..f28dd1b4ab 100644 --- a/src/download/tests/download-curl-resume.rs +++ b/src/download/tests/download-curl-resume.rs @@ -1,25 +1,21 @@ #![cfg(feature = "curl-backend")] extern crate download; -extern crate rustup_mock; extern crate tempdir; +extern crate url; +use std::sync::{Arc, Mutex}; use std::fs::{self, File}; -use std::io::{Read}; +use std::io::{self, Read}; use std::path::Path; use tempdir::TempDir; - -use rustup_mock::http_server; +use url::Url; use download::*; -fn setup(test: &Fn(TempDir, http_server::Server) -> ()) { - let tmp = TempDir::new("rustup-download-test-").expect("creating tempdir for test"); - let served_dir = &tmp.path().join("test-files"); - fs::DirBuilder::new().create(served_dir).expect("setting up a folder to server files from"); - let server = http_server::Server::serve_from(served_dir).expect("setting up http server for test"); - test(tmp, server); +fn tmp_dir() -> TempDir { + TempDir::new("rustup-download-test-").expect("creating tempdir for test") } fn file_contents(path: &Path) -> String { @@ -28,60 +24,75 @@ fn file_contents(path: &Path) -> String { result } -#[test] -fn when_download_is_interrupted_partial_file_is_left_on_disk() { - setup(&|tmpdir, mut server| { - let target_path = tmpdir.path().join("downloaded"); - server.put_file_from_bytes("test-file", b"12345"); +pub fn write_file(path: &Path, contents: &str) { + let mut file = fs::OpenOptions::new() + .write(true) + .truncate(true) + .create(true) + .open(path) + .expect("writing test data"); - server.stop_after_bytes(3); - download_to_path_with_backend( - Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None) - .expect("Test download failed"); + io::Write::write_all(&mut file, contents.as_bytes()).expect("writing test data"); - assert_eq!(file_contents(&target_path), "123"); - }); + file.sync_data().expect("writing test data"); } #[test] -fn download_interrupted_and_resumed() { - setup(&|tmpdir, mut server| { - let target_path = tmpdir.path().join("downloaded"); - - server.put_file_from_bytes("test-file", b"12345"); - - server.stop_after_bytes(3); - download_to_path_with_backend( - Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None) - .expect("Test download failed"); - - server.stop_after_bytes(2); - download_to_path_with_backend( - Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, None) +fn partially_downloaded_file_gets_resumed_from_byte_offset() { + let tmpdir = tmp_dir(); + let from_path = tmpdir.path().join("download-source"); + write_file(&from_path, "xxx45"); + + let target_path = tmpdir.path().join("downloaded"); + write_file(&target_path, "123"); + + let from_url = Url::from_file_path(&from_path).unwrap(); + download_to_path_with_backend( + Backend::Curl, + &from_url, + &target_path, + true, + None) .expect("Test download failed"); - assert_eq!(file_contents(&target_path), "12345"); - }); + assert_eq!(file_contents(&target_path), "12345"); } #[test] -fn resuming_download_with_callback_that_needs_to_read_contents() { - setup(&|tmpdir, mut server| { - let target_path = tmpdir.path().join("downloaded"); +fn callback_gets_all_data_as_if_the_download_happened_all_at_once() { + let tmpdir = tmp_dir(); - server.put_file_from_bytes("test-file", b"12345"); + let from_path = tmpdir.path().join("download-source"); + write_file(&from_path, "xxx45"); + + let target_path = tmpdir.path().join("downloaded"); + write_file(&target_path, "123"); + + let from_url = Url::from_file_path(&from_path).unwrap(); + + let received_in_callback = Arc::new(Mutex::new(Vec::new())); + + download_to_path_with_backend(Backend::Curl, + &from_url, + &target_path, + true, + Some(&|msg| { + match msg { + Event::DownloadDataReceived(data) => { + for b in data.iter() { + received_in_callback.lock().unwrap().push(b.clone()); + } + } + _ => {} + } - server.stop_after_bytes(3); - download_to_path_with_backend( - Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, Some(&|_| {Ok(())})) - .expect("Test download failed"); - server.stop_after_bytes(2); - download_to_path_with_backend( - Backend::Curl, &server.address().join("test-file").unwrap(), &target_path, true, Some(&|_| {Ok(())})) + Ok(()) + })) .expect("Test download failed"); - assert_eq!(file_contents(&target_path), "12345"); - }); + let ref observed_bytes = *received_in_callback.lock().unwrap(); + assert_eq!(observed_bytes, &vec![b'1', b'2', b'3', b'4', b'5']); + assert_eq!(file_contents(&target_path), "12345"); } diff --git a/src/rustup-mock/Cargo.toml b/src/rustup-mock/Cargo.toml index eef2ece98d..d103d530df 100644 --- a/src/rustup-mock/Cargo.toml +++ b/src/rustup-mock/Cargo.toml @@ -22,7 +22,6 @@ toml = "0.1.27" rustup-utils = { path = "../rustup-utils", version = "1.0.0" } sha2 = "0.1.2" wait-timeout = "0.1.3" -hyper = "0.10.5" [target."cfg(windows)".dependencies] winapi = "0.2.8" diff --git a/src/rustup-mock/src/http_server.rs b/src/rustup-mock/src/http_server.rs deleted file mode 100644 index 6875bd2d83..0000000000 --- a/src/rustup-mock/src/http_server.rs +++ /dev/null @@ -1,150 +0,0 @@ - -use std::path::{Path, PathBuf}; -use std::io::{self, Read, Write, Seek, SeekFrom}; -use std::net::SocketAddr; -use std::fs::File; -use std::sync::{Arc, Mutex, MutexGuard}; - -use url; -use hyper; -use hyper::header::{Range, ByteRangeSpec}; -use hyper::server::request::*; -use hyper::uri::RequestUri::*; -use hyper::server::response::*; -use hyper::net::Fresh; -use hyper::status::StatusCode; - -struct ServerImp { - base_path: PathBuf, - max_bytes: Option, -} - -pub struct Server { - server_addr: SocketAddr, - hserver: hyper::server::Listening, - server_imp: Arc>, -} - -impl ServerImp { - fn put_file_from_bytes>(&self, rel_path: T, bytes: &[u8]) { - File::create(&self.base_path.join(rel_path)) - .expect("creating file for sample data") - .write_all(bytes) - .expect("writing sample data"); - } - - fn stop_after_bytes(&mut self, bytes_to_serve: u64) { - self.max_bytes = if bytes_to_serve > 0 { - Some(bytes_to_serve) - } else { - None - }; - } - - fn write_file(&self, path: &Path, mut to: T, requested_range: Option<&Range>) -> Result<(), io::Error> - where T: Write { - - let mut to_send = try!(File::open(path)); - - // Don't need to support every case here, just trying to test the resume-from case - if let Some(requested_range) = requested_range { - match requested_range { - &Range::Bytes(ref byte_ranges) => { - match byte_ranges[0] { - ByteRangeSpec::AllFrom(n) => { - to_send.seek(SeekFrom::Start(n)).expect("Seeking to start of requested byte range"); - }, - _ => { unimplemented!() } - } - } - _ => unimplemented!() - } - - } - - let mut to_send : Box = if let Some(bytes_to_serve) = self.max_bytes { - Box::new(to_send.take(bytes_to_serve)) - } else { - Box::new(to_send) - }; - - io::copy(&mut to_send, &mut to).expect("Copying from file to response"); - Ok(()) - } - - fn write_file_to_response(&self, path: &Path, mut response: Response, request: Request) -> Result<(), io::Error> { - if !path.exists() { - *response.status_mut() = StatusCode::NotFound; - return Ok(()); - } - let mut response = response.start().unwrap(); - let byte_ranges = request.headers.get::(); - try!(self.write_file(path, &mut response, byte_ranges)); - Ok(()) - } - - fn serve_file(&self, base_path: &Path, request: Request, mut response: Response) -> Result<(), io::Error> { - match request.uri.clone() { - AbsolutePath(s) => { - let p = base_path.join(&s[1..]); - try!(self.write_file_to_response(&p, response, request)); - } - AbsoluteUri(u) => { - let urlpath = u.path(); - let p = base_path.join(&urlpath[1..]); - try!(self.write_file_to_response(&p, response, request)) - } - _ => { *response.status_mut() = StatusCode::MethodNotAllowed; } - } - - Ok(()) - } -} - -impl Server { - fn imp(&self) -> MutexGuard { - self.server_imp.lock().expect("Lock has been poisoned by failure in another thread - aborting test") - } - - pub fn serve_from(path: &Path) -> Result { - let hserver = try!(hyper::server::Server::http("127.0.0.1:0")); - let base_path = path.to_owned(); - - let server_imp = Arc::new(Mutex::new(ServerImp { - base_path: base_path.clone(), - max_bytes: None, - })); - - let server_clone = server_imp.clone(); - let listening = try!(hserver.handle(move |req: Request, resp: Response| { - server_clone.lock().unwrap().serve_file(&base_path, req, resp).expect("Rendering response"); - })); - - Ok(Server { - server_addr: listening.socket.clone(), - hserver: listening, - server_imp: server_imp, - }) - } - - pub fn put_file_from_bytes>(&self, rel_path: T, bytes: &[u8]) { - self.imp().put_file_from_bytes(rel_path, bytes) - } - - - pub fn address(&self) -> url::Url { - let url_string = format!("http://127.0.0.1:{}", self.server_addr.port()); - url::Url::parse(&url_string).expect("Mistake in url for test server") - } - - pub fn stop_after_bytes(&mut self, bytes_to_serve: u64) { - self.imp().stop_after_bytes(bytes_to_serve) - } - -} - -impl Drop for Server { - fn drop(&mut self) { - self.hserver.close().expect("Unable to shut down server"); - } -} diff --git a/src/rustup-mock/src/lib.rs b/src/rustup-mock/src/lib.rs index b8afb0c477..cd3184195b 100644 --- a/src/rustup-mock/src/lib.rs +++ b/src/rustup-mock/src/lib.rs @@ -13,7 +13,6 @@ extern crate toml; extern crate rustup_utils; extern crate sha2; extern crate wait_timeout; -extern crate hyper; #[cfg(windows)] extern crate winapi; @@ -22,7 +21,6 @@ extern crate winreg; pub mod dist; pub mod clitools; -pub mod http_server; use std::fs::{self, OpenOptions, File}; use std::path::Path; From d9f34d88b8ee2bd77f8fcf24d6f5cdeef593a43c Mon Sep 17 00:00:00 2001 From: James Elford Date: Thu, 30 Mar 2017 00:12:40 +0100 Subject: [PATCH 6/8] Use a larger buffer size when checking previously downloaded file hashes, and make consistent with partial download code. --- src/download/src/lib.rs | 4 ++-- src/rustup-dist/src/dist.rs | 33 +++++++++++++++++++-------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/download/src/lib.rs b/src/download/src/lib.rs index 25a27ac2eb..01855f4277 100644 --- a/src/download/src/lib.rs +++ b/src/download/src/lib.rs @@ -76,7 +76,7 @@ pub fn download_to_path_with_backend( if let Some(cb) = callback { try!(cb(Event::ResumingPartialDownload)); - let mut buf = vec![0; 1024*1024*10]; + let mut buf = vec![0; 32768]; let mut downloaded_so_far = 0; loop { let n = try!(partial.read(&mut buf)); @@ -102,7 +102,7 @@ pub fn download_to_path_with_backend( .create(true) .open(&path) .chain_err(|| "error opening file for download")); - + try!(possible_partial.seek(SeekFrom::End(0))); (possible_partial, downloaded_so_far) diff --git a/src/rustup-dist/src/dist.rs b/src/rustup-dist/src/dist.rs index 48236bf582..31520f8017 100644 --- a/src/rustup-dist/src/dist.rs +++ b/src/rustup-dist/src/dist.rs @@ -502,6 +502,23 @@ impl ops::Deref for File { } } +fn file_hash(path: &Path) -> Result { + let mut hasher = Sha256::new(); + use std::io::Read; + let mut downloaded = try!(fs::File::open(&path).chain_err(|| "opening already downloaded file")); + let mut buf = vec![0; 32768]; + loop { + if let Ok(n) = downloaded.read(&mut buf) { + if n == 0 { break; } + hasher.input(&buf[..n]); + } else { + break; + } + } + + Ok(hasher.result_str()) +} + impl<'a> DownloadCfg<'a> { pub fn download(&self, url: &Url, hash: &str, notify_handler: &'a Fn(Notification)) -> Result { @@ -510,19 +527,7 @@ impl<'a> DownloadCfg<'a> { 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(); + let cached_result = try!(file_hash(&target_file)); if hash == cached_result { notify_handler(Notification::FileAlreadyDownloaded); notify_handler(Notification::ChecksumValid(&url.to_string())); @@ -543,7 +548,7 @@ impl<'a> DownloadCfg<'a> { + ".partial"); let mut hasher = Sha256::new(); - + try!(utils::download_file_with_resume(&url, &partial_file_path, Some(&mut hasher), From 2a029261551634d95a991b5b3d6265d6438088a9 Mon Sep 17 00:00:00 2001 From: James Elford Date: Sat, 1 Apr 2017 01:01:38 +0100 Subject: [PATCH 7/8] Remove unix-only flag on std::io::Error error_chain foreign link I don't know why I put it there in the first place --- src/download/src/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/download/src/errors.rs b/src/download/src/errors.rs index a2431b2f2d..6dfada40cb 100644 --- a/src/download/src/errors.rs +++ b/src/download/src/errors.rs @@ -4,7 +4,7 @@ error_chain! { links { } foreign_links { - Io(::std::io::Error) #[cfg(unix)]; + Io(::std::io::Error); } errors { From c63b2754bfb268388be918baaed2ca4840422244 Mon Sep 17 00:00:00 2001 From: James Elford Date: Sat, 1 Apr 2017 18:35:38 +0100 Subject: [PATCH 8/8] Undo accidental formatting changes --- src/rustup-mock/Cargo.toml | 1 + src/rustup-mock/src/lib.rs | 1 + src/rustup-utils/src/notifications.rs | 1 + src/rustup-utils/src/utils.rs | 356 +++++++++++++------------- 4 files changed, 183 insertions(+), 176 deletions(-) diff --git a/src/rustup-mock/Cargo.toml b/src/rustup-mock/Cargo.toml index d103d530df..568c8bcbed 100644 --- a/src/rustup-mock/Cargo.toml +++ b/src/rustup-mock/Cargo.toml @@ -26,3 +26,4 @@ wait-timeout = "0.1.3" [target."cfg(windows)".dependencies] winapi = "0.2.8" winreg = "0.3.2" + diff --git a/src/rustup-mock/src/lib.rs b/src/rustup-mock/src/lib.rs index cd3184195b..cf2fe3914b 100644 --- a/src/rustup-mock/src/lib.rs +++ b/src/rustup-mock/src/lib.rs @@ -124,3 +124,4 @@ pub fn get_path() -> Option { None } #[cfg(unix)] pub fn restore_path(_: &Option) { } + diff --git a/src/rustup-utils/src/notifications.rs b/src/rustup-utils/src/notifications.rs index abaa9bf14c..8356ad81a6 100644 --- a/src/rustup-utils/src/notifications.rs +++ b/src/rustup-utils/src/notifications.rs @@ -67,3 +67,4 @@ impl<'a> Display for Notification<'a> { } } } + diff --git a/src/rustup-utils/src/utils.rs b/src/rustup-utils/src/utils.rs index 68d14a639b..d2d6da5629 100644 --- a/src/rustup-utils/src/utils.rs +++ b/src/rustup-utils/src/utils.rs @@ -6,7 +6,7 @@ use std::process::Command; use std::ffi::OsString; use std::env; use sha2::Sha256; -use notifications::Notification; +use notifications::{Notification}; use raw; #[cfg(windows)] use winapi::DWORD; @@ -15,8 +15,8 @@ use winreg; use std::cmp::Ord; use url::Url; -pub use raw::{is_directory, is_file, path_exists, if_not_empty, random_string, prefix_arg, has_cmd, - find_cmd}; +pub use raw::{is_directory, is_file, path_exists, if_not_empty, random_string, prefix_arg, + has_cmd, find_cmd}; pub fn ensure_dir_exists(name: &'static str, path: &Path, @@ -24,57 +24,57 @@ pub fn ensure_dir_exists(name: &'static str, -> Result { raw::ensure_dir_exists(path, |p| notify_handler(Notification::CreatingDirectory(name, p))) - .chain_err(|| { - ErrorKind::CreatingDirectory { - name: name, - path: PathBuf::from(path), - } - }) + .chain_err(|| { + ErrorKind::CreatingDirectory { + name: name, + path: PathBuf::from(path), + } + }) } pub fn read_file(name: &'static str, path: &Path) -> Result { raw::read_file(path).chain_err(|| { - ErrorKind::ReadingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::ReadingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn write_file(name: &'static str, path: &Path, contents: &str) -> Result<()> { raw::write_file(path, contents).chain_err(|| { - ErrorKind::WritingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::WritingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn append_file(name: &'static str, path: &Path, line: &str) -> Result<()> { raw::append_file(path, line).chain_err(|| { - ErrorKind::WritingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::WritingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn write_line(name: &'static str, file: &mut File, path: &Path, line: &str) -> Result<()> { writeln!(file, "{}", line).chain_err(|| { - ErrorKind::WritingFile { - name: name, - path: path.to_path_buf(), - } - }) + ErrorKind::WritingFile { + name: name, + path: path.to_path_buf(), + } + }) } pub fn write_str(name: &'static str, file: &mut File, path: &Path, s: &str) -> Result<()> { write!(file, "{}", s).chain_err(|| { - ErrorKind::WritingFile { - name: name, - path: path.to_path_buf(), - } - }) + ErrorKind::WritingFile { + name: name, + path: path.to_path_buf(), + } + }) } pub fn rename_file(name: &'static str, src: &Path, dest: &Path) -> Result<()> { @@ -116,27 +116,27 @@ pub fn match_file Option>(name: &'static str, f: F) -> Result> { raw::match_file(src, f).chain_err(|| { - ErrorKind::ReadingFile { - name: name, - path: PathBuf::from(src), - } - }) + ErrorKind::ReadingFile { + name: name, + path: PathBuf::from(src), + } + }) } pub fn canonicalize_path(path: &Path, notify_handler: &Fn(Notification)) -> PathBuf { fs::canonicalize(path).unwrap_or_else(|_| { - notify_handler(Notification::NoCanonicalPath(path)); - PathBuf::from(path) - }) + notify_handler(Notification::NoCanonicalPath(path)); + PathBuf::from(path) + }) } pub fn tee_file(name: &'static str, path: &Path, w: &mut W) -> Result<()> { raw::tee_file(path, w).chain_err(|| { - ErrorKind::ReadingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::ReadingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn download_file(url: &Url, @@ -158,21 +158,21 @@ pub fn download_file_with_resume(url: &Url, Ok(_) => Ok(()), Err(e) => { let is_client_error = match e.kind() { - &ErrorKind::Download(DEK::HttpStatus(400...499)) => true, + &ErrorKind::Download(DEK::HttpStatus(400 ... 499)) => true, &ErrorKind::Download(DEK::FileNotFound) => true, - _ => false, + _ => false }; Err(e).chain_err(|| if is_client_error { - ErrorKind::DownloadNotExists { - url: url.clone(), - path: path.to_path_buf(), - } - } else { - ErrorKind::DownloadingFile { - url: url.clone(), - path: path.to_path_buf(), - } - }) + ErrorKind::DownloadNotExists { + url: url.clone(), + path: path.to_path_buf(), + } + } else { + ErrorKind::DownloadingFile { + url: url.clone(), + path: path.to_path_buf(), + } + }) } } } @@ -202,7 +202,7 @@ fn download_file_(url: &Url, h.input(data); } } - _ => (), + _ => () } match msg { @@ -243,7 +243,11 @@ pub fn parse_url(url: &str) -> Result { } pub fn cmd_status(name: &'static str, cmd: &mut Command) -> Result<()> { - raw::cmd_status(cmd).chain_err(|| ErrorKind::RunningCommand { name: OsString::from(name) }) + raw::cmd_status(cmd).chain_err(|| { + ErrorKind::RunningCommand { + name: OsString::from(name), + } + }) } pub fn assert_is_file(path: &Path) -> Result<()> { @@ -265,108 +269,105 @@ pub fn assert_is_directory(path: &Path) -> Result<()> { pub fn symlink_dir(src: &Path, dest: &Path, notify_handler: &Fn(Notification)) -> Result<()> { notify_handler(Notification::LinkingDirectory(src, dest)); raw::symlink_dir(src, dest).chain_err(|| { - ErrorKind::LinkingDirectory { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::LinkingDirectory { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) } pub fn hardlink_file(src: &Path, dest: &Path) -> Result<()> { raw::hardlink(src, dest).chain_err(|| { - ErrorKind::LinkingFile { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::LinkingFile { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) } #[cfg(unix)] pub fn symlink_file(src: &Path, dest: &Path) -> Result<()> { ::std::os::unix::fs::symlink(src, dest).chain_err(|| { - ErrorKind::LinkingFile { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::LinkingFile { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) } #[cfg(windows)] pub fn symlink_file(src: &Path, dest: &Path) -> Result<()> { // we are supposed to not use symlink on windows Err(ErrorKind::LinkingFile { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - .into()) + src: PathBuf::from(src), + dest: PathBuf::from(dest), + }.into() + ) } pub fn copy_dir(src: &Path, dest: &Path, notify_handler: &Fn(Notification)) -> Result<()> { notify_handler(Notification::CopyingDirectory(src, dest)); raw::copy_dir(src, dest).chain_err(|| { - ErrorKind::CopyingDirectory { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::CopyingDirectory { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) } pub fn copy_file(src: &Path, dest: &Path) -> Result<()> { fs::copy(src, dest) .chain_err(|| { - ErrorKind::CopyingFile { - src: PathBuf::from(src), - dest: PathBuf::from(dest), - } - }) + ErrorKind::CopyingFile { + src: PathBuf::from(src), + dest: PathBuf::from(dest), + } + }) .map(|_| ()) } -pub fn remove_dir(name: &'static str, - path: &Path, - notify_handler: &Fn(Notification)) - -> Result<()> { +pub fn remove_dir(name: &'static str, path: &Path, notify_handler: &Fn(Notification)) -> Result<()> { notify_handler(Notification::RemovingDirectory(name, path)); raw::remove_dir(path).chain_err(|| { - ErrorKind::RemovingDirectory { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::RemovingDirectory { + name: name, + path: PathBuf::from(path), + } + }) } pub fn remove_file(name: &'static str, path: &Path) -> Result<()> { fs::remove_file(path).chain_err(|| { - ErrorKind::RemovingFile { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::RemovingFile { + name: name, + path: PathBuf::from(path), + } + }) } pub fn read_dir(name: &'static str, path: &Path) -> Result { fs::read_dir(path).chain_err(|| { - ErrorKind::ReadingDirectory { - name: name, - path: PathBuf::from(path), - } - }) + ErrorKind::ReadingDirectory { + name: name, + path: PathBuf::from(path), + } + }) } pub fn open_browser(path: &Path) -> Result<()> { match raw::open_browser(path) { Ok(true) => Ok(()), Ok(false) => Err("no browser installed".into()), - Err(e) => Err(e).chain_err(|| "could not open browser"), + Err(e) => Err(e).chain_err(|| "could not open browser") } } pub fn set_permissions(path: &Path, perms: fs::Permissions) -> Result<()> { fs::set_permissions(path, perms).chain_err(|| { - ErrorKind::SettingPermissions { - path: PathBuf::from(path), - } - }) + ErrorKind::SettingPermissions { + path: PathBuf::from(path), + } + }) } pub fn make_executable(path: &Path) -> Result<()> { @@ -379,10 +380,10 @@ pub fn make_executable(path: &Path) -> Result<()> { use std::os::unix::fs::PermissionsExt; let metadata = try!(fs::metadata(path).chain_err(|| { - ErrorKind::SettingPermissions { - path: PathBuf::from(path), - } - })); + ErrorKind::SettingPermissions { + path: PathBuf::from(path), + } + })); let mut perms = metadata.permissions(); let new_mode = (perms.mode() & !0o777) | 0o755; perms.set_mode(new_mode); @@ -403,9 +404,9 @@ pub fn current_exe() -> Result { pub fn to_absolute>(path: P) -> Result { current_dir().map(|mut v| { - v.push(path); - v - }) + v.push(path); + v + }) } // On windows, unlike std and cargo, multirust does *not* consider the @@ -434,9 +435,7 @@ pub fn home_dir() -> Option { 0 => sz, _ => sz - 1, // sz includes the null terminator } - }, - os2path) - .ok() + }, os2path).ok() }) } @@ -452,7 +451,7 @@ fn fill_utf16_buf(mut f1: F1, f2: F2) -> io::Result F2: FnOnce(&[u16]) -> T { use kernel32::{GetLastError, SetLastError}; - use winapi::ERROR_INSUFFICIENT_BUFFER; + use winapi::{ERROR_INSUFFICIENT_BUFFER}; // Start off with a stack buf but then spill over to the heap if we end up // needing more space. @@ -490,7 +489,7 @@ fn fill_utf16_buf(mut f1: F1, f2: F2) -> io::Result } else if k >= n { n = k; } else { - return Ok(f2(&buf[..k])); + return Ok(f2(&buf[..k])) } } } @@ -511,19 +510,22 @@ pub fn cargo_home() -> Result { // install to the wrong place. This check is to make the // multirust-rs to rustup upgrade seamless. let env_var = if let Some(v) = env_var { - let vv = v.to_string_lossy().to_string(); - if vv.contains(".multirust/cargo") || vv.contains(r".multirust\cargo") || - vv.trim().is_empty() { - None - } else { - Some(v) - } + let vv = v.to_string_lossy().to_string(); + if vv.contains(".multirust/cargo") || + vv.contains(r".multirust\cargo") || + vv.trim().is_empty() { + None + } else { + Some(v) + } } else { None }; let cwd = try!(env::current_dir().chain_err(|| ErrorKind::CargoHome)); - let cargo_home = env_var.clone().map(|home| cwd.join(home)); + let cargo_home = env_var.clone().map(|home| { + cwd.join(home) + }); let user_home = home_dir().map(|p| p.join(".cargo")); cargo_home.or(user_home).ok_or(ErrorKind::CargoHome.into()) } @@ -561,12 +563,15 @@ pub fn do_rustup_home_upgrade() -> bool { } fn rustup_old_version_exists() -> bool { - rustup_dir().map(|p| p.join("rustup-version").exists()).unwrap_or(false) + rustup_dir() + .map(|p| p.join("rustup-version").exists()) + .unwrap_or(false) } fn delete_rustup_dir() -> Result<()> { if let Some(dir) = rustup_dir() { - raw::remove_dir(&dir).chain_err(|| "unable to delete rustup dir")?; + raw::remove_dir(&dir) + .chain_err(|| "unable to delete rustup dir")?; } Ok(()) @@ -575,7 +580,8 @@ pub fn do_rustup_home_upgrade() -> bool { fn rename_rustup_dir_to_rustup_sh() -> Result<()> { let dirs = (rustup_dir(), rustup_sh_dir()); if let (Some(rustup), Some(rustup_sh)) = dirs { - fs::rename(&rustup, &rustup_sh).chain_err(|| "unable to rename rustup dir")?; + fs::rename(&rustup, &rustup_sh) + .chain_err(|| "unable to rename rustup dir")?; } Ok(()) @@ -584,7 +590,8 @@ pub fn do_rustup_home_upgrade() -> bool { fn rename_multirust_dir_to_rustup() -> Result<()> { let dirs = (multirust_dir(), rustup_dir()); if let (Some(rustup), Some(rustup_sh)) = dirs { - fs::rename(&rustup, &rustup_sh).chain_err(|| "unable to rename multirust dir")?; + fs::rename(&rustup, &rustup_sh) + .chain_err(|| "unable to rename multirust dir")?; } Ok(()) @@ -592,9 +599,7 @@ pub fn do_rustup_home_upgrade() -> bool { // If RUSTUP_HOME is set then its default path doesn't matter, so we're // not going to risk doing any I/O work and making a mess. - if rustup_home_is_set() { - return true; - } + if rustup_home_is_set() { return true } // Now we are just trying to get a bogus, rustup.sh-created ~/.rustup out // of the way in the manner that is least likely to take time and generate @@ -623,8 +628,7 @@ pub fn do_rustup_home_upgrade() -> bool { }; // Now we're trying to move ~/.multirust to ~/.rustup - old_rustup_dir_removed && - if multirust_dir_exists() { + old_rustup_dir_removed && if multirust_dir_exists() { if rustup_dir_exists() { // There appears to be both a ~/.multirust dir and a valid ~/.rustup // dir. Most likely because one is a symlink to the other, as configured @@ -652,12 +656,11 @@ pub fn create_rustup_home() -> Result<()> { // If RUSTUP_HOME is set then don't make any assumptions about where it's // ok to put ~/.multirust - if env::var_os("RUSTUP_HOME").is_some() { - return Ok(()); - } + if env::var_os("RUSTUP_HOME").is_some() { return Ok(()) } let home = rustup_home_in_user_dir()?; - fs::create_dir_all(&home).chain_err(|| "unable to create ~/.rustup")?; + fs::create_dir_all(&home) + .chain_err(|| "unable to create ~/.rustup")?; // This is a temporary compatibility symlink create_legacy_multirust_symlink()?; @@ -675,11 +678,9 @@ fn create_legacy_multirust_symlink() -> Result<()> { return Ok(()); } - raw::symlink_dir(&newhome, &oldhome).chain_err(|| { - format!("unable to symlink {} from {}", - newhome.display(), - oldhome.display()) - })?; + raw::symlink_dir(&newhome, &oldhome) + .chain_err(|| format!("unable to symlink {} from {}", + newhome.display(), oldhome.display()))?; Ok(()) } @@ -688,8 +689,8 @@ pub fn delete_legacy_multirust_symlink() -> Result<()> { let oldhome = legacy_multirust_home()?; if oldhome.exists() { - let meta = - fs::symlink_metadata(&oldhome).chain_err(|| "unable to get metadata for ~/.multirust")?; + let meta = fs::symlink_metadata(&oldhome) + .chain_err(|| "unable to get metadata for ~/.multirust")?; if meta.file_type().is_symlink() { // remove_dir handles unlinking symlinks raw::remove_dir(&oldhome) @@ -716,7 +717,9 @@ pub fn multirust_home() -> Result { let use_rustup_dir = do_rustup_home_upgrade(); let cwd = try!(env::current_dir().chain_err(|| ErrorKind::MultirustHome)); - let multirust_home = env::var_os("RUSTUP_HOME").map(|home| cwd.join(home)); + let multirust_home = env::var_os("RUSTUP_HOME").map(|home| { + cwd.join(home) + }); let user_home = if use_rustup_dir { dot_dir(".rustup") } else { @@ -753,8 +756,7 @@ pub fn string_from_winreg_value(val: &winreg::RegValue) -> Option { use std::slice; match val.vtype { - RegType::REG_SZ | - RegType::REG_EXPAND_SZ => { + RegType::REG_SZ | RegType::REG_EXPAND_SZ => { // Copied from winreg let words = unsafe { slice::from_raw_parts(val.bytes.as_ptr() as *const u16, val.bytes.len() / 2) @@ -764,12 +766,10 @@ pub fn string_from_winreg_value(val: &winreg::RegValue) -> Option { } else { return None; }; - while s.ends_with('\u{0}') { - s.pop(); - } + while s.ends_with('\u{0}') {s.pop();} Some(s) } - _ => None, + _ => None } } @@ -814,21 +814,25 @@ mod tests { #[test] fn test_toochain_sort() { - let expected = vec!["stable-x86_64-unknown-linux-gnu", - "beta-x86_64-unknown-linux-gnu", - "nightly-x86_64-unknown-linux-gnu", - "1.0.0-x86_64-unknown-linux-gnu", - "1.2.0-x86_64-unknown-linux-gnu", - "1.8.0-x86_64-unknown-linux-gnu", - "1.10.0-x86_64-unknown-linux-gnu"]; - - let mut v = vec!["1.8.0-x86_64-unknown-linux-gnu", - "1.0.0-x86_64-unknown-linux-gnu", - "nightly-x86_64-unknown-linux-gnu", - "stable-x86_64-unknown-linux-gnu", - "1.10.0-x86_64-unknown-linux-gnu", - "beta-x86_64-unknown-linux-gnu", - "1.2.0-x86_64-unknown-linux-gnu"]; + let expected = vec![ + "stable-x86_64-unknown-linux-gnu", + "beta-x86_64-unknown-linux-gnu", + "nightly-x86_64-unknown-linux-gnu", + "1.0.0-x86_64-unknown-linux-gnu", + "1.2.0-x86_64-unknown-linux-gnu", + "1.8.0-x86_64-unknown-linux-gnu", + "1.10.0-x86_64-unknown-linux-gnu", + ]; + + let mut v = vec![ + "1.8.0-x86_64-unknown-linux-gnu", + "1.0.0-x86_64-unknown-linux-gnu", + "nightly-x86_64-unknown-linux-gnu", + "stable-x86_64-unknown-linux-gnu", + "1.10.0-x86_64-unknown-linux-gnu", + "beta-x86_64-unknown-linux-gnu", + "1.2.0-x86_64-unknown-linux-gnu", + ]; toolchain_sort(&mut v);