Skip to content
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

feat(cli)!: remove implicit toolchain installation #3985

Merged
merged 10 commits into from
Aug 9, 2024
2 changes: 1 addition & 1 deletion src/cli/proxy_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ pub async fn main(arg0: &str, current_dir: PathBuf, process: &Process) -> Result
.collect();

let cfg = set_globals(current_dir, false, true, process)?;
let cmd = cfg.local_toolchain(toolchain).await?.command(arg0)?;
let cmd = cfg.resolve_local_toolchain(toolchain)?.command(arg0)?;
run_command_for_dir(cmd, arg0, &cmd_args)
}
18 changes: 9 additions & 9 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ async fn which(
binary: &str,
toolchain: Option<ResolvableToolchainName>,
) -> Result<utils::ExitCode> {
let binary_path = cfg.resolve_toolchain(toolchain).await?.binary_file(binary);
let binary_path = cfg.resolve_toolchain(toolchain)?.binary_file(binary);

utils::assert_is_file(&binary_path)?;

Expand Down Expand Up @@ -1097,7 +1097,7 @@ async fn target_list(
quiet: bool,
) -> Result<utils::ExitCode> {
// downcasting required because the toolchain files can name any toolchain
let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?;
let distributable = DistributableToolchain::from_partial(toolchain, cfg)?;
common::list_items(
distributable,
|c| {
Expand All @@ -1124,7 +1124,7 @@ async fn target_add(
// isn't a feature yet.
// list_components *and* add_component would both be inappropriate for
// custom toolchains.
let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?;
let distributable = DistributableToolchain::from_partial(toolchain, cfg)?;
let components = distributable.components()?;

if targets.contains(&"all".to_string()) {
Expand Down Expand Up @@ -1168,7 +1168,7 @@ async fn target_remove(
targets: Vec<String>,
toolchain: Option<PartialToolchainDesc>,
) -> Result<utils::ExitCode> {
let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?;
let distributable = DistributableToolchain::from_partial(toolchain, cfg)?;

for target in targets {
let target = TargetTriple::new(target);
Expand Down Expand Up @@ -1204,7 +1204,7 @@ async fn component_list(
quiet: bool,
) -> Result<utils::ExitCode> {
// downcasting required because the toolchain files can name any toolchain
let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?;
let distributable = DistributableToolchain::from_partial(toolchain, cfg)?;
common::list_items(
distributable,
|c| Some(&c.name),
Expand All @@ -1220,7 +1220,7 @@ async fn component_add(
toolchain: Option<PartialToolchainDesc>,
target: Option<String>,
) -> Result<utils::ExitCode> {
let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?;
let distributable = DistributableToolchain::from_partial(toolchain, cfg)?;
let target = get_target(target, &distributable);

for component in &components {
Expand All @@ -1246,7 +1246,7 @@ async fn component_remove(
toolchain: Option<PartialToolchainDesc>,
target: Option<String>,
) -> Result<utils::ExitCode> {
let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?;
let distributable = DistributableToolchain::from_partial(toolchain, cfg)?;
let target = get_target(target, &distributable);

for component in &components {
Expand Down Expand Up @@ -1447,7 +1447,7 @@ async fn doc(
mut topic: Option<&str>,
doc_page: &DocPage,
) -> Result<utils::ExitCode> {
let toolchain = cfg.toolchain_from_partial(toolchain).await?;
let toolchain = cfg.toolchain_from_partial(toolchain)?;

if let Ok(distributable) = DistributableToolchain::try_from(&toolchain) {
if let [_] = distributable
Expand Down Expand Up @@ -1508,7 +1508,7 @@ async fn man(
command: &str,
toolchain: Option<PartialToolchainDesc>,
) -> Result<utils::ExitCode> {
let toolchain = cfg.toolchain_from_partial(toolchain).await?;
let toolchain = cfg.toolchain_from_partial(toolchain)?;
let path = toolchain.man_path();
utils::assert_is_directory(&path)?;

Expand Down
43 changes: 25 additions & 18 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,20 +498,18 @@ impl<'a> Cfg<'a> {
.transpose()?)
}

pub(crate) async fn toolchain_from_partial(
pub(crate) fn toolchain_from_partial(
&self,
toolchain: Option<PartialToolchainDesc>,
) -> anyhow::Result<Toolchain<'_>> {
match toolchain {
let toolchain = match toolchain {
rami3l marked this conversation as resolved.
Show resolved Hide resolved
Some(toolchain) => {
let desc = toolchain.resolve(&self.get_default_host_triple()?)?;
Ok(Toolchain::new(
self,
LocalToolchainName::Named(ToolchainName::Official(desc)),
)?)
Some(LocalToolchainName::Named(ToolchainName::Official(desc)))
}
None => Ok(self.find_or_install_active_toolchain(false).await?.0),
}
None => None,
};
self.local_toolchain(toolchain)
}

pub(crate) fn find_active_toolchain(
Expand Down Expand Up @@ -709,31 +707,40 @@ impl<'a> Cfg<'a> {
Ok(Some(Toolchain::new(self, name)?.rustc_version()))
}

pub(crate) async fn resolve_toolchain(
pub(crate) fn resolve_toolchain(
&self,
name: Option<ResolvableToolchainName>,
) -> Result<Toolchain<'_>> {
Ok(match name {
let toolchain = match name {
rami3l marked this conversation as resolved.
Show resolved Hide resolved
Some(name) => {
let desc = name.resolve(&self.get_default_host_triple()?)?;
Toolchain::new(self, desc.into())?
Some(desc.into())
}
None => self.find_or_install_active_toolchain(false).await?.0,
})
None => None,
};
self.local_toolchain(toolchain)
}

pub(crate) async fn local_toolchain(
pub(crate) fn resolve_local_toolchain(
&self,
name: Option<ResolvableLocalToolchainName>,
) -> Result<Toolchain<'_>> {
let local = name
.map(|name| name.resolve(&self.get_default_host_triple()?))
.transpose()?;
self.local_toolchain(local)
}

Ok(match local {
Some(tc) => Toolchain::from_local(tc, false, self).await?,
None => self.find_or_install_active_toolchain(false).await?.0,
})
fn local_toolchain(&self, name: Option<LocalToolchainName>) -> Result<Toolchain<'_>> {
let toolchain = match name {
Some(tc) => tc,
None => {
self.find_active_toolchain()?
.ok_or_else(|| no_toolchain_error(self.process))?
.0
}
};
Ok(Toolchain::new(self, toolchain)?)
}

#[tracing::instrument(level = "trace", skip_all)]
Expand Down
6 changes: 2 additions & 4 deletions src/toolchain/distributable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,11 @@ pub(crate) struct DistributableToolchain<'a> {
}

impl<'a> DistributableToolchain<'a> {
pub(crate) async fn from_partial(
pub(crate) fn from_partial(
toolchain: Option<PartialToolchainDesc>,
cfg: &'a Cfg<'a>,
) -> anyhow::Result<Self> {
Ok(Self::try_from(
&cfg.toolchain_from_partial(toolchain).await?,
)?)
Ok(Self::try_from(&cfg.toolchain_from_partial(toolchain)?)?)
}

pub(crate) fn new(cfg: &'a Cfg<'a>, desc: ToolchainDesc) -> Result<Self, RustupError> {
Expand Down
4 changes: 4 additions & 0 deletions tests/suite/cli_misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,10 @@ async fn which_asking_uninstalled_toolchain() {
#[tokio::test]
async fn override_by_toolchain_on_the_command_line() {
let mut cx = CliTestContext::new(Scenario::SimpleV2).await;
cx.config
.expect_ok(&["rustup", "toolchain", "install", "stable", "nightly"])
.await;

#[cfg(windows)]
cx.config
.expect_stdout_ok(
Expand Down
71 changes: 25 additions & 46 deletions tests/suite/cli_rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,47 +932,6 @@ async fn list_default_and_override_toolchain() {
.await;
}

#[tokio::test]
async fn heal_damaged_toolchain() {
rami3l marked this conversation as resolved.
Show resolved Hide resolved
let mut cx = CliTestContext::new(Scenario::SimpleV2).await;
cx.config.expect_ok(&["rustup", "default", "nightly"]).await;
cx.config
.expect_not_stderr_ok(&["rustup", "which", "rustc"], "syncing channel updates")
.await;
let manifest_path = format!(
"toolchains/nightly-{}/lib/rustlib/multirust-channel-manifest.toml",
this_host_triple()
);

let mut rustc_path = cx.config.rustupdir.join(
[
"toolchains",
&format!("nightly-{}", this_host_triple()),
"bin",
"rustc",
]
.iter()
.collect::<PathBuf>(),
);

if cfg!(windows) {
rustc_path.set_extension("exe");
}

fs::remove_file(cx.config.rustupdir.join(manifest_path)).unwrap();
cx.config
.expect_ok_ex(
&["rustup", "which", "rustc"],
&format!("{}\n", rustc_path.to_str().unwrap()),
for_host!("info: syncing channel updates for 'nightly-{0}'\n"),
)
.await;
cx.config.expect_ok(&["rustup", "default", "nightly"]).await;
cx.config
.expect_stderr_ok(&["rustup", "which", "rustc"], "syncing channel updates")
.await;
}

#[tokio::test]
#[ignore = "FIXME: Windows shows UNC paths"]
async fn show_toolchain_override() {
Expand Down Expand Up @@ -2133,7 +2092,7 @@ async fn file_override_toml_format_install_both_toolchain_and_components() {
cx.config.expect_ok(&["rustup", "default", "stable"]).await;
}

let cx = cx.with_dist_dir(Scenario::ArchivesV2_2015_01_01);
let mut cx = cx.with_dist_dir(Scenario::ArchivesV2_2015_01_01);
cx.config
.expect_stdout_ok(&["rustc", "--version"], "hash-stable-1.1.0")
.await;
Expand All @@ -2153,6 +2112,9 @@ components = [ "rust-src" ]
)
.unwrap();

cx.config
.expect_ok(&["rustup", "toolchain", "install"])
.await;
cx.config
.expect_stdout_ok(&["rustc", "--version"], "hash-nightly-1")
.await;
Expand Down Expand Up @@ -2180,6 +2142,12 @@ components = [ "rust-src" ]
)
.unwrap();

cx.config
.expect_stderr_ok(
&["rustup", "toolchain", "install"],
"info: installing component 'rust-src'",
)
.await;
cx.config
.expect_stdout_ok(&["rustup", "component", "list"], "rust-src (installed)")
.await;
Expand Down Expand Up @@ -2207,6 +2175,12 @@ targets = [ "arm-linux-androideabi" ]
)
.unwrap();

cx.config
.expect_stderr_ok(
&["rustup", "toolchain", "install"],
"info: installing component 'rust-std' for 'arm-linux-androideabi'",
)
.await;
cx.config
.expect_stdout_ok(
&["rustup", "component", "list"],
Expand All @@ -2233,7 +2207,7 @@ components = [ "rust-bongo" ]

cx.config
.expect_stderr_ok(
&["rustc", "--version"],
&["rustup", "toolchain", "install"],
"warn: Force-skipping unavailable component 'rust-bongo",
)
.await;
Expand Down Expand Up @@ -2263,6 +2237,9 @@ channel = "nightly"
"#,
)
.unwrap();
cx.config
.expect_ok(&["rustup", "toolchain", "install"])
.await;
cx.config
.expect_not_stdout_ok(
&["rustup", "component", "list"],
Expand Down Expand Up @@ -2789,7 +2766,7 @@ async fn dont_warn_on_partial_build() {
/// Checks that `rust-toolchain.toml` files are considered
#[tokio::test]
async fn rust_toolchain_toml() {
let cx = CliTestContext::new(Scenario::SimpleV2).await;
let mut cx = CliTestContext::new(Scenario::SimpleV2).await;
cx.config
.expect_err(
&["rustc", "--version"],
Expand All @@ -2800,7 +2777,9 @@ async fn rust_toolchain_toml() {
let cwd = cx.config.current_dir();
let toolchain_file = cwd.join("rust-toolchain.toml");
raw::write_file(&toolchain_file, "[toolchain]\nchannel = \"nightly\"").unwrap();

cx.config
.expect_ok(&["rustup", "toolchain", "install"])
.await;
cx.config
.expect_stdout_ok(&["rustc", "--version"], "hash-nightly-2")
.await;
Expand Down Expand Up @@ -2831,7 +2810,7 @@ async fn warn_on_duplicate_rust_toolchain_file() {

cx.config
.expect_stderr_ok(
&["rustc", "--version"],
&["rustup", "toolchain", "install"],
&format!(
"warn: both `{0}` and `{1}` exist. Using `{0}`",
toolchain_file_1.canonicalize().unwrap().display(),
Expand Down
22 changes: 9 additions & 13 deletions tests/suite/cli_v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,18 +169,6 @@ async fn remove_toolchain() {
.await;
}

#[tokio::test]
async fn remove_default_toolchain_autoinstalls() {
let mut cx = CliTestContext::new(Scenario::SimpleV1).await;
cx.config.expect_ok(&["rustup", "default", "nightly"]).await;
cx.config
.expect_ok(&["rustup", "toolchain", "remove", "nightly"])
.await;
cx.config
.expect_stderr_ok(&["rustc", "--version"], "info: installing component")
.await;
}

#[tokio::test]
async fn remove_override_toolchain_err_handling() {
let mut cx = CliTestContext::new(Scenario::SimpleV1).await;
Expand All @@ -194,7 +182,15 @@ async fn remove_override_toolchain_err_handling() {
.expect_ok(&["rustup", "toolchain", "remove", "beta"])
.await;
cx.config
.expect_stderr_ok(&["rustc", "--version"], "info: installing component")
.expect_err_ex(
&["rustc", "--version"],
"",
for_host!(
r"error: toolchain 'beta-{0}' is not installed
help: run `rustup toolchain install beta-{0}` to install it
"
),
)
.await;
}

Expand Down
Loading
Loading