-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Implement //@ needs-target-std
compiletest directive
#142297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
To centralize this hack in one place with a backlink to the issue tracking this hack, as this logic is also needed by compiletest to implement a `//@ needs-target-std` directive.
To support tests to condition their (potentially cross-compile) execution based on whether the target supports std.
This PR modifies If appropriate, please update The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable.
67affa4
to
cc61ec7
Compare
Instead of a jumble of `ignore-$target`s, `ignore-none` and `ignore-nvptx`.
cc61ec7
to
c558db3
Compare
Since you're already looking at it, r? Kobzol |
Maybe you could do the usual try build with "annoying CI jobs that cross-compile". Feel free to r=me with or without doing that. |
We have some metadata for that inn the Target json input/output. rust/compiler/rustc_target/src/json.rs Line 101 in 100199c
|
Please see #142296, I've looked at that avenue already. TL;DR not all built-in target specs have this metadata std field
(Which additionally, the |
@bors2 try jobs=test-various,armhf-gnu |
Implement `//@ needs-target-std` compiletest directive Closes #141863. Needed to unblock #139244 and #141856. ### Summary This PR implements a `//@ needs-target-std` compiletest directive that gates test execution based on whether the target supports std or not. For some cases, this should be preferred over e.g. some combination of `//@ ignore-none`, `//@ ignore-nvptx` and more[^none-limit]. ### Implementation limitation Unfortunately, since there is currently [no reliable way to determine from metadata whether a given target supports std or not](#142296), we have to resort to a hack. Bootstrap currently determines whether or not a target supports std by a naive target tuple substring comparison: a target supports std if its target tuple does *not* contain one of `["-none", "nvptx", "switch"]` substrings. This PR simply pulls that hack out into `build_helpers` to avoid reimplementing the same hack in compiletest, and uses that logic to inform `//@ needs-target-std`. ### Auxiliary changes This PR additionally changes a few run-make tests to use `//@ needs-target-std` over an inconsistent combination of target-based `ignore`s. This should help with #139244. --- r? bootstrap [^none-limit]: Notably, `target_os = "none"` is **not** a sufficient condition for "target does not support std" try-job: test-various try-job: armhf-gnu
@rustbot author |
@bors r=Kobzol rollup |
Implement `//@ needs-target-std` compiletest directive Closes rust-lang#141863. Needed to unblock rust-lang#139244 and rust-lang#141856. ### Summary This PR implements a `//@ needs-target-std` compiletest directive that gates test execution based on whether the target supports std or not. For some cases, this should be preferred over e.g. some combination of `//@ ignore-none`, `//@ ignore-nvptx` and more[^none-limit]. ### Implementation limitation Unfortunately, since there is currently [no reliable way to determine from metadata whether a given target supports std or not](rust-lang#142296), we have to resort to a hack. Bootstrap currently determines whether or not a target supports std by a naive target tuple substring comparison: a target supports std if its target tuple does *not* contain one of `["-none", "nvptx", "switch"]` substrings. This PR simply pulls that hack out into `build_helpers` to avoid reimplementing the same hack in compiletest, and uses that logic to inform `//@ needs-target-std`. ### Auxiliary changes This PR additionally changes a few run-make tests to use `//@ needs-target-std` over an inconsistent combination of target-based `ignore`s. This should help with rust-lang#139244. --- r? bootstrap [^none-limit]: Notably, `target_os = "none"` is **not** a sufficient condition for "target does not support std"
Rollup of 13 pull requests Successful merges: - #128425 (Make `missing_fragment_specifier` an unconditional error) - #141639 (Expose discriminant values in stable_mir) - #141967 (Configure bootstrap backport nominations through triagebot) - #142042 (Make E0621 missing lifetime suggestion verbose) - #142176 (tests: Split dont-shuffle-bswaps along opt-levels and arches) - #142235 (Build rustc with assertions in `dist-alt` jobs) - #142248 (Add supported asm types for LoongArch32) - #142272 (tests: Change ABIs in tests to more future-resilient ones) - #142282 (Only run `citool` tests on the `auto` branch) - #142285 (tests: Do not run afoul of asm.validity.non-exhaustive in input-stats) - #142297 (Implement `//@ needs-target-std` compiletest directive) - #142298 (Make loongarch-none target maintainers more easily pingable) - #142306 (Dont unwrap and re-wrap typing envs) Failed merges: - #141942 (Implement representation options to smir) r? `@ghost` `@rustbot` modify labels: rollup
Implement `//@ needs-target-std` compiletest directive Closes rust-lang#141863. Needed to unblock rust-lang#139244 and rust-lang#141856. ### Summary This PR implements a `//@ needs-target-std` compiletest directive that gates test execution based on whether the target supports std or not. For some cases, this should be preferred over e.g. some combination of `//@ ignore-none`, `//@ ignore-nvptx` and more[^none-limit]. ### Implementation limitation Unfortunately, since there is currently [no reliable way to determine from metadata whether a given target supports std or not](rust-lang#142296), we have to resort to a hack. Bootstrap currently determines whether or not a target supports std by a naive target tuple substring comparison: a target supports std if its target tuple does *not* contain one of `["-none", "nvptx", "switch"]` substrings. This PR simply pulls that hack out into `build_helpers` to avoid reimplementing the same hack in compiletest, and uses that logic to inform `//@ needs-target-std`. ### Auxiliary changes This PR additionally changes a few run-make tests to use `//@ needs-target-std` over an inconsistent combination of target-based `ignore`s. This should help with rust-lang#139244. --- r? bootstrap [^none-limit]: Notably, `target_os = "none"` is **not** a sufficient condition for "target does not support std"
Implement `//@ needs-target-std` compiletest directive Closes rust-lang#141863. Needed to unblock rust-lang#139244 and rust-lang#141856. ### Summary This PR implements a `//@ needs-target-std` compiletest directive that gates test execution based on whether the target supports std or not. For some cases, this should be preferred over e.g. some combination of `//@ ignore-none`, `//@ ignore-nvptx` and more[^none-limit]. ### Implementation limitation Unfortunately, since there is currently [no reliable way to determine from metadata whether a given target supports std or not](rust-lang#142296), we have to resort to a hack. Bootstrap currently determines whether or not a target supports std by a naive target tuple substring comparison: a target supports std if its target tuple does *not* contain one of `["-none", "nvptx", "switch"]` substrings. This PR simply pulls that hack out into `build_helpers` to avoid reimplementing the same hack in compiletest, and uses that logic to inform `//@ needs-target-std`. ### Auxiliary changes This PR additionally changes a few run-make tests to use `//@ needs-target-std` over an inconsistent combination of target-based `ignore`s. This should help with rust-lang#139244. --- r? bootstrap [^none-limit]: Notably, `target_os = "none"` is **not** a sufficient condition for "target does not support std"
Rollup of 13 pull requests Successful merges: - #134841 (Look at proc-macro attributes when encountering unknown attribute) - #141639 (Expose discriminant values in stable_mir) - #141967 (Configure bootstrap backport nominations through triagebot) - #142042 (Make E0621 missing lifetime suggestion verbose) - #142176 (tests: Split dont-shuffle-bswaps along opt-levels and arches) - #142248 (Add supported asm types for LoongArch32) - #142272 (tests: Change ABIs in tests to more future-resilient ones) - #142282 (Only run `citool` tests on the `auto` branch) - #142297 (Implement `//@ needs-target-std` compiletest directive) - #142298 (Make loongarch-none target maintainers more easily pingable) - #142306 (Dont unwrap and re-wrap typing envs) - #142324 (Remove unneeded `FunctionCx` from some codegen methods) - #142328 (feat: Add `bit_width` for unsigned integer types) r? `@ghost` `@rustbot` modify labels: rollup
Implement `//@ needs-target-std` compiletest directive Closes rust-lang#141863. Needed to unblock rust-lang#139244 and rust-lang#141856. ### Summary This PR implements a `//@ needs-target-std` compiletest directive that gates test execution based on whether the target supports std or not. For some cases, this should be preferred over e.g. some combination of `//@ ignore-none`, `//@ ignore-nvptx` and more[^none-limit]. ### Implementation limitation Unfortunately, since there is currently [no reliable way to determine from metadata whether a given target supports std or not](rust-lang#142296), we have to resort to a hack. Bootstrap currently determines whether or not a target supports std by a naive target tuple substring comparison: a target supports std if its target tuple does *not* contain one of `["-none", "nvptx", "switch"]` substrings. This PR simply pulls that hack out into `build_helpers` to avoid reimplementing the same hack in compiletest, and uses that logic to inform `//@ needs-target-std`. ### Auxiliary changes This PR additionally changes a few run-make tests to use `//@ needs-target-std` over an inconsistent combination of target-based `ignore`s. This should help with rust-lang#139244. --- r? bootstrap [^none-limit]: Notably, `target_os = "none"` is **not** a sufficient condition for "target does not support std"
Rollup of 9 pull requests Successful merges: - #141967 (Configure bootstrap backport nominations through triagebot) - #142042 (Make E0621 missing lifetime suggestion verbose) - #142272 (tests: Change ABIs in tests to more future-resilient ones) - #142282 (Only run `citool` tests on the `auto` branch) - #142297 (Implement `//@ needs-target-std` compiletest directive) - #142298 (Make loongarch-none target maintainers more easily pingable) - #142306 (Dont unwrap and re-wrap typing envs) - #142324 (Remove unneeded `FunctionCx` from some codegen methods) - #142328 (feat: Add `bit_width` for unsigned integer types) Failed merges: - #141639 (Expose discriminant values in stable_mir) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #142297 - jieyouxu:needs-target-std, r=Kobzol Implement `//@ needs-target-std` compiletest directive Closes #141863. Needed to unblock #139244 and #141856. ### Summary This PR implements a `//@ needs-target-std` compiletest directive that gates test execution based on whether the target supports std or not. For some cases, this should be preferred over e.g. some combination of `//@ ignore-none`, `//@ ignore-nvptx` and more[^none-limit]. ### Implementation limitation Unfortunately, since there is currently [no reliable way to determine from metadata whether a given target supports std or not](#142296), we have to resort to a hack. Bootstrap currently determines whether or not a target supports std by a naive target tuple substring comparison: a target supports std if its target tuple does *not* contain one of `["-none", "nvptx", "switch"]` substrings. This PR simply pulls that hack out into `build_helpers` to avoid reimplementing the same hack in compiletest, and uses that logic to inform `//@ needs-target-std`. ### Auxiliary changes This PR additionally changes a few run-make tests to use `//@ needs-target-std` over an inconsistent combination of target-based `ignore`s. This should help with #139244. --- r? bootstrap [^none-limit]: Notably, `target_os = "none"` is **not** a sufficient condition for "target does not support std"
Rollup of 9 pull requests Successful merges: - rust-lang/rust#141967 (Configure bootstrap backport nominations through triagebot) - rust-lang/rust#142042 (Make E0621 missing lifetime suggestion verbose) - rust-lang/rust#142272 (tests: Change ABIs in tests to more future-resilient ones) - rust-lang/rust#142282 (Only run `citool` tests on the `auto` branch) - rust-lang/rust#142297 (Implement `//@ needs-target-std` compiletest directive) - rust-lang/rust#142298 (Make loongarch-none target maintainers more easily pingable) - rust-lang/rust#142306 (Dont unwrap and re-wrap typing envs) - rust-lang/rust#142324 (Remove unneeded `FunctionCx` from some codegen methods) - rust-lang/rust#142328 (feat: Add `bit_width` for unsigned integer types) Failed merges: - rust-lang/rust#141639 (Expose discriminant values in stable_mir) r? `@ghost` `@rustbot` modify labels: rollup
ignore `run-make` tests that need `std` on targets without `std` In particular, anything that includes `none` in the target triple, and `nvptx64-nvidia-cuda`. Right now we don't cross-compile the `run-make` tests, but we want to in the future. This uses `//@ needs-target-std` introduced in #142297. Useful for #139244 and #141856. The modified files are based on running #141856 locally. It might be that #139244 uncovers some additional files, but that PR needs to be rebased (though actually I'd advice to rebase the non-test changes onto this PR, probably faster that way). r? `@jieyouxu` <details> <summary>vim notes for future me</summary> Make a file with lines like this ``` /home/folkertdev/rust/rust/tests/run-make/export/disambiguator/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/invalid-so/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/no-builtins-attribute/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/export/extern-opt/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/link-dedup/rmake.rs:1:1 ``` then ``` :set errorformat=%f:%l:%c :cfile /tmp/files-to-fix.txt ``` ``` :copen :cnext :cprev ``` are your friends </details> try-job: test-various try-job: armhf-gnu
…ieyouxu ignore `run-make` tests that need `std` on targets without `std` In particular, anything that includes `none` in the target triple, and `nvptx64-nvidia-cuda`. Right now we don't cross-compile the `run-make` tests, but we want to in the future. This uses `//@ needs-target-std` introduced in rust-lang#142297. Useful for rust-lang#139244 and rust-lang#141856. The modified files are based on running rust-lang#141856 locally. It might be that rust-lang#139244 uncovers some additional files, but that PR needs to be rebased (though actually I'd advice to rebase the non-test changes onto this PR, probably faster that way). r? `@jieyouxu` <details> <summary>vim notes for future me</summary> Make a file with lines like this ``` /home/folkertdev/rust/rust/tests/run-make/export/disambiguator/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/invalid-so/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/no-builtins-attribute/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/export/extern-opt/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/link-dedup/rmake.rs:1:1 ``` then ``` :set errorformat=%f:%l:%c :cfile /tmp/files-to-fix.txt ``` ``` :copen :cnext :cprev ``` are your friends </details>
…ieyouxu ignore `run-make` tests that need `std` on targets without `std` In particular, anything that includes `none` in the target triple, and `nvptx64-nvidia-cuda`. Right now we don't cross-compile the `run-make` tests, but we want to in the future. This uses `//@ needs-target-std` introduced in rust-lang#142297. Useful for rust-lang#139244 and rust-lang#141856. The modified files are based on running rust-lang#141856 locally. It might be that rust-lang#139244 uncovers some additional files, but that PR needs to be rebased (though actually I'd advice to rebase the non-test changes onto this PR, probably faster that way). r? ``@jieyouxu`` <details> <summary>vim notes for future me</summary> Make a file with lines like this ``` /home/folkertdev/rust/rust/tests/run-make/export/disambiguator/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/invalid-so/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/no-builtins-attribute/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/export/extern-opt/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/link-dedup/rmake.rs:1:1 ``` then ``` :set errorformat=%f:%l:%c :cfile /tmp/files-to-fix.txt ``` ``` :copen :cnext :cprev ``` are your friends </details>
Rollup merge of #142414 - folkertdev:ignore-nostd-tests, r=jieyouxu ignore `run-make` tests that need `std` on targets without `std` In particular, anything that includes `none` in the target triple, and `nvptx64-nvidia-cuda`. Right now we don't cross-compile the `run-make` tests, but we want to in the future. This uses `//@ needs-target-std` introduced in #142297. Useful for #139244 and #141856. The modified files are based on running #141856 locally. It might be that #139244 uncovers some additional files, but that PR needs to be rebased (though actually I'd advice to rebase the non-test changes onto this PR, probably faster that way). r? ``@jieyouxu`` <details> <summary>vim notes for future me</summary> Make a file with lines like this ``` /home/folkertdev/rust/rust/tests/run-make/export/disambiguator/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/invalid-so/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/no-builtins-attribute/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/export/extern-opt/rmake.rs:1:1 /home/folkertdev/rust/rust/tests/run-make/link-dedup/rmake.rs:1:1 ``` then ``` :set errorformat=%f:%l:%c :cfile /tmp/files-to-fix.txt ``` ``` :copen :cnext :cprev ``` are your friends </details>
Closes #141863.
Needed to unblock #139244 and #141856.
Summary
This PR implements a
//@ needs-target-std
compiletest directive that gates test execution based on whether the target supports std or not. For some cases, this should be preferred over e.g. some combination of//@ ignore-none
,//@ ignore-nvptx
and more1.Implementation limitation
Unfortunately, since there is currently no reliable way to determine from metadata whether a given target supports std or not, we have to resort to a hack. Bootstrap currently determines whether or not a target supports std by a naive target tuple substring comparison: a target supports std if its target tuple does not contain one of
["-none", "nvptx", "switch"]
substrings. This PR simply pulls that hack out intobuild_helpers
to avoid reimplementing the same hack in compiletest, and uses that logic to inform//@ needs-target-std
.Auxiliary changes
This PR additionally changes a few run-make tests to use
//@ needs-target-std
over an inconsistent combination of target-basedignore
s. This should help with #139244.r? bootstrap
Footnotes
Notably,
target_os = "none"
is not a sufficient condition for "target does not support std" ↩