-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove most version detection and conditionals for older versions of Rust #2845
Conversation
r? @Amanieu (rust-highfive has picked a reviewer for you, use r? to override) |
ec4d2a9
to
3154cac
Compare
cc @JohnTitor |
@@ -1,5 +1,7 @@ | |||
//! SGX C types definition | |||
|
|||
use c_void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be pub use
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah nevermind, I see there's one in the crate root.
I'm very disappointed hearing this. This is a significant jump in MSRV which not all parties can deploy quickly. As an example, Debian stable is now at 1.48 which is below 1.56. |
I don't think supporting 1.48 costs us too much, however the current MSRV for libc is 1.13 which puts a huge burden on maintenance since many useful language features are missing (e.g. traits impls for arrays of more than 32 elements). |
I'm going to second @Kixunil here. As is, this will force a lot of ecosystem crates that make MSRV guarantees to bump their minimum versions. This crate should probably move a bit cautiously here because of how foundational it is to the ecosystem. |
It looks like 1.40 could be supported while still getting most of the benefit from this PR. |
@Amanieu nothing wrong with bumping super-ancient 1.13. 1.40 would be perfect for me, but 1.48 is fine too. |
The problem here is that we don't have clear guidelines on when we can stop supporting a version. If we decide to set the MSRV to 1.48 (for example), at what point can we stop supporting this version? |
@Amanieu I think it's probably time for libc to establish an MSRV policy from what I'm seeing here. |
@Amanieu my preference would be "what popular Linux distros (insert list) support in repositories" I specifically care about Debian stable but there are other popular distros too. |
Stepping back to consider other flag day conditions that |
So, I'm actually not so sure on this, as relying on debian stable will cause a huge maintenance burden over time as debian lags further and further behind due to the infrequent release cycle. I think some sort of time-based policy (e.g, last year's worth of releases) may be the best approach here as it keeps some stability but also doesn't lock in MSRV behind what debian is doing. |
@Noah-Kennedy Debian tries to target 2 year release cycle. They would rather put off the release if there are unresolved problems but I don't think that's too bad. A few months shouldn't be a huge problem, I guess. If it gets unreasonably prolonged I wouldn't mind MSRV bump. So perhaps "whatever popular distros support unless they unreasonably prolong releases; minimum 2 years" :) |
Hmm, two years sounds like a good timeline actually for MSRV breaks in libc. |
Honestly, libc should be treated as something that doesn't do anything even mildly breaking outside of fixed release dates every few years. It is one of the core foundational elements of the rust ecosystem, so a change in MSRV here requires everyone to bump MSRV, which is a pretty big deal. This needs to be done with caution, not everyone can update to the newest compiler for entirely valid reasons, and those who have this issue shouldn't be forced to rely on an old version of the rust ecosystem built on an older version of libc. |
And Debian stable's packaged version of the libc crate is 0.2.80, which works on Debian stable's packaged version of rustc. Debian's rustc is for use by Debian packages. Compatibility with the rustc shipped by Debian stable is not a goal crates on crates.io should be striving to meet.
Not at the moment, but the implied MSRV policy of "support whatever's in Debian stable" absolutely would long-term.
I sincerely hope so. MSRV is a prisoner's dilemma: crates keep MSRV low because some other crate asks them to keep MSRV low because some other crate asks them to keep MSRV low, most of which can't actually point to a project that actually needs that MSRV for some reason. As a result, there are far more crates affected than there are users who actually have any desire to use such a Rust version. We discussed possible MSRV policies in the library team meeting. The primary debate was between "support stable and stable-minus-one to allow time for upgrades" and "support a few extra versions back because we're libc", but the debate was between N-1 and (say) N-3 or N-4, not "1-2 years". (To be clear, nobody proposed that we upgrade gratuitously, only that we upgrade when there's a rustc feature we need.) We ended that meeting without setting a concrete MSRV policy, largely because we figured we'd start by making this change and go from there. But it looks like we may need to set a concrete MSRV policy after all.
Yes, they absolutely should. If you run old rustc, run a matching set of old crates, such as those packaged alongside your rustc. Whoever packaged your rustc and your crates will then have validated that your rustc builds your crates. I am entirely in support of distributions that package self-contained versions of rustc and crates and use those to make self-contained packages of Rust software. I run Debian myself, and I appreciate that they package software written in Rust and many other languages. That doesn't mean that crates in the ecosystem need to support running the latest version of a crate with a years-old version of rustc from Debian stable. |
The concrete MSRV policy I would propose:
|
@joshtriplett I think the "Only upgrade if there's a feature we actually need" statement still leaves some ambiguity around what "need" really means, but perhaps that's intentional. I.e. if the lack of a language feature can be worked around, how expensive can that workaround be before the feature is "needed"? Orthogonally, I'd request that a release be cut immediately prior to this PR being merged, and the same for any future MSRV bumps. IMHO a notice period before the MSRV is bumped would also be helpful - it'd give anyone who wants changes made to version of libc supporting a soon-to-be-dropped version of Rust a window to get them submitted. |
The way @joshtriplett describes the downstream use of MSRV largely echoes with me. Anyone has the ability to vendor their dependencies. What MSRV policies across the ecosystem gives is a sliding window of released crates which makes vendoring a specific rustc release possible (in say the debian repository). But the minimal requirement is that the ecosystem can at some point be frozen on a reasonably stable set of package versions which allows everything being vendored to build. The wider the MSRV of all packages the better since it increases the probability that this global set of versions can be found (else package maintainers have to go through the hassle of figuring out how to have more than one version of the same package in parallel). Another potentially large concern in my mind is having a bug patching policy which is friendly to downstreams, such as the one outlined in Tokio. It serves to guide maintainers which minor version of a package to prefer to pick in the hopes that critical issues in the future will be patched and tested against a compatible version of the vendored package. A similar policy for foundational crates such as libc might be worth considering. So far this is all a bit speculative and I'd love to hear more concrete user stories around this. Especially ones where people have been hurt by the lack of a stricter MSRV for some package with no reasonable remediation at hand. |
The proposal of last three stable versions would force Tokio to break its MSRV policy, which is that the MSRV must always be at least six months old. (i.e. our policy is N-4) The Tokio project controls most of the dependencies of Tokio, so until now, this is a policy that we've been able to uphold without issues. I see now that it was a mistake not to consult the libc maintainers before picking this policy. The reason this didn't happen is that libc is such a fundamental crate that I never imagined that libc would pick something weaker. I would really appreciate it if the policy was N-4 rather than N-2. As for debian stable, Tokio doesn't track debian stable, but there are every so often people who complain that we don't. People definitely try to use the cargo from the debian stable repos to compile their projects.
I do have a user story about this, though it was a case where the MSRV being too new was merely annoying and wasted my time, rather than a situation with no reasonable remediation. The story is that I have a custom program I run in a cronscript on a VPS to manage my RSS feeds. Initially I wanted to compile it on my local machine and scp the binary to the server, but the binary didn't work on the server due to a version mismatch in the dynamic libraries (I think it was glibc in this case, but I've run into the same issue with an sql library in the past). I then tried to compile it on the server, but it didn't compile because Tokio's MSRV was too new. Thus, I was forced to install rustup to compile the application. This is annoying because I wanted Rust to be available to all users on the VPS. Looking at the VPS, it appears that I've somehow managed to install it globally, although it appears that I did it wrong, because upgrading rustc failed when I just tried it. |
@joshtriplett perhaps it'll help to describe my use case. I write security-critical software that has to run on a well-secured system which should also be reliable. As such I chose Debian stable as the base system. In principle I have only a single reason against picking whichever version of Rust: it's hard to ensure the installed version is not backdoored unless it came from distribution repository. The alternative for me would be to make effort to make Rustup verify Rust signatures when installing (last time I checked it didn't do that) and then get legit Rustup. Or write my own alternative installer. Packaged crates don't fit my use case well mainly because there's no way to tell So far sticking with MSRV was the solution for me but since it's getting harder over time, I may attempt to do something different. |
(The MSRV policy for libc is currently being discussed in rust-lang/libs-team#72) |
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at <rust-lang#2845>. I just updated to remove all new uses of `libc_union` introduced since then. Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at <rust-lang#2845>. I just updated to remove all new uses of `libc_union` introduced since then. Co-authored-by: Josh Triplett <josh@joshtriplett.org>
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org>
#4040 raised the MSRV to 1.63. I was able to cherry pick most of these commits to remove the config, with the exception of those that we need to somehow patch |
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d)
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test.
This is mostly taken from Josh's work at [1], I just updated to account for conflicts and new uses of `libc_align`. [1]: rust-lang#2845 Co-authored-by: Josh Triplett <josh@joshtriplett.org> (apply <rust-lang#4057> to `main`) (cherry picked from commit b5b553d) As part of this update, drop the `align` feature from Cargo.toml and libc-test. Changes are reduced in this cherry pick compared to the original commit because some of this was already applied to `main`.
Per library team decision, drop most of the conditionals, simplifying the code
substantially. Update the README accordingly.
I left the conditional for 1.62, since that just shipped, and the conditional for 1.51 pending ratification of a libc MSRV policy.
Deferred while discussing libc MSRV policy: