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()?; 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), } } } 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", + ); }) } 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", diff --git a/tests/mock/clitools.rs b/tests/mock/clitools.rs index 6de9e65dee..58a2a1a8e3 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); @@ -427,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(),