-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Warn when installing with a non-default toolchain #16131
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||
| use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; | ||||||||||||||
| use std::path::{Path, PathBuf}; | ||||||||||||||
| use std::sync::Arc; | ||||||||||||||
| use std::{env, fs}; | ||||||||||||||
| use std::{env, fmt, fs}; | ||||||||||||||
|
|
||||||||||||||
| use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, UnitOutput}; | ||||||||||||||
| use crate::core::{Dependency, Edition, Package, PackageId, SourceId, Target, Workspace}; | ||||||||||||||
|
|
@@ -14,6 +14,7 @@ use crate::util::errors::CargoResult; | |||||||||||||
| use crate::util::{Filesystem, GlobalContext, Rustc}; | ||||||||||||||
| use crate::{drop_println, ops}; | ||||||||||||||
|
|
||||||||||||||
| use annotate_snippets::Level; | ||||||||||||||
| use anyhow::{Context as _, bail}; | ||||||||||||||
| use cargo_util::paths; | ||||||||||||||
| use cargo_util_schemas::core::PartialVersion; | ||||||||||||||
|
|
@@ -39,6 +40,26 @@ impl Drop for Transaction { | |||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| enum RustupToolchainSource { | ||||||||||||||
| Default, | ||||||||||||||
| Environment, | ||||||||||||||
| CommandLine, | ||||||||||||||
| OverrideDB, | ||||||||||||||
| ToolchainFile, | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| impl fmt::Display for RustupToolchainSource { | ||||||||||||||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||||||||||||||
| f.write_str(match self { | ||||||||||||||
| Self::Default => "default", | ||||||||||||||
| Self::Environment => "env", | ||||||||||||||
| Self::CommandLine => "cli", | ||||||||||||||
| Self::OverrideDB => "path-override", | ||||||||||||||
| Self::ToolchainFile => "toolchain-file", | ||||||||||||||
| }) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| struct InstallablePackage<'gctx> { | ||||||||||||||
| gctx: &'gctx GlobalContext, | ||||||||||||||
| opts: ops::CompileOptions, | ||||||||||||||
|
|
@@ -316,6 +337,28 @@ impl<'gctx> InstallablePackage<'gctx> { | |||||||||||||
| fn install_one(mut self, dry_run: bool) -> CargoResult<bool> { | ||||||||||||||
| self.gctx.shell().status("Installing", &self.pkg)?; | ||||||||||||||
|
|
||||||||||||||
| if let Some(source) = self.get_rustup_toolchain_source() | ||||||||||||||
| && !matches!( | ||||||||||||||
| source, | ||||||||||||||
| RustupToolchainSource::Default | RustupToolchainSource::CommandLine | ||||||||||||||
| ) | ||||||||||||||
| { | ||||||||||||||
| let maybe_toolchain = self | ||||||||||||||
| .gctx | ||||||||||||||
| .get_env("RUSTUP_TOOLCHAIN") | ||||||||||||||
| .ok() | ||||||||||||||
| .map(|toolchain| format!(" `{toolchain}`")) | ||||||||||||||
| .unwrap_or_default(); | ||||||||||||||
| let report = &[Level::WARNING | ||||||||||||||
| .primary_title(format!( | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not too obvious but we are limiting ourselves to |
||||||||||||||
| "using non-default toolchain{maybe_toolchain} overridden by {source}" | ||||||||||||||
| )) | ||||||||||||||
| .element(Level::HELP.message(format!( | ||||||||||||||
| "Use `cargo +stable install` if you meant to use the stable toolchain." | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please change I really wish we had [ If its purely for diagnostic purposes, we could run |
||||||||||||||
| )))]; | ||||||||||||||
| self.gctx.shell().print_report(report, false)?; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| let dst = self.root.join("bin").into_path_unlocked(); | ||||||||||||||
|
|
||||||||||||||
| let mut td_opt = None; | ||||||||||||||
|
|
@@ -585,6 +628,20 @@ impl<'gctx> InstallablePackage<'gctx> { | |||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| fn get_rustup_toolchain_source(&self) -> Option<RustupToolchainSource> { | ||||||||||||||
| self.gctx | ||||||||||||||
| .get_env("RUSTUP_TOOLCHAIN_SOURCE") | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My gut reaction is we should make an exception and read from Lines 75 to 78 in 40076ea
but then we have this Lines 362 to 363 in 40076ea
|
||||||||||||||
| .ok() | ||||||||||||||
| .and_then(|val| match val { | ||||||||||||||
| "default" => Some(RustupToolchainSource::Default), | ||||||||||||||
| "env" => Some(RustupToolchainSource::Environment), | ||||||||||||||
| "cli" => Some(RustupToolchainSource::CommandLine), | ||||||||||||||
| "path-override" => Some(RustupToolchainSource::OverrideDB), | ||||||||||||||
| "toolchain-file" => Some(RustupToolchainSource::ToolchainFile), | ||||||||||||||
| _ => None, | ||||||||||||||
| }) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| fn check_yanked_install(&self) -> CargoResult<()> { | ||||||||||||||
| if self.ws.ignore_lock() || !self.ws.root().join("Cargo.lock").exists() { | ||||||||||||||
| return Ok(()); | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ use std::path::PathBuf; | |
| use std::thread; | ||
|
|
||
| use crate::prelude::*; | ||
| use crate::utils::cargo_process; | ||
| use crate::utils::{cargo_process, pkg}; | ||
| use cargo_test_support::compare::assert_e2e; | ||
| use cargo_test_support::cross_compile; | ||
| use cargo_test_support::git; | ||
|
|
@@ -21,16 +21,6 @@ use crate::utils::cross_compile::disabled as cross_compile_disabled; | |
| use cargo_test_support::install::{assert_has_installed_exe, assert_has_not_installed_exe, exe}; | ||
| use cargo_test_support::paths; | ||
|
|
||
| fn pkg(name: &str, vers: &str) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For myself, I'm not too much of a fan of us reusing like this as it hides in the test what is happening and gets annoying when you just need a little customization. Just directly publish the needed package inside of your test. |
||
| Package::new(name, vers) | ||
| .file("src/lib.rs", "") | ||
| .file( | ||
| "src/main.rs", | ||
| &format!("extern crate {}; fn main() {{}}", name), | ||
| ) | ||
| .publish(); | ||
| } | ||
|
|
||
| #[cargo_test] | ||
| fn simple() { | ||
| pkg("foo", "0.0.1"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,8 +8,9 @@ use std::path::{Path, PathBuf}; | |||||||||||||||
|
|
||||||||||||||||
| use crate::prelude::*; | ||||||||||||||||
| use crate::utils::cargo_process; | ||||||||||||||||
| use cargo_test_support::paths::{home, root}; | ||||||||||||||||
| use cargo_test_support::{process, project, str}; | ||||||||||||||||
| use cargo_test_support::install::assert_has_installed_exe; | ||||||||||||||||
| use cargo_test_support::paths::{cargo_home, home, root}; | ||||||||||||||||
| use cargo_test_support::{execs, process, project, str}; | ||||||||||||||||
|
|
||||||||||||||||
| /// Helper to generate an executable. | ||||||||||||||||
| fn make_exe(dest: &Path, name: &str, contents: &str, env: &[(&str, PathBuf)]) -> PathBuf { | ||||||||||||||||
|
|
@@ -55,6 +56,13 @@ fn real_rustc_wrapper(bin_dir: &Path, message: &str) -> PathBuf { | |||||||||||||||
| // The toolchain rustc needs to call the real rustc. In order to do that, | ||||||||||||||||
| // it needs to restore or clear the RUSTUP environment variables so that | ||||||||||||||||
| // if rustup is installed, it will call the correct rustc. | ||||||||||||||||
| let rustup_toolchain_source_setup = match std::env::var_os("RUSTUP_TOOLCHAIN_SOURCE") { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should be doing anything based on the rustup we are running in See also #16131 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the risk of sounding like an excuse, I was following the example of the code just below this: cargo/tests/testsuite/rustup.rs Lines 65 to 71 in ff31445
Should that following code be modified? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems suspicious and I'd like to better understand why we do this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just FYI, I'm willing to make changes not directly related #11036. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would you feel about rustc's path being determined in Cargo's build script rather than via environment variables in this function? That is, I'm proposing that /// Creates an executable which prints a message and then runs the *real* rustc.
fn real_rustc_wrapper(bin_dir: &Path, message: &str) -> PathBuf {
let real_rustc = PathBuf::from(env!("RUSTC"));
let env = vec![("CARGO_RUSTUP_TEST_real_rustc", real_rustc)];
make_exe(
bin_dir,
"rustc",
&format!(
r#"
eprintln!("{message}");
let r = std::process::Command::new(env!("CARGO_RUSTUP_TEST_real_rustc"))
.args(std::env::args_os().skip(1))
.status();
std::process::exit(r.unwrap().code().unwrap_or(2));
"#
),
&env,
)
}EDIT: Also, please tell me whether you think a separate PR would be appropriate. |
||||||||||||||||
| Some(t) => format!( | ||||||||||||||||
| ".env(\"RUSTUP_TOOLCHAIN_SOURCE\", \"{}\")", | ||||||||||||||||
| t.into_string().unwrap() | ||||||||||||||||
| ), | ||||||||||||||||
| None => format!(".env_remove(\"RUSTUP_TOOLCHAIN_SOURCE\")"), | ||||||||||||||||
| }; | ||||||||||||||||
| let rustup_toolchain_setup = match std::env::var_os("RUSTUP_TOOLCHAIN") { | ||||||||||||||||
| Some(t) => format!( | ||||||||||||||||
| ".env(\"RUSTUP_TOOLCHAIN\", \"{}\")", | ||||||||||||||||
|
|
@@ -78,6 +86,7 @@ fn real_rustc_wrapper(bin_dir: &Path, message: &str) -> PathBuf { | |||||||||||||||
| eprintln!("{message}"); | ||||||||||||||||
| let r = std::process::Command::new(env!("CARGO_RUSTUP_TEST_real_rustc")) | ||||||||||||||||
| .args(std::env::args_os().skip(1)) | ||||||||||||||||
| {rustup_toolchain_source_setup} | ||||||||||||||||
| {rustup_toolchain_setup} | ||||||||||||||||
| {rustup_home_setup} | ||||||||||||||||
| .status(); | ||||||||||||||||
|
|
@@ -91,7 +100,13 @@ fn real_rustc_wrapper(bin_dir: &Path, message: &str) -> PathBuf { | |||||||||||||||
| /// Creates a simulation of a rustup environment with `~/.cargo/bin` and | ||||||||||||||||
| /// `~/.rustup` directories populated with some executables that simulate | ||||||||||||||||
| /// rustup. | ||||||||||||||||
| fn simulated_rustup_environment() -> RustupEnvironment { | ||||||||||||||||
| /// | ||||||||||||||||
| /// Arguments | ||||||||||||||||
| /// | ||||||||||||||||
| /// - `proxy_calls_cargo`: if true, the cargo proxy calls the cargo under test; | ||||||||||||||||
| /// otherwise, the cargo proxy calls an executable that panics immediately | ||||||||||||||||
| /// - `env_setup`: environment variable setup the proxy should perform | ||||||||||||||||
| fn simulated_rustup_environment(proxy_calls_cargo: bool, env_setup: &str) -> RustupEnvironment { | ||||||||||||||||
|
Comment on lines
+104
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These make the tests much harder to read and not thrilled about injecting code directly into things. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would you prefer instead? |
||||||||||||||||
| // Set up ~/.rustup/toolchains/test-toolchain/bin with a custom rustc and cargo. | ||||||||||||||||
| let rustup_home = home().join(".rustup"); | ||||||||||||||||
| let toolchain_bin = rustup_home | ||||||||||||||||
|
|
@@ -100,42 +115,58 @@ fn simulated_rustup_environment() -> RustupEnvironment { | |||||||||||||||
| .join("bin"); | ||||||||||||||||
| toolchain_bin.mkdir_p(); | ||||||||||||||||
| let rustc_toolchain_exe = real_rustc_wrapper(&toolchain_bin, "real rustc running"); | ||||||||||||||||
| let cargo_toolchain_exe = make_exe( | ||||||||||||||||
| &toolchain_bin, | ||||||||||||||||
| "cargo", | ||||||||||||||||
| r#"panic!("cargo toolchain should not be called");"#, | ||||||||||||||||
| &[], | ||||||||||||||||
| ); | ||||||||||||||||
| let cargo_toolchain_exe = if proxy_calls_cargo { | ||||||||||||||||
| crate::utils::cargo_exe() | ||||||||||||||||
| } else { | ||||||||||||||||
| make_exe( | ||||||||||||||||
| &toolchain_bin, | ||||||||||||||||
| "cargo", | ||||||||||||||||
| r#"panic!("cargo toolchain should not be called");"#, | ||||||||||||||||
| &[], | ||||||||||||||||
| ) | ||||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| // Set up ~/.cargo/bin with a typical set of rustup proxies. | ||||||||||||||||
| let cargo_bin = home().join(".cargo").join("bin"); | ||||||||||||||||
| cargo_bin.mkdir_p(); | ||||||||||||||||
|
|
||||||||||||||||
| let rustc_proxy = make_exe( | ||||||||||||||||
| let proxy = make_exe( | ||||||||||||||||
| &cargo_bin, | ||||||||||||||||
| "rustc", | ||||||||||||||||
| &format!( | ||||||||||||||||
| r#" | ||||||||||||||||
| match std::env::args().next().unwrap().as_ref() {{ | ||||||||||||||||
| "rustc" => {{}} | ||||||||||||||||
| arg => panic!("proxy only supports rustc, got {{arg:?}}"), | ||||||||||||||||
| }} | ||||||||||||||||
| eprintln!("rustc proxy running"); | ||||||||||||||||
| let r = std::process::Command::new(env!("CARGO_RUSTUP_TEST_rustc_toolchain_exe")) | ||||||||||||||||
| let file_stem = std::path::PathBuf::from(std::env::args().next().unwrap()) | ||||||||||||||||
| .file_stem() | ||||||||||||||||
| .map(ToOwned::to_owned) | ||||||||||||||||
| .unwrap(); | ||||||||||||||||
| let program = match file_stem.to_str().unwrap() {{ | ||||||||||||||||
| "cargo" => env!("CARGO_RUSTUP_TEST_cargo_toolchain_exe"), | ||||||||||||||||
| "rustc" => env!("CARGO_RUSTUP_TEST_rustc_toolchain_exe"), | ||||||||||||||||
| arg => panic!("proxy only supports cargo and rustc, got {{arg:?}}"), | ||||||||||||||||
| }}; | ||||||||||||||||
| eprintln!("`{{program}}` proxy running"); | ||||||||||||||||
| let r = std::process::Command::new(program) | ||||||||||||||||
| .args(std::env::args_os().skip(1)) | ||||||||||||||||
| {env_setup} | ||||||||||||||||
| .status(); | ||||||||||||||||
| std::process::exit(r.unwrap().code().unwrap_or(2)); | ||||||||||||||||
| "# | ||||||||||||||||
| ), | ||||||||||||||||
| &[("CARGO_RUSTUP_TEST_rustc_toolchain_exe", rustc_toolchain_exe)], | ||||||||||||||||
| &[ | ||||||||||||||||
| ("CARGO_RUSTUP_TEST_rustc_toolchain_exe", rustc_toolchain_exe), | ||||||||||||||||
| ( | ||||||||||||||||
| "CARGO_RUSTUP_TEST_cargo_toolchain_exe", | ||||||||||||||||
| cargo_toolchain_exe.clone(), | ||||||||||||||||
| ), | ||||||||||||||||
| ], | ||||||||||||||||
| ); | ||||||||||||||||
| fs::hard_link( | ||||||||||||||||
| &rustc_proxy, | ||||||||||||||||
| &proxy, | ||||||||||||||||
| cargo_bin.join("cargo").with_extension(EXE_EXTENSION), | ||||||||||||||||
| ) | ||||||||||||||||
| .unwrap(); | ||||||||||||||||
| fs::hard_link( | ||||||||||||||||
| &rustc_proxy, | ||||||||||||||||
| &proxy, | ||||||||||||||||
| cargo_bin.join("rustup").with_extension(EXE_EXTENSION), | ||||||||||||||||
| ) | ||||||||||||||||
| .unwrap(); | ||||||||||||||||
|
|
@@ -154,14 +185,15 @@ fn typical_rustup() { | |||||||||||||||
| cargo_bin, | ||||||||||||||||
| rustup_home, | ||||||||||||||||
| cargo_toolchain_exe, | ||||||||||||||||
| } = simulated_rustup_environment(); | ||||||||||||||||
| } = simulated_rustup_environment(false, ""); | ||||||||||||||||
|
|
||||||||||||||||
| // Set up a project and run a normal cargo build. | ||||||||||||||||
| let p = project().file("src/lib.rs", "").build(); | ||||||||||||||||
| // The path is modified so that cargo will call `rustc` from | ||||||||||||||||
| // `~/.cargo/bin/rustc to use our custom rustup proxies. | ||||||||||||||||
| let path = prepend_path(&cargo_bin); | ||||||||||||||||
| p.cargo("check") | ||||||||||||||||
| .env("RUSTUP_TOOLCHAIN_SOURCE", "default") | ||||||||||||||||
| .env("RUSTUP_TOOLCHAIN", "test-toolchain") | ||||||||||||||||
| .env("RUSTUP_HOME", &rustup_home) | ||||||||||||||||
| .env("PATH", &path) | ||||||||||||||||
|
|
@@ -179,6 +211,7 @@ real rustc running | |||||||||||||||
| p.build_dir().rm_rf(); | ||||||||||||||||
|
|
||||||||||||||||
| p.cargo("check") | ||||||||||||||||
| .env("RUSTUP_TOOLCHAIN_SOURCE", "default") | ||||||||||||||||
| .env("RUSTUP_TOOLCHAIN", "test-toolchain") | ||||||||||||||||
| .env("RUSTUP_HOME", &rustup_home) | ||||||||||||||||
| .env("PATH", &path) | ||||||||||||||||
|
|
@@ -202,7 +235,7 @@ fn custom_calls_other_cargo() { | |||||||||||||||
| cargo_bin, | ||||||||||||||||
| rustup_home, | ||||||||||||||||
| cargo_toolchain_exe: _, | ||||||||||||||||
| } = simulated_rustup_environment(); | ||||||||||||||||
| } = simulated_rustup_environment(false, ""); | ||||||||||||||||
|
|
||||||||||||||||
| // Create a directory with a custom toolchain (outside of the rustup universe). | ||||||||||||||||
| let custom_bin = root().join("custom-bin"); | ||||||||||||||||
|
|
@@ -249,6 +282,7 @@ fn custom_calls_other_cargo() { | |||||||||||||||
| // Set these to simulate what would happen when running under rustup. | ||||||||||||||||
| // We want to make sure that cargo-custom does not try to use the | ||||||||||||||||
| // rustup proxies. | ||||||||||||||||
| .env("RUSTUP_TOOLCHAIN_SOURCE", "default") | ||||||||||||||||
| .env("RUSTUP_TOOLCHAIN", "test-toolchain") | ||||||||||||||||
| .env("RUSTUP_HOME", &rustup_home) | ||||||||||||||||
| .with_stderr_data(str![[r#" | ||||||||||||||||
|
|
@@ -260,3 +294,43 @@ custom toolchain rustc running | |||||||||||||||
| "#]]) | ||||||||||||||||
| .run(); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Performs a `cargo install` with a non-default toolchain in a simulated | ||||||||||||||||
| /// rustup environment. The purpose is to verify the warning that is emitted. | ||||||||||||||||
| #[cargo_test] | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We prefer tests for a behavior change on its own commit first, so that in the next fix impl commit we can see the behavior change through test assertion/snapshot diff. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that with what was said, each commit should pass tests. As the last commit has no test changes, I'm assuming that instead you added all tests as if feature is implemented and put them in the test commit. See https://doc.crates.io/contrib/process/working-on-cargo.html#submitting-a-pull-request for more details |
||||||||||||||||
| fn cargo_install_with_non_default_toolchain() { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should likely have
|
||||||||||||||||
| let RustupEnvironment { | ||||||||||||||||
| cargo_bin, | ||||||||||||||||
| rustup_home: _, | ||||||||||||||||
| cargo_toolchain_exe: _, | ||||||||||||||||
| } = simulated_rustup_environment( | ||||||||||||||||
| true, | ||||||||||||||||
| ".env(\"RUSTUP_TOOLCHAIN_SOURCE\", \"env\") | ||||||||||||||||
| .env(\"RUSTUP_TOOLCHAIN\", \"test-toolchain\")", | ||||||||||||||||
| ); | ||||||||||||||||
|
|
||||||||||||||||
| crate::utils::pkg("foo", "0.0.1"); | ||||||||||||||||
|
|
||||||||||||||||
| let mut p = process(cargo_bin.join("cargo")); | ||||||||||||||||
| p.arg_line("install foo"); | ||||||||||||||||
| execs() | ||||||||||||||||
| .with_process_builder(p) | ||||||||||||||||
| .with_stderr_data(str![[r#" | ||||||||||||||||
| `[..]/cargo[EXE]` proxy running | ||||||||||||||||
| [UPDATING] `dummy-registry` index | ||||||||||||||||
| [DOWNLOADING] crates ... | ||||||||||||||||
| [DOWNLOADED] foo v0.0.1 (registry `dummy-registry`) | ||||||||||||||||
| [INSTALLING] foo v0.0.1 | ||||||||||||||||
| [WARNING] using non-default toolchain `test-toolchain` overridden by env | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the new rustup is released, to avoid it causing annoyances in our tests, I suspect we want to update cargo/crates/cargo-test-support/src/lib.rs Line 1377 in 40076ea
RUSTUP_* and env_remove them from the binary being executed
|
||||||||||||||||
| | | ||||||||||||||||
| = [HELP] Use `cargo +stable install` if you meant to use the stable toolchain. | ||||||||||||||||
| [COMPILING] foo v0.0.1 | ||||||||||||||||
| [FINISHED] `release` profile [optimized] target(s) in [ELAPSED]s | ||||||||||||||||
| [INSTALLING] [ROOT]/home/.cargo/bin/foo[EXE] | ||||||||||||||||
| [INSTALLED] package `foo v0.0.1` (executable `foo[EXE]`) | ||||||||||||||||
| [WARNING] be sure to add `[ROOT]/home/.cargo/bin` to your PATH to be able to run the installed binaries | ||||||||||||||||
|
|
||||||||||||||||
| "#]]) | ||||||||||||||||
| .run(); | ||||||||||||||||
| assert_has_installed_exe(cargo_home(), "foo"); | ||||||||||||||||
| } | ||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Downside to
matches!is we can miss it if new values are added. For now, that is not likely to happen as this is all for one feature. That might not always be the case.I would
&str->RustupToolchainSourceto a function onget_rustup_toolchain_source(shouldn't touchgctx)RustupToolchainSourcethat lts is ask the question we are asking here and uses amatchwith every case enumerated