From 416db312e8f2ed8f7f16ccb58668040d3447f75b Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Mon, 16 Dec 2019 20:57:17 +0000 Subject: [PATCH 1/6] clitools: Fix support for checking stderr in ok/err The expect_not_stderr_ok() implementation incorrectly expected an error. This commit introduces expect_not_stderr_err() for that specific case, and fixes the test in expect_not_stderr_ok() Signed-off-by: Daniel Silverstone --- tests/mock/clitools.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/mock/clitools.rs b/tests/mock/clitools.rs index 6de9e65dee..84ad45f895 100644 --- a/tests/mock/clitools.rs +++ b/tests/mock/clitools.rs @@ -229,6 +229,16 @@ pub fn expect_not_stdout_ok(config: &Config, args: &[&str], expected: &str) { } pub fn expect_not_stderr_ok(config: &Config, args: &[&str], expected: &str) { + let out = run(config, args[0], &args[1..], &[]); + if !out.ok || out.stderr.contains(expected) { + print_command(args, &out); + println!("expected.ok: false"); + print_indented("expected.stderr.does_not_contain", expected); + panic!(); + } +} + +pub fn expect_not_stderr_err(config: &Config, args: &[&str], expected: &str) { let out = run(config, args[0], &args[1..], &[]); if out.ok || out.stderr.contains(expected) { print_command(args, &out); From 46214a2ad2735dd586f14f9657502ecc0acbd1d5 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Mon, 16 Dec 2019 20:58:29 +0000 Subject: [PATCH 2/6] cli-v2: Fix uses of expect_not_stderr_ok These tests actually wanted to be expect_not_stderr_err Signed-off-by: Daniel Silverstone --- tests/cli-v2.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/cli-v2.rs b/tests/cli-v2.rs index 89d16d9c2c..d5853220e9 100644 --- a/tests/cli-v2.rs +++ b/tests/cli-v2.rs @@ -5,7 +5,7 @@ pub mod mock; use crate::mock::clitools::{ self, expect_component_executable, expect_component_not_executable, expect_err, - expect_not_stderr_ok, expect_not_stdout_ok, expect_ok, expect_ok_ex, expect_stderr_ok, + expect_not_stderr_err, expect_not_stdout_ok, expect_ok, expect_ok_ex, expect_stderr_ok, expect_stdout_ok, set_current_dist_date, this_host_triple, Config, Scenario, }; use std::fs; @@ -1124,7 +1124,7 @@ fn add_component_suggest_best_match() { &["rustup", "component", "add", "rustd"], "did you mean 'rustc'?", ); - expect_not_stderr_ok( + expect_not_stderr_err( config, &["rustup", "component", "add", "potato"], "did you mean", @@ -1136,7 +1136,7 @@ fn add_component_suggest_best_match() { fn remove_component_suggest_best_match() { setup(&|config| { expect_ok(config, &["rustup", "default", "nightly"]); - expect_not_stderr_ok( + expect_not_stderr_err( config, &["rustup", "component", "remove", "rsl"], "did you mean 'rls'?", @@ -1175,7 +1175,7 @@ fn add_target_suggest_best_match() { ], &format!("did you mean '{}'", clitools::CROSS_ARCH1), ); - expect_not_stderr_ok( + expect_not_stderr_err( config, &["rustup", "target", "add", "potato"], "did you mean", @@ -1187,7 +1187,7 @@ fn add_target_suggest_best_match() { fn remove_target_suggest_best_match() { setup(&|config| { expect_ok(config, &["rustup", "default", "nightly"]); - expect_not_stderr_ok( + expect_not_stderr_err( config, &[ "rustup", From 11899ced7fe55a95d2b5f390c0c1018932ce47f7 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Mon, 16 Dec 2019 21:00:06 +0000 Subject: [PATCH 3/6] notifications: Add PlainVerboseMessage notification So that we can send simple verbose messages at runtime, add a plain verbose notification message type. Signed-off-by: Daniel Silverstone --- src/notifications.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/notifications.rs b/src/notifications.rs index ece041a24c..3722c1f5c0 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -32,6 +32,7 @@ pub enum Notification<'a> { NonFatalError(&'a Error), UpgradeRemovesToolchains, MissingFileDuringSelfUninstall(PathBuf), + PlainVerboseMessage(&'a str), } impl<'a> From> for Notification<'a> { @@ -64,6 +65,7 @@ impl<'a> Notification<'a> { | UpdatingToolchain(_) | ReadMetadataVersion(_) | InstalledToolchain(_) + | PlainVerboseMessage(_) | UpdateHashMatches => NotificationLevel::Verbose, SetDefaultToolchain(_) | SetOverrideToolchain(_, _) @@ -127,6 +129,7 @@ impl<'a> Display for Notification<'a> { "expected file does not exist to uninstall: {}", p.display() ), + PlainVerboseMessage(r) => write!(f, "{}", r), } } } From 4f0f6b1118f10101d5a15615d0348615f939c280 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Mon, 16 Dec 2019 21:00:39 +0000 Subject: [PATCH 4/6] rustup_mode: Change deprecation messages from warnings to verbose We decided that the deprecation messages ought to be verbose messages (i.e. only in --verbose mode) rather than warnings. Fixes: #2149 Signed-off-by: Daniel Silverstone --- src/cli/rustup_mode.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 65ed42ffac..8f439223cb 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -9,6 +9,7 @@ use clap::{App, AppSettings, Arg, ArgGroup, ArgMatches, Shell, SubCommand}; use rustup::dist::dist::{PartialTargetTriple, PartialToolchainDesc, Profile, TargetTriple}; use rustup::dist::manifest::Component; use rustup::utils::utils::{self, ExitCode}; +use rustup::Notification; use rustup::{command, Cfg, Toolchain}; use std::error::Error; use std::fmt; @@ -27,12 +28,23 @@ fn handle_epipe(res: Result<()>) -> Result<()> { } } -fn deprecated(instead: &str, cfg: A, matches: B, callee: F) -> Result<()> +fn deprecated(instead: &str, cfg: &mut Cfg, matches: B, callee: F) -> Result<()> where - F: FnOnce(A, B) -> Result<()>, + F: FnOnce(&mut Cfg, B) -> Result<()>, { - warn!("Use of deprecated command line interface."); - warn!(" Please use `rustup {}` instead", instead); + (cfg.notify_handler)(Notification::PlainVerboseMessage( + "Use of (currently) unmaintained command line interface.", + )); + (cfg.notify_handler)(Notification::PlainVerboseMessage( + "The exact API of this command may change without warning", + )); + (cfg.notify_handler)(Notification::PlainVerboseMessage( + "Eventually this command will be a true alias. Until then:", + )); + (cfg.notify_handler)(Notification::PlainVerboseMessage(&format!( + " Please use `rustup {}` instead", + instead + ))); callee(cfg, matches) } @@ -66,7 +78,7 @@ pub fn main() -> Result<()> { ("install", Some(m)) => deprecated("toolchain install", cfg, m, update)?, ("update", Some(m)) => update(cfg, m)?, ("check", Some(_)) => check_updates(cfg)?, - ("uninstall", Some(m)) => deprecated("toolchain uninstall", &*cfg, m, toolchain_remove)?, + ("uninstall", Some(m)) => deprecated("toolchain uninstall", cfg, m, toolchain_remove)?, ("default", Some(m)) => default_(cfg, m)?, ("toolchain", Some(c)) => match c.subcommand() { ("install", Some(m)) => update(cfg, m)?, @@ -1200,7 +1212,7 @@ fn toolchain_link(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> { .map_err(Into::into) } -fn toolchain_remove(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> { +fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result<()> { for toolchain in m.values_of("toolchain").unwrap() { let toolchain = cfg.get_toolchain(toolchain, false)?; toolchain.remove()?; From 255dad7b5ad52d9c161d4bf04593ab3ca62d5a04 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Mon, 16 Dec 2019 21:01:29 +0000 Subject: [PATCH 5/6] cli-misc: Update deprecated_interfaces test for new behaviour Signed-off-by: Daniel Silverstone --- tests/cli-misc.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/tests/cli-misc.rs b/tests/cli-misc.rs index 487e5a29fd..d8dedac179 100644 --- a/tests/cli-misc.rs +++ b/tests/cli-misc.rs @@ -4,9 +4,10 @@ pub mod mock; use crate::mock::clitools::{ - self, expect_component_executable, expect_component_not_executable, expect_err, expect_ok, - expect_ok_contains, expect_ok_eq, expect_ok_ex, expect_stderr_ok, expect_stdout_ok, run, - set_current_dist_date, this_host_triple, Config, Scenario, + self, expect_component_executable, expect_component_not_executable, expect_err, + expect_not_stderr_ok, expect_ok, expect_ok_contains, expect_ok_eq, expect_ok_ex, + expect_stderr_ok, expect_stdout_ok, run, set_current_dist_date, this_host_triple, Config, + Scenario, }; use rustup::utils::{raw, utils}; @@ -998,17 +999,35 @@ fn toolchain_link_then_list_verbose() { #[test] fn deprecated_interfaces() { setup(&|config| { + // In verbose mode we want the deprecated interfaces to complain expect_ok_contains( config, - &["rustup", "install", "nightly", "--no-self-update"], + &[ + "rustup", + "--verbose", + "install", + "nightly", + "--no-self-update", + ], "", "Please use `rustup toolchain install` instead", ); expect_ok_contains( config, - &["rustup", "uninstall", "nightly"], + &["rustup", "--verbose", "uninstall", "nightly"], "", "Please use `rustup toolchain uninstall` instead", ); + // But if not verbose then they should *NOT* complain + expect_not_stderr_ok( + config, + &["rustup", "install", "nightly", "--no-self-update"], + "Please use `rustup toolchain install` instead", + ); + expect_not_stderr_ok( + config, + &["rustup", "uninstall", "nightly"], + "Please use `rustup toolchain uninstall` instead", + ); }) } From 4560e851eb055a07497a4af06db8637d38a7a743 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Mon, 16 Dec 2019 21:10:59 +0000 Subject: [PATCH 6/6] clitools: Attempt to mitigate ETXTBSY some more Somehow sometimes we still get ETXTBSY on Travis test suite runs This attempts to mitigate that a little more Signed-off-by: Daniel Silverstone --- tests/mock/clitools.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/tests/mock/clitools.rs b/tests/mock/clitools.rs index 84ad45f895..58a2a1a8e3 100644 --- a/tests/mock/clitools.rs +++ b/tests/mock/clitools.rs @@ -437,10 +437,27 @@ where } println!("running {:?}", cmd); - let lock = cmd_lock().read().unwrap(); - let out = cmd.output(); - drop(lock); - let out = out.expect("failed to run test command"); + let mut retries = 8; + let out = loop { + let lock = cmd_lock().read().unwrap(); + let out = cmd.output(); + drop(lock); + match out { + Ok(out) => break out, + Err(e) => { + retries -= 1; + if retries > 0 + && e.kind() == std::io::ErrorKind::Other + && format!("{}", e).contains("os error 26") + { + // This is a ETXTBSY situation + std::thread::sleep(std::time::Duration::from_millis(250)); + } else { + panic!("Unable to run test command: {:?}", e); + } + } + } + }; let output = SanitizedOutput { ok: out.status.success(),