From 93a5569c5e1117a407f8eb017294c0cd2e49ca76 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Tue, 21 Nov 2017 16:26:58 +1300 Subject: [PATCH] Add rustfmt to the tools list --- Cargo.lock | 18 +++++++-------- src/rustup-cli/self_update.rs | 27 ++++++++++++++++++++-- src/rustup-utils/src/utils.rs | 10 ++++++++ src/rustup-win-installer/src/lib.rs | 2 +- tests/cli-self-upd.rs | 36 +++++++++++++++++++++++++---- 5 files changed, 76 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5057bca291..d0fcd6812f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,12 +1,3 @@ -[root] -name = "rustup-win-installer" -version = "1.7.0" -dependencies = [ - "gcc 0.3.54 (registry+https://github.com/rust-lang/crates.io-index)", - "rustup 1.7.0", - "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", -] - [[package]] name = "adler32" version = "1.0.2" @@ -899,6 +890,15 @@ dependencies = [ "winreg 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "rustup-win-installer" +version = "1.7.0" +dependencies = [ + "gcc 0.3.54 (registry+https://github.com/rust-lang/crates.io-index)", + "rustup 1.7.0", + "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "safemem" version = "0.2.0" diff --git a/src/rustup-cli/self_update.rs b/src/rustup-cli/self_update.rs index 272e2e7480..d73c3c6e7d 100644 --- a/src/rustup-cli/self_update.rs +++ b/src/rustup-cli/self_update.rs @@ -184,7 +184,7 @@ Studio 2013 and during install select the "C++ tools": _Install the C++ build tools before proceeding_. -If you will be targetting the GNU ABI or otherwise know what you are +If you will be targeting the GNU ABI or otherwise know what you are doing then it is fine to continue installation without the build tools, but otherwise, install the C++ build tools before proceeding. "#; @@ -192,6 +192,11 @@ tools, but otherwise, install the C++ build tools before proceeding. static TOOLS: &'static [&'static str] = &["rustc", "rustdoc", "cargo", "rust-lldb", "rust-gdb", "rls"]; +// Tools which are commonly installed by Cargo as well as rustup. We take a bit +// more care with these to ensure we don't overwrite the user's previous +// installation. +static DUP_TOOLS: &'static [&'static str] = &["rustfmt", "cargo-fmt"]; + static UPDATE_ROOT: &'static str = "https://static.rust-lang.org/rustup"; @@ -646,13 +651,31 @@ fn install_bins() -> Result<()> { try!(utils::copy_file(this_exe_path, rustup_path)); try!(utils::make_executable(rustup_path)); + // Record the size of the known links, then when we get files which may or + // may not be links, we compare their size. Same size means probably a link. + let mut file_size = 0; + // Try to hardlink all the Rust exes to the rustup exe. Some systems, // like Android, does not support hardlinks, so we fallback to symlinks. for tool in TOOLS { let ref tool_path = bin_path.join(&format!("{}{}", tool, EXE_SUFFIX)); + if tool_path.exists() { + file_size = utils::file_size(tool_path)?; + } try!(utils::hard_or_symlink_file(rustup_path, tool_path)); } + for tool in DUP_TOOLS { + let ref tool_path = bin_path.join(&format!("{}{}", tool, EXE_SUFFIX)); + if tool_path.exists() && (file_size == 0 || utils::file_size(tool_path)? != file_size) { + warn!("tool `{}` is already installed, remove it from `{}`, then run `rustup update` \ + to have rustup manage this tool.", + tool, bin_path.to_string_lossy()); + } else { + try!(utils::hard_or_symlink_file(rustup_path, tool_path)); + } + } + Ok(()) } @@ -752,7 +775,7 @@ pub fn uninstall(no_prompt: bool) -> Result<()> { // Then everything in bin except rustup and tools. These can't be unlinked // until this process exits (on windows). - let tools = TOOLS.iter().map(|t| format!("{}{}", t, EXE_SUFFIX)); + let tools = TOOLS.iter().chain(DUP_TOOLS.iter()).map(|t| format!("{}{}", t, EXE_SUFFIX)); let tools: Vec<_> = tools.chain(vec![format!("rustup{}", EXE_SUFFIX)]).collect(); for dirent in try!(fs::read_dir(&cargo_home.join("bin")).chain_err(|| read_dir_err)) { let dirent = try!(dirent.chain_err(|| read_dir_err)); diff --git a/src/rustup-utils/src/utils.rs b/src/rustup-utils/src/utils.rs index 8df5704c4f..fa4bd2822e 100644 --- a/src/rustup-utils/src/utils.rs +++ b/src/rustup-utils/src/utils.rs @@ -383,6 +383,16 @@ pub fn set_permissions(path: &Path, perms: fs::Permissions) -> Result<()> { }) } +pub fn file_size(path: &Path) -> Result { + let metadata = fs::metadata(path).chain_err(|| { + ErrorKind::ReadingFile { + name: "metadata for", + path: PathBuf::from(path), + } + })?; + Ok(metadata.len()) +} + pub fn make_executable(path: &Path) -> Result<()> { #[cfg(windows)] fn inner(_: &Path) -> Result<()> { diff --git a/src/rustup-win-installer/src/lib.rs b/src/rustup-win-installer/src/lib.rs index 90d2e3bb07..beee59f3f7 100644 --- a/src/rustup-win-installer/src/lib.rs +++ b/src/rustup-win-installer/src/lib.rs @@ -16,7 +16,7 @@ pub const LOGMSG_STANDARD: i32 = 2; // TODO: share this with self_update.rs static TOOLS: &'static [&'static str] - = &["rustc", "rustdoc", "cargo", "rust-lldb", "rust-gdb", "rls"]; + = &["rustc", "rustdoc", "cargo", "rust-lldb", "rust-gdb", "rls", "rustfmt", "cargo-fmt"]; #[no_mangle] /// This is be run as a `deferred` action after `InstallFiles` on install and upgrade diff --git a/tests/cli-self-upd.rs b/tests/cli-self-upd.rs index d6fa9a72ec..c8bc9c4b45 100644 --- a/tests/cli-self-upd.rs +++ b/tests/cli-self-upd.rs @@ -27,16 +27,13 @@ use std::process::Command; use remove_dir_all::remove_dir_all; use rustup_mock::clitools::{self, Config, Scenario, expect_ok, expect_ok_ex, - expect_stdout_ok, + expect_stdout_ok, expect_stderr_ok, expect_err, expect_err_ex, this_host_triple}; use rustup_mock::dist::{calc_hash}; use rustup_mock::{get_path, restore_path}; use rustup_utils::{utils, raw}; -#[cfg(windows)] -use rustup_mock::clitools::expect_stderr_ok; - macro_rules! for_host { ($s: expr) => (&format!($s, this_host_triple())) } const TEST_VERSION: &'static str = "1.1.1"; @@ -49,7 +46,7 @@ pub fn setup(f: &Fn(&Config)) { } let _g = LOCK.lock(); - // An windows these tests mess with the user's PATH. Save + // On windows these tests mess with the user's PATH. Save // and restore them here to keep from trashing things. let saved_path = get_path(); let _g = scopeguard::guard(saved_path, |p| restore_path(p)); @@ -1249,3 +1246,32 @@ fn rls_proxy_set_up_after_update() { assert!(rls_path.exists()); }); } + + +#[test] +fn update_does_not_overwrite_rustfmt() { + update_setup(&|config, _| { + expect_ok(config, &["rustup-init", "-y"]); + let ref rustfmt_path = config.cargodir.join(format!("bin/rustfmt{}", EXE_SUFFIX)); + raw::write_file(rustfmt_path, "").unwrap(); + assert_eq!(utils::file_size(rustfmt_path).unwrap(), 0); + + expect_stderr_ok(config, &["rustup", "self", "update"], + "`rustfmt` is already installed"); + expect_ok(config, &["rustup", "self", "update"]); + assert!(rustfmt_path.exists()); + assert_eq!(utils::file_size(rustfmt_path).unwrap(), 0); + + + // We run the test twice because the first time around none of the shims + // exist, and we want to check that out check for rustfmt works if there + // are shims or not. + let ref rustdoc_path = config.cargodir.join(format!("bin/rustdoc{}", EXE_SUFFIX)); + assert!(rustdoc_path.exists()); + + expect_stderr_ok(config, &["rustup", "self", "update"], + "`rustfmt` is already installed"); + assert!(rustfmt_path.exists()); + assert_eq!(utils::file_size(rustfmt_path).unwrap(), 0); + }); +}