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.

Sign up for GitHub

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

Jump to bottom

Cargo standard library dependencies #5

Open
ehuss opened this issue Jul 20, 2019 · 8 comments
Open

Cargo standard library dependencies #5

ehuss opened this issue Jul 20, 2019 · 8 comments
Labels
RFC Discussion for a possible new RFC stabilization blocker This needs a resolution before stabilization

Comments

@ehuss
Copy link
Contributor

ehuss commented Jul 20, 2019

This issue is for tracking the writing of an RFC for extending Cargo.toml to specify dependencies on standard library crates.

There are a few needs and requirements:

  • Opt-in to building the standard library instead of using pre-built artifacts.
  • Specifying alternate standard library implementations.
  • Create a way for Cargo to pass --extern=foo to rustc to specify explicit dependencies to add to the extern prelude, even for pre-built artifacts. This can help obviate the need for extern crate for crates like proc-macro.
    • We may also consider this to be implicit in some cases. For example, proc-macro could be automatically added with proc-macro = true.

There have been a few different proposals for syntax, please see the following:

Some issues to consider:

  • How does cargo treat multiple crates in a graph declaring dependencies on the standard library?
  • How to balance implicit vs explicit dependencies?
  • How to express a dependency on a pre-built artifact vs building one from source.
@ehuss ehuss added the RFC Discussion for a possible new RFC label Jul 20, 2019
@SimonSapin
Copy link

Some thoughts:

  • How does cargo treat multiple crates in a graph declaring dependencies on the standard library?

Same as when multiple crates in the graph declare a dependency on a given crates.io crate: unify them. Except that the standard library doesn’t have a version number that is meaningful to Cargo, so the version key should not be allowed and you can’t end up with two copies because versions incompatible with each other were requested.

For feature flags specifically (#4), this means taking the union of all requested features.

  • How to balance implicit vs explicit dependencies?

This one is tough because we’re stuck with what’s already stable. Most existing stable crates depend on libstd implicitly, so it’s tempting to declare that every crate does unless it opts out.

But we also have existing stable #![no_std] crate that do not have this explicit opt out (since the opt out mechanism doesn’t exist yet), but are still compatible with targets that do not have a libstd at all. Cargo needs to continue to support those as well.

  • How to express a dependency on a pre-built artifact vs building one from source.

I think there should not be direct control over this. Cargo should decide based on other factors what libstd it wants (profile settings #2, target #3, feature flags #4, etc), then see if a binary is already available with that configuration, and if not fall back to compiling it.

Any difference between a binary shipped through rustup and one rebuilt with the same configuration is a bug. Perhaps a reproducibility bug rust-lang/rust#34902, or perhaps another criteria should be added to this "configuration" concept.

@SimonSapin SimonSapin mentioned this issue Jul 23, 2019
Open
@Ericson2314
Copy link

This one is tough because we’re stuck with what’s already stable. Most existing stable crates depend on libstd implicitly, so it’s tempting to declare that every crate does unless it opts out.

Just doing that is the easiest plan.

But we also have existing stable #![no_std] crate that do not have this explicit opt out (since the opt out mechanism doesn’t exist yet), but are still compatible with targets that do not have a libstd at all. Cargo needs to continue to support those as well.

Hmm on platforms with std the extra implicit dependency is fine. On platforms without std...I hope we can quickly transition most crates, and seeing that those platforms have the most to gain from this the breakage will be worth it.

The only other alternative I see would be making a breaking change in the Cargo.toml format (requiring all deps explicitly) and allowing Cargo to parse old versions. That's nice, though crates using the new format drop do support for old rustc/cargo.

Trying to have Cargo "fish out" the #![no std] sounds like a nightmare. I absolutely recommend against that. [If anything, I rather go in the opposite direction where #![no std] becomes as deprecated as extern crate.]

I think there should not be direct control over this.

Amen!

@Ericson2314 Ericson2314 mentioned this issue Jul 23, 2019
Open
@ehuss ehuss mentioned this issue Aug 5, 2019
Merged
bors added a commit to rust-lang/cargo that referenced this issue Sep 3, 2019
@bors
Auto merge of #7216 - ehuss:build-std, r=alexcrichton
49fbf52 
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.
This was referenced Sep 6, 2019
Open
Open
Closed
Closed
@alexcrichton alexcrichton mentioned this issue Oct 2, 2019
Closed
@ehuss ehuss mentioned this issue Oct 3, 2019
Merged
@jethrogb
Copy link

Opt-in to building the standard library instead of using pre-built artifacts.

This seems like something that shouldn't just be part of the regular dependency specification.

@Ericson2314 Ericson2314 mentioned this issue Jun 14, 2020
Closed
@clangenb clangenb mentioned this issue Jun 21, 2021
Closed
@PoignardAzur
Copy link

But we also have existing stable #![no_std] crate that do not have this explicit opt out (since the opt out mechanism doesn’t exist yet), but are still compatible with targets that do not have a libstd at all. Cargo needs to continue to support those as well.

A thought occurs: maybe it would be a good thing to add a no_std flag to cargo right now, even if that flag doesn't do anything yet, so that when a later version comes out and does use the flag, crates can use it without bumping MSRV.

(my understanding is that was the reasoning for the rust-version flag?)

@tarcieri
Copy link

tarcieri commented Oct 14, 2022 •
edited
Loading

An idea I posted to IRLO awhile ago: https://internals.rust-lang.org/t/pre-pre-rfc-making-std-dependent-cargo-features-a-first-class-concept/10828

Pave the cowpaths we have today, making the alloc and std features first-class (probably at an edition boundary).

They could be unconditionally linked by default just like today, but adding a std feature at all could automatically make a crate no_std, and when the std feature is enabled, automatically link std the same way specifying extern crate std would in a no_std crate, and ditto for alloc.

This requires no new syntax or concepts, just semantic changes that would eliminate the existing boilerplate.

Existing code would work just fine, but linters could warn you existing no_std and extern crate std directives are no longer necessary and redundant.

Bonus points: this gets rid of the last remaining use cases for extern crate, I believe.

@Lokathor
Copy link

first of all extern crate is still useful for forcing a crate to be linked in.

but second, some crates are no_std but don't have a std feature (or alloc feature), so you must design around that case too.

@tarcieri
Copy link

tarcieri commented Oct 14, 2022 •
edited
Loading

Yeah sure, but the boilerplate for #![no_std] along with a feature-gated extern crate std in the event you want to support both a no_std and std profile is a lot which it’d be nice to replace with declarative first-class features, the same way the 2018 edition removed the need for routine extern crate for every crate dependency.

Also sidebar, but the no_std approach is very much antithetical to the additive nature of features: it’s something you add which subtracts functionality. This has lead to countless bugs I’ve had to endure where people either add a subtractive no_std crate feature or add broken cfg(not(feature = “std”)) gating leading to code which is broken when the std feature is disabled.

So even if you don’t like my suggestion for first-class alloc and std crate features, I would implore you to make the Cargo linkage to libraries like alloc and std linking additive rather than subtractive, just like crate features, with defaults that can be removed/overridden, rather than having to actively declare what you don’t want.

@Rahix
Copy link

Rahix commented Oct 14, 2022

first of all extern crate is still useful for forcing a crate to be linked in.

@Lokathor

use crate_name as _;

or just

use crate_name;

also works fine for that purpose.

@ehuss ehuss added the stabilization blocker This needs a resolution before stabilization label May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
RFC Discussion for a possible new RFC stabilization blocker This needs a resolution before stabilization
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@tarcieri @ehuss @SimonSapin @Ericson2314 @jethrogb @Lokathor @Rahix @PoignardAzur