Skip to content
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

Fixup some crate flags. #28

Open
ehuss opened this issue Sep 6, 2019 · 3 comments
Open

Fixup some crate flags. #28

ehuss opened this issue Sep 6, 2019 · 3 comments
Labels
implementation Implementation exploration and tracking issues stabilization blocker This needs a resolution before stabilization

Comments

@ehuss
Copy link
Contributor

ehuss commented Sep 6, 2019

There are some crates that require special compiler flags. The current implementation does not do a good job of passing these flags. It would be ideal if these could be pushed towards some kind of config somewhere. Currently they are scattered in the bootstrap code.

Some that I can think of is:

@ehuss ehuss added the implementation Implementation exploration and tracking issues label Sep 6, 2019
@alexcrichton
Copy link
Member

One issue here is that when considering stabilization of -Z build-std in one form or another this'll be adding an implicit contract with upstream rust-lang/rust about building libstd and the fanciness we can control flags with. That's just always a thing we'd have to deal with though.

To actually deal with the specific issues, though:

  • debug-assertions and compiler-builtins - I would ideally hope that we could handle this by configuration in Cargo.toml. We actually already tell Cargo to disable debug assertions everywhere and then they're selectively enabled via src/bootstrap/bin/rustc.rs. In general bin/rustc.rs is sort of a mess and poses a big hazard for std-aware Cargo because libstd is only built by this shim, never natively by Cargo. I would ideally like to remove the need for bin/rustc.rs entirely, but it will take us some time to get there.

  • panic=abort and panic_abort - this may actually be possible to fix by just handling panic modes better (Support different panic strategies. #29), and it should be the case that if you compile panic_abort with the unwind strategy it still works, it's just not too useful.

I think all in all I'd sort of see this issue as we "we should delete bin/rustc.rs", or at least to the best extent that we can.

@alexcrichton
Copy link
Member

As an update on this, rust-lang/rust#64316 is an attempt to move away from src/bootstrap/bin/rustc.rs as much as possible, partly to prepare for -Zbuild-std where Cargo isn't always passing loads of fancy flags to libstd.

The current shim as of now is largely just a glorified wrapper for dealing with bootstrapping and a few miscellaneous flags, plus the exact two use cases mentioned in the description of this issue. All other flags are passed in one of two locations:

  • First is in Builder::cargo, which while a massive method largely just deals with the various build configurations that you can request via config.toml. There is still a risk, however, that special flags are required to build libstd which are only set on CI by accident. As of right this red hot second everything falls into the category of "dylib goop" or "build configuration that Cargo already does" so we're in the clear. Long-term I think we'll want to test, on each commit, that Cargo can indeed build libstd.

  • Second is in std_cargo which is where std-specific settings are configured. This has some sanitizer nonsense as well as things related to Figure out how to handle target-specific settings. #30, but otherwise we're good for now.

I've also been trying to figure out what to do about compiler-builtins, but the only reason it's compiled with panic=abort and debug-assertions off was to fix linking issues. I can't for the life of me actually reproduce these linking issues, and I'm not entirely certain as to why. It may be a case though where we can largely sweep it under the rug until it actually comes up. In any case it should be transparently handled so fixing it wouldn't exactly constitute a breaking change.

@tmiasko
Copy link

tmiasko commented Mar 14, 2020

The rust-lang/compiler-builtins#347 is one example of linking issues encountered when build-std is combined with fat LTO on a profile with debug assertions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implementation Implementation exploration and tracking issues stabilization blocker This needs a resolution before stabilization
Projects
None yet
Development

No branches or pull requests

3 participants