Skip to content

compiletest: investigate if it's possible to do run-time capability detection against *target* #135928

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
jieyouxu opened this issue Jan 23, 2025 · 1 comment
Labels
A-compiletest Area: The compiletest test runner A-cross Area: Cross compilation C-enhancement Category: An issue proposing an enhancement or a PR with one. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-needs-design This issue needs exploration and design to see how and if we can fix/implement it T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Jan 23, 2025

Some tests are conditioned on capabilities, which are most accurately detected at run-time against the target under cross-compilation scenarios (compared to having to maintain allowlists or denylists of targets). However, implementing this is non-trivial:

  • You have to build and run the capability run-time test as an executable on the target, not the host. This may involve having to work with custom test runners (e.g. wasmtime) or remote-test-server.
  • The target may not support std!
@jieyouxu jieyouxu added A-compiletest Area: The compiletest test runner A-cross Area: Cross compilation C-enhancement Category: An issue proposing an enhancement or a PR with one. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 23, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 23, 2025
@jieyouxu jieyouxu added E-needs-design This issue needs exploration and design to see how and if we can fix/implement it E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. C-enhancement Category: An issue proposing an enhancement or a PR with one. A-cross Area: Cross compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-compiletest Area: The compiletest test runner and removed E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. C-enhancement Category: An issue proposing an enhancement or a PR with one. A-cross Area: Cross compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-compiletest Area: The compiletest test runner needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 23, 2025
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 23, 2025
…<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
@jieyouxu
Copy link
Member Author

Alternatively, another print flag: rustc --print=target-supports-std-subprocess.

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 23, 2025
…<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
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 24, 2025
…r=oli-obk

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. 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
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 24, 2025
…r=oli-obk

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. 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
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 24, 2025
Rollup merge of rust-lang#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 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. 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-compiletest Area: The compiletest test runner A-cross Area: Cross compilation C-enhancement Category: An issue proposing an enhancement or a PR with one. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-needs-design This issue needs exploration and design to see how and if we can fix/implement it T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
No open projects
Development

No branches or pull requests

2 participants