From 6d132d9a5d526ca74db4ca4396e7e5f8c88573ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Mon, 23 Sep 2024 07:41:58 +0000 Subject: [PATCH 1/4] Pass bootstrap cargo when `--stage 0` and `COMPILETST_FORCE_STAGE0` And stop passing `BOOTSTRAP_CARGO` as an env var, instead the provided cargo should go through `--cargo-path.` --- src/bootstrap/src/core/build_steps/test.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 8f076e5554ee5..dcbaf20796faa 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1730,8 +1730,15 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the let is_rustdoc = suite.ends_with("rustdoc-ui") || suite.ends_with("rustdoc-js"); if mode == "run-make" { - let cargo = builder.ensure(tool::Cargo { compiler, target: compiler.host }); - cmd.arg("--cargo-path").arg(cargo); + let cargo_path = if builder.top_stage == 0 { + // If we're using `--stage 0`, we should provide the bootstrap cargo. + builder.initial_cargo.clone() + } else { + // We need to properly build cargo using the suitable stage compiler. + builder.ensure(tool::Cargo { compiler, target: compiler.host }) + }; + + cmd.arg("--cargo-path").arg(cargo_path); } // Avoid depending on rustdoc when we don't need it. @@ -2088,8 +2095,6 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the cmd.arg("--rustfix-coverage"); } - cmd.env("BOOTSTRAP_CARGO", &builder.initial_cargo); - cmd.arg("--channel").arg(&builder.config.channel); if !builder.config.omit_git_hash { From 705ab171a4cc872f79753cf2b83a967baa67b7a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Tue, 24 Sep 2024 19:02:06 +0800 Subject: [PATCH 2/4] Fix tool cargo being off-by-one from rustc staging Previously if you pass compiler stage 1 to `tool::Cargo`, it will build stage2 rustc and give you back a cargo built with stage2 rustc, which is not what we want. This commit adds a hack that chops off a stage from the compiler passed to `tool::Cargo`, meaning that we will get a cargo built with stage 1 compiler, avoiding unnecessary and incorrect build of stage2 rustc and the cargo built by that. --- src/bootstrap/src/core/build_steps/test.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index dcbaf20796faa..870fe6a9f1658 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1735,6 +1735,14 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the builder.initial_cargo.clone() } else { // We need to properly build cargo using the suitable stage compiler. + + // HACK: currently tool stages are off-by-one compared to compiler stages, i.e. if + // you give `tool::Cargo` a stage 1 rustc, it will cause stage 2 rustc to be built + // and produce a cargo built with stage 2 rustc. To fix this, we need to chop off + // the compiler stage by 1 to align with expected `./x test run-make --stage N` + // behavior, i.e. we need to pass `N - 1` compiler stage to cargo. See also Miri + // which does a similar hack. + let compiler = builder.compiler(builder.top_stage - 1, compiler.host); builder.ensure(tool::Cargo { compiler, target: compiler.host }) }; From 53897921bdd74979ecd7b0e3c649d8cddffeb783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Mon, 23 Sep 2024 07:54:05 +0000 Subject: [PATCH 3/4] Fix `run-make-support` to respect per-stage cargo --- src/tools/run-make-support/src/external_deps/cargo.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tools/run-make-support/src/external_deps/cargo.rs b/src/tools/run-make-support/src/external_deps/cargo.rs index b0e045dc80bf8..e91d101cb995b 100644 --- a/src/tools/run-make-support/src/external_deps/cargo.rs +++ b/src/tools/run-make-support/src/external_deps/cargo.rs @@ -1,7 +1,8 @@ use crate::command::Command; use crate::env_var; -/// Returns a command that can be used to invoke Cargo. +/// Returns a command that can be used to invoke cargo. The cargo is provided by compiletest +/// through the `CARGO` env var. pub fn cargo() -> Command { - Command::new(env_var("BOOTSTRAP_CARGO")) + Command::new(env_var("CARGO")) } From f5482167286424fd25266d8202e8ded6655cc9ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Mon, 23 Sep 2024 07:54:31 +0000 Subject: [PATCH 4/4] Update `run-make` tests to use `cargo` wrapper cmd --- tests/run-make/compiler-builtins/rmake.rs | 56 +++++++++------------ tests/run-make/thumb-none-cortex-m/rmake.rs | 46 ++++++----------- tests/run-make/thumb-none-qemu/rmake.rs | 53 +++++++------------ 3 files changed, 58 insertions(+), 97 deletions(-) diff --git a/tests/run-make/compiler-builtins/rmake.rs b/tests/run-make/compiler-builtins/rmake.rs index daa12d2986eb9..10093db2258df 100644 --- a/tests/run-make/compiler-builtins/rmake.rs +++ b/tests/run-make/compiler-builtins/rmake.rs @@ -15,46 +15,36 @@ #![deny(warnings)] use std::collections::HashSet; -use std::path::PathBuf; use run_make_support::object::read::Object; use run_make_support::object::read::archive::ArchiveFile; use run_make_support::object::{ObjectSection, ObjectSymbol, RelocationTarget}; use run_make_support::rfs::{read, read_dir}; -use run_make_support::{cmd, env_var, object}; +use run_make_support::{cargo, object, path, target}; fn main() { - let target_dir = PathBuf::from("target"); - let target = env_var("TARGET"); - - println!("Testing compiler_builtins for {}", target); - - let manifest_path = PathBuf::from("Cargo.toml"); - - let path = env_var("PATH"); - let rustc = env_var("RUSTC"); - let cargo = env_var("CARGO"); - let mut cmd = cmd(cargo); - cmd.args(&[ - "build", - "--manifest-path", - manifest_path.to_str().unwrap(), - "-Zbuild-std=core", - "--target", - &target, - ]) - .env("PATH", path) - .env("RUSTC", rustc) - .env("RUSTFLAGS", "-Copt-level=0 -Cdebug-assertions=yes") - .env("CARGO_TARGET_DIR", &target_dir) - .env("RUSTC_BOOTSTRAP", "1") - // Visual Studio 2022 requires that the LIB env var be set so it can - // find the Windows SDK. - .env("LIB", std::env::var("LIB").unwrap_or_default()); - - cmd.run(); - - let rlibs_path = target_dir.join(target).join("debug").join("deps"); + let target_dir = path("target"); + + println!("Testing compiler_builtins for {}", target()); + + cargo() + .args(&[ + "build", + "--manifest-path", + "Cargo.toml", + "-Zbuild-std=core", + "--target", + &target(), + ]) + .env("RUSTFLAGS", "-Copt-level=0 -Cdebug-assertions=yes") + .env("CARGO_TARGET_DIR", &target_dir) + .env("RUSTC_BOOTSTRAP", "1") + // Visual Studio 2022 requires that the LIB env var be set so it can + // find the Windows SDK. + .env("LIB", std::env::var("LIB").unwrap_or_default()) + .run(); + + let rlibs_path = target_dir.join(target()).join("debug").join("deps"); let compiler_builtins_rlib = read_dir(rlibs_path) .find_map(|e| { let path = e.unwrap().path(); diff --git a/tests/run-make/thumb-none-cortex-m/rmake.rs b/tests/run-make/thumb-none-cortex-m/rmake.rs index 9112646290f20..27afef874da64 100644 --- a/tests/run-make/thumb-none-cortex-m/rmake.rs +++ b/tests/run-make/thumb-none-cortex-m/rmake.rs @@ -14,10 +14,7 @@ //@ only-thumb -use std::path::PathBuf; - -use run_make_support::rfs::create_dir; -use run_make_support::{cmd, env_var, target}; +use run_make_support::{cargo, cmd, env, env_var, target}; const CRATE: &str = "cortex-m"; const CRATE_URL: &str = "https://github.com/rust-embedded/cortex-m"; @@ -28,32 +25,21 @@ fn main() { // See below link for git usage: // https://stackoverflow.com/questions/3489173#14091182 cmd("git").args(["clone", CRATE_URL, CRATE]).run(); - std::env::set_current_dir(CRATE).unwrap(); + env::set_current_dir(CRATE); cmd("git").args(["reset", "--hard", CRATE_SHA1]).run(); - let target_dir = PathBuf::from("target"); - let manifest_path = PathBuf::from("Cargo.toml"); - - let path = env_var("PATH"); - let rustc = env_var("RUSTC"); - let cargo = env_var("CARGO"); - // FIXME: extract cargo invocations to a proper command - // https://github.com/rust-lang/rust/issues/128734 - let mut cmd = cmd(cargo); - cmd.args(&[ - "build", - "--manifest-path", - manifest_path.to_str().unwrap(), - "-Zbuild-std=core", - "--target", - &target(), - ]) - .env("PATH", path) - .env("RUSTC", rustc) - .env("CARGO_TARGET_DIR", &target_dir) - // Don't make lints fatal, but they need to at least warn - // or they break Cargo's target info parsing. - .env("RUSTFLAGS", "-Copt-level=0 -Cdebug-assertions=yes --cap-lints=warn"); - - cmd.run(); + cargo() + .args(&[ + "build", + "--manifest-path", + "Cargo.toml", + "-Zbuild-std=core", + "--target", + &target(), + ]) + .env("CARGO_TARGET_DIR", "target") + // Don't make lints fatal, but they need to at least warn + // or they break Cargo's target info parsing. + .env("RUSTFLAGS", "-Copt-level=0 -Cdebug-assertions=yes --cap-lints=warn") + .run(); } diff --git a/tests/run-make/thumb-none-qemu/rmake.rs b/tests/run-make/thumb-none-qemu/rmake.rs index d0f42bc880897..a505bb013f9b7 100644 --- a/tests/run-make/thumb-none-qemu/rmake.rs +++ b/tests/run-make/thumb-none-qemu/rmake.rs @@ -14,49 +14,34 @@ //! //! FIXME: https://github.com/rust-lang/rust/issues/128733 this test uses external //! dependencies, and needs an active internet connection -//! -//! FIXME: https://github.com/rust-lang/rust/issues/128734 extract bootstrap cargo -//! to a proper command //@ only-thumb use std::path::PathBuf; -use run_make_support::{cmd, env_var, path_helpers, target}; +use run_make_support::{cargo, cmd, env_var, path, target}; const CRATE: &str = "example"; fn main() { std::env::set_current_dir(CRATE).unwrap(); - let bootstrap_cargo = env_var("BOOTSTRAP_CARGO"); - let path = env_var("PATH"); - let rustc = env_var("RUSTC"); - - let target_dir = path_helpers::path("target"); - let manifest_path = path_helpers::path("Cargo.toml"); - - let debug = { - let mut cmd = cmd(&bootstrap_cargo); - cmd.args(&["run", "--target", &target()]) - .env("RUSTFLAGS", "-C linker=arm-none-eabi-ld -C link-arg=-Tlink.x") - .env("CARGO_TARGET_DIR", &target_dir) - .env("PATH", &path) - .env("RUSTC", &rustc); - cmd.run() - }; - - debug.assert_stdout_contains("x = 42"); - - let release = { - let mut cmd = cmd(&bootstrap_cargo); - cmd.args(&["run", "--release", "--target", &target()]) - .env("RUSTFLAGS", "-C linker=arm-none-eabi-ld -C link-arg=-Tlink.x") - .env("CARGO_TARGET_DIR", &target_dir) - .env("PATH", &path) - .env("RUSTC", &rustc); - cmd.run() - }; - - release.assert_stdout_contains("x = 42"); + let target_dir = path("target"); + let manifest_path = path("Cargo.toml"); + + // Debug + cargo() + .args(&["run", "--target", &target()]) + .env("RUSTFLAGS", "-C linker=arm-none-eabi-ld -C link-arg=-Tlink.x") + .env("CARGO_TARGET_DIR", &target_dir) + .run() + .assert_stdout_contains("x = 42"); + + // Release + cargo() + .args(&["run", "--release", "--target", &target()]) + .env("RUSTFLAGS", "-C linker=arm-none-eabi-ld -C link-arg=-Tlink.x") + .env("CARGO_TARGET_DIR", &target_dir) + .run() + .assert_stdout_contains("x = 42"); }