Skip to content

Commit

Permalink
Auto merge of rust-lang#135926 - jieyouxu:needs-subprocess-thread, r=…
Browse files Browse the repository at this point in the history
…<try>

Implement `needs-subprocess` directive, and cleanup a bunch of tests to use `needs-{subprocess,threads}`

### Summary

Closes rust-lang#128295.

- Implements `//@ needs-subprocess` directive in compiletest as requested in rust-lang#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 rust-lang#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.
- 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
  • Loading branch information
bors committed Jan 23, 2025
2 parents cf577f3 + b3f4819 commit 8622479
Show file tree
Hide file tree
Showing 211 changed files with 268 additions and 328 deletions.
3 changes: 2 additions & 1 deletion src/doc/rustc-dev-guide/src/tests/directives.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ for more details.
| Directive | Explanation | Supported test suites | Possible values |
|-----------------------------------|--------------------------------------------------------------------------------------------------------------------------|----------------------------------------------|-----------------------------------------------------------------------------------------|
| `check-run-results` | Check run test binary `run-{pass,fail}` output snapshot | `ui`, `crashes`, `incremental` if `run-pass` | N/A |
| `error-pattern` | Check that output contains a specific string | `ui`, `crashes`, `incremental` if `run-pass` | String |
| `error-pattern` | Check that output contains a specific string | `ui`, `crashes`, `incremental` if `run-pass` | String |
| `regex-error-pattern` | Check that output contains a regex pattern | `ui`, `crashes`, `incremental` if `run-pass` | Regex |
| `check-stdout` | Check `stdout` against `error-pattern`s from running test binary[^check_stdout] | `ui`, `crashes`, `incremental` | N/A |
| `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 |
Expand Down Expand Up @@ -176,6 +176,7 @@ settings:
- `needs-rust-lld` — ignores if the rust lld support is not enabled (`rust.lld =
true` in `config.toml`)
- `needs-threads` — ignores if the target does not have threading support
- `needs-subprocess` — ignores if the target does not have subprocess support
- `needs-symlink` — ignores if the target does not support symlinks. This can be
the case on Windows if the developer did not enable privileged symlink
permissions.
Expand Down
11 changes: 11 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,17 @@ impl Config {
git_merge_commit_email: &self.git_merge_commit_email,
}
}

pub fn has_subprocess_support(&self) -> bool {
// FIXME(#135928): compiletest is always a **host** tool. Building and running an
// capability detection executable against the **target** is not trivial. The short term
// solution here is to hard-code some targets to allow/deny, unfortunately.

let unsupported_target = self.target_cfg().env == "sgx"
|| matches!(self.target_cfg().arch.as_str(), "wasm32" | "wasm64")
|| self.target_cfg().os == "emscripten";
!unsupported_target
}
}

/// Known widths of `target_has_atomic`.
Expand Down
1 change: 1 addition & 0 deletions src/tools/compiletest/src/directive-list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"needs-sanitizer-support",
"needs-sanitizer-thread",
"needs-std-debug-assertions",
"needs-subprocess",
"needs-symlink",
"needs-target-has-atomic",
"needs-threads",
Expand Down
6 changes: 6 additions & 0 deletions src/tools/compiletest/src/header/needs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ pub(super) fn handle_needs(
condition: config.has_threads(),
ignore_reason: "ignored on targets without threading support",
},
Need {
name: "needs-subprocess",
condition: config.has_subprocess_support(),
ignore_reason: "ignored on targets without subprocess support",
},
Need {
name: "needs-unwind",
condition: config.can_unwind(),
Expand Down Expand Up @@ -351,6 +356,7 @@ fn find_dlltool(config: &Config) -> bool {
dlltool_found
}

// FIXME:
#[cfg(windows)]
fn has_symlinks() -> bool {
if std::env::var_os("CI").is_some() {
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/abi/homogenous-floats-target-feature-mixup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
// without #[repr(simd)]

//@ run-pass
//@ ignore-wasm32 no processes
//@ ignore-sgx no processes
//@ needs-subprocess

#![feature(avx512_target_feature)]

Expand Down
3 changes: 1 addition & 2 deletions tests/ui/abi/segfault-no-out-of-stack.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//@ run-pass
//@ ignore-wasm32 can't run commands
//@ ignore-sgx no processes
//@ needs-subprocess
//@ ignore-fuchsia must translate zircon signal to SIGSEGV/SIGBUS, FIXME (#58590)

#![feature(rustc_private)]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/abi/stack-probes-lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//@[aarch64] only-aarch64
//@[x32] only-x86
//@[x64] only-x86_64
//@ ignore-sgx no processes
//@ needs-subprocess
//@ ignore-musl FIXME #31506
//@ ignore-fuchsia no exception handler registered for segfault
//@ compile-flags: -C lto
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/abi/stack-probes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
//@[aarch64] only-aarch64
//@[x32] only-x86
//@[x64] only-x86_64
//@ ignore-emscripten no processes
//@ ignore-sgx no processes
//@ needs-subprocess
//@ ignore-fuchsia no exception handler registered for segfault
//@ ignore-nto Crash analysis impossible at SIGSEGV in QNX Neutrino
//@ ignore-ios Stack probes are enabled, but the SIGSEGV handler isn't
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/alloc-error/default-alloc-error-hook.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//@ run-pass
//@ ignore-wasm32 no processes
//@ ignore-sgx no processes
//@ needs-subprocess

use std::alloc::{Layout, handle_alloc_error};
use std::env;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/array-slice-vec/bounds-check-no-overflow.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-fail
//@ error-pattern:index out of bounds
//@ ignore-emscripten no processes
//@ needs-subprocess

use std::mem::size_of;

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/array-slice-vec/box-of-array-of-drop-1.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//@ run-pass
//@ needs-unwind
//@ needs-threads

#![allow(overflowing_literals)]

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

//@ ignore-emscripten no threads support

use std::thread;
use std::sync::atomic::{AtomicUsize, Ordering};

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/array-slice-vec/box-of-array-of-drop-2.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//@ run-pass
//@ needs-unwind
//@ needs-threads

#![allow(overflowing_literals)]

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

//@ ignore-emscripten no threads support

use std::thread;
use std::sync::atomic::{AtomicUsize, Ordering};

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/array-slice-vec/dst-raw-slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

//@ run-fail
//@ error-pattern:index out of bounds
//@ ignore-emscripten no processes
//@ needs-subprocess

#[allow(unconditional_panic)]
fn main() {
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/array-slice-vec/nested-vec-3.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//@ run-pass
//@ needs-unwind
#![allow(overflowing_literals)]
//@ needs-threads

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

// Test that using the `vec!` macro nested within itself works when
// the contents implement Drop and we hit a panic in the middle of
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/array-slice-vec/slice-panic-1.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//@ run-pass
//@ needs-unwind

//@ ignore-emscripten no threads support
//@ needs-threads

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

Expand Down
3 changes: 1 addition & 2 deletions tests/ui/array-slice-vec/slice-panic-2.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//@ run-pass
//@ needs-unwind

//@ ignore-emscripten no threads support
//@ needs-threads

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

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/array-slice-vec/vec-overrun.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-fail
//@ error-pattern:index out of bounds: the len is 1 but the index is 2
//@ ignore-emscripten no processes
//@ needs-subprocess

fn main() {
let v: Vec<isize> = vec![10];
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/backtrace/backtrace.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
//@ run-pass
//@ ignore-android FIXME #17520
//@ ignore-wasm32 spawning processes is not supported
//@ needs-subprocess
//@ ignore-openbsd no support for libbacktrace without filename
//@ ignore-sgx no processes
//@ ignore-msvc see #62897 and `backtrace-debuginfo.rs` test
//@ ignore-fuchsia Backtraces not symbolized
//@ compile-flags:-g
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/backtrace/std-backtrace.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
//@ run-pass
//@ ignore-android FIXME #17520
//@ ignore-wasm32 spawning processes is not supported
//@ needs-subprocess
//@ ignore-openbsd no support for libbacktrace without filename
//@ ignore-sgx no processes
//@ ignore-fuchsia Backtraces not symbolized
//@ compile-flags:-g
//@ compile-flags:-Cstrip=none
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/binop/binop-fail-3.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-fail
//@ error-pattern:quux
//@ ignore-emscripten no processes
//@ needs-subprocess

fn foo() -> ! {
panic!("quux");
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/binop/binop-panic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-fail
//@ error-pattern:quux
//@ ignore-emscripten no processes
//@ needs-subprocess

fn my_err(s: String) -> ! {
println!("{}", s);
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/borrowck/borrowck-local-borrow.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-fail
//@ error-pattern:panic 1
//@ ignore-emscripten no processes
//@ needs-subprocess

fn main() {
let x = 2;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/borrowck/issue-28934.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

//@ run-fail
//@ error-pattern:explicit panic
//@ ignore-emscripten no processes
//@ needs-subprocess

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

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/box/unit/unwind-unique.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-pass
//@ needs-unwind
//@ ignore-emscripten no threads support
//@ needs-threads

use std::thread;

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/cleanup-rvalue-temp-during-incomplete-alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// It's unclear how likely such a bug is to recur, but it seems like a
// scenario worth testing.

//@ ignore-emscripten no threads support
//@ needs-threads

use std::thread;

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/closures/diverging-closure.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-fail
//@ error-pattern:oops
//@ ignore-emscripten no processes
//@ needs-subprocess

fn main() {
let func = || -> ! {
Expand Down
5 changes: 2 additions & 3 deletions tests/ui/command/command-argv0.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
//@ run-pass

//@ ignore-windows - this is a unix-specific test
//@ ignore-wasm32 no processes
//@ ignore-sgx no processes
//@ only-unix (this is a unix-specific test)
//@ needs-subprocess
use std::env;
use std::os::unix::process::CommandExt;
use std::process::Command;
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/command/command-current-dir.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//@ run-pass
//@ no-prefer-dynamic We move the binary around, so do not depend dynamically on libstd
//@ ignore-wasm32 no processes
//@ ignore-sgx no processes
//@ needs-subprocess
//@ ignore-fuchsia Needs directory creation privilege

use std::env;
Expand Down
8 changes: 2 additions & 6 deletions tests/ui/command/command-exec.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
//@ run-pass

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

#![feature(process_exec)]

use std::env;
use std::os::unix::process::CommandExt;
use std::process::Command;
Expand Down
10 changes: 4 additions & 6 deletions tests/ui/command/command-pre-exec.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
//@ run-pass

#![allow(stable_features)]
//@ ignore-windows - this is a unix-specific test
//@ ignore-wasm32 no processes
//@ ignore-sgx no processes
//@ only-unix (this is a unix-specific test)
//@ needs-subprocess
//@ ignore-fuchsia no execvp syscall
#![feature(process_exec, rustc_private)]

#![feature(rustc_private)]

extern crate libc;

Expand Down
5 changes: 2 additions & 3 deletions tests/ui/command/command-setgroups.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
//@ run-pass
//@ ignore-windows - this is a unix-specific test
//@ ignore-wasm32
//@ ignore-sgx
//@ only-unix (this is a unix-specific test)
//@ ignore-musl - returns dummy result for _SC_NGROUPS_MAX
//@ ignore-nto - does not have `/bin/id`, expects groups to be i32 (not u32)
//@ needs-subprocess

#![feature(rustc_private)]
#![feature(setgroups)]
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/command/command-uid-gid.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
//@ run-pass
//@ ignore-android
//@ ignore-emscripten
//@ ignore-sgx
//@ ignore-fuchsia no '/bin/sh', '/bin/ls'
//@ needs-subprocess

#![feature(rustc_private)]

Expand Down
3 changes: 1 addition & 2 deletions tests/ui/command/issue-10626.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//@ run-pass
//@ ignore-wasm32 no processes
//@ ignore-sgx no processes
//@ needs-subprocess

// Make sure that if a process doesn't have its stdio/stderr descriptors set up
// that we don't die in a large ball of fire
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/issue-29798.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-fail
//@ error-pattern:index out of bounds: the len is 5 but the index is 5
//@ ignore-emscripten no processes
//@ needs-subprocess

const fn test(x: usize) -> i32 {
[42;5][x]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/coroutine/coroutine-resume-after-panic.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@ run-fail
//@ needs-unwind
//@ error-pattern:coroutine resumed after panicking
//@ ignore-emscripten no processes
//@ needs-subprocess

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

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/drop/drop-trait-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#![allow(dead_code)]
#![allow(unused_assignments)]
#![allow(unused_variables)]
//@ ignore-emscripten no threads support
//@ needs-threads
//@ needs-unwind

use std::thread;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/drop/terminate-in-initializer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-pass
//@ needs-unwind
//@ ignore-emscripten no threads support
//@ needs-threads

// Issue #787
// Don't try to clean up uninitialized locals
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/expr/if/expr-if-panic-fn.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-fail
//@ error-pattern:explicit panic
//@ ignore-emscripten no processes
//@ needs-subprocess

fn f() -> ! {
panic!()
Expand Down
Loading

0 comments on commit 8622479

Please sign in to comment.