Skip to content

Commit

Permalink
fix!(config): re-enable active toolchain installation in `Cfg::find_a…
Browse files Browse the repository at this point in the history
…ctive_toolchain()` with optional opt-out
  • Loading branch information
rami3l authored and djc committed Mar 4, 2025
1 parent 8431f90 commit 95da617
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 43 deletions.
2 changes: 1 addition & 1 deletion src/cli/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ pub(crate) async fn list_toolchains(
let default_toolchain_name = cfg.get_default()?;
let active_toolchain_name: Option<ToolchainName> =
if let Ok(Some((LocalToolchainName::Named(toolchain), _reason))) =
cfg.find_active_toolchain().await
cfg.find_active_toolchain(None).await
{
Some(toolchain)
} else {
Expand Down
8 changes: 4 additions & 4 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ async fn default_(
}
};

if let Some((toolchain, reason)) = cfg.find_active_toolchain().await? {
if let Some((toolchain, reason)) = cfg.find_active_toolchain(None).await? {
if !matches!(reason, ActiveReason::Default) {
info!("note that the toolchain '{toolchain}' is currently in use ({reason})");
}
Expand Down Expand Up @@ -964,7 +964,7 @@ async fn show(cfg: &Cfg<'_>, verbose: bool) -> Result<utils::ExitCode> {
let installed_toolchains = cfg.list_toolchains()?;
let active_toolchain_and_reason: Option<(ToolchainName, ActiveReason)> =
if let Ok(Some((LocalToolchainName::Named(toolchain_name), reason))) =
cfg.find_active_toolchain().await
cfg.find_active_toolchain(None).await
{
Some((toolchain_name, reason))
} else {
Expand Down Expand Up @@ -1084,7 +1084,7 @@ async fn show(cfg: &Cfg<'_>, verbose: bool) -> Result<utils::ExitCode> {

#[tracing::instrument(level = "trace", skip_all)]
async fn show_active_toolchain(cfg: &Cfg<'_>, verbose: bool) -> Result<utils::ExitCode> {
match cfg.find_active_toolchain().await? {
match cfg.find_active_toolchain(None).await? {
Some((toolchain_name, reason)) => {
let toolchain = Toolchain::with_reason(cfg, toolchain_name.clone(), &reason)?;
if verbose {
Expand Down Expand Up @@ -1322,7 +1322,7 @@ async fn toolchain_link(
async fn toolchain_remove(cfg: &mut Cfg<'_>, opts: UninstallOpts) -> Result<utils::ExitCode> {
let default_toolchain = cfg.get_default().ok().flatten();
let active_toolchain = cfg
.find_active_toolchain()
.find_active_toolchain(Some(false))
.await
.ok()
.flatten()
Expand Down
68 changes: 58 additions & 10 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,15 +516,63 @@ impl<'a> Cfg<'a> {

pub(crate) async fn find_active_toolchain(
&self,
force_install_active: Option<bool>,
) -> Result<Option<(LocalToolchainName, ActiveReason)>> {
Ok(
if let Some((override_config, reason)) = self.find_override_config()? {
Some((override_config.into_local_toolchain_name(), reason))
} else {
self.get_default()?
.map(|x| (x.into(), ActiveReason::Default))
},
)
let (components, targets, profile, toolchain, reason) = match self.find_override_config()? {
Some((
OverrideCfg::Official {
components,
targets,
profile,
toolchain,
},
reason,
)) => (components, targets, profile, toolchain, reason),
Some((override_config, reason)) => {
return Ok(Some((override_config.into_local_toolchain_name(), reason)));
}
None => {
return Ok(self
.get_default()?
.map(|x| (x.into(), ActiveReason::Default)));
}
};

let should_install_active = force_install_active.unwrap_or_else(|| {
self.process
.var("RUSTUP_AUTO_INSTALL")
.map_or(true, |it| it != "0")
});

if !should_install_active {
return Ok(Some(((&toolchain).into(), reason)));
}

let components = components.iter().map(AsRef::as_ref).collect::<Vec<_>>();
let targets = targets.iter().map(AsRef::as_ref).collect::<Vec<_>>();
match DistributableToolchain::new(self, toolchain.clone()) {
Err(RustupError::ToolchainNotInstalled { .. }) => {
DistributableToolchain::install(
self,
&toolchain,
&components,
&targets,
profile.unwrap_or_default(),
false,
)
.await?;
}
Ok(mut distributable) => {
if !distributable.components_exist(&components, &targets)? {
distributable
.update(&components, &targets, profile.unwrap_or_default())
.await?;
}
}
Err(e) => return Err(e.into()),
};

Ok(Some(((&toolchain).into(), reason)))
}

fn find_override_config(&self) -> Result<Option<(OverrideCfg, ActiveReason)>> {
Expand Down Expand Up @@ -709,7 +757,7 @@ impl<'a> Cfg<'a> {
self.set_toolchain_override(&ResolvableToolchainName::try_from(&t[1..])?);
}

let Some((name, _)) = self.find_active_toolchain().await? else {
let Some((name, _)) = self.find_active_toolchain(None).await? else {
return Ok(None);
};
Ok(Some(Toolchain::new(self, name)?.rustc_version()))
Expand Down Expand Up @@ -739,7 +787,7 @@ impl<'a> Cfg<'a> {
let toolchain = match name {
Some(tc) => tc,
None => {
self.find_active_toolchain()
self.find_active_toolchain(None)
.await?
.ok_or_else(|| no_toolchain_error(self.process))?
.0
Expand Down
20 changes: 8 additions & 12 deletions tests/suite/cli_rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1052,12 +1052,12 @@ async fn show_toolchain_override_not_installed() {
.expect_ok(&["rustup", "toolchain", "remove", "nightly"])
.await;
let out = cx.config.run("rustup", ["show"], &[]).await;
assert!(!out.ok);
assert!(out.ok);
assert!(
out.stderr
!out.stderr
.contains("is not installed: the directory override for")
);
assert!(!out.stderr.contains("info: installing component 'rustc'"));
assert!(out.stderr.contains("info: installing component 'rustc'"));
}

#[tokio::test]
Expand Down Expand Up @@ -1137,7 +1137,7 @@ async fn show_toolchain_env_not_installed() {
.run("rustup", ["show"], &[("RUSTUP_TOOLCHAIN", "nightly")])
.await;

assert!(!out.ok);
assert!(out.ok);

let expected_out = for_host_and_home!(
cx.config,
Expand All @@ -1149,17 +1149,13 @@ installed toolchains
active toolchain
----------------
name: nightly-{0}
active because: overridden by environment variable RUSTUP_TOOLCHAIN
installed targets:
{0}
"
);
assert!(&out.stdout == expected_out);
assert!(
out.stderr
== format!(
"error: override toolchain 'nightly-{}' is not installed: \
the RUSTUP_TOOLCHAIN environment variable specifies an uninstalled toolchain\n",
this_host_triple()
)
);
}

#[tokio::test]
Expand Down
10 changes: 3 additions & 7 deletions tests/suite/cli_v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,10 @@ async fn remove_override_toolchain_err_handling() {
.expect_ok(&["rustup", "toolchain", "remove", "beta"])
.await;
cx.config
.expect_err_ex(
.expect_ok_contains(
&["rustc", "--version"],
"",
for_host!(
r"error: toolchain 'beta-{0}' is not installed
help: run `rustup toolchain install beta-{0}` to install it
"
),
"1.2.0 (hash-beta-1.2.0)",
"info: downloading component 'rust'",
)
.await;
}
Expand Down
15 changes: 6 additions & 9 deletions tests/suite/cli_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,14 +327,10 @@ async fn remove_override_toolchain_err_handling() {
.expect_ok(&["rustup", "toolchain", "remove", "beta"])
.await;
cx.config
.expect_err_ex(
.expect_ok_contains(
&["rustc", "--version"],
"",
for_host!(
r"error: toolchain 'beta-{0}' is not installed
help: run `rustup toolchain install beta-{0}` to install it
"
),
"1.2.0 (hash-beta-1.2.0)",
"info: downloading component 'rustc'",
)
.await;
}
Expand All @@ -346,9 +342,10 @@ async fn file_override_toolchain_err_handling() {
let toolchain_file = cwd.join("rust-toolchain");
rustup::utils::raw::write_file(&toolchain_file, "beta").unwrap();
cx.config
.expect_err(
.expect_ok_contains(
&["rustc", "--version"],
for_host!("toolchain 'beta-{0}' is not installed"),
"1.2.0 (hash-beta-1.2.0)",
"info: downloading component 'rustc'",
)
.await;
}
Expand Down

0 comments on commit 95da617

Please sign in to comment.