Closed
Bug 1370874
Opened 7 years ago
Closed 7 years ago
Require Rust 1.18
Categories
(Firefox Build System :: General, enhancement)
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.
Reporter | ||
Comment 1•7 years ago
|
||
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
Assignee | ||
Comment 2•7 years ago
|
||
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
Reporter | ||
Comment 3•7 years ago
|
||
I had the build times in mind, and struct field reordering to minimise padding.
https://github.com/rust-lang/rust/pull/40377
Comment 4•7 years ago
|
||
Neither of those *require* 1.18. We can update most builds to 1.18, but still stick to require 1.15.
Comment 5•7 years ago
|
||
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.)
Comment 6•7 years ago
|
||
Dropping <1.18 would be optional, of course. Do it if it buys us something. Hold off if cost to support it is minimal.
Comment 7•7 years ago
|
||
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.
Reporter | ||
Comment 8•7 years ago
|
||
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.
Comment 9•7 years ago
|
||
Updated•7 years ago
|
Summary: Require Rust 1.18 once it's out → Require Rust 1.18
Comment 10•7 years ago
|
||
please remember about 3tiers platform for Rust. I need sometimes to correctly make a package for OpenBSD. Next week should be ok.
Comment 11•7 years ago
|
||
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.
Assignee | ||
Comment 12•7 years ago
|
||
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. :)
Reporter | ||
Comment 13•7 years ago
|
||
(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.
Comment 14•7 years ago
|
||
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.
Reporter | ||
Comment 15•7 years ago
|
||
There is no compelling reason to stay on 1.15.1 either though.
Comment 16•7 years ago
|
||
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.
Assignee | ||
Comment 17•7 years ago
|
||
Thanks, :semarie. Those details help me understand where you're coming from. Good luck with the ABI target versioning!
Comment 18•7 years ago
|
||
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'.
Comment 19•7 years ago
|
||
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?
Comment 20•7 years ago
|
||
> 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.
Comment 21•7 years ago
|
||
(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.
Comment 22•7 years ago
|
||
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.
Comment 23•7 years ago
|
||
(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)
Comment 24•7 years ago
|
||
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)
Comment 25•7 years ago
|
||
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
Comment 26•7 years ago
|
||
(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?
Comment 27•7 years ago
|
||
(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.
Comment 28•7 years ago
|
||
(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)
Comment 29•7 years ago
|
||
(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?
Comment 30•7 years ago
|
||
(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.
Comment 31•7 years ago
|
||
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.
Comment 32•7 years ago
|
||
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.
Assignee | ||
Comment 33•7 years ago
|
||
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. :)
Comment 34•7 years ago
|
||
Thanks for writing that up, Ralph!
Assignee | ||
Comment 35•7 years ago
|
||
This is resolved by bug 1383311 which bumped the required version to 1.19.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•