Skip to content

Commit e96bb6a

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #135926 - jieyouxu:needs-subprocess-thread, r=oli-obk
Implement `needs-subprocess` directive, and cleanup a bunch of tests to use `needs-{subprocess,threads}` ### Summary Closes #128295. - Implements `//@ needs-subprocess` directive in compiletest as requested in #128295. However, compiletest is a host tool, so we can't just try to spawn process because that spawns the process on *host*, not the *target*, under cross-compilation scenarios. - The short-term solution is to add *Yet Another* list of allow-list targets. - The long-term solution is to first check if a `$target` supports std, then try to run a binary to do run-time capability detection *on the target*. But that is tricky because you have to build-and-run a binary *for the target*. - This PR picks the short-term solution, because the long-term solution is highly non-trivial, and it's already an improvement over individual `ignore-*`s all over the place. - Opened an issue about the long-term solution in #135928. - Documents `//@ needs-subprocess` in rustc-dev-guide. - Replace `ignore-{wasm,wasm32,emscripten,sgx}` with `needs-{subprocess,threads}` where suitable in tests. - Some drive-by test changes as I was trying to figure out if I could use `needs-{subprocess,threads}` and found some bits needlessly distracting. Count of tests that use `ignore-{wasm,wasm32,emscripten,sgx}` before and after this PR: | State | `ignore-sgx` | `ignore-wasm` | `ignore-emscripten` | | - | - | - | - | | Before this PR | 96 | 88 | 207 | | After this PR | 36 | 38 | 61 | <details> <summary>Commands used to find out locally</summary> ``` --- before [17:40] Joe:rust (fresh) | rg --no-ignore -l "ignore-sgx" tests | wc -l 96 [17:40] Joe:rust (fresh) | rg --no-ignore -l "ignore-wasm" tests | wc -l 88 [17:40] Joe:rust (fresh) | rg --no-ignore -l "ignore-emscripten" tests | wc -l 207 --- after [17:39] Joe:rust (needs-subprocess-thread) | rg --no-ignore -l "ignore-sgx" tests | wc -l 36 [17:39] Joe:rust (needs-subprocess-thread) | rg --no-ignore -l "ignore-wasm" tests | wc -l 38 [17:39] Joe:rust (needs-subprocess-thread) | rg --no-ignore -l "ignore-emscripten" tests | wc -l 61 ``` </details> ### Review advice - Best reviewed commit-by-commit. - Non-trivial test changes (not mechanically simple replacements) are split into individual commits to help with review. Their individual commit messages give some basic description of the changes. - I *could* split some test changes out into another PR, but I found that I needed to change some tests to `needs-threads`, some to `needs-subprocess`, and some needed to use *both*, so they might conflict and become very annoying. --- r? ``@ghost`` (need to run try jobs) try-job: x86_64-msvc-1 try-job: i686-msvc-1 try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: aarch64-gnu try-job: test-various try-job: armhf-gnu
2 parents 135cd69 + 071ad37 commit e96bb6a

File tree

214 files changed

+292
-334
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

214 files changed

+292
-334
lines changed

Diff for: ‎src/doc/rustc-dev-guide/src/tests/directives.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ for more details.
9494
| Directive | Explanation | Supported test suites | Possible values |
9595
|-----------------------------------|--------------------------------------------------------------------------------------------------------------------------|----------------------------------------------|-----------------------------------------------------------------------------------------|
9696
| `check-run-results` | Check run test binary `run-{pass,fail}` output snapshot | `ui`, `crashes`, `incremental` if `run-pass` | N/A |
97-
| `error-pattern` | Check that output contains a specific string | `ui`, `crashes`, `incremental` if `run-pass` | String |
97+
| `error-pattern` | Check that output contains a specific string | `ui`, `crashes`, `incremental` if `run-pass` | String |
9898
| `regex-error-pattern` | Check that output contains a regex pattern | `ui`, `crashes`, `incremental` if `run-pass` | Regex |
9999
| `check-stdout` | Check `stdout` against `error-pattern`s from running test binary[^check_stdout] | `ui`, `crashes`, `incremental` | N/A |
100100
| `normalize-stderr-32bit` | Normalize actual stderr (for 32-bit platforms) with a rule `"<raw>" -> "<normalized>"` before comparing against snapshot | `ui`, `incremental` | `"<RAW>" -> "<NORMALIZED>"`, `<RAW>`/`<NORMALIZED>` is regex capture and replace syntax |
@@ -176,6 +176,7 @@ settings:
176176
- `needs-rust-lld` — ignores if the rust lld support is not enabled (`rust.lld =
177177
true` in `config.toml`)
178178
- `needs-threads` — ignores if the target does not have threading support
179+
- `needs-subprocess` — ignores if the target does not have subprocess support
179180
- `needs-symlink` — ignores if the target does not support symlinks. This can be
180181
the case on Windows if the developer did not enable privileged symlink
181182
permissions.

Diff for: ‎src/tools/compiletest/src/common.rs

+11
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,17 @@ impl Config {
488488
git_merge_commit_email: &self.git_merge_commit_email,
489489
}
490490
}
491+
492+
pub fn has_subprocess_support(&self) -> bool {
493+
// FIXME(#135928): compiletest is always a **host** tool. Building and running an
494+
// capability detection executable against the **target** is not trivial. The short term
495+
// solution here is to hard-code some targets to allow/deny, unfortunately.
496+
497+
let unsupported_target = self.target_cfg().env == "sgx"
498+
|| matches!(self.target_cfg().arch.as_str(), "wasm32" | "wasm64")
499+
|| self.target_cfg().os == "emscripten";
500+
!unsupported_target
501+
}
491502
}
492503

493504
/// Known widths of `target_has_atomic`.

Diff for: ‎src/tools/compiletest/src/directive-list.rs

+1
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
152152
"needs-sanitizer-support",
153153
"needs-sanitizer-thread",
154154
"needs-std-debug-assertions",
155+
"needs-subprocess",
155156
"needs-symlink",
156157
"needs-target-has-atomic",
157158
"needs-threads",

Diff for: ‎src/tools/compiletest/src/header/needs.rs

+8
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ pub(super) fn handle_needs(
9494
condition: config.has_threads(),
9595
ignore_reason: "ignored on targets without threading support",
9696
},
97+
Need {
98+
name: "needs-subprocess",
99+
condition: config.has_subprocess_support(),
100+
ignore_reason: "ignored on targets without subprocess support",
101+
},
97102
Need {
98103
name: "needs-unwind",
99104
condition: config.can_unwind(),
@@ -351,6 +356,9 @@ fn find_dlltool(config: &Config) -> bool {
351356
dlltool_found
352357
}
353358

359+
// FIXME(#135928): this is actually not quite right because this detection is run on the **host**.
360+
// This however still helps the case of windows -> windows local development in case symlinks are
361+
// not available.
354362
#[cfg(windows)]
355363
fn has_symlinks() -> bool {
356364
if std::env::var_os("CI").is_some() {

Diff for: ‎src/tools/tidy/src/issues.txt

-1
Original file line numberDiff line numberDiff line change
@@ -2204,7 +2204,6 @@ ui/issues/issue-3895.rs
22042204
ui/issues/issue-38954.rs
22052205
ui/issues/issue-38987.rs
22062206
ui/issues/issue-39089.rs
2207-
ui/issues/issue-39175.rs
22082207
ui/issues/issue-39211.rs
22092208
ui/issues/issue-39367.rs
22102209
ui/issues/issue-39548.rs

Diff for: ‎src/tools/tidy/src/ui_tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use ignore::Walk;
1717
const ENTRY_LIMIT: u32 = 901;
1818
// FIXME: The following limits should be reduced eventually.
1919

20-
const ISSUES_ENTRY_LIMIT: u32 = 1664;
20+
const ISSUES_ENTRY_LIMIT: u32 = 1662;
2121

2222
const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
2323
"rs", // test source files

Diff for: ‎tests/ui/abi/homogenous-floats-target-feature-mixup.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55
// without #[repr(simd)]
66

77
//@ run-pass
8-
//@ ignore-wasm32 no processes
9-
//@ ignore-sgx no processes
8+
//@ needs-subprocess
109

1110
#![feature(avx512_target_feature)]
1211

Diff for: ‎tests/ui/abi/segfault-no-out-of-stack.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//@ run-pass
2-
//@ ignore-wasm32 can't run commands
3-
//@ ignore-sgx no processes
2+
//@ needs-subprocess
43
//@ ignore-fuchsia must translate zircon signal to SIGSEGV/SIGBUS, FIXME (#58590)
54

65
#![feature(rustc_private)]

Diff for: ‎tests/ui/abi/stack-probes-lto.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//@[aarch64] only-aarch64
44
//@[x32] only-x86
55
//@[x64] only-x86_64
6-
//@ ignore-sgx no processes
6+
//@ needs-subprocess
77
//@ ignore-musl FIXME #31506
88
//@ ignore-fuchsia no exception handler registered for segfault
99
//@ compile-flags: -C lto

Diff for: ‎tests/ui/abi/stack-probes.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
//@[aarch64] only-aarch64
44
//@[x32] only-x86
55
//@[x64] only-x86_64
6-
//@ ignore-emscripten no processes
7-
//@ ignore-sgx no processes
6+
//@ needs-subprocess
87
//@ ignore-fuchsia no exception handler registered for segfault
98
//@ ignore-nto Crash analysis impossible at SIGSEGV in QNX Neutrino
109
//@ ignore-ios Stack probes are enabled, but the SIGSEGV handler isn't

Diff for: ‎tests/ui/alloc-error/default-alloc-error-hook.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//@ run-pass
2-
//@ ignore-wasm32 no processes
3-
//@ ignore-sgx no processes
2+
//@ needs-subprocess
43

54
use std::alloc::{Layout, handle_alloc_error};
65
use std::env;

Diff for: ‎tests/ui/array-slice-vec/bounds-check-no-overflow.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ run-fail
22
//@ error-pattern:index out of bounds
3-
//@ ignore-emscripten no processes
3+
//@ needs-subprocess
44

55
use std::mem::size_of;
66

Diff for: ‎tests/ui/array-slice-vec/box-of-array-of-drop-1.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
//@ run-pass
22
//@ needs-unwind
3+
//@ needs-threads
4+
35
#![allow(overflowing_literals)]
46

57
// Test that we cleanup a fixed size Box<[D; k]> properly when D has a
68
// destructor.
79

8-
//@ ignore-emscripten no threads support
9-
1010
use std::thread;
1111
use std::sync::atomic::{AtomicUsize, Ordering};
1212

Diff for: ‎tests/ui/array-slice-vec/box-of-array-of-drop-2.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
//@ run-pass
22
//@ needs-unwind
3+
//@ needs-threads
4+
35
#![allow(overflowing_literals)]
46

57
// Test that we cleanup dynamic sized Box<[D]> properly when D has a
68
// destructor.
79

8-
//@ ignore-emscripten no threads support
9-
1010
use std::thread;
1111
use std::sync::atomic::{AtomicUsize, Ordering};
1212

Diff for: ‎tests/ui/array-slice-vec/dst-raw-slice.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
//@ run-fail
44
//@ error-pattern:index out of bounds
5-
//@ ignore-emscripten no processes
5+
//@ needs-subprocess
66

77
#[allow(unconditional_panic)]
88
fn main() {

Diff for: ‎tests/ui/array-slice-vec/nested-vec-3.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
//@ run-pass
22
//@ needs-unwind
3-
#![allow(overflowing_literals)]
3+
//@ needs-threads
44

5-
//@ ignore-emscripten no threads support
5+
#![allow(overflowing_literals)]
66

77
// Test that using the `vec!` macro nested within itself works when
88
// the contents implement Drop and we hit a panic in the middle of

Diff for: ‎tests/ui/array-slice-vec/slice-panic-1.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//@ run-pass
22
//@ needs-unwind
3-
4-
//@ ignore-emscripten no threads support
3+
//@ needs-threads
54

65
// Test that if a slicing expr[..] fails, the correct cleanups happen.
76

Diff for: ‎tests/ui/array-slice-vec/slice-panic-2.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//@ run-pass
22
//@ needs-unwind
3-
4-
//@ ignore-emscripten no threads support
3+
//@ needs-threads
54

65
// Test that if a slicing expr[..] fails, the correct cleanups happen.
76

Diff for: ‎tests/ui/array-slice-vec/vec-overrun.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ run-fail
22
//@ error-pattern:index out of bounds: the len is 1 but the index is 2
3-
//@ ignore-emscripten no processes
3+
//@ needs-subprocess
44

55
fn main() {
66
let v: Vec<isize> = vec![10];

Diff for: ‎tests/ui/backtrace/backtrace.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
//@ run-pass
22
//@ ignore-android FIXME #17520
3-
//@ ignore-wasm32 spawning processes is not supported
3+
//@ needs-subprocess
44
//@ ignore-openbsd no support for libbacktrace without filename
5-
//@ ignore-sgx no processes
65
//@ ignore-msvc see #62897 and `backtrace-debuginfo.rs` test
76
//@ ignore-fuchsia Backtraces not symbolized
87
//@ compile-flags:-g

Diff for: ‎tests/ui/backtrace/std-backtrace.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
//@ run-pass
22
//@ ignore-android FIXME #17520
3-
//@ ignore-wasm32 spawning processes is not supported
3+
//@ needs-subprocess
44
//@ ignore-openbsd no support for libbacktrace without filename
5-
//@ ignore-sgx no processes
65
//@ ignore-fuchsia Backtraces not symbolized
76
//@ compile-flags:-g
87
//@ compile-flags:-Cstrip=none

Diff for: ‎tests/ui/binop/binop-fail-3.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ run-fail
22
//@ error-pattern:quux
3-
//@ ignore-emscripten no processes
3+
//@ needs-subprocess
44

55
fn foo() -> ! {
66
panic!("quux");

Diff for: ‎tests/ui/binop/binop-panic.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ run-fail
22
//@ error-pattern:quux
3-
//@ ignore-emscripten no processes
3+
//@ needs-subprocess
44

55
fn my_err(s: String) -> ! {
66
println!("{}", s);

Diff for: ‎tests/ui/borrowck/borrowck-local-borrow.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ run-fail
22
//@ error-pattern:panic 1
3-
//@ ignore-emscripten no processes
3+
//@ needs-subprocess
44

55
fn main() {
66
let x = 2;

Diff for: ‎tests/ui/borrowck/issue-28934.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
//@ run-fail
55
//@ error-pattern:explicit panic
6-
//@ ignore-emscripten no processes
6+
//@ needs-subprocess
77

88
struct Parser<'i: 't, 't>(&'i u8, &'t u8);
99

Diff for: ‎tests/ui/box/unit/unwind-unique.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ run-pass
22
//@ needs-unwind
3-
//@ ignore-emscripten no threads support
3+
//@ needs-threads
44

55
use std::thread;
66

Diff for: ‎tests/ui/cleanup-rvalue-temp-during-incomplete-alloc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
// It's unclear how likely such a bug is to recur, but it seems like a
2121
// scenario worth testing.
2222

23-
//@ ignore-emscripten no threads support
23+
//@ needs-threads
2424

2525
use std::thread;
2626

Diff for: ‎tests/ui/closures/diverging-closure.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ run-fail
22
//@ error-pattern:oops
3-
//@ ignore-emscripten no processes
3+
//@ needs-subprocess
44

55
fn main() {
66
let func = || -> ! {

Diff for: ‎tests/ui/command/command-argv0.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
//@ run-pass
22

3-
//@ ignore-windows - this is a unix-specific test
4-
//@ ignore-wasm32 no processes
5-
//@ ignore-sgx no processes
3+
//@ only-unix (this is a unix-specific test)
4+
//@ needs-subprocess
65
use std::env;
76
use std::os::unix::process::CommandExt;
87
use std::process::Command;

Diff for: ‎tests/ui/command/command-current-dir.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//@ run-pass
22
//@ no-prefer-dynamic We move the binary around, so do not depend dynamically on libstd
3-
//@ ignore-wasm32 no processes
4-
//@ ignore-sgx no processes
3+
//@ needs-subprocess
54
//@ ignore-fuchsia Needs directory creation privilege
65

76
use std::env;

Diff for: ‎tests/ui/command/command-exec.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
11
//@ run-pass
22

3-
#![allow(stable_features)]
4-
//@ ignore-windows - this is a unix-specific test
5-
//@ ignore-wasm32 no processes
6-
//@ ignore-sgx no processes
3+
//@ only-unix (this is a unix-specific test)
4+
//@ needs-subprocess
75
//@ ignore-fuchsia no execvp syscall provided
86

9-
#![feature(process_exec)]
10-
117
use std::env;
128
use std::os::unix::process::CommandExt;
139
use std::process::Command;

Diff for: ‎tests/ui/command/command-pre-exec.rs

+4-6
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
//@ run-pass
2-
3-
#![allow(stable_features)]
4-
//@ ignore-windows - this is a unix-specific test
5-
//@ ignore-wasm32 no processes
6-
//@ ignore-sgx no processes
2+
//@ only-unix (this is a unix-specific test)
3+
//@ needs-subprocess
74
//@ ignore-fuchsia no execvp syscall
8-
#![feature(process_exec, rustc_private)]
5+
6+
#![feature(rustc_private)]
97

108
extern crate libc;
119

Diff for: ‎tests/ui/command/command-setgroups.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
//@ run-pass
2-
//@ ignore-windows - this is a unix-specific test
3-
//@ ignore-wasm32
4-
//@ ignore-sgx
2+
//@ only-unix (this is a unix-specific test)
53
//@ ignore-musl - returns dummy result for _SC_NGROUPS_MAX
64
//@ ignore-nto - does not have `/bin/id`, expects groups to be i32 (not u32)
5+
//@ needs-subprocess
76

87
#![feature(rustc_private)]
98
#![feature(setgroups)]

Diff for: ‎tests/ui/command/command-uid-gid.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
//@ run-pass
22
//@ ignore-android
3-
//@ ignore-emscripten
4-
//@ ignore-sgx
53
//@ ignore-fuchsia no '/bin/sh', '/bin/ls'
4+
//@ needs-subprocess
65

76
#![feature(rustc_private)]
87

Diff for: ‎tests/ui/command/issue-10626.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//@ run-pass
2-
//@ ignore-wasm32 no processes
3-
//@ ignore-sgx no processes
2+
//@ needs-subprocess
43

54
// Make sure that if a process doesn't have its stdio/stderr descriptors set up
65
// that we don't die in a large ball of fire

Diff for: ‎tests/ui/consts/issue-29798.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ run-fail
22
//@ error-pattern:index out of bounds: the len is 5 but the index is 5
3-
//@ ignore-emscripten no processes
3+
//@ needs-subprocess
44

55
const fn test(x: usize) -> i32 {
66
[42;5][x]

Diff for: ‎tests/ui/coroutine/coroutine-resume-after-panic.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//@ run-fail
22
//@ needs-unwind
33
//@ error-pattern:coroutine resumed after panicking
4-
//@ ignore-emscripten no processes
4+
//@ needs-subprocess
55

66
// Test that we get the correct message for resuming a panicked coroutine.
77

Diff for: ‎tests/ui/drop/drop-trait-enum.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#![allow(dead_code)]
33
#![allow(unused_assignments)]
44
#![allow(unused_variables)]
5-
//@ ignore-emscripten no threads support
5+
//@ needs-threads
66
//@ needs-unwind
77

88
use std::thread;

0 commit comments

Comments
 (0)