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

Cargo.toml should document MSRV #3253

Open
riptl opened this issue Jan 27, 2025 · 7 comments
Open

Cargo.toml should document MSRV #3253

riptl opened this issue Jan 27, 2025 · 7 comments

Comments

@riptl
Copy link
Contributor

riptl commented Jan 27, 2025

error: could not compile `mdbx-sys` (lib) due to 121 previous errors
warning: build failed, waiting for other jobs to finish...
error: failed to compile `nimiq-client v1.0.6 (/home/nimiq/core-rs/client)`, intermediate artifacts can be found at `/home/nimiq/core-rs/target`.

Possibly a MSRV issue

I tried to compile with cargo-1.79.0-2.el9.x86_64

build.log

@Eligioo
Copy link
Member

Eligioo commented Jan 27, 2025

The MSRV is of the nimiq-client is 1.82.0.

1.82.0 introduces unsafe extern blocks (RFC3484) which are used by libmdbx 0.5.3.

@Eligioo Eligioo closed this as completed Jan 27, 2025
@riptl
Copy link
Contributor Author

riptl commented Jan 27, 2025

Thank you, @Eligioo. I suggest adding rust-version to Cargo.toml. This would turn such confusing build error spam into a more human-readable error.

@riptl
Copy link
Contributor Author

riptl commented Jan 27, 2025

On second thought, it would make more sense to add it to mdbx-sys instead.
Looks like the rust-version attribute was recently removed 🤔 vorot93/libmdbx-rs@5132006

@riptl
Copy link
Contributor Author

riptl commented Jan 27, 2025

@Eligioo Ok, hmm, I think the flaw lies in bindgen 0.71.0.

In Rust 2021, using C FFI extern blocks without unsafe does not fail the build.
In Rust 2024, unsafe extern blocks become mandatory.

The Rust maintainers seem to encourage early adoption in Rust 2021 via code warnings.

Writing an extern block without the unsafe keyword is provided for compatibility only, and will eventually generate a warning.

Therefore, bindgen introduced a change that tries to guess whether the compiler supports. If it fails detection, then it seems to assumes unsafe extern is supported (rust-lang/rust-bindgen#3052 (comment)).
When used in build.rs it causes non-deterministic code generation.

IMHO the root cause is bindgen introducing breaking changes due to broken compiler feature detection in a minor release.

I don't think the MSRV needs to be quite as high. If we can lower it (by fixing bindgen's unsafe extern detection), this allows people to build Nimiq with the Rust toolchain from the operating system's package manager.

@hrxi
Copy link
Contributor

hrxi commented Jan 27, 2025

rust-version should probably be added for better error messages.

@hrxi hrxi reopened this Jan 27, 2025
@hrxi hrxi changed the title Nimiq v1.0.6 build failure (mdbx-sys / bindgen) Cargo.toml should document MSRV Jan 27, 2025
@hrxi
Copy link
Contributor

hrxi commented Jan 27, 2025

If we can lower it (by fixing bindgen's unsafe extern detection), this allows people to build Nimiq with the Rust toolchain from the operating system's package manager.

Which Linux distro do you use?

@riptl
Copy link
Contributor Author

riptl commented Jan 27, 2025

Which Linux distro do you use?

@hrxi Rocky Linux 9.5 (which is like RHEL 9)

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

No branches or pull requests

3 participants