Conversation
Supporting multiple Ubuntu versions means (possibly) having to support multiple rustc versions. Explicitly defining a MSRV helps us control how far we can bump our dependencies.
e750257 to
70cb768
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1365 +/- ##
==========================================
- Coverage 86.35% 86.24% -0.11%
==========================================
Files 99 99
Lines 6690 6690
Branches 111 111
==========================================
- Hits 5777 5770 -7
- Misses 857 860 +3
- Partials 56 60 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nss/Cargo.toml
Outdated
| name = "nss" | ||
| version = "0.1.0" | ||
| edition = "2021" | ||
| rust-version = "1.82.0" |
There was a problem hiding this comment.
From the docs:
"3" (edition = "2024" default, requires Rust 1.84+): Change the default for resolver.incompatible-rust-versions from allow to fallback
Running cargo check locally on the branch fails with this error
authd test-pr-1365-local ✗ cargo +1.82.0 check
error: failed to parse manifest at `/home/nooreldeenmansour/Documents/Projects/authd/Cargo.toml`
Caused by:
feature `edition2024` is required
The package requires the Cargo feature called `edition2024`, but that feature is not stabilized in this version of Cargo (1.82.0 (8f40fc59f 2024-08-21)).
Consider trying a newer version of Cargo (this may require the nightly release).
See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#edition-2024 for more information about the status of this feature.
Should rust-version be bumped or that would be breaking?
There was a problem hiding this comment.
Nicely spotted! We do have Rust 1.84 in Noble, so I think we can bump our MSRV now. I'll update the PR to also bump the version to 1.84.
There was a problem hiding this comment.
Should be fixed now!
There was a problem hiding this comment.
It seems to me that in Noble, the available Rust versions (deb) is up to 1.82. According to this link https://documentation.ubuntu.com/ubuntu-for-developers/reference/availability/rust/#ubuntu-rust-deb-packages. Unsure if there is an available backport for 1.8.4
Is this CI failure related? https://github.com/canonical/authd/actions/runs/23800681979/job/69359973291?pr=1365#step:5:6194
There was a problem hiding this comment.
Hmm... This weird. Looking at https://launchpad.net/ubuntu/noble/+package/rustc-1.84, you can see that the 1.84 version exists in the archive.
There was a problem hiding this comment.
You are correct. The Ubuntu documentation is seemingly out-of-sync. or I likely misunderstood it. Anyway, I wonder what is the cause of this CI failure. It seems to be related to the dh-cargo/debian cargo wrapper?
I can see different cargo versions being installed in the CI job:
Setting up cargo-1.84 (1.84.1+dfsg0ubuntu1~bpo2-0ubuntu2.24.04) ...
Setting up debhelper (13.14.1ubuntu5) ...
Setting up dh-exec (0.29build1) ...
Setting up cargo (1.75.0+dfsg0ubuntu1-0ubuntu7.1) ...
Setting up dh-cargo (31ubuntu1) ...
In order to rely on some important resolver updates of Cargo (especially when it comes to respecting the MSRV when bumping updates), we need Rust 2024 edition, which requires Rust >= 1.84. Since Noble has cargo-1.84 available, we can safely bump the Rust version without risking FTBFS due to versioning issues.
Some important changes to how MSRV is handled were introduced in Rust 2024. We need to switch to resolver version 3 in order to have access to those changes. More information can be found in: https://rust-lang.github.io/rfcs/3537-msrv-resolver.html Switching resolvers sometimes require some code changes, which are integrated in this commit as a result of running "cargo fix --edition"
70cb768 to
a50dad3
Compare
| members = ["nss/"] | ||
| exclude = ["vendor_rust/", "authd-oidc-brokers/third_party/libhimmelblau/", "parts/libhimmelblau/build"] | ||
| resolver = "2" | ||
| resolver = "3" |
There was a problem hiding this comment.
This is causing the deb build to fail on Noble:
error: failed to parse manifest at
/home/runner/work/authd/authd/Cargo.toml
Caused by:
resolversetting3is not valid, valid options are "1" or "2"
There was a problem hiding this comment.
Yeah... This is due to dh-cargo hardcoding the cargo bin path in it's script (whilst also downloading the default cargo package from the archive).
There was a problem hiding this comment.
Related, Max Gilmour pointed out that rustc-1.91 was backported to Noble and should not be affected by that bug.
There was a problem hiding this comment.
Although the bugfix that you've mentioned is good news, it doesn't help us, sadly. The new cargo versions use a different binary name (i.e. cargo-1.91) and the cargo wrapper is hardcoded to use /usr/bin/cargo, as you can see here, for example:
if subcmd == "clean":
logrun(["env", "RUST_BACKTRACE=1", "/usr/bin/cargo"] + list(newargs), check=True)
if os.path.exists(cargo_home):
shutil.rmtree(cargo_home)
return 0
There was a problem hiding this comment.
As mentioned elsewhere, the cargo wrapper shipped with cargo-1.91 also hardcodes /usr/bin/cargo, so we still can't use a newer cargo version on Noble. Until that's fixed, you could try using the regenerate-cargo-lock-file-on-noble script from my devscripts to generate a Cargo.lock that's compatible with the cargo version we use on Noble.
There was a problem hiding this comment.
I think it wouldn't make much sense in the context of this PR, especially since (before resolver = 3) cargo doesn't really respect the rust_version setting anyway when adding/bumping dependencies... Up until resolver = 2 (if I understood correctly), it will only throw an error when compiling the crate. I'd say we keep this open (maybe move it back to draft?) until we can get this hardcoding issue fixed and then come back to this. Wdyt?
There was a problem hiding this comment.
especially since (before resolver = 3) cargo doesn't really respect the rust_version setting anyway when adding/bumping dependencies
ah, I was not aware. then it indeed doesn't make sense to merge this with resolver = 2.
Supporting multiple Ubuntu versions means (possibly) supporting multiple rustc versions. Explicitly defining a MSRV helps us control how far we can bump our dependencies.
UDENG-9430