Check build target supports std when building with -Zbuild-std=std

[harmou01]

What does this PR try to resolve?

Ensures that Cargo first verifies whether a given target supports building the standard library when the -Zbuild-std=std option is passed to Cargo (see issue here). This information can be obtained by querying rustc --print=target-spec-json. The target spec "metadata" contains an Option<bool> determining whether the target supports building std.

In the case this value is false, cargo will stop the build. This avoids the numerous "use of unstable library" errors, giving a cleaner, and simpler, "building std is not supported on this target".

How should we test and review this PR?

It can be manually tested by running cargo build --target <target> -Zbuild-std=std. If a target who's target-spec marks std as false is passed, cargo will exit. This works with multiple --target's, and if any of them don't support std, cargo will exit.

Additional Information

This change relies on two things:

• The target-spec metadata in rustc needs to be filled out. Currently, most fields are None, which is treated as OK with this change. Meaning this can be merged before the rustc changes.
• The new test case added with this change will fail without at least aarch64-unknown-none having it's target-spec metadata completed.
• Some targets have std support marked as "?". If this state is properly represented in the target-spec in the future, it needs to be determined whether this is allowed, so the build can continue, or whether it fails.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) some time within the next two weeks.

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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[harmou01]

r? @ehuss

[rustbot]

Could not assign reviewer from: ehuss.
User(s) ehuss are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

[ehuss]

Thanks for the PR! Unfortunately I'm not sure this is something we can move forward with in the short term. The --print target-spec-json option would need to be stabilized in order for this to be supported in build-std. I'm not sure how feasible that is. My impression is that it seems like that will be difficult, since some of the fields are somewhat inherently unstable.

There is some more information at rust-lang/rust#38338 and rust-lang/wg-cargo-std-aware#6.

I would suggest getting in contact with the compiler team to suss out the viability of that. There might be other alternatives, such as providing other rustc options for querying/extracting information like this.

[adamgemmell]

I didn't think that target-spec-json would need to be stable because Cargo and Rustc are versioned together - this creates a contract between the two, but one that can be changed, similar to the contract between Rustc and std (or stdarch as a closer example). Would a test in Rust that tests this integration alleviate your concerns? We only need to parse part of the spec.

Otherwise I doubt the spec can be stabilised, so I think we'd need some other mechanism of getting target info from Rustc. I think this might be useful for things like getting the default panic or unwind strategy too.

[ehuss]

I didn't think that target-spec-json would need to be stable because Cargo and Rustc are versioned together - this creates a contract between the two, but one that can be changed, similar to the contract between Rustc and std (or stdarch as a closer example).

They aren't exactly tightly coupled with each other. Cargo is expected to work on the previous two versions of rustc (although build-std makes that complicated). Also, since cargo is developed in a separate repository, it makes it difficult to synchronize changes, since a change in rustc could immediately break development in cargo.

---

I think it might be possible to support this on an advisory basis, but not something we can strictly rely upon. It would likely need a different approach than what is taken in this PR, though. A few things to consider:

• I'm uncomfortable with setting RUSTC_BOOTSTRAP in TargetInfo::new, since that could leak exposure of unstable options on stable (and generally uncomfortable setting it anywhere). I would suggest collecting the JSON information in a separate call to cached_output. That would also avoid needing to guess where the JSON boundaries are.
• Ignore all errors from rustc, to handle the case where target-spec-json changes.
• Ignore all errors from deserialization, to also handle the case if target-spec-json changes.

However, I'm reluctant to do that since I think it introduces risk of breaking cargo's CI and development (since cargo's testsuite could fail). I'm not sure how a test in rust-lang/rust would help, since if the compiler needs to make a change to the target-spec-json output or CLI option, there would be a problem where you can't update both rustc and cargo at the same time. That has been a fundamental problem that has prevented us from adding build-std tests to rust-lang/rust.

I would suggest opening a dialog with the compiler team to see if they have any input on how cargo and rustc can coordinate to obtain this information in a way that would be reliable.

[bors]

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

[adamgemmell]

I appreciate the detailed answer, thanks. Agreed on the RUSTC_BOOTSTRAP=1 hack. On checking the target-spec-json tracking issue it does seem like there's some support for stabilising the print option which surprised me - I think I had it mixed up in my head with the idea of providing the JSON as an input target.

Considering the case where it was stabilised with all fields, current and future, optional. In the case of this PR, it would mean that Cargo handles the "not found" case for the Metadata::std field, probably by printing out a warning, but not error out or break Cargo's CI. Then we'd basically have to wait for a bug report from someone who got the warning and parse the new format while continuing to support the old one.

This seems not ideal, but might it be good enough? The alternative would be a stricter form of stabilisation that makes all current fields mandatory, any future fields optional and all enums non-exhaustive and strings have an undefined format.

[weihanglo]

Regardless of my review, Eric's concern in #14183 (comment) should be deeply considered.

[harmou01]

@weihanglo Regarding Eric's concerns about the use of the --print=target-spec-json and the possibility of it changing, @adamgemmell has an RFC open to add a --print=std-support flag, which gives us a simpler and more reliable method for determining std support. We wouldn't have to grab the entire target-spec-json anymore, and therefore wouldn't risk things changing between versions, because rustc would always give a true/false answer. I think this should address Eric's concerns.

The second rustc query wouldn't be needed either if this RFC is implemented, we could revert back to collecting everything with a single query.

rust-lang/rfcs#3693

[adamgemmell]

rust-lang/rfcs#3693

To elaborate on this we tried to make this patch improve things without waiting for the RFC's proposal to become stable by removing RUSTC_BOOTSTRAP and making Cargo tolerate all failure cases. If you'd feel more comfortable waiting for that solution then we're fine with that of course.

[weihanglo]

Thanks for working through the feedback—looks great now!