Skip to content

Explicitly define our Rust MSRV #1365

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.

Sign up for GitHub

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

Open
denisonbarbosa wants to merge 3 commits into
base: main
Choose a base branch
from
+9 −7
Open

Explicitly define our Rust MSRV #1365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3c5472
Add package.rust-version to NSS manifest
denisonbarbosa Mar 25, 2026
05e505f
Bump Rust MSRV to 1.84
denisonbarbosa Mar 31, 2026
a50dad3
Use resolver '3' for the workspace
denisonbarbosa Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[workspace]
members = ["nss/"]
exclude = ["vendor_rust/", "authd-oidc-brokers/third_party/libhimmelblau/", "parts/libhimmelblau/build"]
resolver = "2"
resolver = "3"
Copy link
Copy Markdown
Contributor

@adombeck adombeck Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:
resolver setting 3 is not valid, valid options are "1" or "2"

Copy link
Copy Markdown
Member Author

@denisonbarbosa denisonbarbosa Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Contributor

@adombeck adombeck Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related, Max Gilmour pointed out that rustc-1.91 was backported to Noble and should not be affected by that bug.

Copy link
Copy Markdown
Member Author

@denisonbarbosa denisonbarbosa Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

@adombeck adombeck Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

@denisonbarbosa denisonbarbosa Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor

@adombeck adombeck Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


[profile.release]
lto = "thin"
Expand Down
2 changes: 1 addition & 1 deletion debian/control
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Build-Depends: debhelper-compat (= 13),
dh-exec,
dh-golang,
dctrl-tools,
cargo (>= 1.82) | cargo-1.82,
cargo (>= 1.84) | cargo-1.84,
# FIXME: We need cargo-vendor-filterer starting from plucky, but noble isn't ready yet
# so workaround it, making it kind of optional, and requiring it only on versions after
# noble (controlled via base-files version that matches the one in noble).
Expand Down
1 change: 1 addition & 0 deletions nss/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
name = "nss"
version = "0.1.0"
edition = "2021"
rust-version = "1.84.0"

[lib]
crate-type = ["cdylib"]
Expand Down
11 changes: 6 additions & 5 deletions nss/src/logs/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ fn init_sys_logger(log_level: LevelFilter) {
pid: std::process::id(),
};

let logger = if let Ok(l) = syslog::unix(formatter) {
l
} else {
eprintln!("failed to create syslog logger");
return;
let logger = match syslog::unix(formatter) {
Ok(l) => l,
_ => {
eprintln!("failed to create syslog logger");
return;
}
};

if let Err(err) = log::set_boxed_logger(Box::new(BasicLogger::new(logger))) {
Expand Down
Loading