Skip to content

Demote deprecation #2162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
}

Expand Down Expand Up @@ -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)?,
Expand Down Expand Up @@ -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()?;
Expand Down
3 changes: 3 additions & 0 deletions src/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -64,6 +65,7 @@ impl<'a> Notification<'a> {
| UpdatingToolchain(_)
| ReadMetadataVersion(_)
| InstalledToolchain(_)
| PlainVerboseMessage(_)
| UpdateHashMatches => NotificationLevel::Verbose,
SetDefaultToolchain(_)
| SetOverrideToolchain(_, _)
Expand Down Expand Up @@ -127,6 +129,7 @@ impl<'a> Display for Notification<'a> {
"expected file does not exist to uninstall: {}",
p.display()
),
PlainVerboseMessage(r) => write!(f, "{}", r),
}
}
}
29 changes: 24 additions & 5 deletions tests/cli-misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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",
);
})
}
10 changes: 5 additions & 5 deletions tests/cli-v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand All @@ -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'?",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
35 changes: 31 additions & 4 deletions tests/mock/clitools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(),
Expand Down