Skip to content

Commit 2568493

Browse files
Rollup merge of rust-lang#136581 - jieyouxu:makefile-be-gone, r=Kobzol
Retire the legacy `Makefile`-based `run-make` test infra The final piece of [porting run-make tests to use Rust rust-lang#121876](rust-lang#121876). Closes rust-lang#121876. Closes rust-lang#40713. Closes rust-lang#81791 (no longer using `wc`). Closes rust-lang#56475 (no longer a problem in current form of that test; we don't ignore the test on `aarch64-unknown-linux-gnu`). ### Summary This PR removes the legacy `Makefile`-based `run-make` test infra which has served us well over the years. The legacy infra is no longer needed since we ported all of `Makefile`-based `run-make` tests to the new `rmake.rs` infra. Additionally, this PR: - Removes `tests/run-make/tools.mk` since no more `Makefile`-based tests remain. - Updates `tests/run-make/README.md` and rustc-dev-guide docs to remove mention about `Makefile`-based `run-make` tests - Update test suite requirements in rustc-dev-guide on Windows to no longer need MSYS2 (they should also now run successfully on native Windows MSVC). - Update `triagebot.toml` to stop backlinking to rust-lang#121876. **Thanks to everyone who helped in this effort to modernize the `run-make` test infra and test suite!** r? bootstrap
2 parents f9e0239 + 95b030f commit 2568493

File tree

18 files changed

+28
-734
lines changed

18 files changed

+28
-734
lines changed

src/doc/rustc-dev-guide/src/tests/compiletest.md

+1-29
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ The following test suites are available, with links for more information:
7474

7575
### General purpose test suite
7676

77-
[`run-make`](#run-make-tests) are general purpose tests using Rust programs (or
78-
Makefiles (legacy)).
77+
[`run-make`](#run-make-tests) are general purpose tests using Rust programs.
7978

8079
### Rustdoc test suites
8180

@@ -396,14 +395,6 @@ your test, causing separate files to be generated for 32bit and 64bit systems.
396395

397396
### `run-make` tests
398397

399-
> **Note on phasing out `Makefile`s**
400-
>
401-
> We are planning to migrate all existing Makefile-based `run-make` tests
402-
> to Rust programs. You should not be adding new Makefile-based `run-make`
403-
> tests.
404-
>
405-
> See <https://github.com/rust-lang/rust/issues/121876>.
406-
407398
The tests in [`tests/run-make`] are general-purpose tests using Rust *recipes*,
408399
which are small programs (`rmake.rs`) allowing arbitrary Rust code such as
409400
`rustc` invocations, and is supported by a [`run_make_support`] library. Using
@@ -424,11 +415,6 @@ Compiletest directives like `//@ only-<target>` or `//@ ignore-<target>` are
424415
supported in `rmake.rs`, like in UI tests. However, revisions or building
425416
auxiliary via directives are not currently supported.
426417

427-
Two `run-make` tests are ported over to Rust recipes as examples:
428-
429-
- <https://github.com/rust-lang/rust/tree/master/tests/run-make/CURRENT_RUSTC_VERSION>
430-
- <https://github.com/rust-lang/rust/tree/master/tests/run-make/a-b-a-linker-guard>
431-
432418
#### Quickly check if `rmake.rs` tests can be compiled
433419

434420
You can quickly check if `rmake.rs` tests can be compiled without having to
@@ -481,20 +467,6 @@ Then add a corresponding entry to `"rust-analyzer.linkedProjects"`
481467
],
482468
```
483469

484-
#### Using Makefiles (legacy)
485-
486-
<div class="warning">
487-
You should avoid writing new Makefile-based `run-make` tests.
488-
</div>
489-
490-
Each test should be in a separate directory with a `Makefile` indicating the
491-
commands to run.
492-
493-
There is a [`tools.mk`] Makefile which you can include which provides a bunch of
494-
utilities to make it easier to run commands and compare outputs. Take a look at
495-
some of the other tests for some examples on how to get started.
496-
497-
[`tools.mk`]: https://github.com/rust-lang/rust/blob/master/tests/run-make/tools.mk
498470
[`tests/run-make`]: https://github.com/rust-lang/rust/tree/master/tests/run-make
499471
[`run_make_support`]: https://github.com/rust-lang/rust/tree/master/src/tools/run-make-support
500472

src/doc/rustc-dev-guide/src/tests/directives.md

+1-6
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@
66
FIXME(jieyouxu) completely revise this chapter.
77
-->
88

9-
Directives are special comments that tell compiletest how to build and interpret
10-
a test. They must appear before the Rust source in the test. They may also
11-
appear in `rmake.rs` or legacy Makefiles for [run-make
12-
tests](compiletest.md#run-make-tests).
9+
Directives are special comments that tell compiletest how to build and interpret a test. They must appear before the Rust source in the test. They may also appear in `rmake.rs` [run-make tests](compiletest.md#run-make-tests).
1310

1411
They are normally put after the short comment that explains the point of this
1512
test. Compiletest test suites use `//@` to signal that a comment is a directive.
@@ -222,8 +219,6 @@ The following directives will check LLVM support:
222219
[`aarch64-gnu-debug`]), which only runs a
223220
subset of `run-make` tests. Other tests with this directive will not
224221
run at all, which is usually not what you want.
225-
- Notably, the [`aarch64-gnu-debug`] CI job *currently* only runs `run-make`
226-
tests which additionally contain `clang` in their test name.
227222

228223
See also [Debuginfo tests](compiletest.md#debuginfo-tests) for directives for
229224
ignoring debuggers.

src/doc/rustc-dev-guide/src/tests/running.md

-25
Original file line numberDiff line numberDiff line change
@@ -238,30 +238,6 @@ This is much faster, but doesn't always work. For example, some tests include
238238
directives that specify specific compiler flags, or which rely on other crates,
239239
and they may not run the same without those options.
240240

241-
## Running `run-make` tests
242-
243-
### Windows
244-
245-
Running the `run-make` test suite on Windows is a currently bit more involved.
246-
There are numerous prerequisites and environmental requirements:
247-
248-
- Install msys2: <https://www.msys2.org/>
249-
- Specify `MSYS2_PATH_TYPE=inherit` in `msys2.ini` in the msys2 installation directory, run the
250-
following with `MSYS2 MSYS`:
251-
- `pacman -Syuu`
252-
- `pacman -S make`
253-
- `pacman -S diffutils`
254-
- `pacman -S binutils`
255-
- `./x test run-make` (`./x test tests/run-make` doesn't work)
256-
257-
There is [on-going work][port-run-make] to not rely on `Makefile`s in the
258-
run-make test suite. Once this work is completed, you can run the entire
259-
`run-make` test suite on native Windows inside `cmd` or `PowerShell` without
260-
needing to install and use MSYS2. As of <!--date-check --> Oct 2024, it is
261-
already possible to run the vast majority of the `run-make` test suite outside
262-
of MSYS2, but there will be failures for the tests that still use `Makefile`s
263-
due to not finding `make`.
264-
265241
## Running tests on a remote machine
266242

267243
Tests may be run on a remote machine (e.g. to test builds for a different
@@ -406,4 +382,3 @@ If you encounter bugs or problems, don't hesitate to open issues on the
406382
repository](https://github.com/rust-lang/rustc_codegen_gcc/).
407383

408384
[`tests/ui`]: https://github.com/rust-lang/rust/tree/master/tests/ui
409-
[port-run-make]: https://github.com/rust-lang/rust/issues/121876

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
};

0 commit comments

Comments
 (0)