From d3f1618a73d3f14bfdd28d62c1d933755ccbad3e Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Fri, 14 Jun 2024 12:02:45 +0200 Subject: [PATCH 1/5] fix python bootstrap tests requiring a downloaded stage0 --- src/bootstrap/bootstrap_test.py | 19 +++++++++++++++++++ src/bootstrap/src/core/build_steps/test.rs | 2 ++ 2 files changed, 21 insertions(+) diff --git a/src/bootstrap/bootstrap_test.py b/src/bootstrap/bootstrap_test.py index 6da410ed2f2..706f2c5bf07 100644 --- a/src/bootstrap/bootstrap_test.py +++ b/src/bootstrap/bootstrap_test.py @@ -138,6 +138,25 @@ class BuildBootstrap(unittest.TestCase): if env is None: env = {} + # This test ends up invoking build_bootstrap_cmd, which searches for + # the Cargo binary and errors out if it cannot be found. This is not a + # problem in most cases, but there is a scenario where it would cause + # the test to fail. + # + # When a custom local Cargo is configured in config.toml (with the + # build.cargo setting), no Cargo is downloaded to any location known by + # bootstrap, and bootstrap relies on that setting to find it. + # + # In this test though we are not using the config.toml of the caller: + # we are generating a blank one instead. If we don't set build.cargo in + # it, the test will have no way to find Cargo, failing the test. + cargo_bin = os.environ.get("BOOTSTRAP_TEST_CARGO_BIN") + if cargo_bin is not None: + configure_args += ["--set", "build.cargo=" + cargo_bin] + rustc_bin = os.environ.get("BOOTSTRAP_TEST_RUSTC_BIN") + if rustc_bin is not None: + configure_args += ["--set", "build.rustc=" + rustc_bin] + env = env.copy() env["PATH"] = os.environ["PATH"] diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 29cd90222b2..22a3f1efa29 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -3040,6 +3040,8 @@ impl Step for Bootstrap { .args(["-m", "unittest", "bootstrap_test.py"]) .env("BUILD_DIR", &builder.out) .env("BUILD_PLATFORM", builder.build.build.triple) + .env("BOOTSTRAP_TEST_RUSTC_BIN", &builder.initial_rustc) + .env("BOOTSTRAP_TEST_CARGO_BIN", &builder.initial_cargo) .current_dir(builder.src.join("src/bootstrap/")); // NOTE: we intentionally don't pass test_args here because the args for unittest and cargo test are mutually incompatible. // Use `python -m unittest` manually if you want to pass arguments. From fcb6ff5171fd41c988a6c449f76ff34c2c2d5a43 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Fri, 14 Jun 2024 12:34:22 +0200 Subject: [PATCH 2/5] fix rust bootstrap tests requiring a downloaded stage0 --- src/bootstrap/src/core/config/config.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 36b44d0169c..6664c5b451c 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1189,7 +1189,19 @@ impl Config { pub fn parse(args: &[String]) -> Config { #[cfg(test)] fn get_toml(_: &Path) -> TomlConfig { - TomlConfig::default() + let mut default = TomlConfig::default(); + + // When configuring bootstrap for tests, make sure to set the rustc and Cargo to the + // same ones used to call the tests. If we don't do that, bootstrap will use its own + // detection logic to find a suitable rustc and Cargo, which doesn't work when the + // caller is specìfying a custom local rustc or Cargo in their config.toml. + default.build = Some(Build { + rustc: std::env::var_os("RUSTC").map(|v| v.into()), + cargo: std::env::var_os("CARGO").map(|v| v.into()), + ..Build::default() + }); + + default } #[cfg(not(test))] From 198c809dd1b06d398c44f570c9635af640895ce9 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 8 Jul 2024 15:16:28 +0200 Subject: [PATCH 3/5] set the correct executable for initial_{rustc,cargo} Due to the way the paths initial_rustc and initial_cargo were constructed before this commit, they mixed \ and / for path separators and they omitted the .exe suffix. This worked fine up until now, as Windows is capable of handling the mixed path separators and the Command::new API adds the ".exe" suffix if missing from the executable. This resulted in paths that didn't actually exist on disk though, due to the missing .exe suffix. This commit fixes that by adding the .exe suffix to initial_rustc and initial_cargo when --build is Windows. --- src/bootstrap/src/core/config/config.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 6664c5b451c..882bae8aeb6 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1452,6 +1452,11 @@ impl Config { config.out = crate::utils::helpers::absolute(&config.out); } + // Hacky way to determine the executable suffix for the build target. We cannot use + // std::env::consts::EXE_SUFFIX as the build target might not be the target bootstrap was + // compiled with. + let initial_exe_suffix = if config.build.triple.contains("windows") { ".exe" } else { "" }; + config.initial_rustc = if let Some(rustc) = rustc { if !flags.skip_stage0_validation { config.check_stage0_version(&rustc, "rustc"); @@ -1459,7 +1464,12 @@ impl Config { rustc } else { config.download_beta_toolchain(); - config.out.join(config.build.triple).join("stage0/bin/rustc") + config + .out + .join(config.build.triple) + .join("stage0") + .join("bin") + .join(format!("rustc{initial_exe_suffix}")) }; config.initial_cargo = if let Some(cargo) = cargo { @@ -1469,7 +1479,12 @@ impl Config { cargo } else { config.download_beta_toolchain(); - config.out.join(config.build.triple).join("stage0/bin/cargo") + config + .out + .join(config.build.triple) + .join("stage0") + .join("bin") + .join(format!("cargo{initial_exe_suffix}")) }; // NOTE: it's important this comes *after* we set `initial_rustc` just above. From b4b2643c111714b541db1c18c178ad0ce654238b Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 8 Jul 2024 17:24:28 +0200 Subject: [PATCH 4/5] set the correct rustc and cargo even for tests invoking parse_inner --- src/bootstrap/src/core/config/config.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 882bae8aeb6..458120fd202 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1189,19 +1189,7 @@ impl Config { pub fn parse(args: &[String]) -> Config { #[cfg(test)] fn get_toml(_: &Path) -> TomlConfig { - let mut default = TomlConfig::default(); - - // When configuring bootstrap for tests, make sure to set the rustc and Cargo to the - // same ones used to call the tests. If we don't do that, bootstrap will use its own - // detection logic to find a suitable rustc and Cargo, which doesn't work when the - // caller is specìfying a custom local rustc or Cargo in their config.toml. - default.build = Some(Build { - rustc: std::env::var_os("RUSTC").map(|v| v.into()), - cargo: std::env::var_os("CARGO").map(|v| v.into()), - ..Build::default() - }); - - default + TomlConfig::default() } #[cfg(not(test))] @@ -1341,6 +1329,17 @@ impl Config { TomlConfig::default() }; + if cfg!(test) { + // When configuring bootstrap for tests, make sure to set the rustc and Cargo to the + // same ones used to call the tests (if custom ones are not defined in the toml). If we + // don't do that, bootstrap will use its own detection logic to find a suitable rustc + // and Cargo, which doesn't work when the caller is specìfying a custom local rustc or + // Cargo in their config.toml. + let build = toml.build.get_or_insert_with(Default::default); + build.rustc = build.rustc.take().or(std::env::var_os("RUSTC").map(|p| p.into())); + build.cargo = build.cargo.take().or(std::env::var_os("CARGO").map(|p| p.into())); + } + if let Some(include) = &toml.profile { // Allows creating alias for profile names, allowing // profiles to be renamed while maintaining back compatibility From 9cd1d253a60aca27086da57b68c24b0924277d58 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 10 Jul 2024 09:27:30 +0200 Subject: [PATCH 5/5] use utils::helpers::exe --- src/bootstrap/src/core/config/config.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 458120fd202..00acd3dabec 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1451,11 +1451,6 @@ impl Config { config.out = crate::utils::helpers::absolute(&config.out); } - // Hacky way to determine the executable suffix for the build target. We cannot use - // std::env::consts::EXE_SUFFIX as the build target might not be the target bootstrap was - // compiled with. - let initial_exe_suffix = if config.build.triple.contains("windows") { ".exe" } else { "" }; - config.initial_rustc = if let Some(rustc) = rustc { if !flags.skip_stage0_validation { config.check_stage0_version(&rustc, "rustc"); @@ -1468,7 +1463,7 @@ impl Config { .join(config.build.triple) .join("stage0") .join("bin") - .join(format!("rustc{initial_exe_suffix}")) + .join(exe("rustc", config.build)) }; config.initial_cargo = if let Some(cargo) = cargo { @@ -1483,7 +1478,7 @@ impl Config { .join(config.build.triple) .join("stage0") .join("bin") - .join(format!("cargo{initial_exe_suffix}")) + .join(exe("cargo", config.build)) }; // NOTE: it's important this comes *after* we set `initial_rustc` just above.