-
Notifications
You must be signed in to change notification settings - Fork 89
build: add infrastructure for rust (v2) #399
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
Conversation
Looking at https://github.com/bus1/dbus-broker/actions/runs/14931302421 it fails to build with the OSS-Fuzz toolchain as expected. It should be possible to switch to the rust builder image and install bindgen-cli there to get it to work for the time being but generally projects written in both C and Rust built with meson aren't natively supported there. I wonder if there are plans to keep fuzzing the C code and start fuzzing the fuzzable Rust code once it's introduced or should it be disabled on OSS-Fuzz until it's all settled? |
to make it more or less work with bus1/dbus-broker#399
The change in bc02e79 should not be needed in 1.8.1, hopefully, so maybe just change the version comparison once the release is out. |
src/meson.build
Outdated
mod_rust.doctest( | ||
'rbus-doctests', | ||
rust_bus, | ||
suite: 'unit', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest using a separate suite here. I'm not sure how your CI works, but some projects do not copy over the individual .o files from the build
stage to the test
stage. That may prevent you from running the doctests in the test
stage, and having a separate suite helps moving them to the build
stage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, rustdoc
does not allow to split the build stage from the test stage, does it? So you have to call rustdoc --test [...]
at the time of running the tests, which then needs access to the crate it documents, as this is usually linked into the doc-tests. I see.
I am kinda leaning towards just making doctests build-time optional, then it is easy to disable when anyone runs into such problems.
But putting it into a separate suite is definitely nice, so it can be easily deselected, given its runtime requirements. Thanks for the hint!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, rustdoc does not allow to split the build stage from the test stage, does it?
Yes that's the issue. I meant build and test stage as CI concepts (I'm more familiar with Gitlab but GitHub should have something similar).
I am kinda leaning towards just making doctests build-time optional, then it is easy to disable when anyone runs into such problems.
Yes that also makes sense and would also be a workaround for any issues with cross toolchain files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, thanks a lot!
I'm going to fuzz dbus-broker downstream (by analogy with dbus-daemon google/oss-fuzz#8699) so I'll go ahead and close google/oss-fuzz#13299. When/if it breaks there it should be possible to disable it by removing the project. |
I think it would be great to add things like clippy and rustfmt eventually. Looking at mesonbuild/meson#13914 it seems Diff in /dbus-broker/src/rlib.rs at line 20:
#[allow(dead_code)]
#[allow(non_camel_case_types)]
#[allow(non_snake_case)]
-pub mod generated {
-}
+pub mod generated {}
#[cfg(test)]
mod test {
Diff in /dbus-broker/src/rlib.rs at line 28:
// Simple dummy to show the test-suite in the test results, even if
// other tests are conditionally disabled.
#[test]
- fn availability() {
- }
+ fn availability() {}
}
|
You can also check QEMU's master branch for how it enables "meson devenv cargo FOO" (where FOO is usually fmt). We use it to run rustfmt in CI. |
Thanks! I just took a quick look but it seems it should probably be possible to borrow something from their CI: https://github.com/qemu/qemu/blob/bc98ffdc7577e55ab8373c579c28fe24d600c40f/.gitlab-ci.d/static_checks.yml#L54. I saw MIRI in the docs as well. |
Extend the build system to compile Rust sources in our tree into an internal, static library. This is then linked into `libbus.a`, which itself is our static library that contains everything but the entrypoints. This does not add any Rust code, but merely extends the build-system to support adding Rust code. Additionally, we add infrastructure to call `rust-bindgen` on our own C headers, so we get clean access to it from Rust. Note that this significantly bumps the build-time requirements of the project: - Meson 1.7.0 (January 2025): Needed for support of Rust Edition 2024 - Rust 1.85 (February 2025): Needed for support of Rust Edition 2024 - rust-bindgen 0.71 (December 2024): Needed for `--rust-edition` and `--rust-target` configuration. Signed-off-by: David Rheinsberg <[email protected]>
Instead of one static library with all Rust code, split it into two: 1) An rlib Rust library with all Rust code. 2) A staticlib Rust library that links the rlib and provides C stubs. This split allows us to link the Rust library to other Rust code, like doctests or unittests. It also keeps the C-API separate from the Rust code and provides a clean path forward. Signed-off-by: David Rheinsberg <[email protected]>
Export the generated symbols from our rlib, to ensure the C API can use them as well. While at it, ensure we suppress common warnings with the C naming scheme we use. Signed-off-by: David Rheinsberg <[email protected]>
Ensure that we do not use assignments in the bindgen options. Meson parses these options to avoid adding duplicates, so lets provide them cleanly. Additionally, add `--use-core` now that we have rlib as `no_std`. Signed-off-by: David Rheinsberg <[email protected]>
Ensure that we generate doctest targets with the new Meson version. This will test-run all Rust code in the documentation. Signed-off-by: David Rheinsberg <[email protected]>
With everything settled, we can now reduce the build requirements and turn some features into optional additions: - Use Rust-2021 over Rust-2024, since we do not benefit much from the new edition. - Reduce meson requirements to 1.3 and guard `mod_rust` additions by a version check. - Reduce Rust MSV to 1.74. - Reduce Bindgen MSV to 0.60 to at least get --allowlist-file. Signed-off-by: David Rheinsberg <[email protected]>
Use plain version numbers in MSVs and require all call-sites to add comparators, if necessary. Signed-off-by: David Rheinsberg <[email protected]>
Ensure that we do not run doctests when cross-compiling. This can easily fail and there is no suitable way for us to check whether `rustdoc` is available: mesonbuild/meson#14583 Signed-off-by: David Rheinsberg <[email protected]>
Move the rust doctests into their own test suite, to allow downstream users to select/deselect it. Signed-off-by: David Rheinsberg <[email protected]>
Guard the doctests by a new meson option called `doctest`. The tests differ from standard unit tests in several ways, and thus can much more easily break setups: - doctests run the Rust compiler at test-time, rather than at build-time - doctests run on the build machine, rather than on the host, but can be compile-tested for the host Signed-off-by: David Rheinsberg <[email protected]>
Thanks for the comments! Lets add static analyzers and other tools in a follow-up. |
I don't intend to enable To be honest, I believe deciding whether a statement should be a single line or multiple should be at the discretion of the author, not purely based on number of characters. |
Thanks a lot! I merged this PR so we have a baseline Rust integration that we can base other work like oss-fuzz on. If your branch works with this PR, I'd be glad to see it merged in oss-fuzz. I know that fuzzing Rust code will take more effort, but we should at least try to keep fuzzing the message-generator as we did before. |
Various IDEs run
It works in the sense that dbus-broker compiles. |
An IDE that reformats an entire file when you touch it? Sounds horrid :-/ I think it would be easier to just put
I have tried that before. For instance, in r-efi (https://crates.io/crates/r-efi) we used to do this, but we just kept adding annotations and it become really ugly. Note that we are not alone on this. The official rust style-guide explicitly leaves decision on what a "short item" is up to an implementer (https://doc.rust-lang.org/nightly/style-guide/). And there are many, many discussion in the rustfmt tracker about this. We explicitly follow the Rust style guide (I think?), I just dislike the decisions rustfmt did when implementing it (which are in no way mandated by the style guide).
I see. Thanks for tracking it! |
I think it should be OK in terms of preventing various tools from automatically complaining about/fixing things. https://github.com/rust-lang/rustfmt/blob/master/Configurations.md#ignore allows skipping whole directories without touching code but it's in nightly only. Just to clarify I'm not disputing the decision. It's just that I think it would be great if it was communicated via skipping/ignoring rustfmt explicitly. |
to make it compile with bus1/dbus-broker#399
I reopened google/oss-fuzz#13299 and dropped the second commit (where this PR was pulled instead of the main branch for testing purposes). It should keep failing until it's merged but it shouldn't be annoying hopefully since the build failures aren't reported by OSS-Fuzz for some reason: google/oss-fuzz#13289 (comment). Once it's merged the https://github.com/bus1/dbus-broker/actions/workflows/fuzz.yml workflow can be turned on again too. |
FWIW @bonzini I took a look at QEMU too and as far as I understand currently it builds because rust is skipped
It would need switching to the rust-builder image too once it's enabled (though looking at https://lore.kernel.org/qemu-devel/[email protected]/ it seems to be delayed). |
Extend the Meson build system to compile an internal Rust library and link that into libbus.a, our internal utility library that is linked into all dbus-broker binaries.
The individual commits have extensive information on, and background for, the individual changes. To summarize, we now use the builtin Rust support of Meson to compile a static library that uses no external dependencies but standard Rust core and alloc, as well as the runtime-integration of std. This avoids the previous dance via meson-cargo, and it makes the build fully independent of Cargo.
This series merely extends the build system, but adds no meaningful Rust code. This will be added in other PRs.
(v2: rebased on the recent CI changes)