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

[DO NOT LOOK] a bunch of compiletest-related changes #136437

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 26 additions & 14 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1646,18 +1646,22 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// part test the *API* of the compiler, not how it compiles a given file. As a result, we
// can run them against the stage 1 sources as long as we build them with the stage 0
// bootstrap compiler.
// NOTE: Only stage 1 is special cased because we need the rustc_private artifacts to match the
// running compiler in stage 2 when plugins run.
let stage_id = if suite == "ui-fulldeps" && compiler.stage == 1 {
// At stage 0 (stage - 1) we are using the beta compiler. Using `self.target` can lead finding
// an incorrect compiler path on cross-targets, as the stage 0 beta compiler is always equal
// to `build.build` in the configuration.
//
// NOTE: Only stage 1 is special cased because we need the `rustc_private` artifacts to
// match the running compiler in stage 2 when plugins run.
let (stage_id, test_stage) = if suite == "ui-fulldeps" && compiler.stage == 1 {
// At stage 0 (stage - 1) we are using the beta compiler. Using `self.target` can lead
// finding an incorrect compiler path on cross-targets, as the stage 0 beta compiler is
// always equal to `build.build` in the configuration.
let build = builder.build.build;

compiler = builder.compiler(compiler.stage - 1, build);
format!("stage{}-{}", compiler.stage + 1, build)

let test_stage = compiler.stage + 1;

(format!("stage{}-{}", test_stage, build), test_stage)
} else {
format!("stage{}-{}", compiler.stage, target)
(format!("stage{}-{}", compiler.stage, target), compiler.stage)
};

if suite.ends_with("fulldeps") {
Expand Down Expand Up @@ -1757,22 +1761,30 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
cmd.arg("--coverage-dump-path").arg(coverage_dump);
}

cmd.arg("--src-base").arg(builder.src.join("tests").join(suite));
cmd.arg("--build-base").arg(testdir(builder, compiler.host).join(suite));
cmd.arg("--src-root").arg(&builder.src);
cmd.arg("--src-test-suite-root").arg(builder.src.join("tests").join(suite));
// Note: this is the *root* build directory, does not include target triple!
cmd.arg("--build-root").arg(&builder.out);
cmd.arg("--build-test-suite-root").arg(testdir(builder, compiler.host).join(suite));

// When top stage is 0, that means that we're testing an externally provided compiler.
// In that case we need to use its specific sysroot for tests to pass.
let sysroot = if builder.top_stage == 0 {
builder.initial_sysroot.clone()
} else {
builder.sysroot(compiler).to_path_buf()
builder.sysroot(compiler)
};
cmd.arg("--sysroot-base").arg(sysroot);
cmd.arg("--sysroot").arg(sysroot);

cmd.arg("--stage-id").arg(stage_id);
cmd.arg("--stage").arg(test_stage.to_string());

cmd.arg("--host").arg(&*compiler.host.triple);
cmd.arg("--target").arg(target.rustc_target_arg());

cmd.arg("--suite").arg(suite);
cmd.arg("--mode").arg(mode);
cmd.arg("--target").arg(target.rustc_target_arg());
cmd.arg("--host").arg(&*compiler.host.triple);

cmd.arg("--llvm-filecheck").arg(builder.llvm_filecheck(builder.config.build));

if builder.build.config.llvm_enzyme {
Expand Down
27 changes: 17 additions & 10 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,22 @@ pub struct Config {
/// `None` then these tests will be ignored.
pub run_clang_based_tests_with: Option<String>,

/// The directory containing the tests to run
pub src_base: PathBuf,

/// The directory where programs should be built
pub build_base: PathBuf,

/// The directory containing the compiler sysroot
pub sysroot_base: PathBuf,

/// Path to directory containing rust-lang/rust sources.
pub src_root: PathBuf,
/// Path to the directory containg the test suite sources. Expected to be a subdirectory of
/// `src_root`.
pub src_test_suite_root: PathBuf,

/// Bootstrap build directory.
pub build_root: PathBuf,
/// Bootstrap test suite build directory. Expected to be a subdirectory of `build_root`.
pub build_test_suite_root: PathBuf,
Comment on lines -218 to +227
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are terribly named, and looking at the runtest impls, it has confused multiple people. src_base is not sources root, it's the directory $SRC_ROOT/tests/<test-suite>/. build_base is not build directory root, it's $BUILD/<host-triple>/test/<test-suite>.


/// The directory containing the compiler sysroot.
pub sysroot: PathBuf,

/// Stage number.
pub stage: u32,
/// The name of the stage being built (stage1, etc)
pub stage_id: String,

Expand Down Expand Up @@ -802,7 +809,7 @@ pub const UI_COVERAGE_MAP: &str = "cov-map";
/// /path/to/build/host-tuple/test/ui/relative/
/// This is created early when tests are collected to avoid race conditions.
pub fn output_relative_path(config: &Config, relative_dir: &Path) -> PathBuf {
config.build_base.join(relative_dir)
config.build_test_suite_root.join(relative_dir)
}

/// Generates a unique name for the test, such as `testname.revision.mode`.
Expand Down
42 changes: 16 additions & 26 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1034,19 +1034,6 @@ impl Config {
}
}

pub fn find_rust_src_root(&self) -> Option<PathBuf> {
let mut path = self.src_base.clone();
let path_postfix = Path::new("src/etc/lldb_batchmode.py");

while path.pop() {
if path.join(&path_postfix).is_file() {
return Some(path);
}
}

None
}
Comment on lines -1037 to -1048
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to use a heuristic to find source root, when we can just provide it through bootstrap.


fn parse_edition(&self, line: &str) -> Option<String> {
self.parse_name_value_directive(line, "edition")
}
Expand Down Expand Up @@ -1093,9 +1080,9 @@ impl Config {

fn expand_variables(mut value: String, config: &Config) -> String {
const CWD: &str = "{{cwd}}";
const SRC_BASE: &str = "{{src-base}}";
const BUILD_BASE: &str = "{{build-base}}";
const RUST_SRC_BASE: &str = "{{rust-src-base}}";
const TEST_SUITE_SRC_BASE: &str = "{{test-suite-src-base}}";
const TEST_SUITE_BUILD_BASE: &str = "{{test-suite-build-base}}";
const SYSROOT_RUST_SRC_BASE: &str = "{{sysroot-rust-src-base}}";
const SYSROOT_BASE: &str = "{{sysroot-base}}";
const TARGET_LINKER: &str = "{{target-linker}}";
const TARGET: &str = "{{target}}";
Expand All @@ -1105,16 +1092,17 @@ fn expand_variables(mut value: String, config: &Config) -> String {
value = value.replace(CWD, &cwd.to_string_lossy());
}

if value.contains(SRC_BASE) {
value = value.replace(SRC_BASE, &config.src_base.to_string_lossy());
if value.contains(TEST_SUITE_SRC_BASE) {
value = value.replace(TEST_SUITE_SRC_BASE, &config.src_test_suite_root.to_string_lossy());
}

if value.contains(BUILD_BASE) {
value = value.replace(BUILD_BASE, &config.build_base.to_string_lossy());
if value.contains(TEST_SUITE_BUILD_BASE) {
value =
value.replace(TEST_SUITE_BUILD_BASE, &config.build_test_suite_root.to_string_lossy());
}

if value.contains(SYSROOT_BASE) {
value = value.replace(SYSROOT_BASE, &config.sysroot_base.to_string_lossy());
value = value.replace(SYSROOT_BASE, &config.sysroot.to_string_lossy());
}

if value.contains(TARGET_LINKER) {
Expand All @@ -1125,11 +1113,13 @@ fn expand_variables(mut value: String, config: &Config) -> String {
value = value.replace(TARGET, &config.target);
}

if value.contains(RUST_SRC_BASE) {
let src_base = config.sysroot_base.join("lib/rustlib/src/rust");
src_base.try_exists().expect(&*format!("{} should exists", src_base.display()));
let src_base = src_base.read_link().unwrap_or(src_base);
value = value.replace(RUST_SRC_BASE, &src_base.to_string_lossy());
if value.contains(SYSROOT_RUST_SRC_BASE) {
let sysroot_rust_src_base = config.sysroot.join("lib/rustlib/src/rust");
sysroot_rust_src_base
.try_exists()
.expect(&*format!("{} should exists", sysroot_rust_src_base.display()));
let src_base = sysroot_rust_src_base.read_link().unwrap_or(sysroot_rust_src_base);
value = value.replace(SYSROOT_RUST_SRC_BASE, &src_base.to_string_lossy());
}

value
Expand Down
2 changes: 2 additions & 0 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ impl ConfigBuilder {
"--android-cross-path=",
"--stage-id",
self.stage_id.as_deref().unwrap_or("stage2-x86_64-unknown-linux-gnu"),
"--stage",
"2",
"--channel",
self.channel.as_deref().unwrap_or("nightly"),
"--host",
Expand Down
96 changes: 64 additions & 32 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,17 @@ pub fn parse_config(args: Vec<String>) -> Config {
.optopt("", "jsondoclint-path", "path to jsondoclint to use for doc tests", "PATH")
.optopt("", "run-clang-based-tests-with", "path to Clang executable", "PATH")
.optopt("", "llvm-filecheck", "path to LLVM's FileCheck binary", "DIR")
.reqopt("", "src-base", "directory to scan for test files", "PATH")
.reqopt("", "build-base", "directory to deposit test outputs", "PATH")
.reqopt("", "sysroot-base", "directory containing the compiler sysroot", "PATH")
.reqopt("", "src-root", "root directory of rust-lang/rust source files", "PATH")
.reqopt(
"",
"src-test-suite-root",
"root directory of the test suite (e.g. `tests/ui/`)",
"PATH",
)
.reqopt("", "build-root", "build directory", "PATH")
.reqopt("", "build-test-suite-root", "directory to deposit test outputs", "PATH")
.reqopt("", "sysroot", "directory containing the compiler sysroot", "PATH")
.reqopt("", "stage", "test stage", "N")
.reqopt("", "stage-id", "the target-stage identifier", "stageN-TARGET")
.reqopt(
"",
Expand Down Expand Up @@ -242,7 +250,12 @@ pub fn parse_config(args: Vec<String>) -> Config {
|| header::extract_llvm_version_from_binary(&matches.opt_str("llvm-filecheck")?),
);

let src_base = opt_path(matches, "src-base");
let stage = if let Some(ref stage) = matches.opt_str("stage") {
stage.parse::<u32>().expect("expected `--stage` to be a non-negative integer")
} else {
panic!("expected `--stage` number")
};

let run_ignored = matches.opt_present("ignored");
let with_rustc_debug_assertions = matches.opt_present("with-rustc-debug-assertions");
let with_std_debug_assertions = matches.opt_present("with-std-debug-assertions");
Expand Down Expand Up @@ -308,10 +321,18 @@ pub fn parse_config(args: Vec<String>) -> Config {
run_clang_based_tests_with: matches.opt_str("run-clang-based-tests-with"),
llvm_filecheck: matches.opt_str("llvm-filecheck").map(PathBuf::from),
llvm_bin_dir: matches.opt_str("llvm-bin-dir").map(PathBuf::from),
src_base,
build_base: opt_path(matches, "build-base"),
sysroot_base: opt_path(matches, "sysroot-base"),

src_root: opt_path(matches, "src-root"),
src_test_suite_root: opt_path(matches, "src-test-suite-root"),

build_root: opt_path(matches, "build-root"),
build_test_suite_root: opt_path(matches, "build-test-suite-root"),

sysroot: opt_path(matches, "sysroot"),

stage,
stage_id: matches.opt_str("stage-id").unwrap(),

mode,
suite: matches.opt_str("suite").unwrap(),
debugger: matches.opt_str("debugger").map(|debugger| {
Expand Down Expand Up @@ -413,9 +434,16 @@ pub fn log_config(config: &Config) {
logv(c, format!("rustc_path: {:?}", config.rustc_path.display()));
logv(c, format!("cargo_path: {:?}", config.cargo_path));
logv(c, format!("rustdoc_path: {:?}", config.rustdoc_path));
logv(c, format!("src_base: {:?}", config.src_base.display()));
logv(c, format!("build_base: {:?}", config.build_base.display()));

logv(c, format!("src_root: {:?}", config.src_root.display()));
logv(c, format!("src_test_suite_root: {:?}", config.src_test_suite_root.display()));

logv(c, format!("build_root: {:?}", config.build_root.display()));
logv(c, format!("build_test_suite_root: {:?}", config.build_test_suite_root.display()));

logv(c, format!("stage: {}", config.stage));
logv(c, format!("stage_id: {}", config.stage_id));

logv(c, format!("mode: {}", config.mode));
logv(c, format!("run_ignored: {}", config.run_ignored));
logv(c, format!("filters: {:?}", config.filters));
Expand Down Expand Up @@ -463,7 +491,7 @@ pub fn run_tests(config: Arc<Config>) {
// we first make sure that the coverage file does not exist.
// It will be created later on.
if config.rustfix_coverage {
let mut coverage_file_path = config.build_base.clone();
let mut coverage_file_path = config.build_test_suite_root.clone();
coverage_file_path.push("rustfix_missing_coverage.txt");
if coverage_file_path.exists() {
if let Err(e) = fs::remove_file(&coverage_file_path) {
Expand Down Expand Up @@ -610,20 +638,29 @@ struct TestCollector {
/// regardless of whether any filters/tests were specified on the command-line,
/// because filtering is handled later by libtest.
pub fn collect_and_make_tests(config: Arc<Config>) -> Vec<test::TestDescAndFn> {
debug!("making tests from {:?}", config.src_base.display());
debug!("making tests from {:?}", config.src_root.display());
let common_inputs_stamp = common_inputs_stamp(&config);
let modified_tests = modified_tests(&config, &config.src_base).unwrap_or_else(|err| {
panic!("modified_tests got error from dir: {}, error: {}", config.src_base.display(), err)
});
let modified_tests =
modified_tests(&config, &config.src_test_suite_root).unwrap_or_else(|err| {
panic!(
"modified_tests got error from dir: {}, error: {}",
config.src_root.display(),
err
)
});
let cache = HeadersCache::load(&config);

let cx = TestCollectorCx { config, cache, common_inputs_stamp, modified_tests };
let mut collector =
TestCollector { tests: vec![], found_path_stems: HashSet::new(), poisoned: false };

collect_tests_from_dir(&cx, &mut collector, &cx.config.src_base, Path::new("")).unwrap_or_else(
|reason| panic!("Could not read tests from {}: {reason}", cx.config.src_base.display()),
);
collect_tests_from_dir(&cx, &mut collector, &cx.config.src_test_suite_root, Path::new(""))
.unwrap_or_else(|reason| {
panic!(
"Could not read tests from {}: {reason}",
cx.config.src_test_suite_root.display()
)
});

let TestCollector { tests, found_path_stems, poisoned } = collector;

Expand All @@ -645,8 +682,7 @@ pub fn collect_and_make_tests(config: Arc<Config>) -> Vec<test::TestDescAndFn> {
/// common to some subset of tests, and are hopefully unlikely to be modified
/// while working on other tests.)
fn common_inputs_stamp(config: &Config) -> Stamp {
let rust_src_dir = config.find_rust_src_root().expect("Could not find Rust source root");

let src_root = &config.src_root;
let mut stamp = Stamp::from_path(&config.rustc_path);

// Relevant pretty printer files
Expand All @@ -660,17 +696,17 @@ fn common_inputs_stamp(config: &Config) -> Stamp {
"src/etc/lldb_providers.py",
];
for file in &pretty_printer_files {
let path = rust_src_dir.join(file);
let path = src_root.join(file);
stamp.add_path(&path);
}

stamp.add_dir(&rust_src_dir.join("src/etc/natvis"));
stamp.add_dir(&src_root.join("src").join("etc").join("natvis"));

stamp.add_dir(&config.run_lib_path);

if let Some(ref rustdoc_path) = config.rustdoc_path {
stamp.add_path(&rustdoc_path);
stamp.add_path(&rust_src_dir.join("src/etc/htmldocck.py"));
stamp.add_path(&src_root.join("src/etc/htmldocck.py"));
}

// Re-run coverage tests if the `coverage-dump` tool was modified,
Expand All @@ -679,10 +715,10 @@ fn common_inputs_stamp(config: &Config) -> Stamp {
stamp.add_path(coverage_dump_path)
}

stamp.add_dir(&rust_src_dir.join("src/tools/run-make-support"));
stamp.add_dir(&src_root.join("src/tools/run-make-support"));

// Compiletest itself.
stamp.add_dir(&rust_src_dir.join("src/tools/compiletest/"));
stamp.add_dir(&src_root.join("src/tools/compiletest/"));

stamp
}
Expand Down Expand Up @@ -923,10 +959,7 @@ fn files_related_to_test(
}

// `minicore.rs` test auxiliary: we need to make sure tests get rerun if this changes.
//
// FIXME(jieyouxu): untangle these paths, we should provide both a path to root `tests/` or
// `tests/auxiliary/` and the test suite in question. `src_base` is also a terrible name.
related.push(config.src_base.parent().unwrap().join("auxiliary").join("minicore.rs"));
related.push(config.src_root.join("tests").join("auxiliary").join("minicore.rs"));

related
}
Expand Down Expand Up @@ -1016,10 +1049,9 @@ fn make_test_name(
testpaths: &TestPaths,
revision: Option<&str>,
) -> test::TestName {
// Print the name of the file, relative to the repository root.
// `src_base` looks like `/path/to/rust/tests/ui`
let root_directory = config.src_base.parent().unwrap().parent().unwrap();
let path = testpaths.file.strip_prefix(root_directory).unwrap();
// Print the name of the file, relative to the repository root. `src_base` looks like
// `/path/to/rust/tests/ui`.
let path = testpaths.file.strip_prefix(&config.src_test_suite_root).unwrap();
let debugger = match config.debugger {
Some(d) => format!("-{}", d),
None => String::new(),
Expand Down
Loading
Loading