Why are both rust-version and rust-toolchain.toml used?

[zacknewman]

What is the point of specifying the minimum rustc version via rust-version in Cargo.toml if you are going to force a specific toolchain via rust-toolchain.toml? Having both means having to keep track of two separate things that frequently go out of sync; and worse, if one updates rust-version such that it's newer than what is in rust-toolchain.toml, one will needlessly install the specified toolchain just for the compiler to fail with a message that the toolchain is not new enough. Seems to me that it's better to remove rust-version from Cargo.toml or allow newer toolchains to be used and remove rust-toolchain.toml instead. I realize that if you have to go back to using nightly, then rust-toolchain.toml is your only option.

[BlackDex]

We want to pin our builds to a specific version, because that is what we have tested. Setting to stable can suddenly break the pipeline or other issues.

The rust-version is the MSRV version, and only determines the lowest version of rust which we support to build Vaultwarden with.

You can override the rust version via +1.62.0 for example, that is where the rust-version comes in and blocks it, or allows it if it is that version or newer. Or, if you didn't used rustup stable could be an old version.

[zacknewman]

I suppose your use of rust-toolchain.toml is unusual as it seems that you use it to mean "the maximum version of the toolchain tested" which doesn't currently have a field in Cargo.toml. My experience with that file in other packages—and why packages rarely have it unless they want to indicate beta/nightly is required—is to indicate the exact version of the toolchain that was used to build and test the crate(s) in the package.

In other words, most crate maintainers based on what you described would simply use rust-version with no attempt to indicate what the maximum version of the toolchain was used and tested especially since you don't test all versions in between but only the minimum and "maximum" (at the time) versions. The logic there is that only building and testing using rust-version is sufficient since any issues that come from using a newer toolchain is a bug in rustc/cargo/std per the SemVer requirements and consequently should be addressed at this lower level.

[BlackDex]

And there is where you make a mistake, Vaultwarden isn't a crate it's an application. And we have had several cases in the past where using stable and not a fixed tested version caused issues.

Also, the toolchain file will override the default set for that user.
And if that user uses stable or nightly as default we want to make sure the same version is used at all steps and locations.

[zacknewman]

Au contraire, you are mistaken; applications are crates: "A crate can come in one of two forms: a binary crate or a library crate" (emphasis added). It is not a coincidence that crates/applications far more popular than Vaultwarden make no use of rust-toolchain.toml (e.g., ripgrep). Again, when you state rust-version you are stating that all versions greater than or equal to it are supported which means it is perfectly fine to use stable; and if issues arise, there is a more foundational issue at the rustc/std level which should be addressed there. When packages specify rust-version, that is not an indication that all versions greater than or equal the specified version have been tested since that is the whole point of SemVer.

If you don't want to support stable, then you should not use rust-version. By having rust-toolchain.toml, you force people to have to opt-out of installing another separate toolchain since they likely only have stable and maybe nightly. You are misusing/abusing the intent of rust-toolchain.toml to mean something that it is not intended to mean (i.e., "maximum version").

"And we have had several cases in the past where using stable and not a fixed tested version caused issues." This makes it seem like stable is not supported which in turn means you shouldn't be using rust-version.

[BlackDex]

We can discuss semantics here until we weigh zero grams.

We have our reasons to ensure stability and the same version across all contributor's, you may disagree. That's fine. I have never said we do not support stable, that's your assumption.

If you have a constructive solution, then please advise. But do know we want to have a fixed version for our builds.

[zacknewman]

I do have a constructive solution: either remove rust-version or rust-toolchain.toml. The current use of both rust-version and rust-toolchain.toml are not correct. You are misusing rust-version to mean only the minimum version supported without any claim about newer versions. You are misusing rust-toolchain.toml to represent the maximum version supported without any claim about older versions. You then are using the conjunction of them to (incorrectly since you don't test all versions in between) to represent the range of versions that are supported.

If all versions greater than or equal to the version specified by rust-version are supported (including stable), then only use that since any issues related to the use of stable or other toolchains newer than rust-version are allowed. If that is too "dangerous" and you don't trust SemVer adherence (because for example you've run into issues in the past with the use of stable), then you must not use rust-version but instead only use rust-toolchain.toml.

That is to say, there is nothing wrong with having a fixed version. That is definitely the safest path to take, but then one simply uses rust-toolchain.toml only and any use of a different toolchain is at the risk of the user and assisting said person using this different toolchain is at your discretion.

[stefan0xC]

I don't see what the issue is with using both. I mean the initial assumption that they would get out-of-sync somehow (and also frequently) is baseless. Why would you assume that we would ever update rust-version to a higher version than rust-toolchain.toml?

rust-version tells you what the minimum supported rust version is (i.e. you should not compile vaultwarden with a lower version of rust). rust-toolchain.toml tells rustup which toolchain is the preferred version (i.e. using specifically 1.72.0).

Correct me if I'm wrong, but if you don't use rustup the rust-toolchain.toml file will be ignored (in which case the rust-version in Cargo.toml would be a way to tell users that they are using an outdated version of rust).

As @BlackDex already explained (#3907 (comment)), if you don't want to use the suggested toolchain, you can override which version to use (i.e. you can run cargo +stable or cargo +nightly or force a specific toolchain) and also you could ignore the MSRV by calling cargo --ignore-rust-version).

So to me both files are merely suggestions and I also would not say that we guarantee anything with regards to supported versions (this is still an open source project maintained by unpaid volunteers in their free time). It's just that we only test those when building:

vaultwarden/.github/workflows/build.yml

Lines 38 to 41 in bbd630f

	matrix:
	channel:
	- "rust-toolchain" # The version defined in rust-toolchain
	- "msrv" # The supported MSRV

The backwards compatibility claim (that you can compile with newer versions of rust) is one that rust makes (i.e. if you use a newer version than the MSRV and it has a fatal flaw in it that prevents building the project than you should report it to the rust project).

We only test the MSRV to make sure that we don't use the latest features that a new version provides (and we will only accept PRs that rely on it when we have determined that the feature is stable enough, i.e. two versions later when the version is the new MSRV). This would not be possible without rust-version.

[zacknewman]

It's a fact of life that things can go out-of-sync, so relying on that never happening is an additional assumption not much different than assuming a developer writing C won't do a double-free. The "frequently" part of the title is baseless, but only because I was under the incorrect belief that rust-version was supposed to be the same as rust-toolchain.toml for this project.

If one wants to support all versions greater than or equal to a minimum version, then it make sense to only have rust-version. If one only wants to support a specific version, then one only uses rust-toolchain.toml. The use of both is rare, and I don't believe that it is a coincidence. Almost all crates (binary or library) either use one or the other; and when rust-toolchain.toml is used, it's almost always to specify beta/nightly. The way it is used for this crate is unusual.

"So to me both files are merely suggestions and I also would not say that we guarantee anything with regards to supported versions (this is still an open source project maintained by unpaid volunteers in their free time). It's just that we only test those when building: "

And what I am claiming is that using rust-version and rust-toolchain.toml as a way to indicate exactly two toolchains were tested is not how rust-version or rust-toolchain.toml are designed to be used. Obviously this is an open source project, and you are free to do what you want.

Edit
I changed the title to accurately reflect the dialogue. As stated, the reason I titled it the way I did was that I was under the incorrect belief that for this project rust-version and rust-toolchain.toml were supposed to be the same; and that was due to almost never encountering a crate that employed both.

[stefan0xC]

I am assuming that the test would fail and therefore a PR would not be merged. (It could still happen of course but in the same sense that you could also do a double free in Rust when you are using unsafe.) But thanks for changing the title.

As explained by testing the rust-version we prevent ourselves from immediately using the latest features that come with a new version. Because not everyone can always use the latest rust version. You are right that this test will not guarantee that it will build with all newer versions or the in-between version but that is also not something that is really in our control. (So far we had no complaints about that but I could have missed something.)
And with the rust-toolchain we pin the compiler to a known working version. Looking through all the crates cached on my disk, I'd argue that it is unusual to ship a rust-toolchain{,.toml} file in the first place, which is why you might not have encountered many examples that utilizes both. So I don't see how our use of both contradicts the intended use cases of either.

[zacknewman]

Not the same thing. One has to opt into unsafe Rust, and convention dictates that one implies safe Rust when not qualifying it with an adjective. Additionally it is better to not have to "assume" anything if you can. I would have assumed that 1.29.2 was tested on ARMv6 before being released, but that clearly wasn't the case. Point being the more assumptions you make the higher the probability you are wrong (i.e., you violated Occam's razor/law of parsimony).

You don't prevent yourselves from using newer features since you can still pass --ignore-rust-version. If you want to retort, "well we don't do that"; then I'd retort "don't specify a specific toolchain when building since rust-toolchain.toml will be used". In other words, you will prevent yourself from using newer features by making the version in rust-toolchain.toml the version in rust-version.

Technically, you can "control" testing every version of a toolchain with CI tests; so that is not true. Obviously you are not obligated to or even encouraged to, but claiming that is not in your control is technically untrue.

That's fine, then only specify rust-toolchain.toml. This project does not have unique problems; yet it is one of the few that employ rust-toolchain.toml let alone both rust-version and rust-toolchain.toml. That is not a coincidence. The reason people use rust-version is to indicate that versions older may not work but versions as new or newer should work and are supported. Specifying rust-toolchain.toml in addition to that is somewhat at odds since one would normally want to use as many toolchains as possible to catch any would-be regressions upstream which can then be reported which helps the ecosystem as a whole. If that is something you are not interested in, then only specifying rust-toolchain.toml is fine.

[zacknewman]

This has gone long enough though, so we will agree to disagree. Thanks for your time.

[stefan0xC]

Okay. But just in case you want to try again to change our minds (i.e. to drop either the rust-version or the rust-toolchain.toml) I just want to say that you'd need to first understand why we do what we do instead of just asserting that you are right and we are wrong. 😦

[zacknewman]

Show me where I am "asserting" that I am right and you are wrong? I am logically justifying my claims and at no point resorted to axiomatic justifications. Again, your use of rust-version and rust-toolchain.toml is at odds with their use in the ecosystem as a whole. You are using those two to specify that two versions have been tested or really only one version is "really tested" (i.e., rust-toolchain.toml) while the other version is "tested" in the sense cargo check or something doesn't report an issue—I say that since if rust-version were tested as thoroughly as rust-toolchain.toml, then I don't see the point of making them separate versions.

Most projects that specify rust-version will at least test their crate(s) using that exact toolchain. They will additionally probably test newer toolchains, but they make no effort in trying to report all the versions that were actually tested as you seemingly are. If/when issues arise with toolchains newer than what is specified in rust-version, then upstream bugs are raised to the benefit of the ecosystem. They don't supplement rust-version with rust-toolchain.toml because that encourages one to use a specific toolchain which is mildly contradictory with the claim that all versions greater than or equal to rust-version are supported. If you want to only test one version and want to prevent issues from being raised due to upstream regressions, then rust-toolchain.toml should only be used. Again, that will prevent you from using newer features than what the toolchain allows since you would have to opt-into using a different toolchain which is no different than opting-into --ignore-rust-version.

Addendum
As I thought, rust-version doesn't actually prevent you from using newer features. All it does is forces a rustc version check and enforces that version to be as new or newer than what is specified before compiling the code (assuming --ignore-rust-version is not passed). It is on you to actually make sure newer features are not used which is only possible if you force yourself to use that specific rustc version. If that is the case, then again it doesn't make sense that rust-toolchain.toml is a different version since rust-toolchain.toml is the only tool you have to hopefully force a specific toolchain (e.g., to enforce that newer features are not used).

[zacknewman]

And finally despite the incorrect and rude accusation that I am merely asserting to be right, here is the coup de grâce: 1.29.2 does not compile using rustc version 1.69.0 despite Cargo.toml having that version for rust-version again supporting my claim that this project should drop the use of rust-version and only use rust-toolchain.toml. I'll raise a bug report shortly. This suggests that you all are just subtracting two from the minor version of rust-toolchain.toml without actually testing it.

[samlaf]

Wow this thread was friday night popcorn.

[BlackDex]

Watching old movies ha 😉