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

Pre-RFC: std aware Cargo #1

Merged
merged 10 commits into from
Mar 16, 2019
Merged

Pre-RFC: std aware Cargo #1

merged 10 commits into from
Mar 16, 2019

Conversation

jamesmunns
Copy link
Owner

@jamesmunns jamesmunns commented Feb 9, 2019

@jamesmunns
Copy link
Owner Author

CC: @japaric @alexcrichton @yoshuawuyts

I'll plan on posting this to i/u.r-l.o once I have it 100% done, but the bulk of it is ready for review, and I would welcome comments!

@jamesmunns
Copy link
Owner Author

Note to self: Probably need to clarify items around precompiled versions of core and std, and whether they should be rebuilt if the profile settings are set, for example with opt-level=3. Maybe only if overrides are used?

@yoshuawuyts
Copy link

Overall I found this quite approachable, and the RFC makes sense to me. Thanks for writing this @jamesmunns!

@alexcrichton
Copy link

Thanks for this! Some thoughts I had while reading:

  • How does this interact with targets that have precompiled binaries available? For example if you cargo build --target i686-unknown-linux-gnu on OSX, would Cargo somehow ask you to prefer rustup or would core/compiler_builtins be built from source?

  • How does Cargo actually acquire the source code for the core/compiler-builtin crates?

  • How does the dependency on core get turned off for core itself?

  • As a caveat, we'll want to be sure to use Cargo to build std/core/etc, especially with the lock file present already in the source (as std depends on crates.io, and we'll want to lock down those versions)

  • This may want to mention briefly how Cargo deduces that it should compile core/compiler-builtins. This is probably implicit, but it may be good to explicitly mention too. I think we'll also want to consider the story for core/std as well so Cargo can eventually deduce what set of stable sysroot crates it should compile.

  • We'll almost for sure want to have a global cache of binaries for libstd/libcore/etc, and that may want to be mentioned here as well

@jamesmunns
Copy link
Owner Author

jamesmunns commented Feb 11, 2019

Hey @alexcrichton, here are a couple notes (some answers, some open questions), If you don't have any further comments, this is how I will address the items you mentioned:

  • How does this interact with targets that have precompiled binaries available? For example if you cargo build --target i686-unknown-linux-gnu on OSX, would Cargo somehow ask you to prefer rustup or would core/compiler_builtins be built from source?

Currently I've written that core/std would be rebuilt when a .json target is provided, but that isn't quite good enough. I think any of the following would trigger cargo to rebuild core/std (and if none of these are true, a precompiled version is used):

  • A custom target json is used
  • The root crate has modified the feature flags of core or std
  • The root crate has set certain profile settings, such as opt-level, etc.
    • Do we want to make this dependent on RFC 2822, e.g. make the user use a profile override, instead of just normal profile settings?
  • The user has specified a patch.sysroot.
    • Should std be rebuilt to use the patched core if specified? Is linking to the new core enough?
  • How does Cargo actually acquire the source code for the core/compiler-builtin crates?

Yup, this is an open question. I would say using rustup on day one is fine, and making the "cargo should lazily but automatically get its dependencies" should be a separate RFC.

  • How does the dependency on core get turned off for core itself?

I'm afraid I don't understand this. Is there something like a stage0 core used to build core?

  • As a caveat, we'll want to be sure to use Cargo to build std/core/etc, especially with the lock file present already in the source (as std depends on crates.io, and we'll want to lock down those versions)

Ah, this is a good point. I'm not sure how this will work if the user tries to patch certain dependencies that are shared with core/std. I'd say in the most conservative mode, we can say that these dependencies are just part of core/std, and cannot be touched. Maybe add the ability to patch core/std deps as a nightly feature in the future?

  • This may want to mention briefly how Cargo deduces that it should compile core/compiler-builtins. This is probably implicit, but it may be good to explicitly mention too. I think we'll also want to consider the story for core/std as well so Cargo can eventually deduce what set of stable sysroot crates it should compile.

I think I touched on this above, but you're right, I do think we need a "don't use core" for #![no_core] uses, and a "don't use std" for #![no_std]. Do you have any suggestions for existing flags we can use? optional = true seems close, but really we want omitted = true or something like that in cargo.toml.

  • We'll almost for sure want to have a global cache of binaries for libstd/libcore/etc, and that may want to be mentioned here as well

Hmm, I do think this could help, though if you allow for profile modifications to trigger a rebuild, you might end up with a lot of libcore/libstds and friends.

@alexcrichton
Copy link

I think any of the following would trigger cargo to rebuild core/std

Ok cool that sounds pretty good to me. For profile settings though I think we'll need to find a good middle ground because if you tweak profile settings today it doesn't already recompile libstd/libcore, and it may not always be desired that they're recompiled. For some use cases, however, you definitely do want to recompile!

I think we'd definitely recompile std if core is patched!

I would say using rustup on day one is fine, and making the "cargo should lazily but automatically get its dependencies" should be a separate RFC.

Ok cool that makes sense to me. The answer I think is then "Cargo looks for a special location in the rustc sysroot and then errors out if it isn't there" and presumably Cargo could provide a nice-ish error message talking about rustup.

I'm not sure how this will work if the user tries to patch certain dependencies that are shared with core/std.

Yeah I think patching is definitely gonna happen later in the future. Now that I think about this we may just need to be more principled about the crates.io dependencies for libstd on crates.io, accurately doing major version bumps and such. In any case, something to worry about later!

Do you have any suggestions for existing flags we can use?

I don't think there's actually much to reuse, but one possible option is that by default crates depend on std/core. If you mention anything, though, then the implicit dependencies are broken. For example this crate depends on std/core default features:

[dependencies]

whereas this crate only depends on core:

[dependencies]
core = { sysroot = true }

(or something like that)

I'm not sure if this is the best design though, and seems like something good to discuss on the RFC!

Hmm, I do think this could help, though if you allow for profile modifications to trigger a rebuild, you might end up with a lot of libcore/libstds and friends.

Heh true! We have a lot of binaries already though :)

@jamesmunns
Copy link
Owner Author

Yeah, for the profile items, we might want a new profile flag like "apply-to-sysroot", which is false by default, but can be opted in to, forcing the rebuild of std. This would look like:

[profile.release]
opt-level = 'z'
apply-to-sysroot = true

For the source stuff, I think looking in the right place in sysroot, and erroring with a "you need to provide the source to rebuild sysroot. Install using rustup with rustup component add rust-src" gets the point across.

I think the rest of the items are good to discuss in the RFC, I'll make sure they all make it into the open questions section :)

Feel free to tag anyone else you think would have good input on this, I'll be plugging it at the next Embedded-WG meeting, and I hope to have this ready to share (as a pre-rfc) more publicly (on u/i.r-l.o) by the middle of this week.

@alexcrichton should this be a rust-lang/rfcs PR, or should it go to rust-lang/cargo?

@alexcrichton
Copy link

All sounds good to me! We don't have a ton of precedent of using rust-lang/cargo for Cargo RFCs, so let's probably stick to rust-lang/rfcs for now

@japaric
Copy link

japaric commented Feb 11, 2019

@jamesmunns Thanks for writing this! It looks good to me. 🚀

"Cargo looks for a special location in the rustc sysroot and then errors out if it isn't there"

(I wonder if we'll want some tamper protection mechanism for the rust-src component. It'd be easy to modify the source on disk to e.g. wrap unstable API in stable API.)


* Supporting new/arbitrary targets, such as those defined by a ".json" file
* Making modifications to `core` or `std` through use of feature flags
* Users who would like to make different optimizations to `core` or `std`, such as `opt-level = 'z'`, with `panic = "abort"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add that we'd really like to be able to have debug_assertions in core, alloc, and std. See threads like https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927?u=scottmcm

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @scottmcm, is this something you would see as a compile time configuration option of core, std, alloc, etc?

This RFC isn't meant to propose which features should or should not be available, but instead is focused on the method for selecting them.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jamesmunns Yes, I do see it as a compile-time configuration option. (Ditto for having overflow checks on by default in debug but not in release -- right now that's hacked together with #[rustc_inherit_overflow_checks], which I'd love to be able to remove.)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I'm fine with the RFC not saying how those particular things should be accomplished, but I think they're valuable examples since it's the Motivation section.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scottmcm Would you be interested in PR-ing (or suggesting text) for a section in "Future possibilities" to discuss a listing of possible flags to stabilize? I don't think this should impact the RFC, but might be good to centralize a list of things people are thinking of.

In general, I want to make it clear in that section that "this is a list of possible flags to stabilize, but this RFC does not guarantee any of the following will be stabilized".

Feel free to fork my repo and submit a PR to this branch, and I can accept it there.

[summary]: #summary

Currently, the `core` and `std` components of Rust are handled in a different way than Cargo handles other crate dependencies. This causes issues for non-mainstream targets, such as WASM, Embedded, and new not-yet-tier-1 targets. The following RFC proposes a roadmap to address these concerns in a consistent and incremental process.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to give a brief summary of the "how it is achieved" here as well.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a little bit worried about repeating myself as this is explained without detail (in the Guide Level), and explained with detail (in the Reference Level).

I am unsure how to summarize without repeating the content of the Guide Level Explanation verbatim. I am open to suggestions!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to think of something to suggest.. :)

text/0000-std-aware-cargo.md Outdated Show resolved Hide resolved
text/0000-std-aware-cargo.md Outdated Show resolved Hide resolved
text/0000-std-aware-cargo.md Outdated Show resolved Hide resolved
text/0000-std-aware-cargo.md Outdated Show resolved Hide resolved
The stabilization of a `core` feature flag would require a process similar to the stabilization of a feature in the language:

* Any new feature begins as unstable, requiring a nightly compiler
* When the feature is sufficiently explored, an RFC/PR can be made to `libcore` to promote this feature to stable
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we would add new feature flags to Cargo.toml of libcore? You may want to clarify this with less familiar readers...

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I hadn't actually considered that far. I would suppose that would be one possible way of implementation.


#### Implementation of Stable Features

There would be some mechanism of differentiating between flags used to build core, sorting them into the groups `unstable` and `stable`. This RFC does not prescribe a certain way of implementation.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair; but you may want to suggest a few possible mechanisms to make this implementable.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps! But I am not an active developer of core or std, so I was trying to avoid something that makes sense to me, but doesn't make sense in practice.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair :) Maybe someone else can help out here? e.g. @alexcrichton?

[prior-art]: #prior-art

* https://github.com/rust-lang/rfcs/pull/1133
* https://github.com/japaric/xargo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to elaborate on these links and give a summary :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust-lang/cargo#2768 you might also want to link and also rust-lang/cargo#5002 and rust-lang/cargo#5003


## Should Cargo obtain or verify the source code for `libcore` or `libstd`?

Right now we depend on `rustup` to obtain the correct source code for these libraries, and we rely on the user not to tamper with the contents. Are these reasonable decisions?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discuss possible ways to avoid tampering? If we rely on users not tampering, how do we communicate this effectively?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the first thing we should decide is IF we should try to detect tampering, and whether that makes sense when the user owns their own PC anyway.

I am of the opinion that we shouldn't (as I think it is a cat and mouse game), however this was brought up by multiple people :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the first thing we should decide is IF we should try to detect tampering, and whether that makes sense when the user owns their own PC anyway.

Right, but we should at least be sure that we can detect tampering.. ;) IOW, let's not decide we want it and find out later that we cannot.

I am of the opinion that we shouldn't (as I think it is a cat and mouse game), however this was brought up by multiple people :)

Things like people wanting to use RUSTC_BOOTSTRAP on stable make me less sure; but maybe we can use social instead of technical means to discourage tampering?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO: Changing libcore/libstd would only affect the binaries produced by building this project, and would not be easy to "accidentally" end up with (except as a consumer of that binary). At the end of the day, anyone can patch the rust compiler for their own builds and distributions, which would defeat any measures we put in place.

Even if we bake in the CRCs of the source files into rustc, people can patch the rustc binary, or rebuild their own compiler. At the end of the day, the compiler is code running on their computer, which we can't do much about.

At best, I think tamper detection would serve only as a warning, "you changed the source, this is not supported behavior".

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At best, I think tamper detection would serve only as a warning, "you changed the source, this is not supported behavior".

If it's not too troublesome performance and implementation-wise, that seems like a decent solution; at least we have communicated what we do and do not consider stable in a direct way then. :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I am just trying to avoid wasted engineering time, and even just baking the CRC of the total source into the compiler seems like it may be more trouble than it is worth. Let's see what other people's feedback is!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah fair; for now you could discuss this as a possibility :)


By using stable feature flags for `std`, we could say that `std` as a crate with `default-features = false` would essentially be `no_core`, or with `features = ["core"]`, we would be the same as `no_std`.

This abstraction may not map to the actual implementation of `libcore` or `libstd`, but instead be an abstraction layer for the end developer.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go into how we could architect such an abstraction layer?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I was unclear, but the "abstraction layer" is that everything except std goes away from the user's perspective, and today's no_core and no_std are just std with no/fewer features set.

I call it an abstraction layer, as under the hood it is unlikely (or undesirable) to try and merge libcore, liballoc, libstd, et. al due to a number of reasons (discussed by the libs team at all-hands).

Let me know how much of that you would like me to add to the RFC or expand upon :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know how much of that you would like me to add to the RFC or expand upon :)

Mostly, I think it would be good to consider how we might architect the facade that core and alloc would presumably turn into... possibly with some code snippets as examples.

Centril and others added 4 commits February 14, 2019 20:00
Co-Authored-By: jamesmunns <james@onevariable.com>
Co-Authored-By: jamesmunns <james@onevariable.com>
Suggestion cleanups

Co-Authored-By: jamesmunns <james@onevariable.com>
@timClicks
Copy link

Currently, the core and std components of Rust are handled in a different way than Cargo handles other crate dependencies.

"Currently" will slip over time. Perhaps pin to a specific version? "As of Rust 1.34, ..."

@timClicks
Copy link

Stabilization of JSON target format

As the custom target json files would become part of the stable interface of Cargo. The format used by this JSON file must become stabilized, and further changes must be made in a backwards compatible way to guarantee stability.

Sorry to bikeshed, but I guess I might as well ask the question. Is JSON already deeply baked in? It has always struck me as a bit strange to use format for configuration, when Cargo uses TOML.


#### Stabilization of JSON target format

As the custom target json files would become part of the stable interface of Cargo. The format used by this JSON file must become stabilized, and further changes must be made in a backwards compatible way to guarantee stability.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always wondered why JSON is used for target specifications. I assume for historical reasons?

I know that it would be more work, but how about switching to TOML? It would be consistent with the other configuration files used by Rust (Cargo.toml, Cargo.lock, .cargo/config) and has the big advantage that it supports comments, which seems very useful because some target options are anything but self-documenting. It would also allow grouping the options in sections, e.g. for separating architecture options (data-layout etc) from build options (linker-flavor etc) if we like.

@phil-opp
Copy link

cc @Ericson2314, who created rust-lang#1133

@Ericson2314
Copy link

Ericson2314 commented Feb 14, 2019

Thanks @phil-opp, I'll try to get to this soon.

@jamesmunns
Copy link
Owner Author

Regarding TOML vs JSON syntax (CC @timClicks and @phil-opp), I would probably agree with you, though I'd prefer not to tack that on to this RFC. I think since this only touches components that haven't been stabilized, it might be easier to submit an RFC addressing changing JSON => TOML (extending/replacing RFC0131 ), separately from this RFC.

@jamesmunns
Copy link
Owner Author

Also thank you @phil-opp, I was looking for that RFC, and failed to find it! I'll plan on reviewing it, and see if I need to make any modifications to my RFC, or at least address any open concerns.

@Ericson2314 it would be great to have any feedback from your perspective as well :)

@Ericson2314
Copy link

Ericson2314 commented Feb 14, 2019

@jamesmunns Yeah everything looks good, and the last paragraph confirmed you've got the right narrative in your head :).

To reiterate to everyone else, the "lodestar" that's best to keep in mind is that eventually we want standard libraries to become as unspecial as special. Specifically the sysroot should indeed eventually go away as we instead have a generic mechanism for caching (and distributing prebuilt) binaries. Likewise it's important than old crates that don't list their stdlib dependencies work as if they do.

In the middle term, yes, we may not able to hit all these goals at once, but in the super close term, I think we should in fact not worry about any intermediate steps either. Specifically, we can punt on worrying about distributing std sources and working with the sysroot by focusing on two specific use-cases:

  1. Rust's own build system. Use this to avoid the shim creates and extra bootstrapping we do today.
  2. Freestanding development, where we can assume [patch] is used to smuggle in possibly-modified stdlib sources, and the prebuilt binaries are worthless anyways.

To my delight, the RFC as totally on board with all that, with the JSON target script being an elegant trick to opt-out of normal sysroot stuff absent anything else, and the [patch] use-case explicitly called out.

I linked it above, but my old PR rust-lang/cargo#2768 that aimed to address just those two use-cases, contra my RFC, may be of some use in fleshing out the details. (Thought it could just be too bitrotted to help!)

@Ericson2314
Copy link

To the concerns about lockfiles, I would again punt on this as only a matter of sysroots and caching. But looking ahead, the general idea is twofold:

  1. If we have prebuilt binaries, we ought to "trim" the lock file to just the build artifacts and their dependencies. Then, when looking at the cache to see if a build step can be stepped, we can check whether the trimmed lock file of each pre-built binary is compatible with the current goals. The trimming then just removes spurious information that would make that compatibility check needlessly more conservative.

  2. During dependency resolution, we may wish to bias the solver in to steer it towards cache hits per the above. This is the general mechanism to steer the user towards prebuilt binaries while neither complicating the soundness check above ("are the lockfiles compatible?") nor making Cargo impure ("just use these binaries; don't care how they are built").

@Ericson2314
Copy link

Oh, what's the point of patch.sysroot vs normal patch. Aren't std and core reserved names on crates.io so ther's no ambiguity?

I ask because I think this is the one exception in the RFC to not baking "sysroot" into any exposed interfaces.

@comex
Copy link

comex commented Feb 14, 2019

Terrible idea: It might be cute to allow adding !std or !core as a dependency name, along the lines of !Sized.

@elichai
Copy link

elichai commented Feb 14, 2019

I love it!
This could be life changing to projects like https://github.com/baidu/rust-sgx-sdk

@IsaacWoods
Copy link

Along with the previously-mentioned inconsistency of using JSON, I've always found the target specification format pretty arcane, and not very documented (as someone who's used them a fair amount). There are also a few options that are very LLVM-specific (data-layout and llvm-target come to mind) - should this / how will this work as alternative backends such as Cranelift are added? Are we explicitly tied to LLVM on stable for anything else atm, or would this set a precedent?

I think we should at least review and document the current situation before stabilisation (preferably separately, so as to not detract from how awesome getting this working would be).

Apart from that, I'm very excited to see this make progress and the overall plan is what I've been hoping for for a long while! 🎉

@Ericson2314
Copy link

I think Cargo could rely on there being a target file that Rustc understands without making it's contents stable, no? That's all that's needed to switch on/off the sysroot stuff, not actually inspection.

@IsaacWoods
Copy link

The other option, of course, would be to at first only support custom target specifications on nightly (and not stabilise any of that stuff yet), and only let people build custom sysroot crates for targets that don't currently provide official builds (all the Tier 3 targets, if I'm not mistaken) on stable. Custom targets could then be stabilised separately, at a later date.

@timClicks
Copy link

There are also a few options that are very LLVM-specific (data-layout and llvm-target come to mind) - should this / how will this work as alternative backends such as Cranelift are added? Are we explicitly tied to LLVM on stable for anything else atm, or would this set a precedent?

These seem like very important considerations before the spec could be stablized. Could this RFC potentially proceed with a statement like "currently-accepted JSON or whatever standard later established"? Perhaps an idea would be to mandate necessary attributes, but not the specific format that they're encoded in?


In some cases, it may be desirable to modify `core` in set of predefined manners. For example, on some targets it may be necessary to have lighter weight machinery for `fmt`.

This step would provide a path for stabilization of compile time `core` features, which would be a subset of all total compile time features of `core`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While not essential, I have an additional use case that fits with Rust's larger goals: reducing the complexity for the beginner.

To someone new to Rust, requiring the "nightly compiler" to embark on embedded development can feel unsettling. Nightly feels advanced & dangerous, stable feels safer and more secure. ("I thought embedded was a great fit for Rust, why can't the stable compiler version handle that yet?") It also increases the teaching complexity, as I've encountered writing some drafts of Rust in Action content.

Copy link

@Ericson2314 Ericson2314 Feb 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the thing. A lot the reason we don't just stabilize everything needed for low-level is it's more complex than it needs to be. There's so many things that are just....needlessly different between "regular" and embedded development. The needless barriers between standard libraries and regular libraries are just one example of this.

The switch to unstable Rustc feels spooky, but ultimately just means that more things are available. Everything that works with stable Rust also works with unstable Rust. It's more spooky than actually dangerous.

If you students ask, tell them it's so future students get a smoother experience and we aren't stuck in a situation that cannot improve like Clang/GCC and C/C++.

@jethrogb
Copy link

jethrogb commented Feb 15, 2019

The RFC mentions in a couple of places that things are meant to be configured in the "root" crate. This is not how normal feature resolution works in Cargo. What would happen if a dependency specifies a dependency on core/std and sets features? Are you intending there to be special logic for core/std? I agree with @Ericson2314 that things should be as un-special as possible. We need a general mechanism to make global decisions (rust-lang-nursery/portability-wg#3 / rust-lang#2492).

Not sure if this next question needs to be resolved in the RFC, but I figured I'd bring it to your attention. When patching the sysroot, how would you like to handle rustc-std-workspace-core? I ran into this issue in Xargo. I've come up with different approaches to fix this, not sure which one is best:

  1. If there is a std crate in the dependency tree, compute ../tools/rustc-std-workspace-core from that path (or use the same git) and use that as the [patch].

  2. If there is a std crate in the dependency tree, find the spec of the core dependency, then have cargo generate a rustc-std-workspace-core crate in a tempdir with that core. Then, use that temp crate as the [patch]. I suppose this temp crate could be virtual in Cargo's memory.

  3. Require users specify their own rustc-std-workspace-core when patching the sysroot.

  4. Maybe this problem just goes away if Cargo knows about core?

  5. Possible alternatives I haven't thought of.


### Caveats

As stability guarantees cannot be made around modified versions of `core` or `std`, a nightly compiler must always be used.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why replacing core or std with a custom version requires nightly? How is it different from no_std and cargo source replacement which are both available on stable and allow similar effects? It makes core and std special again because the user has the freedom to use custom source for any other crate but for some reason is not allowed to use custom source for core or std.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that just replacing core or std with a custom version should be fine on stable, those custom versions should not be allowed to bypass stability like the standard version, so they will almost certainly require nightly to compile, but that's separate from the act of replacement.

3. Extend the new behaviors described in step 1 and 2 for `std` (and `alloc`).
4. Allow the user to provide their own custom source versions of `core` and `std`, allowing for deep customizations when necessary. This will require a nightly version of the compiler.

As a new concept, the items above propose the existence of "stable features" for `core` and `std`. These features would be considered stable with the same degree of guarantees made for stability in the rest of the language. These features would allow configuration of certain functionalities of `core` or `std`, in a way decided at compile time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How strong would the testing guarantees of stable features be? Do stable features mean a commitment to testing every combination of stable features on every tier-1 platform? If so, then that blows up the cost of CI by a factor of 2n.

@Ericson2314
Copy link

@japaric yeah rustc-std-workspace-core gets to go away.

@granthusbands
Copy link

Echoing some comments on specific changes, I question why a nightly compiler is always going to be needed. Once 'std'-aware Cargo and Rust are stable, surely one shouldn't then need a Nightly compiler to use feature flags? As stable as Nightly might be, it's still not the stable version.

@jamesmunns
Copy link
Owner Author

jamesmunns commented Feb 15, 2019

Okay, here are a couple notes and responses, based on topics.

Misc note to @Ericson2314: The use of a target file is not the only way cargo could decide to recompile core and std. See here and following lines for more details.

Misc note to @granthusbands: A nightly compiler is only required in some cases, which is discussed in most use case areas of the detailed design. With regards to feature flags, currently none of the feature flags in std or core have been committed as stable, because setting them requires a nightly compiler. This RFC proposes a way to mark feature flags as stable, which would allow them to be set or not set by a stable compiler.

@Ericson2314's previous efforts

  • RFC1133 - This RFC from 2015 proposed making cargo aware of std. I still need to review in more detail to find the parts and syntax that may solve some open questions.
  • Cargo Issue 5002 - This issue proposed a syntax for explicit dependency on std
  • Cargo Issue 5003 - This issue discussed how to be backwards compatible with crates that don't explicitly mention std

Crate Root Dependency

  • As a note, some parts of my design do still keep core and std as only "configurable" by the root crate. This is a bit of a special case (these sections would be ignored in non-root crate Cargo.toml's).
  • Also as an open discussion point, I am not sure how to specify that a crate is no_std or no_core in that crate's Cargo.toml
    • Perhaps we could add flags to Cargo.toml like std = true, core = false? This would be a binary on-off (and std without core wouldn't be valid), but all other configuration would be ignored (like setting features) when not in root position?
  • @jethrogb pointed out here that we may not to make core or std a special case.

Sysroot

  • @Ericson2314 asked about the patch.sysroot syntax. Currently patch uses the form patch.$source, such as crates-io (not the name of the crate to be patched). sysroot in this context was just the first descriptive category I could think of, I am open to other names.

Lockfile

  • @Ericson2314 suggested not to worry about lockfiles, however currently lockfiles are not observed when building library crates. @alexcrichton has voiced concern that crates defined in rust-lang/rust, such as libcore and libstd, may have looser Cargo.toml definitions than are desired, and may lean on the Cargo.lock file to get things right. This may be an issue.

Defining non-dependencies on core or std

  • @comex suggested !core and !std similar to !Sized

Target File Format

  • A number of people have expressed a lack of love for the JSON format, preferring TOML format instead
  • @IsaacWoods rightly pointed out that the current format is very heavily dependent on LLVM specs and format, which may not play well with alternative compiler backends.
  • @IsaacWoods also suggested NOT stabilizing the target file format as a dependency of this RFC, and instead leave the use of the target file format as a nightly-only feature
  • I am of the opinion we should have a separate RFC to change/stabilize that format, and this RFC should be "blind" to the contents of the target file (in whatever format it has)

Embedded Development

  • A number of people have voiced concern that nightly is required for embedded development. While it is true that only Cortex-M targets have been added to the list of official targets, any embedded platform may currently add a build of core to the CI, and add support for their platform to the list of rustup targets. This is certainly not the most scalable approach, however. Building no_std binaries, which are needed for development of embedded firmware, stabilized in Rust 1.31. CC @timClicks, @Ericson2314

Allowing for custom core and std with a stable compiler

@Riateche and @Nemo157 wondered why nightly was required for custom core or std. I discussed this with @alexcrichton, but it is currently not possible to build core (or maybe even std) without unstable features, as there are some compiler intrinsics and "magic" that are necessary (the format macros and box keyword come to mind).

I initially wrote the RFC in this manner, however I was later convinced this was not possible to do.

I am of the opinion that if you could, then it should be allowed to use a stable compiler, but that might be too theoretical for this RFC. I could include it in "Future Possibilities"

Cost of stable core and std features

  • @porglezomp pointed out that if we are to test all combinations of features to guarantee stability, this could have a 2^n effect on CI build times

Edit: grammatical fixes

@Ericson2314
Copy link

Ah thanks for reminding me how [patch] works. The $source here is chiefly a namespace thing: Cargo assumes crates from different sources are not "different versions of the same thing" and makes no attempt to unify them.

With that in mind, I do not think standard library crates should be separately namespaced. I do like the std facade approach, which means creating more creates behind std (along with using existing ones like hashbrown and parking_lot for this purpose). One would think that would require namespacing to work out, but the crucial thing to keep in mind is such creates ought to be taken crates.io, and if they are born or submodule'd inside the rust-lang/rust repo be [patched] in crate.io crates until they can be migrated to crates.io. To facilitate those migrations, we should only have one namespace, and so use [patch.crates-io] for std, core, and alloc too.

@SimonSapin
Copy link

but the crucial thing to keep in mind is such creates ought to be taken crates.io, […] we should only have one namespace, and so use [patch.crates-io] for std, core, and alloc too.

I disagree with both this premise and this conclusion. Any given version of the standard library (whatever its split into crates like std v.s. core) is fundamentally tied to the version of the compiler that it was shipped with. It is an entirely separate kind a dependency, similar to how path and git dependencies are different kinds from each other and from registries (crates.io).

@Ericson2314
Copy link

@SimonSapin Yes that's true today, but say std can become a regular crates.io crate with all compiler-specific stuff in core? The set of libraries provided by the compiler is an implementation specific detail. It doesn't really make sense for packages to specify this library must or must not come with the library, unless that package also wants to tie itself to a specific Rust implementation (including specific version of that implementation).

But perhaps we can have our cake an eat it too. instead of, yes, lying, that std/core come from crates.io today, we can introduce union registries, and say the default one should be crates.io ∪ compiler-specific. This is like creating a new blank registry, and then [patch]ing in compiler-specific libs with top 1, and crates.io libs with second highest priority. Then packages will still be flexible about whether their dependencies are provided by crates.io or the compiler, but Cargo is also honest about the provenance of each package with the explicit indirection.

@Ericson2314
Copy link

What's blocking this from becoming a regular RFC PR?

@jamesmunns
Copy link
Owner Author

Thank you everyone involved! Discussion seems stable, so I will be merging this PR, and I will be opening an official RFC for this (after a quick rebase). I'll follow up with a link to the actual RFC PR shortly.

@jamesmunns jamesmunns merged commit e214666 into master Mar 16, 2019
@jamesmunns
Copy link
Owner Author

This is now a full RFC! 🎉 See RFC2663 for further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.