Skip to content

Commit 59dc6d6

Browse files
committed
compiletest: remove legacy Makefile-based run-make support
1 parent cb08599 commit 59dc6d6

File tree

4 files changed

+18
-214
lines changed

4 files changed

+18
-214
lines changed

src/tools/compiletest/src/header.rs

+7-9
Original file line numberDiff line numberDiff line change
@@ -709,11 +709,11 @@ impl TestProps {
709709
/// returns a struct containing various parts of the directive.
710710
fn line_directive<'line>(
711711
line_number: usize,
712-
comment: &str,
713712
original_line: &'line str,
714713
) -> Option<DirectiveLine<'line>> {
715714
// Ignore lines that don't start with the comment prefix.
716-
let after_comment = original_line.trim_start().strip_prefix(comment)?.trim_start();
715+
let after_comment =
716+
original_line.trim_start().strip_prefix(COMPILETEST_DIRECTIVE_PREFIX)?.trim_start();
717717

718718
let revision;
719719
let raw_directive;
@@ -722,7 +722,7 @@ fn line_directive<'line>(
722722
// A comment like `//@[foo]` only applies to revision `foo`.
723723
let Some((line_revision, after_close_bracket)) = after_open_bracket.split_once(']') else {
724724
panic!(
725-
"malformed condition directive: expected `{comment}[foo]`, found `{original_line}`"
725+
"malformed condition directive: expected `{COMPILETEST_DIRECTIVE_PREFIX}[foo]`, found `{original_line}`"
726726
)
727727
};
728728

@@ -836,6 +836,8 @@ pub(crate) fn check_directive<'a>(
836836
CheckDirectiveResult { is_known_directive: is_known(&directive_name), trailing_directive }
837837
}
838838

839+
const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@";
840+
839841
fn iter_header(
840842
mode: Mode,
841843
_suite: &str,
@@ -849,8 +851,7 @@ fn iter_header(
849851
}
850852

851853
// Coverage tests in coverage-run mode always have these extra directives, without needing to
852-
// specify them manually in every test file. (Some of the comments below have been copied over
853-
// from the old `tests/run-make/coverage-reports/Makefile`, which no longer exists.)
854+
// specify them manually in every test file.
854855
//
855856
// FIXME(jieyouxu): I feel like there's a better way to do this, leaving for later.
856857
if mode == Mode::CoverageRun {
@@ -867,9 +868,6 @@ fn iter_header(
867868
}
868869
}
869870

870-
// NOTE(jieyouxu): once we get rid of `Makefile`s we can unconditionally check for `//@`.
871-
let comment = if testfile.extension().is_some_and(|e| e == "rs") { "//@" } else { "#" };
872-
873871
let mut rdr = BufReader::with_capacity(1024, rdr);
874872
let mut ln = String::new();
875873
let mut line_number = 0;
@@ -882,7 +880,7 @@ fn iter_header(
882880
}
883881
let ln = ln.trim();
884882

885-
let Some(directive_line) = line_directive(line_number, comment, ln) else {
883+
let Some(directive_line) = line_directive(line_number, ln) else {
886884
continue;
887885
};
888886

src/tools/compiletest/src/header/tests.rs

-9
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,6 @@ fn check_ignore(config: &Config, contents: &str) -> bool {
239239
d.ignore
240240
}
241241

242-
fn parse_makefile(config: &Config, contents: &str) -> EarlyProps {
243-
let bytes = contents.as_bytes();
244-
EarlyProps::from_reader(config, Path::new("Makefile"), bytes)
245-
}
246-
247242
#[test]
248243
fn should_fail() {
249244
let config: Config = cfg().build();
@@ -261,10 +256,6 @@ fn revisions() {
261256
let config: Config = cfg().build();
262257

263258
assert_eq!(parse_rs(&config, "//@ revisions: a b c").revisions, vec!["a", "b", "c"],);
264-
assert_eq!(
265-
parse_makefile(&config, "# revisions: hello there").revisions,
266-
vec!["hello", "there"],
267-
);
268259
}
269260

270261
#[test]

src/tools/compiletest/src/lib.rs

+11-32
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pub mod util;
2121

2222
use core::panic;
2323
use std::collections::HashSet;
24-
use std::ffi::{OsStr, OsString};
24+
use std::ffi::OsString;
2525
use std::io::{self, ErrorKind};
2626
use std::path::{Path, PathBuf};
2727
use std::process::{Command, Stdio};
@@ -268,12 +268,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
268268
let path = Path::new(f);
269269
let mut iter = path.iter().skip(1);
270270

271-
// We skip the test folder and check if the user passed `rmake.rs` or `Makefile`.
272-
if iter
273-
.next()
274-
.is_some_and(|s| s == OsStr::new("rmake.rs") || s == OsStr::new("Makefile"))
275-
&& iter.next().is_none()
276-
{
271+
// We skip the test folder and check if the user passed `rmake.rs`.
272+
if iter.next().is_some_and(|s| s == "rmake.rs") && iter.next().is_none() {
277273
path.parent().unwrap().to_str().unwrap().to_string()
278274
} else {
279275
f.to_string()
@@ -776,16 +772,9 @@ fn collect_tests_from_dir(
776772
return Ok(());
777773
}
778774

779-
// For run-make tests, a "test file" is actually a directory that contains
780-
// an `rmake.rs` or `Makefile`"
775+
// For run-make tests, a "test file" is actually a directory that contains an `rmake.rs`.
781776
if cx.config.mode == Mode::RunMake {
782-
if dir.join("Makefile").exists() && dir.join("rmake.rs").exists() {
783-
return Err(io::Error::other(
784-
"run-make tests cannot have both `Makefile` and `rmake.rs`",
785-
));
786-
}
787-
788-
if dir.join("Makefile").exists() || dir.join("rmake.rs").exists() {
777+
if dir.join("rmake.rs").exists() {
789778
let paths = TestPaths {
790779
file: dir.to_path_buf(),
791780
relative_dir: relative_dir_path.parent().unwrap().to_path_buf(),
@@ -854,24 +843,14 @@ pub fn is_test(file_name: &OsString) -> bool {
854843
!invalid_prefixes.iter().any(|p| file_name.starts_with(p))
855844
}
856845

857-
/// For a single test file, creates one or more test structures (one per revision)
858-
/// that can be handed over to libtest to run, possibly in parallel.
846+
/// For a single test file, creates one or more test structures (one per revision) that can be
847+
/// handed over to libtest to run, possibly in parallel.
859848
fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &TestPaths) {
860-
// For run-make tests, each "test file" is actually a _directory_ containing
861-
// an `rmake.rs` or `Makefile`. But for the purposes of directive parsing,
862-
// we want to look at that recipe file, not the directory itself.
849+
// For run-make tests, each "test file" is actually a _directory_ containing an `rmake.rs`. But
850+
// for the purposes of directive parsing, we want to look at that recipe file, not the directory
851+
// itself.
863852
let test_path = if cx.config.mode == Mode::RunMake {
864-
if testpaths.file.join("rmake.rs").exists() && testpaths.file.join("Makefile").exists() {
865-
panic!("run-make tests cannot have both `rmake.rs` and `Makefile`");
866-
}
867-
868-
if testpaths.file.join("rmake.rs").exists() {
869-
// Parse directives in rmake.rs.
870-
testpaths.file.join("rmake.rs")
871-
} else {
872-
// Parse directives in the Makefile.
873-
testpaths.file.join("Makefile")
874-
}
853+
testpaths.file.join("rmake.rs")
875854
} else {
876855
PathBuf::from(&testpaths.file)
877856
};

src/tools/compiletest/src/runtest/run_make.rs

-164
Original file line numberDiff line numberDiff line change
@@ -9,168 +9,6 @@ use crate::util::{copy_dir_all, dylib_env_var};
99

1010
impl TestCx<'_> {
1111
pub(super) fn run_rmake_test(&self) {
12-
let test_dir = &self.testpaths.file;
13-
if test_dir.join("rmake.rs").exists() {
14-
self.run_rmake_v2_test();
15-
} else if test_dir.join("Makefile").exists() {
16-
self.run_rmake_legacy_test();
17-
} else {
18-
self.fatal("failed to find either `rmake.rs` or `Makefile`")
19-
}
20-
}
21-
22-
fn run_rmake_legacy_test(&self) {
23-
let cwd = env::current_dir().unwrap();
24-
25-
// FIXME(Zalathar): This should probably be `output_base_dir` to avoid
26-
// an unnecessary extra subdirectory, but since legacy Makefile tests
27-
// are hopefully going away, it seems safer to leave this perilous code
28-
// as-is until it can all be deleted.
29-
let tmpdir = cwd.join(self.output_base_name());
30-
ignore_not_found(|| recursive_remove(&tmpdir)).unwrap();
31-
32-
fs::create_dir_all(&tmpdir).unwrap();
33-
34-
let host = &self.config.host;
35-
let make = if host.contains("dragonfly")
36-
|| host.contains("freebsd")
37-
|| host.contains("netbsd")
38-
|| host.contains("openbsd")
39-
|| host.contains("aix")
40-
{
41-
"gmake"
42-
} else {
43-
"make"
44-
};
45-
46-
let mut cmd = Command::new(make);
47-
cmd.current_dir(&self.testpaths.file)
48-
.stdout(Stdio::piped())
49-
.stderr(Stdio::piped())
50-
.env("TARGET", &self.config.target)
51-
.env("PYTHON", &self.config.python)
52-
.env("S", &self.config.src_root)
53-
.env("RUST_BUILD_STAGE", &self.config.stage_id)
54-
.env("RUSTC", cwd.join(&self.config.rustc_path))
55-
.env("TMPDIR", &tmpdir)
56-
.env("LD_LIB_PATH_ENVVAR", dylib_env_var())
57-
.env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path))
58-
.env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path))
59-
.env("LLVM_COMPONENTS", &self.config.llvm_components)
60-
// We for sure don't want these tests to run in parallel, so make
61-
// sure they don't have access to these vars if we run via `make`
62-
// at the top level
63-
.env_remove("MAKEFLAGS")
64-
.env_remove("MFLAGS")
65-
.env_remove("CARGO_MAKEFLAGS");
66-
67-
if let Some(ref cargo) = self.config.cargo_path {
68-
cmd.env("CARGO", cwd.join(cargo));
69-
}
70-
71-
if let Some(ref rustdoc) = self.config.rustdoc_path {
72-
cmd.env("RUSTDOC", cwd.join(rustdoc));
73-
}
74-
75-
if let Some(ref node) = self.config.nodejs {
76-
cmd.env("NODE", node);
77-
}
78-
79-
if let Some(ref linker) = self.config.target_linker {
80-
cmd.env("RUSTC_LINKER", linker);
81-
}
82-
83-
if let Some(ref clang) = self.config.run_clang_based_tests_with {
84-
cmd.env("CLANG", clang);
85-
}
86-
87-
if let Some(ref filecheck) = self.config.llvm_filecheck {
88-
cmd.env("LLVM_FILECHECK", filecheck);
89-
}
90-
91-
if let Some(ref llvm_bin_dir) = self.config.llvm_bin_dir {
92-
cmd.env("LLVM_BIN_DIR", llvm_bin_dir);
93-
}
94-
95-
if let Some(ref remote_test_client) = self.config.remote_test_client {
96-
cmd.env("REMOTE_TEST_CLIENT", remote_test_client);
97-
}
98-
99-
// We don't want RUSTFLAGS set from the outside to interfere with
100-
// compiler flags set in the test cases:
101-
cmd.env_remove("RUSTFLAGS");
102-
103-
// Use dynamic musl for tests because static doesn't allow creating dylibs
104-
if self.config.host.contains("musl") {
105-
cmd.env("RUSTFLAGS", "-Ctarget-feature=-crt-static").env("IS_MUSL_HOST", "1");
106-
}
107-
108-
if self.config.bless {
109-
cmd.env("RUSTC_BLESS_TEST", "--bless");
110-
// Assume this option is active if the environment variable is "defined", with _any_ value.
111-
// As an example, a `Makefile` can use this option by:
112-
//
113-
// ifdef RUSTC_BLESS_TEST
114-
// cp "$(TMPDIR)"/actual_something.ext expected_something.ext
115-
// else
116-
// $(DIFF) expected_something.ext "$(TMPDIR)"/actual_something.ext
117-
// endif
118-
}
119-
120-
if self.config.target.contains("msvc") && !self.config.cc.is_empty() {
121-
// We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe`
122-
// and that `lib.exe` lives next to it.
123-
let lib = Path::new(&self.config.cc).parent().unwrap().join("lib.exe");
124-
125-
// MSYS doesn't like passing flags of the form `/foo` as it thinks it's
126-
// a path and instead passes `C:\msys64\foo`, so convert all
127-
// `/`-arguments to MSVC here to `-` arguments.
128-
let cflags = self
129-
.config
130-
.cflags
131-
.split(' ')
132-
.map(|s| s.replace("/", "-"))
133-
.collect::<Vec<_>>()
134-
.join(" ");
135-
let cxxflags = self
136-
.config
137-
.cxxflags
138-
.split(' ')
139-
.map(|s| s.replace("/", "-"))
140-
.collect::<Vec<_>>()
141-
.join(" ");
142-
143-
cmd.env("IS_MSVC", "1")
144-
.env("IS_WINDOWS", "1")
145-
.env("MSVC_LIB", format!("'{}' -nologo", lib.display()))
146-
.env("MSVC_LIB_PATH", format!("{}", lib.display()))
147-
.env("CC", format!("'{}' {}", self.config.cc, cflags))
148-
.env("CXX", format!("'{}' {}", &self.config.cxx, cxxflags));
149-
} else {
150-
cmd.env("CC", format!("{} {}", self.config.cc, self.config.cflags))
151-
.env("CXX", format!("{} {}", self.config.cxx, self.config.cxxflags))
152-
.env("AR", &self.config.ar);
153-
154-
if self.config.target.contains("windows") {
155-
cmd.env("IS_WINDOWS", "1");
156-
}
157-
}
158-
159-
let (output, truncated) =
160-
self.read2_abbreviated(cmd.spawn().expect("failed to spawn `make`"));
161-
if !output.status.success() {
162-
let res = ProcRes {
163-
status: output.status,
164-
stdout: String::from_utf8_lossy(&output.stdout).into_owned(),
165-
stderr: String::from_utf8_lossy(&output.stderr).into_owned(),
166-
truncated,
167-
cmdline: format!("{:?}", cmd),
168-
};
169-
self.fatal_proc_rec("make failed", &res);
170-
}
171-
}
172-
173-
fn run_rmake_v2_test(&self) {
17412
// For `run-make` V2, we need to perform 2 steps to build and run a `run-make` V2 recipe
17513
// (`rmake.rs`) to run the actual tests. The support library is already built as a tool rust
17614
// library and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`.
@@ -191,8 +29,6 @@ impl TestCx<'_> {
19129
// recipes to `remove_dir_all($TMPDIR)` without running into issues related trying to remove
19230
// a currently running executable because the recipe executable is not under the
19331
// `rmake_out/` directory.
194-
//
195-
// This setup intentionally diverges from legacy Makefile run-make tests.
19632
let base_dir = self.output_base_dir();
19733
ignore_not_found(|| recursive_remove(&base_dir)).unwrap();
19834

0 commit comments

Comments
 (0)