Closed Bug 1370874 Opened 7 years ago Closed 7 years ago

Require Rust 1.18

Categories

(Firefox Build System :: General, enhancement)

1.0 Branch
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: nox, Assigned: rillian)

References

Details

I would like to bump the minimum required Rust version to 1.17, to be able to use the latest syntax elisions that were introduced in that version. Is there any specific reason we need to keep on supporting 1.15.1? In general staying on older versions mean we may not be able to keep on updating our dependencies, whether or not there are new features we need in the new versions.
Just remembered that 1.18 is due next week, and has features that would be more compelling to the bumping-wary people, updating accordingly.
Summary: Require Rust 1.17 → Require Rust 1.18 once it's out
I believe 1.18.0 will be released tomorrow. There's a Firefox merge to beta on Monday though, so I was planning to wait until next week before updating. Can you be specific about what features in 1.18 you see as compelling? For me the 17% improvement in build times is a reason to update, given we're about to enable stylo by default.
Assignee: nobody → giles
I had the build times in mind, and struct field reordering to minimise padding. https://github.com/rust-lang/rust/pull/40377
Neither of those *require* 1.18. We can update most builds to 1.18, but still stick to require 1.15.
Generally speaking, if supporting an older version is causing too much of a burden, I think we should bump the minimum version requirement with few questions asked: we shouldn't be inconveniencing developers with legacy requirements if it can be avoided. Given that Firefox uplift is Monday, at this point we should definitely defer the upgrade to 1.18 and preserve 1.15.1 support until Firefox 56 next week. That will mean 54 and 55 will both work with 1.15.1. We can upgrade to 1.18 and drop <1.18 at the beginning of the cycle, which is when we should be making these invasive changes (so they have as much time to bake as possible). (I think upgrading toolchains at this point in the cycle would violate the new "soft freeze" policy.)
Dropping <1.18 would be optional, of course. Do it if it buys us something. Hold off if cost to support it is minimal.
Struct shorthand is not IMHO a reason enough to want to drop support for 1.15. They are easy to revert. If there's a long pole of changes required due to the large rust ecosystem jumping on the bandwagon of new syntax, then sure, we can bump the requirement to 1.17, but not before having tried without whether there's friction or not. Note I'm saying 1.17, not 1.18. Upgrading the requirement and upgrading the version used to build the shipped version of Firefox don't have to be the same version.
AFAIK, when servo-vcs-sync was discussed during workweeks, it was deemed reasonable for Servo to bump Rust stable whenever a new version is released on the sole basis that a new version is out, because 6 weeks was long enough of a bump cycle.
Summary: Require Rust 1.18 once it's out → Require Rust 1.18
Depends on: 1371366
Depends on: 1371406
please remember about 3tiers platform for Rust. I need sometimes to correctly make a package for OpenBSD. Next week should be ok.
seconded: as soon as you hard-require the very latest rust du jour which was released 6 hours ago, you're giving the middle finger to everyone trying to build m-c on their exotic configurations outside of mozilla builders. Please think of the kittens.
If I may moderate the hyperbole a bit, there are a several different issues with tier-3 platform development, and I think it helps to be clear which one you're concerned about. 1. Your system doesn't have new enough Rust. This is actually the majority position for all developers. The Rust project provides the `rustup` tool to make it easy to work with upstream toolchains build. Generally people are using this to work with rust code, and having it available means you have the same option as developers working on tier-1 platforms. This has been the situation with other rapidly-evolving language systems like ruby and node for a long time. 2. Your system doesn't have a Rust installer. If you can't use rustup because there are no installer builds for your platform, you should fix that. It's an important escape hatch, will give you better testing of the rust toolchain, earlier bug reports on it, and will allow you to contribute code to Firefox and other projects depending on Rust without blocking on packaging. 3. You want to test your system's packaging of Rust. This is great! But there's a fairness issue. It's not reasonable to ask people using a packing channel with lower latency to block until yours has packages available. This is basically what tier-3 means. We've decided (collectively) that the allocation of resources means it's the tier-3 platform which blocks while workarounds are implemented, rather than the tier-1 platforms. New Rust releases also aren't surprises. There's a regular schedule, and you can anticipate when new packages will be needed. 4. Rust doesn't support your platform at all. At this point supporting Firefox means getting Rust running. Firefox 52 esr can still be built without Rust language support but Rust code is now a requirement for Firefox development, for better or worse. I do appreciate the work tier-3 maintainers do, and we're not rejecting your efforts. We're all trying to get work done and make Firefox better. If you have more specific concerns about the cost up upgrading Rust, I'm happy to hear them and help work towards a smoother upgrade path for your platform. Maintaining a separate bootstrap chain for Rust is valuable. We just can't stop development because it takes 10 days for package promotion to the Debian testing distribution, or because travis doesn't have OpenBSD images. Faster development == more kittens, if you will. :)
(In reply to Landry Breuil (:gaston) from comment #11) > seconded: as soon as you hard-require the very latest rust du jour which was > released 6 hours ago, you're giving the middle finger to everyone trying to > build m-c on their exotic configurations outside of mozilla builders. Please > think of the kittens. Note that this patch was originally about requiring 1.17, which was released quite a longer time than 6 hours ago. While I do agree that requiring Rust 1.18 6 hours after it is released is over the top, I strongly disagree that requiring 1.17 now that 1.18 is out is a bad idea. I always think of the kittens, even going as far as getting up every day the past 5 years to make an insulin shot to my diabetic cat.
There is still to this day, no compelling reason to *require* 1.17 or 1.18. Again, we can update the builds to use it, but keep the requirement to what it currently is (1.15.1), until something *needs* to land that *actively* requires 1.17, and can't be trivially fixed to be backwards compatible.
There is no compelling reason to stay on 1.15.1 either though.
As maintainer of Rust for OpenBSD, I will comment a bit. > If you can't use rustup because there are no installer builds for your platform, you should fix that. The rustup tool is great, but doesn't fit well with BSD platform. First, it requires to provide a crosscompiling environment to build OpenBSD binaries from Linux environment. I will not explain deeply the issues with that, but it isn't a trivial task. Secondly, rustc targets "openbsd" and not a specific version (like "openbsd6.1"). Due to BSD model (potential binary incompatibility between versions) it is unsuitable for us. Currently, I am working on this specific problem, to permit rustc to targeting a specific OS version, but it takes time: https://internals.rust-lang.org/t/pre-rfc-target-extension-dealing-with-breaking-changes-at-os-level/5289/20. And please note, that even with a crosscompile env and a way to targets specific OS version, rustup will *not* be able to target the right version of OpenBSD for developpement: we uses -current head where libc library is moving, and I strongly doubt to be able to provide to Rust developers a moving crosscompile environnment. To make clear the current problem with Rust 1.18: I have a regular build of Rust nightly/beta branches. I know building Rust 1.18 isn't a problem. The stable (1.18) was tagged only some days ago (Tue, 06 Jun 2017) and after a quick check, I saw I will have to adapt (again) the packaging due to some build error (the way to build rustc or cargo changes just every release actually). The problem is I am not at home to working on this issue (yes, I am working on Rust only on my hobbies time). Having firefox to requires Rust 1.17 isn't a problem: it is packaged and available. Requiring 1.18 is more problematic: after packaging it, it still requires to be incorporated on OpenBSD port tree (this step requires reviewing). The gap between tagging and releasing (2 days for 1.18) is really short to expect tier-3 to be able to provide an installer.
Thanks, :semarie. Those details help me understand where you're coming from. Good luck with the ABI target versioning!
On another note, requiring a newer rust for the sake of requiring it (without providing fallback codepaths/handling for 'stable/older' rust) will prevent ppl to build newer versions of firefox on 'stable' environments. When firefox is a leaf package in a dependency tree, it's easy to update it, less when it's rust which stuff depends on.. Of course you're going to say 'but those users should just use ESR if they want supported versions in a stable environment', to which i'll reply 'meh'.
I have raised bug 1374669 because I have just got bitten with bug 1374666. The problem Rust has a "fast" release cycle and we need to have a decent upgrade policy. There appears to be a disconnect with the way that Servo is doing things (using latest stable release) and how we are doing things in automation (using 1.15). The original bug was to upgrade to 1.17, what is the reason for not doing this?
> Servo is doing things (using latest stable release) Servo is on a pinned Nigthly. For "build-geckolib" specifically a different compiler is used. It’s currently 1.16, but we should probably align with Gecko’s minimum. > and how we are doing things in automation (using 1.15). With bug 1371406 most Gecko automation is now on 1.18. Only some specific builds use 1.15.1 to check that the minimum version still works.
(In reply to Simon Sapin (:SimonSapin) from comment #20) > > and how we are doing things in automation (using 1.15). > > With bug 1371406 most Gecko automation is now on 1.18. Only some specific > builds use 1.15.1 to check that the minimum version still works. And if it doesnt then you get backed out... so we can't claim 1.18 support when if you use those features you can't land code. This is the disconnect I mean.
Using a more recent compiler can get you things like better compile time, better optimizations, etc. But yes, in terms of language features and standard library features, my understanding is that the current policy only allows using those available in the version specified by rustc_min_version in build/moz.configure/rust.configure, which is currently 1.15.1. This bug is about raising that minimum.
(In reply to Sebastien Marie [:semarie] from comment #10) > please remember about 3tiers platform for Rust. I need sometimes to > correctly make a package for OpenBSD. Next week should be ok. Sebastien, how is the OpenBSD package for 1.18 going?
Flags: needinfo?(semarie)
For OpenBSD, Rust 1.18 is packaged. So it isn't a problem for -current (developpment head). But please keep also in mind the issue mentioned by :landry : requiring higher version for Rust makes firefox distribution more complex on 'stable' environement. As example we provide update for firefox for OpenBSD 6.1 (stable and latest release). The Rust version here is 1.16. With a higher version requirement, it will not be updated anymore. Personally I think the version should be updated for feature requirement and not just for better compile time.
Flags: needinfo?(semarie)
I added uses of std::ptr::{read,write}_unaligned in bindgen, which are needed to deal with some C++ bitfields safely. I didn't realize they were only stable from rust 1.17[1], so now the bindgen update is blocked on this, or on requiring at least 1.17 :(. [1]: https://github.com/rust-lang/rust/commit/d38ea8b371f781aa4b0689c46ede75b5385fedba
(In reply to Emilio Cobos Álvarez [:emilio] from comment #25) > I added uses of std::ptr::{read,write}_unaligned in bindgen, which are > needed to deal with some C++ bitfields safely. I didn't realize they were > only stable from rust 1.17[1], so now the bindgen update is blocked on this, > or on requiring at least 1.17 :(. Is it possible to just manually write out the equivalent of {read,write}_unaligned where bindgen uses those functions?
(In reply to Nathan Froyd [:froydnj] from comment #26) > (In reply to Emilio Cobos Álvarez [:emilio] from comment #25) > > I added uses of std::ptr::{read,write}_unaligned in bindgen, which are > > needed to deal with some C++ bitfields safely. I didn't realize they were > > only stable from rust 1.17[1], so now the bindgen update is blocked on this, > > or on requiring at least 1.17 :(. > > Is it possible to just manually write out the equivalent of > {read,write}_unaligned where bindgen uses those functions? It's a compiler intrinsic, so no, or at least not in a way I know about.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #27) > (In reply to Nathan Froyd [:froydnj] from comment #26) > > (In reply to Emilio Cobos Álvarez [:emilio] from comment #25) > > > I added uses of std::ptr::{read,write}_unaligned in bindgen, which are > > > needed to deal with some C++ bitfields safely. I didn't realize they were > > > only stable from rust 1.17[1], so now the bindgen update is blocked on this, > > > or on requiring at least 1.17 :(. > > > > Is it possible to just manually write out the equivalent of > > {read,write}_unaligned where bindgen uses those functions? > > It's a compiler intrinsic, so no, or at least not in a way I know about. (Well, I guess I could do the arithmetic in each generated bitfield accessor, but that sounds fairly sub-optimal)
(In reply to Emilio Cobos Álvarez [:emilio] from comment #27) > (In reply to Nathan Froyd [:froydnj] from comment #26) > > (In reply to Emilio Cobos Álvarez [:emilio] from comment #25) > > > I added uses of std::ptr::{read,write}_unaligned in bindgen, which are > > > needed to deal with some C++ bitfields safely. I didn't realize they were > > > only stable from rust 1.17[1], so now the bindgen update is blocked on this, > > > or on requiring at least 1.17 :(. > > > > Is it possible to just manually write out the equivalent of > > {read,write}_unaligned where bindgen uses those functions? > > It's a compiler intrinsic, so no, or at least not in a way I know about. Everything in here: https://github.com/rust-lang/rust/blob/d38ea8b371f781aa4b0689c46ede75b5385fedba/src/libcore/ptr.rs#L171-L179 looks like pretty bog-standard Rust. copy_nonoverlapping is a compiler intrinsic under the hood, but it looks like it's perfectly usable from normal Rust code...am I missing something?
(In reply to Nathan Froyd [:froydnj] from comment #29) > (In reply to Emilio Cobos Álvarez [:emilio] from comment #27) > > (In reply to Nathan Froyd [:froydnj] from comment #26) > > > (In reply to Emilio Cobos Álvarez [:emilio] from comment #25) > > > > I added uses of std::ptr::{read,write}_unaligned in bindgen, which are > > > > needed to deal with some C++ bitfields safely. I didn't realize they were > > > > only stable from rust 1.17[1], so now the bindgen update is blocked on this, > > > > or on requiring at least 1.17 :(. > > > > > > Is it possible to just manually write out the equivalent of > > > {read,write}_unaligned where bindgen uses those functions? > > > > It's a compiler intrinsic, so no, or at least not in a way I know about. > > Everything in here: > > https://github.com/rust-lang/rust/blob/ > d38ea8b371f781aa4b0689c46ede75b5385fedba/src/libcore/ptr.rs#L171-L179 > > looks like pretty bog-standard Rust. copy_nonoverlapping is a compiler > intrinsic under the hood, but it looks like it's perfectly usable from > normal Rust code...am I missing something? Huh, you're right... I'll update bindgen to do that manually I guess, though I don't really love it, it's probably just easier.
Not using the nice-to-have struct literal syntactic sugar is one thing, but if we’re at the point where we’re considering duplicating code from the standard library I think that’s more than enough of a reason to raise the minimum. At least to 1.17, released 8 weeks ago.
Depends on: 1374807
No longer blocks: 1365254
I tend to agree with :SimonSapin : duplicating code isn't a good option. If having the really latest Rust version isn't right (from my point of vue), keeping a too older one isn't better. But collateral damages should be considered when switching (even if there aren't avoided). Some policy about Rust version updating would be welcome: it would make more clear what to expect.
We talked about this in several meetings recently. I've proposed we skip updating to 1.18.0 and go straight to 1.19.0 at the start of Firefox 57 development on August 3. We would bump the required version two weeks after each stable Rust release comes out from that point on. I think this is the right balance between the needs of all the contributors I've heard from. I've written up the detailed arguments at https://public.etherpad-mozilla.org/p/rust-update-policy Please read those before arguing for a different policy. :)
Thanks for writing that up, Ralph!
Depends on: 1382743
This is resolved by bug 1383311 which bumped the required version to 1.19.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.