-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add support for custom JSON targets when using build-std. #120232
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
To make sure I understand the problem this is solving, you have custom target spec JSON not named after the triplet for the target? E.g., it looks to me like making the file x86_64-linux-special-gnu.json would probably also make all of these build scripts work? (Doesn't make this change a bad one but want to make sure I understand the problem) |
@Mark-Simulacrum Yes, renaming the JSON target file to include "linux" in the filename would allow the target to build (in the linked issue, the author is using |
☔ The latest upstream changes (presumably #117958) made this pull request unmergeable. Please resolve the merge conflicts. |
4146e40
to
797cf59
Compare
@bors r+ rollup This looks good to me. Thanks! |
Add support for custom JSON targets when using build-std. Currently, when building with `build-std`, some library build scripts check properties of the target by inspecting the target triple at `env::TARGET`, which is simply set to the filename of the JSON file when using JSON target files. This patch alters these build scripts to use `env::CARGO_CFG_*` to fetch target information instead, allowing JSON target files describing platforms without `restricted_std` to build correctly when using `-Z build-std`. There are some weak assertions here (for example, `nintendo && newlib`), however this seems at least a marginal improvement on the existing solution. Fixes rust-lang/wg-cargo-std-aware#60.
library/std/build.rs
Outdated
@@ -48,7 +53,7 @@ fn main() { | |||
// - mipsel-sony-psp | |||
// - nvptx64-nvidia-cuda | |||
// - arch=avr | |||
// - JSON targets | |||
// - JSON targets not describing an excluded target above. |
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.
As I understand it restricted-std is meant to prevent being able to use libstd on stable for targets that we don't actually support libstd support for once -Zbuild-std
is stabilized. When using custom json targets, IMO it shouldn't be stable as libstd changes may break support in the future even if it happens to work currently.
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.
Hm, I think that seems reasonable. We have a long-standing issue about making custom target specs entirely unstable (#71009), which seems like the better fix to me. Or an explicit guard here if we want (not sure how feasible that is). I'd be ok cutting this specific change.
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.
Giving `--target=file.json some unstable-machinery at the CLI level, to me, would clearly imply that std doesn't support JSON targets in the same way Rust doesn't.
At the moment I'm not sure how a build script would detect a JSON target without the old broken TARGET behaviour
Add support for custom JSON targets when using build-std. Currently, when building with `build-std`, some library build scripts check properties of the target by inspecting the target triple at `env::TARGET`, which is simply set to the filename of the JSON file when using JSON target files. This patch alters these build scripts to use `env::CARGO_CFG_*` to fetch target information instead, allowing JSON target files describing platforms without `restricted_std` to build correctly when using `-Z build-std`. There are some weak assertions here (for example, `nintendo && newlib`), however this seems at least a marginal improvement on the existing solution. Fixes rust-lang/wg-cargo-std-aware#60.
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#116677 (References refer to allocated objects) - rust-lang#120232 (Add support for custom JSON targets when using build-std.) - rust-lang#120266 (Improve documentation for [A]Rc::into_inner) - rust-lang#120358 (Bump Fuchsia, build tests, and use 8 core bots) - rust-lang#120373 (Adjust Behaviour of `read_dir` and `ReadDir` in Windows Implementation: Check Whether Path to Search In Exists) - rust-lang#120402 (Make the coroutine def id of an async closure the child of the closure def id) - rust-lang#120420 (Stop using derivative in rustc_pattern_analysis) - rust-lang#120425 (Remove unnecessary unit returns in query declarations) r? `@ghost` `@rustbot` modify labels: rollup
Add support for custom JSON targets when using build-std. Currently, when building with `build-std`, some library build scripts check properties of the target by inspecting the target triple at `env::TARGET`, which is simply set to the filename of the JSON file when using JSON target files. This patch alters these build scripts to use `env::CARGO_CFG_*` to fetch target information instead, allowing JSON target files describing platforms without `restricted_std` to build correctly when using `-Z build-std`. There are some weak assertions here (for example, `nintendo && newlib`), however this seems at least a marginal improvement on the existing solution. Fixes rust-lang/wg-cargo-std-aware#60.
Add support for custom JSON targets when using build-std. Currently, when building with `build-std`, some library build scripts check properties of the target by inspecting the target triple at `env::TARGET`, which is simply set to the filename of the JSON file when using JSON target files. This patch alters these build scripts to use `env::CARGO_CFG_*` to fetch target information instead, allowing JSON target files describing platforms without `restricted_std` to build correctly when using `-Z build-std`. There are some weak assertions here (for example, `nintendo && newlib`), however this seems at least a marginal improvement on the existing solution. Fixes rust-lang/wg-cargo-std-aware#60.
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#116677 (References refer to allocated objects) - rust-lang#118533 (Suppress unhelpful diagnostics for unresolved top level attributes) - rust-lang#120232 (Add support for custom JSON targets when using build-std.) - rust-lang#120238 (Always check the result of `pthread_mutex_lock`) - rust-lang#120266 (Improve documentation for [A]Rc::into_inner) - rust-lang#120373 (Adjust Behaviour of `read_dir` and `ReadDir` in Windows Implementation: Check Whether Path to Search In Exists) r? `@ghost` `@rustbot` modify labels: rollup
Add support for custom JSON targets when using build-std. Currently, when building with `build-std`, some library build scripts check properties of the target by inspecting the target triple at `env::TARGET`, which is simply set to the filename of the JSON file when using JSON target files. This patch alters these build scripts to use `env::CARGO_CFG_*` to fetch target information instead, allowing JSON target files describing platforms without `restricted_std` to build correctly when using `-Z build-std`. There are some weak assertions here (for example, `nintendo && newlib`), however this seems at least a marginal improvement on the existing solution. Fixes rust-lang/wg-cargo-std-aware#60.
@bors rollup=iffy |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#116677 (References refer to allocated objects) - rust-lang#118533 (Suppress unhelpful diagnostics for unresolved top level attributes) - rust-lang#119991 (Reject infinitely-sized reads from io::Repeat) - rust-lang#120232 (Add support for custom JSON targets when using build-std.) - rust-lang#120358 (Bump Fuchsia, build tests, and use 8 core bots) - rust-lang#120424 (raw pointer metadata API: data address -> data pointer) - rust-lang#120434 (Revert outdated version of "Add the wasm32-wasi-preview2 target") - rust-lang#120443 (Fixes footnote handling in rustdoc) r? `@ghost` `@rustbot` modify labels: rollup
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
Fixed the typo, and the new patch should also fix the test failures - all |
Currently, when building with `build-std`, some library build scripts check properties of the target by inspecting the target triple at `env::TARGET`, which is simply set to the filename of the JSON file when using JSON target files. This patch alters these build scripts to use `env::CARGO_CFG_*` to fetch target information instead, allowing JSON target files describing platforms without `restricted_std` to build correctly when using `-Z build-std`. Fixes wg-cargo-std-aware/rust-lang#60.
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (0cbef48): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 666.058s -> 668.04s (0.30%) |
This PR aims to pin down exactly what restricted_std is meant to achieve and what it isn't. This commit fixes rust-lang/wg-cargo-std-aware#87 by explaining why the error appears and what the choices the user has. The error describes how std cannot function without knowing about some form of OS/platform support. Any features of std that work without an OS should be moved to core/alloc (see rust-lang#27242 rust-lang#103765). Note that the message says "platform" and "environment" because, since rust-lang#120232, libstd can be built for some JSON targets. This is still unsupported (all JSON targets probably should be unstable rust-lang/wg-cargo-std-aware#90), but a JSON target with the right configuration should hopefully have some partial libstd support. I propose closing rust-lang/wg-cargo-std-aware#69 as "Won't fix" since any support of std without properly configured os, vendor or env fields is very fragile considering future upgrades of Rust or dependencies. In addition there's no likely path to it being fixed long term (making std buildable for all targets being the only solution). This is distinct from tier 3 platforms with limited std support implemented (and as such aren't restricted_std) because these platforms can conceptually work in the future and std support should mainly improve over time. The alternative to closing rust-lang/wg-cargo-std-aware#69 is a new crate feature for std which escapes the restricted_std mechanism in build.rs. It could be used with the -Zbuild-std-features flag if we keep it permanently unstable, which I hope we can do anyway. A minor side-effect in this scenario is that std wouldn't be marked as unstable if documentation for it were generated with build-std.
…d-std, r=ehuss Document restricted_std This PR aims to pin down exactly what restricted_std is meant to achieve and what it isn't. This commit fixes rust-lang/wg-cargo-std-aware#87 by explaining why the error appears and what the choices the user has. The error describes how std cannot function without knowing about some form of OS/platform support. Any features of std that work without an OS should be moved to core/alloc (see rust-lang#27242 rust-lang#103765). Note that the message says "platform" and "environment" because, since rust-lang#120232, libstd can be built for some JSON targets. This is still unsupported (all JSON targets probably should be unstable rust-lang/wg-cargo-std-aware#90), but a JSON target with the right configuration should hopefully have some partial libstd support. I propose closing rust-lang/wg-cargo-std-aware#69 as "Won't fix" since any support of std without properly configured os, vendor or env fields is very fragile considering future upgrades of Rust or dependencies. In addition there's no likely path to it being fixed long term (making std buildable for all targets being the only solution). This is distinct from tier 3 platforms with limited std support implemented (and as such aren't restricted_std) because these platforms can conceptually work in the future and std support should mainly improve over time. The alternative to closing rust-lang/wg-cargo-std-aware#69 is a new crate feature for std which escapes the restricted_std mechanism in build.rs. It could be used with the -Zbuild-std-features flag if we keep it permanently unstable, which I hope we can do anyway. A minor side-effect in this scenario is that std wouldn't be marked as unstable if documentation for it were generated with build-std. cc `@ehuss`
…d-std, r=ehuss Document restricted_std This PR aims to pin down exactly what restricted_std is meant to achieve and what it isn't. This commit fixes rust-lang/wg-cargo-std-aware#87 by explaining why the error appears and what the choices the user has. The error describes how std cannot function without knowing about some form of OS/platform support. Any features of std that work without an OS should be moved to core/alloc (see rust-lang#27242 rust-lang#103765). Note that the message says "platform" and "environment" because, since rust-lang#120232, libstd can be built for some JSON targets. This is still unsupported (all JSON targets probably should be unstable rust-lang/wg-cargo-std-aware#90), but a JSON target with the right configuration should hopefully have some partial libstd support. I propose closing rust-lang/wg-cargo-std-aware#69 as "Won't fix" since any support of std without properly configured os, vendor or env fields is very fragile considering future upgrades of Rust or dependencies. In addition there's no likely path to it being fixed long term (making std buildable for all targets being the only solution). This is distinct from tier 3 platforms with limited std support implemented (and as such aren't restricted_std) because these platforms can conceptually work in the future and std support should mainly improve over time. The alternative to closing rust-lang/wg-cargo-std-aware#69 is a new crate feature for std which escapes the restricted_std mechanism in build.rs. It could be used with the -Zbuild-std-features flag if we keep it permanently unstable, which I hope we can do anyway. A minor side-effect in this scenario is that std wouldn't be marked as unstable if documentation for it were generated with build-std. cc ``@ehuss``
…d-std, r=ehuss Document restricted_std This PR aims to pin down exactly what restricted_std is meant to achieve and what it isn't. This commit fixes rust-lang/wg-cargo-std-aware#87 by explaining why the error appears and what the choices the user has. The error describes how std cannot function without knowing about some form of OS/platform support. Any features of std that work without an OS should be moved to core/alloc (see rust-lang#27242 rust-lang#103765). Note that the message says "platform" and "environment" because, since rust-lang#120232, libstd can be built for some JSON targets. This is still unsupported (all JSON targets probably should be unstable rust-lang/wg-cargo-std-aware#90), but a JSON target with the right configuration should hopefully have some partial libstd support. I propose closing rust-lang/wg-cargo-std-aware#69 as "Won't fix" since any support of std without properly configured os, vendor or env fields is very fragile considering future upgrades of Rust or dependencies. In addition there's no likely path to it being fixed long term (making std buildable for all targets being the only solution). This is distinct from tier 3 platforms with limited std support implemented (and as such aren't restricted_std) because these platforms can conceptually work in the future and std support should mainly improve over time. The alternative to closing rust-lang/wg-cargo-std-aware#69 is a new crate feature for std which escapes the restricted_std mechanism in build.rs. It could be used with the -Zbuild-std-features flag if we keep it permanently unstable, which I hope we can do anyway. A minor side-effect in this scenario is that std wouldn't be marked as unstable if documentation for it were generated with build-std. cc ```@ehuss```
Rollup merge of rust-lang#123360 - adamgemmell:dev/adagem01/restricted-std, r=ehuss Document restricted_std This PR aims to pin down exactly what restricted_std is meant to achieve and what it isn't. This commit fixes rust-lang/wg-cargo-std-aware#87 by explaining why the error appears and what the choices the user has. The error describes how std cannot function without knowing about some form of OS/platform support. Any features of std that work without an OS should be moved to core/alloc (see rust-lang#27242 rust-lang#103765). Note that the message says "platform" and "environment" because, since rust-lang#120232, libstd can be built for some JSON targets. This is still unsupported (all JSON targets probably should be unstable rust-lang/wg-cargo-std-aware#90), but a JSON target with the right configuration should hopefully have some partial libstd support. I propose closing rust-lang/wg-cargo-std-aware#69 as "Won't fix" since any support of std without properly configured os, vendor or env fields is very fragile considering future upgrades of Rust or dependencies. In addition there's no likely path to it being fixed long term (making std buildable for all targets being the only solution). This is distinct from tier 3 platforms with limited std support implemented (and as such aren't restricted_std) because these platforms can conceptually work in the future and std support should mainly improve over time. The alternative to closing rust-lang/wg-cargo-std-aware#69 is a new crate feature for std which escapes the restricted_std mechanism in build.rs. It could be used with the -Zbuild-std-features flag if we keep it permanently unstable, which I hope we can do anyway. A minor side-effect in this scenario is that std wouldn't be marked as unstable if documentation for it were generated with build-std. cc ```@ehuss```
Currently, when building with
build-std
, some library build scripts check properties of the target by inspecting the target triple atenv::TARGET
, which is simply set to the filename of the JSON file when using JSON target files.This patch alters these build scripts to use
env::CARGO_CFG_*
to fetch target information instead, allowing JSON target files describing platforms withoutrestricted_std
to build correctly when using-Z build-std
. There are some weak assertions here (for example,nintendo && newlib
), however this seems at least a marginal improvement on the existing solution.Fixes rust-lang/wg-cargo-std-aware#60.