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

Pin rust version #5680

Conversation

faern
Copy link
Member

@faern faern commented Jan 11, 2024

Adds pinning of Rust version to this repository. Still work in progress!

Benefits

  • Developers do not need to manually run rustup update. Cargo will take care of getting the correct version of Rust for every build command
  • Build servers do not need separate logic to update Rust. They will automatically use the version specified in rust-toolchain.toml so bumping that is enough.
  • Building of the containers are not time-sensitive in the same sence. Which Rust version is picked is deterministic compared to just picking whatever happens to be stable at the time.

Cons

  • Since the Rust version is a specific version and not stable, over time developers will sit with many toolchains installed on their systems that they manually need to clean out to free disk space.
  • This is not how rust-toolchain.toml is supposed to be used really, so there might be dragons we have not found yet.
  • Rust targets are per release channel. So if you install the target for x86_64-pc-windows-msvc for Rust 1.77.0 then you will not automatically get it when installing 1.78.0. This is a major pain point with this approach.

This change is Reviewable

Copy link

linear bot commented Jan 11, 2024

@faern faern force-pushed the pin-rust-version-and-allow-automatic-install-of-right-des-554 branch 3 times, most recently from d227e48 to 5630f07 Compare January 11, 2024 15:39
@faern faern changed the title Pin rust version and allow automatic install of right des 554 Pin rust version Jan 16, 2024
@faern faern force-pushed the pin-rust-version-and-allow-automatic-install-of-right-des-554 branch from 5630f07 to 0e56332 Compare April 19, 2024 11:27
@faern
Copy link
Member Author

faern commented Jun 18, 2024

We decided not to do this. The rust-toolchain.toml file was not meant for this usage, and it has more cons than pros.

@faern faern closed this Jun 18, 2024
@ruuda
Copy link

ruuda commented Jun 22, 2024

I was going to open an issue to request to add a rust-toolchain.toml, then I found this.

I’ve been unable to compile the AUR package for more than half a year due to some Rust error that I never had the time to investigate, today I decided to dive deeper. It turns out that this was because the package builds with RUSTUP_TOOLCHAIN=stable, but I had not run rustup update stable for a long time, which meant that it tried to build with a Rust toolchain that was too old to compile android/translations-converter in particular (at least, that is where the error surfaced first).

The biggest problem with stable is that it is a moving target. This makes stable unsuitable for releases: it doesn’t tell you which Rust toolchain the application needs to be compiled with. If I want to build a year-old release and it no longer compiles because there was a breaking change in Cargo or Rust (which does happen), then one has to manually correlate the release date with Rust’s release dates to find the compatible version. And vice versa, what I ran into is that my local view of what stable points to was outdated. (And even then, rustup update stable does not necessarily fix that; the Mullvad release may be a few weeks old, my Rust version could be too recent.)

Since the Rust version is a specific version and not stable, over time developers will sit with many toolchains installed on their systems that they manually need to clean out to free disk space.

This is true, but that is the reality of Rust development nowadays. It is already standard practice to pin specific versions (which although it has downsides, is in my experience way better than not pinning), so if you develop or use any Rust projects, those toolchains are going to be there anyway. In the Git repositories in my homedir, I currently have 15 rustup-toolchain.toml files and 21 rust-toolchain files that together specify 15 different Rust versions (14 pinned to specific versions and one to stable), and that doesn’t even include the versions needed to build older releases of projects.

The converse is also true though, if you use stable, that forces a new toolchain onto people every six weeks, even when the application doesn’t need any of the new Rust features. (And also, unfortunately rustup is not smart enough to make stable a symlink to a specific version, so if stable is currently 1.79.0 and you also have a project that pins to 1.79, you’ll have two copies of it.) Every toolchain update also forces rebuilding all dependencies.

This is not how rust-toolchain.toml is supposed to be used really, so there might be dragons we have not found yet.

How so? This is exactly what it is meant for.

@faern
Copy link
Member Author

faern commented Jun 23, 2024

Do you have an example of a version of this app that does not build with a Rust version newer than whatever was the latest Rust release at the time of us releasing that app version? I'm pretty sure that you can always just use latest stable to build basically all Mullvad app versions coming out prior to that Rust release. In other words, you should always be able to solve your build issues by rustup update.

but I had not run rustup update stable for a long time, which meant that it tried to build with a Rust toolchain that was too old to compile

Sounds like your problems were simply solved by a rustup update? I don't think it's our responsibility to have you upgrade your compiler.

(And even then, rustup update stable does not necessarily fix that; the Mullvad release may be a few weeks old, my Rust version could be too recent.)

Did this actually happen to you, or is this more of a hypothetical scenario? As stated above, this should work fine. Rust is backwards compatible.

because there was a breaking change in Cargo or Rust (which does happen)

Do you have links to things like this? Except some more extreme cases I don't think this happens a lot. Just targeting latest stable is not a controversial way to handle a Rust project.

It is already standard practice to pin specific versions

Do you have links to some prominent projects doing this? I have almost never seen rust-toolchain.toml in the wild, except a few projects depending on nightly features.

For Linux, we do have pinned containers with a correct Rust version if you want to use that: https://github.com/mullvad/mullvadvpn-app/blob/main/building/linux-container-image.txt. For any given commit, the container image pointed to by that file should build the app just fine.

rust-toolchain.toml is not designed to remove the need for developers to upgrade their toolchains. From the documentation on rust-toolchain.toml:

Some projects find themselves ‘pinned’ to a specific release of Rust and want this information reflected in their source repository. This is most often the case for nightly-only software that pins to a revision from the release archives.

We are not pinned to a specific version. We just use the latest stable, which is not at all an uncommon thing to do.

@ruuda
Copy link

ruuda commented Jun 23, 2024

Do you have an example of a version of this app that does not build with a Rust version newer than whatever was the latest Rust release at the time of us releasing that app version?

I didn’t run into that problem with mullvadvpn-app specifically before I commented here, but I build lots of Rust software from source and it’s quite common. It didn’t take me long to find a case in mullvadvpn-app, v2020.4 cannot be compiled with current Rust stable:

$ git checkout 2020.4
HEAD is now at fbf6d44d5 Updating version in package files

# The tag is dated Tue May 12 09:56:23 2020 +0200.
# According to https://releases.rs/docs/1.44.0/, 1.44 was released June 4th,
# so the latest stable at the time was 1.43.1 from May 7th.
# Rustup complains though that the 'cargo' component is not applicable to this release,
# so we go with 1.43.0.
$ cargo +1.43.0 check
    (some output omitted)
    Finished dev [unoptimized + debuginfo] target(s) in 24.96s

# Now let's try with 1.79, which is stable at the time of writing.
$ cargo +1.43.0 clean
$ cargo +1.79 check
(some output omitted)
    Checking tokio-io v0.1.12
    Checking rand v0.5.6
error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> /home/ruud/.cargo/registry/src/index.crates.io-6f17d22bba15001f/socket2-0.3.11/src/sockaddr.rs:156:9
    |
156 |         mem::transmute::<SocketAddrV4, sockaddr_in>(v4);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: source type: `SocketAddrV4` (48 bits)
    = note: target type: `sockaddr_in` (128 bits)

   Compiling ring v0.16.12
    Checking tokio-service v0.1.0
    Checking rand v0.7.2
For more information about this error, try `rustc --explain E0512`.
error: could not compile `socket2` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...

This version is 4 years old by now, but the Rust version on which it breaks first is 1.64, which was released August 2022, only 2 years after tag was created.

(Semi-related, many older versions cannot be built any more because they use Git dependencies and the pinned commits can no longer be obtained.)

I don't think it's our responsibility to have you upgrade your compiler.

Of course, but it’s nice that Rustup automates the process if there is a rust-toolchain.toml. Now that I am looking into it, there is a MSRV in Cargo.toml, so that is helpful to manually find a compatible version, and on recent Rust versions it will also refuse to build when the toolchain is too old, so at least it will remind you update the toolchain, and the problem that I originally ran into, with my stable pointing to a version that is too old, should not happen again. Thanks!

Do you have links to some prominent projects doing this?

  • denoland/deno (the most starred Rust repository on GitHub after Rust itself)
  • FuelLabs/fuel-core
  • zed-industries/zed
  • dani-garcia/vaultwarden
  • helix-editor/helix
  • nushell/nushell (This one also specifies the rationale in a comment, they track stable minus two releases to give packagers time to package the new toolchain.)
  • solana-labs/solana (In this project in particular, before they had the rust-toolchain.toml in place, there was a release that compiled fine with multiple rustc versions, but for some rustc versions the binary would segfault, because a dependency relied on how rustc laid out a struct in memory, and it changed between releases. That involved unsafe code and maybe that library was at fault, but still, these things happen in practice.)
  • aptos-labs/aptos-core

All of these pin to stable versions, not nightly. These are cherry-picked of course, there are also prominent projects that don’t pin to a specific version, especially simple applications with fewer dependencies. (E.g. sharkdp/fd, though it does specify an MSRV.) There are also a few popular projects that don’t pin in rust-toolchain.toml, but that do specify a specific MSRV in Cargo.toml and test for that on CI (e.g. Alacritty, Ripgrep).

Now that I’m looking into this, mulvadvpn-app also started doing this for recent releases, so that is very helpful! Future toolchains breaking old code tends to happen only over longer time scales that are rarely a problem when packaging, it’s more of an issue when building historical versions, so this is already a big help.

If the toolchain that I tried to use to build 2024.3 had been recent enough to support the rust-version key, it would have failed with a much clearer error, so going forward this should be less of a problem.

We are not pinned to a specific version. We just use the latest stable, which is not at all an uncommon thing to do.

Projects that pin generally also track a recent stable, they just bump the pinned version regularly. This ensures that they can update at a time when it’s convenient for them, rather than having the update forced onto them by Rust’s release cycle.

Edit November 2024: The recent timepocalypse (rust-lang/rust#127343) is another example of a breaking change that had very widespread impact and broke many applications that compiled fine 4 months earlier.

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.

2 participants