Merge pull request #2162 from kinnison/demote-deprecation

Demote deprecation

Author: kinnison

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<F, A, B>(instead: &str, cfg: A, matches: B, callee: F) -> Result<()>
+fn deprecated<F, B>(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()?;

src/notifications.rs

@@ -32,6 +32,7 @@ pub enum Notification<'a> {
     NonFatalError(&'a Error),
     UpgradeRemovesToolchains,
     MissingFileDuringSelfUninstall(PathBuf),
+    PlainVerboseMessage(&'a str),
 }
 
 impl<'a> From<crate::dist::Notification<'a>> 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),
         }
     }
 }

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",
+        );
     })
 }

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",

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(),