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

Enable rlib-only libstd build (no dylib) #56443

Open
RalfJung opened this issue Dec 2, 2018 · 7 comments
Open

Enable rlib-only libstd build (no dylib) #56443

RalfJung opened this issue Dec 2, 2018 · 7 comments
Labels
A-driver Area: rustc_driver that ties everything together into the `rustc` compiler C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Dec 2, 2018

For cross-building libstd (for use e.g. with miri), it would be great if one could make an rlib-only libstd build. This is because building a dylib invokes the native linker, and that fails when the target architectures is too foreign and/or not supported by the installed C toolchain. (Currently, we build libstd both as rlib and dylib.)

So my goal was to equip libstd with a feature flag to control whether it is built as a dylib (next to the rlib) or not. To this end, I patched collect_crate_types such that the crate types passed on the command-line and the one given as attributes are merged (instead of command-line taking precedence). That means I can use #![cfg_attr(dylib, crate-type = "dylib")] to make a dylib feature enable a dylib build. That seems to work, however, there is a problem: During bootstrap, rustbuild uses cargo's stamp file to determine which files to copy into the sysroot, and that stamp file now no longer includes the libstd.so, breaking the build.

I wonder if there is maybe a better way to achieve an rlib-only libstd build?

Cc @eddyb @alexcrichton any advice?

@eddyb
Copy link
Member

eddyb commented Dec 2, 2018

cc @rust-lang/compiler This interaction between CLI flags and attributes seems really unfortunate.
I wonder if Cargo could benefit from a --print option for this, that would be handled after macro expansion, but it would have to happen at the same time as compiling the crate (as to avoid doing parsing and expansion twice), so it'd be more like --emit=dep-info.

@alexcrichton
Copy link
Member

This is currently a limitation of the compiler and we can't conditionally produce a dynamic library. The CLI flags passed to rustc trump whatever is listed in the crate (attribute-wise), and there's no way to have Cargo conditionally produce a dynamic library for the standard library, unfortunately.

We could consider adding a feature to rustc for this though! (it's certainly been asked for before)

@RalfJung
Copy link
Member Author

RalfJung commented Dec 4, 2018

@alexcrichton I have a branch that makes CLI flags not trump attributes any more.

The problem is, as described in my first post: With that patch, rustbuild no longer copies the libstd.so into the sysroot because it is not listed in the stamp file.

@alexcrichton
Copy link
Member

Yeah crate type is tricky where it can't really be configured without Cargo's knowledge as Cargo moves around files and need to know what's being output. I think this would need to be a Cargo feature rather than a rustc feature to implement this

@RalfJung
Copy link
Member Author

@alexcrichton suggested it might be possible to do at least a metadata-only build, using essentially cargo check.

I tried this, but at least for cross-building purposes that currently does not work because since compiler_builtins got moved out of the rustc workspace, the c feature is forced to be enabled. (It can be controlled in libstd, but not in liballoc which sets rustc-dep-of-std.)

@jonas-schievink jonas-schievink added A-driver Area: rustc_driver that ties everything together into the `rustc` compiler T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Aug 6, 2019
bors added a commit to rust-lang/cargo that referenced this issue Sep 3, 2019
Basic standard library support.

This is not intended to be useful to anyone. If people want to try it, that's great, but do not rely on this. This is only for experimenting and setting up for future work.

This adds a flag `-Zbuild-std` to build the standard library with a project. The flag can also take optional comma-separated crate names, like `-Zbuild-std=core`. Default is `std,core,panic_unwind,compiler_builtins`.

Closes rust-lang/wg-cargo-std-aware#10.

Note: I can probably break some of the refactoring into smaller PRs if necessary.

## Overview
The general concept here is to use two resolvers, and to combine everything in the Unit graph. There are a number of changes to support this:

- A synthetic workspace for the standard library is created to set up the patches and members correctly.
- Decouple `unit_dependencies` from `Context` to make it easier to manage.
- Add `features` to `Unit` to keep it unique and to remove the need to query a resolver.
- Add a `UnitDep` struct which encodes the edges between `Unit`s. This removes the need to query a resolver for `extern_crate_name` and `public`.
- Remove `Resolver` from `BuildContext` to avoid any confusion and to keep the complexity focused in `unit_dependencies`.
- Remove `Links` from `Context` since it used the resolver. Adjusted so that instead of checking links at runtime, they are all checked at once in the beginning. Note that it does not check links for the standard lib, but it should be safe? I think `compiler-rt` is the only `links`?

I currently went with a strategy of linking the standard library dependencies using `--extern` (instead of `--sysroot` or `-L`). This has some benefits but some significant drawbacks. See below for some questions.

## For future PRs
- Add Cargo.toml support. See rust-lang/wg-cargo-std-aware#5
- Source is not downloaded. It assumes you have run `rustup component add rust-src`. See rust-lang/wg-cargo-std-aware#11
- `cargo metadata` does not include any information about std. I don't know how this should work.
- `cargo clean` is not std-aware.
- `cargo fetch` does not fetch std dependencies.
- `cargo vendor` does not vendor std dependencies.
- `cargo pkgid` is not std-aware.
- `--target` is required on the command-line. This should default to host-as-target.
- `-p` is not std aware.
- A synthetic `Cargo.toml` workspace is created which has to know about things like `rustc-std-workspace-core`. Perhaps rust-lang/rust should publish the source with this `Cargo.toml` already created?
- `compiler_builtins` uses default features (pure Rust implementation, etc.). See rust-lang/wg-cargo-std-aware#15
    - `compiler_builtins` may need to be built without debug assertions, see [this](https://github.com/rust-lang/rust/blob/8e917f48382c6afaf50568263b89d35fba5d98e4/src/bootstrap/bin/rustc.rs#L210-L214). Could maybe use profile overrides.
- Panic issues:
    - `panic_abort` is not yet supported, though it should probably be easy. It could maybe look at the profile to determine which panic implementation to use? This requires more hard-coding in Cargo to know about rustc implementation details.
    - [This](https://github.com/rust-lang/rust/blob/8e917f48382c6afaf50568263b89d35fba5d98e4/src/bootstrap/bin/rustc.rs#L186-L201) should probably be handled where `panic` is set for `panic_abort` and `compiler_builtins`. I would like to get a test case for it. This can maybe be done with profile overrides?
- Using two resolvers is quite messy and causes a lot of complications. It would be ideal if it could only use one, though that may not be possible for the foreseeable future. See rust-lang/wg-cargo-std-aware#12
- Features are hard-coded. See rust-lang/wg-cargo-std-aware#13
- Lots of various platform-specific support is not included (musl, wasi, windows-gnu, etc.).
- Default `backtrace` is used with C compiler. See rust-lang/wg-cargo-std-aware#16
- Sanitizers are not built. See rust-lang/wg-cargo-std-aware#17
- proc_macro has some hacky code to synthesize its dependencies. See rust-lang/wg-cargo-std-aware#18. This may not be necessary if this uses `--sysroot` instead.
- Profile overrides cause weird linker errors.
  That is:
  ```toml
  [profile.dev.overrides.std]
  opt-level = 2
  ```
  Using `[profile.dev.overrides."*"]` works. I tried fiddling with it, but couldn't figure it out.
  We may also want to consider altering the syntax for profile overrides. Having to repeat the same profile for `std` and `core` and `alloc` and everything else would not be ideal.
- ~~`Context::unit_deps` does not handle build overrides, see #7215.~~ FIXED

## Questions for this PR
- I went with the strategy of using `--extern` to link the standard lib. This seems to work, and I haven't found any problems, but it seems risky. It also forces Cargo to know about certain implicit dependencies like `compiler_builtins` and `panic_*`. The alternative is to create a sysroot and copy all the crates to that directory and pass `--sysroot`. However, this is complicated by pipelining, which would require special support to copy `.rmeta` files when they are generated. Let me know if you think I should use a different strategy. I'm on the fence here, and I think using `--sysroot` may be safer, but adds more complexity.
    - As an aside, if rustc ever tries to grab a crate from sysroot that was not passed in via `--extern`, then it results in duplicate lang items. For example, saying `extern crate proc_macro;` without specifying `proc_macro` as a dependency. We could prevent rustc from ever trying by passing `--sysroot=/nonexistent` to prevent it from trying. Or add an equivalent flag to rustc.
- How should this be tested? I added a janky integration test, but it has some drawbacks. It requires internet access. It is slow. Since it is slow, it reuses the same target directory for multiple tests which makes it awkward to work with.
    - What interesting things are there to test?
    - We may want to disable the test before merging if it seems too annoying to make it the default. It requires rust-src to be downloaded, and takes several minutes to run, and are somewhat platform-dependent.
- How to test that it is actually linking the correct standard library? I did tests locally with a modified libcore, but I can't think of a good way to do that in the test suite.
- I did not add `__CARGO_DEFAULT_LIB_METADATA` to the hash. I had a hard time coming up with a test case where it would matter.
    - My only thought is that it is a problem because libstd includes a dylib, which prevents the hash from being added to the filename. It does cause recompiles when switching between compilers, for example, when it normally wouldn't.
    - Very dumb question: Why exactly does libstd include a dylib? This can cause issues (see rust-lang/rust#56443).
    - This should probably change, but I want to better understand it first.
- The `bin_nostd` test needs to link libc on linux, and I'm not sure I understand why. I'm concerned there is something wrong there. libstd does not do that AFAIK.
@Ericson2314
Copy link
Contributor

Ericson2314 commented Sep 7, 2019

Can we just have the workspace root decide how things are built? That seems like a non-controversial thing to add to Cargo.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 19, 2019

Looks like rust-lang/cargo#7353 might be related or even solve this issue, but I have not taken a closer look yet.

EDIT: Looks like it's only for cargo's own growing support to build libstd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-driver Area: rustc_driver that ties everything together into the `rustc` compiler C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants