Add build support for Cargo's build-std feature. #74033

ehuss · 2020-07-04T14:29:06Z

This makes some changes to the standard library to make it easier to use with Cargo's build-std feature. The primary goal is to make it so that Cargo and its users do not need to know which crates to build and which features to use for every platform.

Conditional cfgs are adjusted so that there is usually a fall-through for unsupported platforms. Additionally, there is a "restricted-std" feature to mark std as unstable when used with build-std on no_std platforms. There is no intent to stabilize this feature for the foreseeable future.

This borrows some of the implementation for wasm which already does what this needs. More code sharing can be done with some other platforms (there is a lot of duplication with cloudabi, hermit, and sgx), but I figure that can be done in a future PR.

There are some small changes to stable behavior in this PR:

std::env::consts::ARCH on asmjs now reports "wasm32", to match its actual architecture.
Some of the wasm error messages for unsupported features report a slightly different error message so that the code can be reused.

There should otherwise not be any changes to how std is built for distribution via bootstrap.

This does not yet support all platforms when used with build-std.

It doesn't work with 16-bit targets (hashbrown does not support that).
It does not work with JSON spec targets.
- In particular, all target triple snooping will need to be replaced with appropriate target option checking.
Switching to gimli (std: Switch from libbacktrace to gimli #73441) will make cross-building much easier.
There are still a ton of issues on the Cargo side to resolve. A big one is panic strategy support.

Future PRs are intended to address some of these issues.

rust-highfive · 2020-07-04T14:29:09Z

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

ehuss · 2020-07-04T14:29:30Z

@alexcrichton FYI

shepmaster · 2020-07-04T18:06:32Z

This is above my level, so I'm going to...

r? @Mark-Simulacrum

It does not work with JSON spec targets.

Can you expand on that a bit? I use cargo -Z build-std=core --target avr-atmega328p-new.json with my AVR work today. Will this PR prevent me from continuing to do that?

ehuss · 2020-07-04T18:17:17Z

Can you expand on that a bit? I use cargo -Z build-std=core --target avr-atmega328p-new.json with my AVR work today. Will this PR prevent me from continuing to do that?

That should continue to work. The issue is if you specify -Z build-std without "core", it may build incorrectly for JSON targets. The main issue is that some build scripts (like compiler_builtins, backtrace, and libunwind) have checks for things like target.contains("…"), which won't work for a target named foo.json. There's also some target name sniffing done in cargo, too.

shepmaster · 2020-07-04T20:23:56Z

some build scripts (like compiler_builtins, backtrace, and libunwind) have checks for things like target.contains("…"), which won't work for a target named foo.json

Amusingly, I have a WIP commit to teach bootstrap about these differences as well, as part of trying to get an Apple Silicon-native rustc.

alexcrichton

This all looks like fantastic progress to me 👍

src/libpanic_abort/lib.rs

src/libpanic_unwind/lib.rs

src/libstd/lib.rs

src/libstd/build.rs

src/libunwind/lib.rs

alexcrichton · 2020-07-06T14:16:28Z

src/libstd/sys_common/mutex.rs

@@ -17,6 +17,7 @@ impl Mutex {
    /// Also, until `init` is called, behavior is undefined if this
    /// mutex is ever used reentrantly, i.e., `raw_lock` or `try_lock`
    /// are called by the thread currently holding the lock.
+    #[rustc_const_stable(feature = "const_sys_mutex_new", since = "1.0.0")]


Out of curiosity where did this come from?

It is a bit convoluted (and I don't 100% understand the const stability infrastructure).

Mutex::new is used inside a const fn here for std::io::lazy::Lazy::new. The const-stability checks are tied to the overall stability checks. When libstd is marked unstable (via restricted-std), somehow this propagates to const functions as well. Surprisingly this is the only const fn that tripped this issue. The specific error is:

error[E0723]: can only call other const fn within a const fn, but const sys_common::mutex::Mutex::new is not stable as const fn

I believe when a const fn has no stability attributes, it inherits whatever the crate (or module?) stability is. (Sorry, it's been months since I worked on this, so my memory is a bit fuzzy.)

Ah ok no worries, I was a little curious but I'd believe that it falls out of something, and it's internal anyway so it doesn't matter too much.

alexcrichton · 2020-07-06T14:19:14Z

src/libstd/sys_common/mod.rs

@@ -73,6 +74,7 @@ cfg_if::cfg_if! {
    if #[cfg(any(target_os = "cloudabi",
                 target_os = "l4re",
                 target_os = "hermit",
+                 feature = "restricted-std",


I sort of forget what's going on here, could you remind me what's up with this module?

This may be a good candidate for movement to sys instead of sys_common?

I'm not sure I fully understand the question.

There are some things defined in sys_common::net that are used in various other places throughout std. In particular:

libstd/net/addr.rs: Needs LookupHost

libstd/net/tcp.rs: Needs TcpStream and TcpListener

libstd/net/udp.rs: Needs UdpSocket

sys_common/net.rs has implementations that work for almost every platform, except those listed above. cloudabi, wasm, and bare-metal targets don't support these (they have stub implementations). l4re, hermit, and sgx have custom implementations.

If it is moved to sys, I think it would need to use #[path] tricks to share amongst unix, windows, vxworks, wasi, etc. I have no objection to that, but I don't fully understand the philosophy of sys_common. A lot of stuff in there does not appear to be 100% common, so I'm not sure how the line gets drawn.

Hm ok so yeah if you're up for it I think the best way to deal with this is to use #[path] tricks like you are for other platforms. That'll make it easy to avoid code duplication while also making it easy to allow each platform to choose its own implementation.

Otherwise the original intention of sys_common was basically what it says on the tin, everything in common between all sys modules. That being said as you've realized this rarely holds up to be true over time. When you add a new platform that needs almost everything but not quite, it's way easier to add a few #[cfg] than to reorganize to align with the original principles.

Nowadays I sort of feel like sys_common should either be 100% (or maybe 99%) shared or otherwise everything should be shared with #[path].

Would you mind if I defer that to a separate PR? I'd actually like to do a pass of de-duplicating large sections of the sys module, and I think this would fit with that. In particular, there are large chunks of duplicate code in the cloudabi, hermit, sgx, and vxworks modules. It is not clear to me if there was a good reason for that, do you happen to know? Do you think there would be any objections to adding more #[path] attributes to share amongst those?

(This is a practical thing, I was recently looking at making a small change to Command, and saw that I needed to update several identical files.)

Ah yeah that's ok, a separate PR should work fine 👍

If you could, please cc me on the follow-up PR.

alexcrichton · 2020-07-06T14:20:10Z

src/libstd/sys_common/mod.rs

@@ -57,6 +57,7 @@ pub mod mutex;
          target_os = "cloudabi",
          target_os = "hermit",
          target_arch = "wasm32",
+          feature = "restricted-std",


Would it be possible to remove this #[cfg] entirely and simply always include the module? If there's warnings about unused definitions we could add #![allow(warnings)], or we could conditionally do that so one target, like unix, emits warnings but everything else just ignores warnings.

The implementation is not compatible on windows. The Ext traits conflict, and FromInner isn't implemented.

What about changing it to cfg(not(windows))?

Ah ok yeah if that's the case then yeah cfg(not(windows)) I think is a better #[cfg], since that should be applicable for basically everything else.

alexcrichton · 2020-07-07T14:15:53Z

src/libpanic_unwind/lib.rs

+    } else if #[cfg(any(
+        all(target_family = "windows", target_env = "gnu"),
+        target_os = "cloudabi",
+        target_family = "unix",


target_family = "unix" FWIW I think is the same as unix, right?

Yes, it is the same. I have been waffling on which style I like better, and I think I ended up being really inconsistent. I think part of the issue is that I've run into a number of users being confused what cfg(unix) means, which makes me feel it is better to be explicit. But unix and windows is conveniently short.

One thing that surprises me is that it is possible to have family="unix" and os="none" at the same time.

Eh either way is fine by me, I figured unix is a bit more idiomatic though. For unix, target_os = "none" being true that seems like it's either a mistake or something that hasn't been reviewed since it was added

x86_64-linux-kernel is the only one. I'm not sure what values would make sense for that, since it is unusual.

Does the Linux kernel even have an UNIX API inside of itself? That doesn't sound right but maybe I just haven't seen it before.

(To be clear, I'm thinking that family="unix" is as valid for inside an OS kernel as os="thatosname" - if os="none" is used then surely there should also not be a family? Maybe we should enforce that the family is derived from the OS name and therefore means "OS family")

Filed #74247 for further discussion.

~~Not sure if it was related or coincidental but I just saw #74257 being open.~~ Oh, highly relevant, just likely accidentally pointing to the wrong #.

src/libstd/build.rs

src/libstd/lib.rs

alexcrichton · 2020-07-07T14:19:38Z

src/libstd/sys_common/mod.rs

@@ -57,6 +57,7 @@ pub mod mutex;
          target_os = "cloudabi",
          target_os = "hermit",
          target_arch = "wasm32",
+          feature = "restricted-std",


Ah ok yeah if that's the case then yeah cfg(not(windows)) I think is a better #[cfg], since that should be applicable for basically everything else.

alexcrichton · 2020-07-07T14:22:08Z

src/libstd/sys_common/mod.rs

@@ -73,6 +74,7 @@ cfg_if::cfg_if! {
    if #[cfg(any(target_os = "cloudabi",
                 target_os = "l4re",
                 target_os = "hermit",
+                 feature = "restricted-std",


Hm ok so yeah if you're up for it I think the best way to deal with this is to use #[path] tricks like you are for other platforms. That'll make it easy to avoid code duplication while also making it easy to allow each platform to choose its own implementation.

Otherwise the original intention of sys_common was basically what it says on the tin, everything in common between all sys modules. That being said as you've realized this rarely holds up to be true over time. When you add a new platform that needs almost everything but not quite, it's way easier to add a few #[cfg] than to reorganize to align with the original principles.

Nowadays I sort of feel like sys_common should either be 100% (or maybe 99%) shared or otherwise everything should be shared with #[path].

alexcrichton · 2020-07-07T14:22:31Z

src/libstd/sys_common/mutex.rs

@@ -17,6 +17,7 @@ impl Mutex {
    /// Also, until `init` is called, behavior is undefined if this
    /// mutex is ever used reentrantly, i.e., `raw_lock` or `try_lock`
    /// are called by the thread currently holding the lock.
+    #[rustc_const_stable(feature = "const_sys_mutex_new", since = "1.0.0")]


Ah ok no worries, I was a little curious but I'd believe that it falls out of something, and it's internal anyway so it doesn't matter too much.

alexcrichton · 2020-07-07T16:51:23Z

FWIW this is an r+ from me, although I think other folks should likely take a look too

bors · 2020-07-14T23:46:22Z

☔ The latest upstream changes (presumably #74342) made this pull request unmergeable. Please resolve the merge conflicts.

This simplifies the definition for ARCH. Note that this changes asmjs-unknown-emscripten ARCH to `wasm32`, which reflects the actual target arch.

Simplifies some of the expressions, and provides a default.

This allows setting a default abort using the core intrinsic.

Use a fall-through for no_std targets.

ehuss · 2020-07-15T17:02:20Z

I'm thinking this should probably add a test. Would there be any objection to adding a test that is essentially cargo build --target aarch64-unknown-none in src/libtest (or whatever no_std target happens to be readily available)?

alexcrichton · 2020-07-15T18:01:05Z

That sounds pretty reasonable to me, there should be a builder or two with plenty of time to spare to shove that in

Mark-Simulacrum · 2020-07-15T18:57:34Z

r=me modulo a test, though feel free to just r+ without that and add it in a future PR (this PR doesn't make things worse anyway).

ehuss · 2020-07-16T17:31:56Z

I spent some time working on a test, but I don't think there's a builder where it really fits. I'm reluctant to add to one of the dist builders, and none of the others already build a target that seems like a good choice. I'm going to go with my original plan of adding tests to Cargo's CI, and see how it goes in the short term. Eventually will need some tests here, but I suspect that is a long ways off.

@bors r=Mark-Simulacrum

bors · 2020-07-16T17:31:57Z

📌 Commit 3d44d3c has been approved by Mark-Simulacrum

bors · 2020-07-16T17:31:58Z

🌲 The tree is currently closed for pull requests below priority 5, this pull request will be tested once the tree is reopened

…Mark-Simulacrum Add build support for Cargo's build-std feature. This makes some changes to the standard library to make it easier to use with Cargo's build-std feature. The primary goal is to make it so that Cargo and its users do not need to know which crates to build and which features to use for every platform. Conditional cfgs are adjusted so that there is usually a fall-through for unsupported platforms. Additionally, there is a "restricted-std" feature to mark `std` as unstable when used with build-std on no_std platforms. There is no intent to stabilize this feature for the foreseeable future. This borrows some of the implementation for wasm which already does what this needs. More code sharing can be done with some other platforms (there is a lot of duplication with cloudabi, hermit, and sgx), but I figure that can be done in a future PR. There are some small changes to stable behavior in this PR: - `std::env::consts::ARCH` on asmjs now reports "wasm32", to match its actual architecture. - Some of the wasm error messages for unsupported features report a slightly different error message so that the code can be reused. There should otherwise not be any changes to how std is built for distribution via bootstrap. This does not yet support all platforms when used with build-std. - It doesn't work with 16-bit targets (hashbrown does not support that). - It does not work with JSON spec targets. - In particular, all target triple snooping will need to be replaced with appropriate target option checking. - Switching to gimli (rust-lang#73441) will make cross-building *much* easier. - There are still a ton of issues on the Cargo side to resolve. A big one is panic strategy support. Future PRs are intended to address some of these issues.

@ghost

…arth Rollup of 8 pull requests Successful merges: - rust-lang#73101 (Resolve items for cross-crate imports relative to the original module) - rust-lang#73269 (Enable some timeouts in SGX platform) - rust-lang#74033 (Add build support for Cargo's build-std feature.) - rust-lang#74351 (Do not render unstable items for rustc doc) - rust-lang#74357 (Some `Symbol` related improvements) - rust-lang#74371 (Improve ayu rustdoc theme) - rust-lang#74386 (Add RISC-V GNU/Linux to src/tools/build-manifest as a host platform) - rust-lang#74398 (Clean up E0723 explanation) Failed merges: r? @ghost

@rustbot

…mutex-attr-cleanup, r=Mark-Simulacrum Remove unnecessary rustc_const_stable attributes. These attributes were added in rust-lang#74033 (comment) because of [std::io::lazy::Lazy::new](https://github.com/rust-lang/rust/blob/0c03aee8b81185d65b5821518661c30ecdb42de5/src/libstd/io/lazy.rs#L21-L23). But [std::io::lazy::Lazy is gone now](rust-lang#77154), so this can be cleaned up. @rustbot modify labels: +T-libs +C-cleanup

@rustbot

…mutex-attr-cleanup, r=Mark-Simulacrum Remove unnecessary rustc_const_stable attributes. These attributes were added in rust-lang#74033 (comment) because of [std::io::lazy::Lazy::new](https://github.com/rust-lang/rust/blob/0c03aee8b81185d65b5821518661c30ecdb42de5/src/libstd/io/lazy.rs#L21-L23). But [std::io::lazy::Lazy is gone now](rust-lang#77154), so this can be cleaned up. @rustbot modify labels: +T-libs +C-cleanup

@rustbot

…mutex-attr-cleanup, r=Mark-Simulacrum Remove unnecessary rustc_const_stable attributes. These attributes were added in rust-lang#74033 (comment) because of [std::io::lazy::Lazy::new](https://github.com/rust-lang/rust/blob/0c03aee8b81185d65b5821518661c30ecdb42de5/src/libstd/io/lazy.rs#L21-L23). But [std::io::lazy::Lazy is gone now](rust-lang#77154), so this can be cleaned up. @rustbot modify labels: +T-libs +C-cleanup

saethlin · 2022-12-28T04:03:12Z

What does it mean to "mark std as unstable"? And how does the restricted-std system accomplish that? I see a build script that enables a feature... Is this a hack to make sure that compiling std on the unsupported platforms requires a nightly toolchain?

rust-highfive assigned shepmaster Jul 4, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 4, 2020

rust-highfive assigned Mark-Simulacrum and unassigned shepmaster Jul 4, 2020

alexcrichton reviewed Jul 6, 2020

View reviewed changes

ehuss force-pushed the std-compile-all-platforms branch from 64fb64b to 68cff59 Compare July 7, 2020 03:40

alexcrichton reviewed Jul 7, 2020

View reviewed changes

ehuss force-pushed the std-compile-all-platforms branch from 68cff59 to 19a85ab Compare July 7, 2020 17:52

ehuss mentioned this pull request Jul 11, 2020

consider disallowing setting target_family if target_os=none #74247

Open

ehuss added 7 commits July 15, 2020 08:38

Automatically calculate std::env::consts::ARCH.

8c6c1dd

This simplifies the definition for ARCH. Note that this changes asmjs-unknown-emscripten ARCH to `wasm32`, which reflects the actual target arch.

Use cfg_if in libtest.

432b4c1

Simplifies some of the expressions, and provides a default.

Use cfg_if in libpanic_abort.

9e58908

This allows setting a default abort using the core intrinsic.

Introduce restricted-std feature.

6e9a1de

Tweak formatting.

cee9f05

Use an allow-list of platforms that support std.

0eb293d

Use a fall-through for no_std targets.

Simplify os_str_bytes cfg expression.

3d44d3c

ehuss force-pushed the std-compile-all-platforms branch from 19a85ab to 3d44d3c Compare July 15, 2020 17:02

alexcrichton mentioned this pull request Jul 15, 2020

Expand doc section about "what about #![no_std]?" bytecodealliance/wasmtime#2024

Merged

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 15, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 16, 2020

Manishearth mentioned this pull request Jul 16, 2020

Rollup of 9 pull requests #74412

Closed

Manishearth mentioned this pull request Jul 16, 2020

Rollup of 9 pull requests #74414

Closed

Manishearth mentioned this pull request Jul 16, 2020

Rollup of 9 pull requests #74415

Closed

This was referenced Jul 17, 2020

Rollup of 8 pull requests #74421

Closed

Rollup of 8 pull requests #74422

Merged

bors merged commit 5751c7f into rust-lang:master Jul 17, 2020

ehuss mentioned this pull request Sep 18, 2020

Can't build std if I specify target json file: std does not see networking rust-lang/wg-cargo-std-aware#60

Closed

m-ou-se mentioned this pull request Oct 8, 2020

Remove unnecessary rustc_const_stable attributes. #77719

Merged

bjorn3 mentioned this pull request Dec 1, 2022

Fix panic when restricted-std is enabled. #104971

Closed

jyn514 mentioned this pull request Dec 27, 2022

Allow test to be built with build-std #97037

Closed

cuviper added this to the 1.47.0 milestone May 2, 2024

Add build support for Cargo's build-std feature. #74033

Add build support for Cargo's build-std feature. #74033

Conversation

ehuss commented Jul 4, 2020

rust-highfive commented Jul 4, 2020

ehuss commented Jul 4, 2020

shepmaster commented Jul 4, 2020

ehuss commented Jul 4, 2020

shepmaster commented Jul 4, 2020

alexcrichton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyb Jul 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyb Jul 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcrichton commented Jul 7, 2020

bors commented Jul 14, 2020

ehuss commented Jul 15, 2020

alexcrichton commented Jul 15, 2020

Mark-Simulacrum commented Jul 15, 2020

ehuss commented Jul 16, 2020

bors commented Jul 16, 2020

bors commented Jul 16, 2020

saethlin commented Dec 28, 2022

eddyb Jul 11, 2020 •

edited

Loading

eddyb Jul 12, 2020 •

edited

Loading