-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Make Cargo aware of standard library dependencies #1133
Conversation
I clarified and cleaned up some things, but force-pushed as - The ideas are the same, just the wording is changed - At the time of commit, I had not yet submitted the PR
Wow neat |
|
||
Currently, all packages implicitly depend on libstd. This makes Cargo unsuitable for packages that | ||
need a custom-built libstd, or otherwise depend on crates with the same names as libstd and the | ||
crates behind the facade. The proposed fixes also open the door to a future were libstd can be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/were/where
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
A funny 'coin'cidence is that the Java folks at Oracle are currently testing a similar concept (and they needed 9 major versions to get there). 👍 |
The only new interface proposed is a boolean field to the package meta telling Cargo that the | ||
package does not depend on libstd by default. This need not imply Rust's `no_std`, as one might want | ||
to `use` their own build of libstd by default. To disambiguate, this field is called | ||
q`no-implicit-deps`; please, go ahead and bikeshead the name. `no-implicit-deps` is false by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The specific name doesn't concern me too much, but do make it boolean-positive instead of negative. So instead of having no-implicit-deps
and setting it to false
by default, make it use-implicit-deps
and make it true
by default.
Whatever name is bikeshedded should be boolean-positive.
@Valloric in case you don't get a notification from the commit message, I did what you suggested with the exception that the field is just called |
@Ericson2314 That's fine; like I said, I don't personally care much about the name (you'll get more than enough bikeshedding from others), only that a double negative is avoided. |
Also the text states that "std should be on crates.io" – the rust distribution should still contain all of std anyway, right? Otherwise people who download rust to try it during their commute are going to have a hard time. 😄 (Case in point: I recently had to download a doc snapshot from servo so I could continue to hack on rust-clippy when offline, because the standard docs no longer contain the rustc… crates). |
Thanks for the RFC @Ericson2314! I'm always quite eager to improve the cross-compilation experience of Rust, and I think that this will help out embedded targets and such quite a bit! At a high level, could you clarify the motivation for this RFC a bit more in the text? After reading it it's not clear to me what the RFC is trying to achieve. For example:
I think that refining the motivation will help me understand more of the detailed design as well, but I'll make some specific comments below about this.
I'm curious where you came up with this? Cargo doesn't pass
This worries me a bit, I don't think we want the ability to swap in an alternate standard library in a super easy fashion. If you're depending on your own "standard library", then I think it should be just that, a dependency in
Can you clarify why you can't use Cargo for these kinds of projects today? If you use
I also unfortunately don't fully understand what's going on here. Can you elaborate why you want to swap out a separate libstd than the one the compiler may already find elsewhere? If crates can be tracked in Cargo, then I definitely think they should, so I don't follow the motivation to not use
I don't quite understand this comment because this can already be done? I can upload a crate which says Overall I think this RFC feels like it's "bolting on" features after the fact instead of creating a well-integrated and smooth system with Cargo. I believe this is also (depending on the motivation) highly inter-related with #1005, and it may wish to be considered here. I think some refinement of the motivation, however, will help guide this RFC, though! |
tl;dr: - Motivation is significantly expanded - Phase 1 is cut for being too half-assed
Ok, I talked to @alexcrichton for a long while on IRC, and then rewrite the RFC. In short, I massively elaborated the motivation, and cut phase 1. I'm respond to some his points, which will also offer some summery of what we talked about, though go to the IRC logs if you're really curious.
All three of those are valid motivation for this.
I extrapolated incorrectly from his comments in the issue I opened long ago. It's fixed in the current RFC. The larger picture is the same either way however: Cargo allows
I'm OK with having
I agree wholeheartedly, but this is currently not possible if crate names + target platform overlap with anything that is in the sysroot.
This this can only be accomplished with a Cargo config override, which I consider sufficiently hacky. It's possible phase 1 would have allowed this to happen more accidentally, but that's outta here.
I wanted libcore to be cross compiled automatically, but if on some machines the host and target match (i.e. no cross compile), the build will fail. See the expanded motivation for more details.
I meant do this in a such that all deps can be (cross-)compiled automatically. See rewritten RFC and below.
I think I was able to explain my case on IRC. Basically @alexcrichton and I (and presumably you, dear reader, too) would like easy no-brainer cross compiling. If we don't make packages state which of libstd or its backing crates packages they need, future Cargo needs to download/build everything to play it safe, and needs to do that for ever. By treating these things as normal deps, Cargo can do something adhoc like querying a Mozilla build farm, but can seamlessly transition to just building them like any other package if they get Cargoized. Then all that ad hoc functionality can be thrown out the window. Finally, if you use a unstable compiler, you can cross compile your own libstd today, and it is nice to do so. While it would be great to standardize libcore, using it effectively requires a fair amount of lang items, and if those are standardized, perhaps all of libcore's implementation could to[1]. Thus, for the time being, freestanding development is probably going to require a compiler that is capable of building libcore, and it just so happens that those people benefit from that the most, so we might as well give them a way to cross compile their whole project---libcore up---today. [1] @alexcrichton pointed out that with standardization there might be less overlap with the lang needed items to use libcore and the lang items it uses in its implementation, quite possible. |
|
||
|
||
However rather than passing in libstd and its deps, Cargo lets the compiler look for them as need in | ||
the compiler's sysroot [specifically `<sysroot>/lib/<target>`]. This is quite coarse in comparison, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's important to spell out here that it's far from standard practice to dump libraries in the sysroot, and the only stable library in the sysroot today is libstd. We have been very hesitant to stabilize any more than precisely one library for many of these reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused what you'd like me to elaborate on. I already wanted to emphasize we just do this in the case of libstd and its dependencies---in other words that we are so close---just 1 library away!---from not linking with the sysroot at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm re-reading I'm not quite sure what I was thinking... It may have been from the aspect that "and its deps" isn't so relevant in stable Rust today as libstd is the only library that can be implicitly linked to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah OK. I'll clarify the situation for stable Rust.
@Ericson2314 so I'm still a little uneasy about this RFC, so I had another read through the motivation section to see if I could get myself up to speed. First I'll mention that my frame of mind is basically only geared towards the standard library today. Using any other dependency in the distribution is not stable and we're free to break if necessary. Along these lines the, I think that this RFC is proposing is that the following two manifests are the same: implicit-std = true # the default setting implicit-std = false
[dependencies]
std = { rustc-sysroot = true } So given that, I think one part I'm uneasy about is that I don't understand what practical impact this will have if implemented today. The possible benefits I see are:
Other than that though, I can't think of what benefits this may bring (although I may be forgetting some). Does that accurately sum up what you're thinking about this RFC? Or have I forgotten some aspect? |
Go ahead and make me a noop vote |
I think we should keep this RFC open simply as a tradition 😛 |
@rfcbot reviewed |
I do agree we should have a more minimal first step, but I don't think it should be (just) rust-lang/cargo#4959. The most important part of this is the interface for specifying stdlilb deps, so we can cleanly bypass all the crufty sysroot and boostrapping implementation details needed today. Much of the discussion thread of this PR also mistakenly focused on uninteresting implementation minutia. I'm happy to rewrite this or open a new one to that end. |
+1 to the idea that sysroot should die and that stdlib crates should not be much different from crates from crates.io. We still, after several years, would like |
@matklad Yeah that's a great point. When I started preaching death to the sysroot, it was a pretty academic question of avoiding the complexity of needless distinctions. But now with all the new tooling work, be it incremental compilation, IDE support, etc, there's real tangible benefits visible to just about every regular user (far beyond the niche of embedded and OS devs). |
@Ericson2314 I'd like to not lose track of the other issues. I don't think a new RFC is the best way to do that though. Perhaps opening an issue or two on the Cargo repo would be better? Where there are UI things, I'd be keen to experiment on top of the sysroot stuff as a plugin if possible, otherwise as an unstable feature, then RFC later. For other things it would be good to identify small chunks of work which could be isolated and have their own RFCs or implementation. |
@nrc OK made 2 (and found 1). Those basically correspond to what I implemented in my Cargo PR way back when, and can all be handled "decently independently". |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period is now complete. |
Closing the RFC per complete FCP #1133 (comment) with a disposition to close. |
Currently, all packages implicitly depend on libstd. This makes Cargo unsuitable for packages that
need a custom-built libstd, or otherwise depend on crates with the same names as libstd and the
crates behind the facade. The proposed fixes also open the door to a future where libstd can be
Cargoized.
I've been busy with school until very recently, so apologies in advance if it turns out I am not taking into account some recent change.
Rendered