-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Allow setting RUSTC_BOOTSTRAP=1 on specific crates in the dependency graph from the top level of the build #6627
Comments
That’s a feature. I’m against anything that lets you set RUSTC_BOOTSTRAP from within Cargo itself. Your own mach build tool should be setting it on the command-line to Cargo. I’m fine with a comma-separated list of crate names in RUSTC_BOOTSTRAP but my final opinion is that this is an anti-use case and that Firefox in the first place should just be using nightly if it wants nightly features. |
For the record, it has never happened in Servo that we would just accidently start using a nightly feature, and AFAIK it never happened in Gecko either before you started using RUSTC_BOOTSTRAP on build scripts. |
What problem are you seeking to solve by having mach communicate a list of crate names via an environment variable compared to the feature being fully contained in cargo and controllable from the top-level
I guess that means I should not try to change your mind, but I'm still interested in understanding your rationale.
This suggests that you are not against using nightly features in production. If you are not against using nightly features in production, why is it important not to use them with a non-nightly compiler? My clone of Gecko, which is a bit out of date, has 385 crates, though some of them are not code that ships in the product. Currently, 2 of the crates need nightly features. A few months back, the number was slightly larger, but still on the order of "up to 5". Why is it important to deny the QA benefits of the Rust train model and the distro packaging clarity to all these crates if a couple of them use nightly features? What's the point of keeping stable Rust so pure as not to use it? If Firefox were to use nightly Rust, what do you expect would happen to Linux and BSD distros packaging Firefox and Rust? How would what you'd expect to happen be an improvement compared to what this feature request contemplates? |
Just so I understand, you're confused as to why we would want to prohibit a stable compiler from using unstable features? You are not bootstrapping rustc. Don't use RUSTC_BOOTSTRAP. |
@sfackler Firefox doesn't want to use a "stable compiler", Firefox wants to use a "pinned nightly compiler" that happens to be pinned to exactly the same commit as stable release. The problem is that |
Shouldn't production Firefox use production Rust? |
That's why I suggested naming along the lines of (It seems relevant that |
@petrochenkov If you want to use stable as a pinned nightly, set RUSTC_BOOTSTRAP globally. What is requested here is an extension actively catering to that usecase. I maintain my point made in the other issues that this is wilfully trying to tear down guarantees that we have worked hard to set up. Also, I'd like to see examples outside of Firefox where users have complained that a nightly feature broke while they had opted into using nightly. From my experience in the embedded area, people appreciated the clarity and were willing to opt into the breakage. |
I am actively opposed to make it easier to use RUSTC_BOOTSTRAP. |
@glandium recalled otherwise in his earlier comment on your PR. (I don't know specifics either way.)
I suggest approaching this from the point of view of people trying address use cases under constraints rather than people "willfully trying to tear down". (If you've designed a system and people are "holding it wrong" on purpose, it's a good idea to consider how the system isn't meeting their needs as designed.) I reiterate that Firefox used to have nightly Rust features enabled globally and then moved to more granular containment of nightly features. My understanding of the motivation of the change was the opposite of "trying to tear down" Rust's guarantees. I have a hard time understanding how it would be better that what this issues requests, but if the Rust team would be happier with Firefox enabling (For clarity: I'm not a Firefox build system developer. My interest is being able to conform to social norms of crates.io while having a couple of crates build in the context of Firefox with nightly features with minimal need to maintain forks of the crates until
That can be read is implying that in the case of Firefox someone complained about a nightly feature breaking. Did you mean that? If so, what was the complaint?
I note that this feature request is formulated to show clarity of breakage being opt-in. The question is one of granularity of the opt-in.
This statement doesn't explain why and doesn't indicate how you suggest the relevant use cases be addressed. |
This feature request is to make crates pretend they build on stable while they in reality use nightly features. That by definition tears down Rust’s guarantees. |
I think the problem is that cargo builds both end-user apps, like Firefox, and crates. So while it's valid in the context of Firefox's build it's dangerous for the rest of the ecosystem. |
No, the opposite. This is about enabling configuration of whether nightly features are enabled on a per-crate basis but not from the crates themselves—only from the top-level Cargo.toml. |
How is that different from what I said? That just means the top-level crate is the one doing the pretending. Did you mean a workspace-only Cargo.toml file? |
RUSTC_BOOTSTRAP enables people to completely subvert the language and standard library's stability guarantee. Unstable features are unstable, and by definition can't be used on the stable release channel. If you want to use unstable features, you should use an unstable toolchain. It is important to make it significantly painful and awkward to bypass feature gating because it needs to be clear that this is not a thing that people should do. If you gain a lot of value from an unstable feature, you should push on getting it stabilized. |
👆 I'd love to understand why this doesn't (presumably) work for the Firefox devs. |
I see. So if the Firefox can't live without nightly features, has it considered the opposite approach? Requesting/lobbying for a way to use the nightly compiler in "stable compiler mode", i.e so it nightly features aren't enabled? That would resolve the "more granular containment of nightly features" problem. |
@dwijnand it wouldn't solve the issue for Linux distributions who package stable Rust and use it to build packages (like Firefox). |
Right, they'd need a nightly Rust in order to build Firefox (or Firefox would have to restrict itself to stable Rust. |
What you said looked like crates anywhere in the dependency graph could be pretending: i.e. the current
Fair enough for pedantic purposes.
I didn't, but only because I'm not profoundly familiar with all aspects of Cargo. Firefox's top-level
I don't understand your point, considering that crates are not built independently but as part of an application (including a top-level Rust In contrast, these solutions don't enforce unstable feature opt-in as top-level configuration but spread it to the crates in the dependency graph:
In all these cases, in a large project, individual crates in the dependency graph may depend on unstable features without surfacing application-level configuration that would allow clear tracking of how much commitment to potential breakage the app as a whole has.
From the context, I gather "they" refers to Linux distributions. How would it be better for the Rust ecosystem to give Linux distributions a reason to package a Firefox-designated snapshot of nightly Rust?
I'd understand the last sentence as coherent policy if the overall position was "don't use nightly features in production". But as seen in the second-to-last sentence, that doesn't seem to be the position of a number of people commenting here. I don't understand how using a nightly toolchain in production, especially for an app that is packaged for Linux and BSD distributions would be better for the ecosystem than containment of unstable feature use within the context of a Rust team-designated release of Rust. For clarity: The purpose of this feature request is to minimize and contain the usage of unstable features in a scenario where it doesn't make sense to abstain from the use of unstable features completely and when the choice of toolchain isn't a private matter but has implications on downstream packaging. Especially in the context of a position that accepts using unstable features in production by using nightly Rust in production, I think the focus is too much on the notion "subvert". If your position is that using unstable features in production is OK, it seems oddly dogmatic to take a mechanism for narrower unstable feature opt-in as subversion.
I have pushed on getting portable SIMD stabilized. What I haven't done is post- I brought up the issue up in June 2016 and in multiple analogous meetings subsequently. I made my point of view known in the Nov/Dec 2016 internals thread. After the chosen direction was not to aim for better-than-C++ portability right away but to aim for C parity (vendor intrinsics) first, I talked with Rust leadership f2f and by videoconferencing to evangelize how Rust's already-working In the case of Firefox has briefly depended on allocator-related unstable features. Those weren't my area, but in those cases, AFAICT, the issue wasn't insufficient push on stabilization but relatively small differences in deployment timeline. |
"Shouldn't do" and "can't do" are not the same thing. If Firefox wants to use unstable features, distribution packagers should make their choice about how they want to approach that:
|
As a procedural matter, I think at minimum, the two teams (libs + lang) that are most affected by exposing this in cargo should have to approve what is proposed here. The cargo team presumably also needs to be involved but more as a matter of "how do we expose this" rather than "do we expose this"?. |
To me, option 1 seems obviously the worst in terms of Rust ecosystem effects (distro-provided Rust that compiles unstable features without opt in and that doesn't necessarily match any Rust release version). I find it bizarre that multiple people have put it forward as an acceptable or even preferable option. Option 3 is a bad position both practically and politically in terms of advocating Rust relative to C++: That users who obtain Firefox from a distro should get it in a damaged condition, especially relative to the replaced C++ code. In the case of character encoding conversions, the Rust code is slower than the C++ code it replaced if SIMD isn't in use and faster than the C++ code it replaced if SIMD is in use. I don't know if in the allocator customization cases it would have been feasible to treat the nightly features as optional. Option 2 is the least bad of the three. This feature request is about minimizing and containing option 2 to make it even less bad. It looks to me that some comments here consider the acknowledgement of option 2 as worse than failure to minimize and contain it. (When Aaron Turon put option 2 on a slide in his RustFest Rome keynote, I took it as a permission to acknowledge option 2.) |
I am aware of what this feature request is doing. I do not want to minimize or contain option 2. |
A lot of this (IMO) ridiculous back-and-forth could've been avoided had we not done the "environment variable set to magical value (which later got simplified to My original proposal, were it to be streamlined for distros, would become basically this:
|
Do I understand correctly that in your proposal the environment variable would be replaced by a choice of facade executable and both |
Yes, ensuring that distros are providing the exact same version of Rust, in two different modes. |
@eddyb (I'm not actually sure what are the benefits of the "two compiler" scheme that would counterbalance the noticeably increased complexity.) |
The point was that if we had two compilers like @eddyb described, `RUSTC_BOOTSTRAP` wouldn’t exist and no crate would have been published explicitly setting it from its build script.
|
@petrochenkov There is a difference, in that distro packagers would have to set (Just in case the answer has scary implications, can build scripts even set |
Is there a Cargo issue for exposing that functionality via workspace-top-level |
@hsivonen Maybe rust-lang/rust#59169 would help you? |
I don't understand how that answers my question from my comment previous to yours. As far as I can tell, the only mention of "cargo" in that issue is a link back to this issue. |
@hsivonen It was not an answer to your question. What you could do is perhaps use |
|
I want to add from a community perspective that we are fighting an uphill battle to get credibility for stability and reliability in the packager communities and incidents like this are setting us back hard. |
encoding_rs is fixed in Debian unstable: https://tracker.debian.org/news/1041837/accepted-rust-encoding-rs-0815-2-source-into-unstable/ The issue has been caused by rust-lang/rust#57416 - which broke rust-simd (which some dependencies of ripgrep depends on) @skade (hello ;) ) as one of the initial packagers of rust in Debian and maintainer of ripgrep, and others rust-based software, I would like to say that the issue is mostly on Debian side. We made the call to take a more recent version of rust for Firefox ESR. This is what caused the issue. |
rust-simd has always required unstable Rust features. It’s not just new warnings (that you can allow), it’s entire functionality that we’ve widely signaled are not ready and will break. rust-lang/rust#57416 should not have been a surprise to anyone. It would have been best for Debian not to package rust-simd in the first place IMO. |
Sure, not blaming anyone here :) |
Isn't the Debian problem more related to the fact that cargo builds dependencies that aren't used for a given feature? |
@sylvestre Quoting from my reddit post:
Is this analysis correct? I'm not sure if the last point is feasible, though. |
At the end of the day, the crate is lying about it’s build requirements. Yes, there are other parts to this puzzle, but the root cause is a crate that “compiles on stable” but does not actually compile on stable, undermining the concept of stability entirely.
… On Jul 2, 2019, at 4:24 PM, Andreas Rottmann ***@***.***> wrote:
@sylvestre Quoting from my reddit post:
Indeed, it seems there's something amiss in the Debian build process, since actually building encoding-rs with the simd-accel feature enabled would have failed, thus exposing the bogus dependency. So there's some things I see that would need addressing:
On the Rust side, there's no way to mark a crate as nightly-only (or, more generally, the minimum Rust version required, which may be "nightly")
On the Debian side, excluding features that rely on too-new rustc versions, which would obviously always exclude dependencies that rely on nightly.
On the Debian side, building libraries with all features (or combinations thereof, I guess).
Is this analysis correct? I'm not sure if the last point is feasible, though.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Since this would be a fundamental change to Rust's stability guarantees, the right way to propose this is as an RFC (or a pre-RFC on internals if you'd like to be less formal). There is no chance we can implement this in Cargo without an RFC. |
The feature being requested
Please add a cargo capability to designate from the top level of the build which specific crates in the dependency graph are compiled with
RUSTC_BOOTSTRAP=1
even when using a non-nightly compilerUse cases
A piece of software that depends on many Rust crates (such as Firefox) tracks the stable release channel for the Rust compiler. Yet, in some case, it makes sense to use an unstable rust feature nonetheless.
Note that making the first use case go away by stabilizing packed portable SIMD (which would be great!) does not make this feature request moot, since it seems reasonable to expect that the second kind of situation will keep arising from time to time even if there are periods in between when nightly features aren't needed.
Alternative solutions and what's wrong with them
Just use nightly Rust
The general tendency of Rust seems to be that if you want to use a nightly feature, you should be using nightly Rust. This poses problems:
is not just a quick matter of
rustup default nightly` for software that gets packaged downstream.Additionally, the "just use nighly Rust" solution has the problems of the next solution as well.
Set
RUSTC_BOOTSTRAP=1
for the whole build systemThis is what Firefox used to do.
The main problem with this solution is that it's very easy to casually introduce more dependencies on nightly Rust features than was intended. Specifically, if nightly features are enabled for specific purpose that's well-known to people who take care of toolchain issues in the project, there is some chance of tracking the status of each nightly feature of interest in Rust development. If, however, developers casually introduce more dependencies on nightly features, soon no one is tracking which nightly features the project as a whole depends on ending there may be unpleasant surprises if nightly feature breaks and the person who casually started using it hasn't been tracking the status of the feature and hasn't been prepared to deal with the nightly feature changing.
In general, to the extent it is some sort of Rust position that production users of Rust should not be relying on nightly features, it seems incongruent with that position to suggest solutions that easily lead production users to depend on arbitrary nightly features when their use case is about depending on a very small number of them.
Notably, unintentionally adding a dependency on a nightly feature is not necessarily a matter of a developer "unintentionally" typing
#![feature(foo)]
in the code they are writing:#![feature(foo)]
may be buried in a dependency, and with nightly features being behind cargo features in dependencies, it may be even less obvious what nightly features are enabled.Use
println!("cargo:rustc-env=RUSTC_BOOTSTRAP=1");
frombuild.rs
This is what Firefox does now.
This hack avoids the problems of the above two solutions by scoping
RUSTC_BOOTSTRAP=1
to specific crates. It's own problems fundamentally arise from the hack being syntactically part of an individual crate while the the need for the hack is a configuration concern of the top-level application.In a non-
cargo vendor
scenario, putting application-level configuration in a specific crate basically requires maintaining a git fork the crate with an extra change set just for the configuration and the ceremony of telling cargo to replace the crates.io instance of a crate with the forked git instance. Even in acargo vendor
situation is not a matter of maintaining a patch that always gets applied after re-runningcargo vendor
: cargo requires more ceremony to opt out of hash checking for the vendor crates.Even if the effort required to maintain application-level configuration as a change to an individual crate is relatively mild, the path of least maintenance burden still is to make the change in the crates.io crate itself. This poses the risk of other applications unknowingly becoming reliant on configuration meant for a different application. This is probably harmless for crates that are blatantly obviously nightly-only, such as previously the
simd
crate or presently thepacked_simd
crate. It can be problematic, however, on crates that conditionally depend on such crates. For example,encoding_rs
has a cargo featuresimd-accel
for enabling the dependency on, depending onencoding_rs
version,simd
orpacked_simd
and gaining the associated performance benefits. While it should be obvious that this is a cargo feature as opposed to the default state of things precisely because there is some tradeoff (in this case, trading compatibility with future Rust toolchains for performance today), the full implications of the trade-off might not be obvious and someone might enablesimd-accel
with a stable-channel toolchain and be unhappy later when it toolchain update breaks the build.Preferred solution
My preference for solving this would be adding the ability to use the top-level
Cargo.toml
to indicate which crates in the dependency graph are to be compiled with nightly features enabled. While naming is a total bikeshed, I suggest putting something very obvious in the syntax along the lines ofenable_features_that_future_rust_releases_may_break
instead of relying on short but less obvious terms likenightly
orbootstrap
.There is other per-crate configuration tha the top-level
Cargo.toml
should be able to control, such as the optimization level. In principle, it would be good for the solution for this feature request to be consistent with other such configuration, but I also wish that this feature request doesn't get deferred indefinitely due to consistency concerns.Alternative solution
It has been suggested that instead of introducing syntax into the the top-level
Cargo.toml
theRUSTC_BOOTSTRAP
environment variable should be extended to take a list of crate names as an alternative to taking the value1
. While this would address the use case, I consider it better design to recognize the use case and to provide top-levelCargo.toml
syntax (that makes the downsides clear) along with other per-crate configuration than to makeRUSTC_BOOTSTRAP
more magic with a special-case design that sort of tries to avoid recognizing the use case by deliberately keeping the mechanism obscure.Potential downsides
A potential downside is that people use
enable_features_that_future_rust_releases_may_break
and are still unhappy when breakage happens. However, the standard approach of telling people to "just use nightly Rust" already has this problem.The best solution to avoiding unhappiness from changes to features that are explicitly subject to change is to work actively on "subject to change" features so that they actually don't appear de facto unchanging and so that they spend a relatively short time in the "subject to change" state. Fixing that kind of general prioritization issue is outside the scope of this feature request, though.
The text was updated successfully, but these errors were encountered: