Skip to content

Commit

Permalink
Auto merge of rust-lang#113026 - jieyouxu:run-make-v2, r=bjorn3
Browse files Browse the repository at this point in the history
Introduce `run-make` V2 infrastructure, a `run_make_support` library and port over 2 tests as example

See [issue rust-lang#40713: Switch run-make tests from Makefiles to rust](rust-lang#40713) for more context.

### Basic Description of `run-make` V2

`run-make` V2 aims to eliminate the dependency on `make` and `Makefile`s for building `run-make`-style tests. Makefiles are replaced by *recipes* (`rmake.rs`). The current implementation runs `run-make` V2 tests in 3 steps:

1. We build the support library `run_make_support` which the `rmake.rs` recipes depend on as a tool dylib.
2. We build the recipe `rmake.rs` and link in the support library.
3. We run the recipe to build and run the tests.

`rmake.rs` is basically a replacement for `Makefile`, and allows running arbitrary Rust code.

### Planned Changes

- [x] Get rid of the builder style patterns in `rmake_support` and instead use something like

    ```rust
    let output = rustc!(scx, "--cfg x -Cprefer-dynamic -Csymbol-mangling-version=legacy -
    Zunstable-options");
    ```
    as per Nils' suggestion. This can probably use something like `xshell`.
- [x] Make `run_make_support` into a proper crate so it can have external dependencies like `xshell`.
- [x] Instead of having an entire alternative directory `run-make-v2`, change how V2 tests are collected based on presence of `rmake.rs` recipe file. This should ease migration and prevent git history from being messed up by big moves.

### Disclaimer

The current implementation is very much a **very very rough prototype** just to get the 2 example tests working. I would appreciate any feedback on the design and implementation.
  • Loading branch information
bors committed Feb 13, 2024
2 parents a84bb95 + 21df159 commit 89d168d
Show file tree
Hide file tree
Showing 12 changed files with 505 additions and 32 deletions.
13 changes: 13 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3323,6 +3323,13 @@ dependencies = [
"serde_json",
]

[[package]]
name = "run_make_support"
version = "0.0.0"
dependencies = [
"shell-words",
]

[[package]]
name = "rust-demangler"
version = "0.0.1"
Expand Down Expand Up @@ -5034,6 +5041,12 @@ version = "0.1.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "45bb67a18fa91266cc7807181f62f9178a6873bfad7dc788c42e6430db40184f"

[[package]]
name = "shell-words"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "24188a676b6ae68c3b2cb3a01be17fbf7240ce009799bb56d5b1409051e78fde"

[[package]]
name = "shlex"
version = "1.3.0"
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ members = [
"src/tools/clippy",
"src/tools/clippy/clippy_dev",
"src/tools/compiletest",
"src/tools/run-make-support",
"src/tools/error_index_generator",
"src/tools/linkchecker",
"src/tools/lint-docs",
Expand Down
83 changes: 82 additions & 1 deletion src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use crate::core::config::flags::get_completion;
use crate::core::config::flags::Subcommand;
use crate::core::config::TargetSelection;
use crate::utils::cache::{Interned, INTERNER};
use crate::utils::dylib::shared_lib_name;
use crate::utils::exec::BootstrapCommand;
use crate::utils::helpers::{
self, add_link_lib_path, add_rustdoc_cargo_linker_args, dylib_path, dylib_path_var,
Expand Down Expand Up @@ -1327,6 +1328,53 @@ macro_rules! coverage_test_alias {
};
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
pub struct RunMakeSupport {
pub compiler: Compiler,
pub target: TargetSelection,
}

impl Step for RunMakeSupport {
type Output = PathBuf;
const DEFAULT: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.never()
}

fn make_run(run: RunConfig<'_>) {
let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple());
run.builder.ensure(RunMakeSupport { compiler, target: run.build_triple() });
}

fn run(self, builder: &Builder<'_>) -> PathBuf {
builder.ensure(compile::Std::new(self.compiler, self.target));

let cargo = tool::prepare_tool_cargo(
builder,
self.compiler,
Mode::ToolStd,
self.target,
"build",
"src/tools/run-make-support",
SourceType::InTree,
&[],
);

let mut cargo = Command::from(cargo);
cargo.env("RUSTFLAGS", "-C prefer-dynamic");
builder.run(&mut cargo);

let lib_name = shared_lib_name("run_make_support", &self.target.to_string());
let lib = builder.tools_dir(self.compiler).join(&lib_name);

let cargo_out =
builder.cargo_out(self.compiler, Mode::ToolStd, self.target).join(&lib_name);
builder.copy(&cargo_out, &lib);
lib
}
}

default_test!(Ui { path: "tests/ui", mode: "ui", suite: "ui" });

default_test!(RunPassValgrind {
Expand Down Expand Up @@ -1361,7 +1409,40 @@ host_test!(RustdocJson { path: "tests/rustdoc-json", mode: "rustdoc-json", suite

host_test!(Pretty { path: "tests/pretty", mode: "pretty", suite: "pretty" });

default_test!(RunMake { path: "tests/run-make", mode: "run-make", suite: "run-make" });
// Special-handling is needed for `run-make`, so don't use `default_test` for defining `RunMake`
// tests.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct RunMake {
pub compiler: Compiler,
pub target: TargetSelection,
}

impl Step for RunMake {
type Output = ();
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = false;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.suite_path("tests/run-make")
}

fn make_run(run: RunConfig<'_>) {
let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple());
run.builder.ensure(RunMakeSupport { compiler, target: run.build_triple() });
run.builder.ensure(RunMake { compiler, target: run.target });
}

fn run(self, builder: &Builder<'_>) {
builder.ensure(Compiletest {
compiler: self.compiler,
target: self.target,
mode: "run-make",
suite: "run-make",
path: "tests/run-make",
compare_mode: None,
});
}
}

host_test!(RunMakeFullDeps {
path: "tests/run-make-fulldeps",
Expand Down
13 changes: 13 additions & 0 deletions src/bootstrap/src/utils/dylib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,16 @@ pub fn exe(name: &str, target: &str) -> String {
name.to_string()
}
}

/// Given a shared library called `name`, return the filename for the
/// shared library for a particular target.
#[allow(dead_code)]
pub fn shared_lib_name(name: &str, target: &str) -> String {
if target.contains("windows") {
format!("{name}.dll")
} else if target.contains("apple") {
format!("lib{name}.dylib")
} else {
format!("lib{name}.so")
}
}
35 changes: 26 additions & 9 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,13 +655,21 @@ fn collect_tests_from_dir(
return Ok(());
}

if config.mode == Mode::RunMake && dir.join("Makefile").exists() {
let paths = TestPaths {
file: dir.to_path_buf(),
relative_dir: relative_dir_path.parent().unwrap().to_path_buf(),
};
tests.extend(make_test(config, cache, &paths, inputs, poisoned));
return Ok(());
if config.mode == Mode::RunMake {
if dir.join("Makefile").exists() && dir.join("rmake.rs").exists() {
return Err(io::Error::other(
"run-make tests cannot have both `Makefile` and `rmake.rs`",
));
}

if dir.join("Makefile").exists() || dir.join("rmake.rs").exists() {
let paths = TestPaths {
file: dir.to_path_buf(),
relative_dir: relative_dir_path.parent().unwrap().to_path_buf(),
};
tests.extend(make_test(config, cache, &paths, inputs, poisoned));
return Ok(());
}
}

// If we find a test foo/bar.rs, we have to build the
Expand Down Expand Up @@ -733,8 +741,17 @@ fn make_test(
poisoned: &mut bool,
) -> Vec<test::TestDescAndFn> {
let test_path = if config.mode == Mode::RunMake {
// Parse directives in the Makefile
testpaths.file.join("Makefile")
if testpaths.file.join("rmake.rs").exists() && testpaths.file.join("Makefile").exists() {
panic!("run-make tests cannot have both `rmake.rs` and `Makefile`");
}

if testpaths.file.join("rmake.rs").exists() {
// Parse directives in rmake.rs.
testpaths.file.join("rmake.rs")
} else {
// Parse directives in the Makefile.
testpaths.file.join("Makefile")
}
} else {
PathBuf::from(&testpaths.file)
};
Expand Down
Loading

0 comments on commit 89d168d

Please sign in to comment.