Skip to content

Commit b3ba955

Browse files
committed
rustup-init: Support updating the installed toolchain
If the user specifies any of --default-toolchain (not none) or --target or --component when invoking `rustup-init` then we will update the existing installation toolchains, rather than the previous behaviour which was to skip toolchain installation. This is nominally a breaking change but likely ends up adhering closer to the principle of least surprise. At this point, the profile will not be applied, so a change of profile from minimal to default will not cause additional components to end up being installed. Signed-off-by: Daniel Silverstone <[email protected]>
1 parent fff860b commit b3ba955

File tree

3 files changed

+68
-23
lines changed

3 files changed

+68
-23
lines changed

src/cli/self_update.rs

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ use std::process::{self, Command};
4747

4848
pub struct InstallOpts<'a> {
4949
pub default_host_triple: Option<String>,
50-
pub default_toolchain: String,
50+
pub default_toolchain: Option<String>,
5151
pub profile: String,
5252
pub no_modify_path: bool,
5353
pub components: &'a [&'a str],
@@ -283,9 +283,9 @@ pub fn install(no_prompt: bool, verbose: bool, quiet: bool, mut opts: InstallOpt
283283
}
284284
utils::create_rustup_home()?;
285285
maybe_install_rust(
286-
&opts.default_toolchain,
286+
opts.default_toolchain.as_deref(),
287287
&opts.profile,
288-
opts.default_host_triple.as_ref(),
288+
opts.default_host_triple.as_deref(),
289289
opts.components,
290290
opts.targets,
291291
verbose,
@@ -440,10 +440,10 @@ fn do_pre_install_options_sanity_checks(opts: &InstallOpts) -> Result<()> {
440440
.as_ref()
441441
.map(|s| dist::TargetTriple::new(s))
442442
.unwrap_or_else(TargetTriple::from_host_or_build);
443-
let toolchain_to_use = if opts.default_toolchain == "none" {
444-
"stable"
445-
} else {
446-
&opts.default_toolchain
443+
let toolchain_to_use = match &opts.default_toolchain {
444+
None => "stable",
445+
Some(s) if s == "none" => "stable",
446+
Some(s) => &s,
447447
};
448448
let partial_channel = dist::PartialToolchainDesc::from_str(toolchain_to_use)?;
449449
let resolved = partial_channel.resolve(&host_triple)?.to_string();
@@ -590,7 +590,9 @@ fn current_install_opts(opts: &InstallOpts) -> String {
590590
.as_ref()
591591
.map(|s| TargetTriple::new(s))
592592
.unwrap_or_else(TargetTriple::from_host_or_build),
593-
opts.default_toolchain,
593+
opts.default_toolchain
594+
.as_deref()
595+
.unwrap_or("stable (default)"),
594596
opts.profile,
595597
if !opts.no_modify_path { "yes" } else { "no" }
596598
)
@@ -612,10 +614,10 @@ fn customize_install(mut opts: InstallOpts) -> Result<InstallOpts> {
612614
.unwrap_or_else(|| TargetTriple::from_host_or_build().to_string()),
613615
)?);
614616

615-
opts.default_toolchain = common::question_str(
617+
opts.default_toolchain = Some(common::question_str(
616618
"Default toolchain? (stable/beta/nightly/none)",
617-
&opts.default_toolchain,
618-
)?;
619+
opts.default_toolchain.as_deref().unwrap_or("stable"),
620+
)?);
619621

620622
opts.profile = common::question_str(
621623
&format!(
@@ -727,9 +729,9 @@ pub fn install_proxies() -> Result<()> {
727729
}
728730

729731
fn maybe_install_rust(
730-
toolchain_str: &str,
732+
toolchain: Option<&str>,
731733
profile_str: &str,
732-
default_host_triple: Option<&String>,
734+
default_host_triple: Option<&str>,
733735
components: &[&str],
734736
targets: &[&str],
735737
verbose: bool,
@@ -746,21 +748,29 @@ fn maybe_install_rust(
746748
info!("default host triple is {}", cfg.get_default_host_triple()?);
747749
}
748750

749-
// If there is already an install, then `toolchain_str` may not be
750-
// a toolchain the user actually wants. Don't do anything. FIXME:
751-
// This logic should be part of InstallOpts so that it isn't
752-
// possible to select a toolchain then have it not be installed.
753-
if toolchain_str == "none" {
751+
let user_specified_something =
752+
toolchain.is_some() || !targets.is_empty() || !components.is_empty();
753+
754+
// If the user specified they want no toolchain, we skip this, otherwise
755+
// if they specify something directly, or we have no default, then we install
756+
// a toolchain (updating if it's already present) and then if neither of
757+
// those are true, we have a user who doesn't mind, and already has an
758+
// install, so we leave their setup alone.
759+
if toolchain == Some("none") {
754760
info!("skipping toolchain installation");
755761
println!();
756-
} else if cfg.find_default()?.is_none() {
762+
} else if user_specified_something || cfg.find_default()?.is_none() {
763+
let toolchain_str = toolchain.unwrap_or("stable");
757764
let toolchain = cfg.get_toolchain(toolchain_str, false)?;
765+
if toolchain.exists() {
766+
warn!("Updating existing toolchain, profile choice will be ignored");
767+
}
758768
let status = toolchain.install_from_dist(true, false, components, targets)?;
759769
cfg.set_default(toolchain_str)?;
760770
println!();
761771
common::show_channel_update(&cfg, toolchain_str, Ok(status))?;
762772
} else {
763-
info!("updating existing rustup installation");
773+
info!("updating existing rustup installation - leaving toolchains alone");
764774
println!();
765775
}
766776

src/cli/setup_mode.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ pub fn main() -> Result<()> {
8888
let verbose = matches.is_present("verbose");
8989
let quiet = matches.is_present("quiet");
9090
let default_host = matches.value_of("default-host").map(ToOwned::to_owned);
91-
let default_toolchain = matches.value_of("default-toolchain").unwrap_or("stable");
91+
let default_toolchain = matches.value_of("default-toolchain").map(ToOwned::to_owned);
9292
let profile = matches
9393
.value_of("profile")
9494
.expect("Unreachable: Clap should supply a default");

tests/cli-self-upd.rs

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -892,8 +892,43 @@ fn reinstall_exact() {
892892
expect_stderr_ok(
893893
config,
894894
&["rustup-init", "-y"],
895-
r"info: updating existing rustup installation
896-
",
895+
r"info: updating existing rustup installation - leaving toolchains alone",
896+
);
897+
});
898+
}
899+
900+
#[test]
901+
fn reinstall_specifying_toolchain() {
902+
setup(&|config| {
903+
expect_ok(config, &["rustup-init", "-y"]);
904+
expect_stdout_ok(
905+
config,
906+
&["rustup-init", "-y", "--default-toolchain=stable"],
907+
r"stable unchanged - 1.1.0",
908+
);
909+
});
910+
}
911+
912+
#[test]
913+
fn reinstall_specifying_component() {
914+
setup(&|config| {
915+
expect_ok(config, &["rustup-init", "-y", "--component=rls"]);
916+
expect_stdout_ok(
917+
config,
918+
&["rustup-init", "-y", "--default-toolchain=stable"],
919+
r"stable unchanged - 1.1.0",
920+
);
921+
});
922+
}
923+
924+
#[test]
925+
fn reinstall_specifying_different_toolchain() {
926+
setup(&|config| {
927+
expect_ok(config, &["rustup-init", "-y"]);
928+
expect_stderr_ok(
929+
config,
930+
&["rustup-init", "-y", "--default-toolchain=nightly"],
931+
r"info: default toolchain set to 'nightly'",
897932
);
898933
});
899934
}

0 commit comments

Comments
 (0)