Skip to content

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

Merged
merged 10 commits into from
Jun 17, 2025
Merged

build: add infrastructure for rust (v2) #399

merged 10 commits into from
Jun 17, 2025

Conversation

dvdhrm
Copy link
Member

@dvdhrm dvdhrm commented May 9, 2025

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)

@evverx
Copy link
Contributor

evverx commented May 10, 2025

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?

evverx added a commit to evverx/oss-fuzz that referenced this pull request May 10, 2025
@bonzini
Copy link

bonzini commented May 12, 2025

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',
Copy link

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.

Copy link
Member Author

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!

Copy link

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.

Copy link
Member Author

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!

@evverx
Copy link
Contributor

evverx commented Jun 5, 2025

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.

@evverx
Copy link
Contributor

evverx commented Jun 11, 2025

I think it would be great to add things like clippy and rustfmt eventually. Looking at mesonbuild/meson#13914 it seems ninja clippy is implemented so it can probably be added to the actions right away. rustfmt is in the works as far as I can see (mesonbuild/meson#13932). FWIW it complains

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() {}
 }
 

@bonzini
Copy link

bonzini commented Jun 11, 2025

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.

@evverx
Copy link
Contributor

evverx commented Jun 11, 2025

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.

dvdhrm added 10 commits June 16, 2025 15:11
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]>
@dvdhrm dvdhrm merged commit d1454f9 into bus1:main Jun 17, 2025
29 checks passed
@dvdhrm
Copy link
Member Author

dvdhrm commented Jun 17, 2025

Thanks for the comments! Lets add static analyzers and other tools in a follow-up.

@dvdhrm
Copy link
Member Author

dvdhrm commented Jun 17, 2025

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() {}
 }
 

I don't intend to enable rustfmt. I generally like the idea, but it does not allow any control over line-splits, but instead enforces strict character-limits.

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.

@dvdhrm
Copy link
Member Author

dvdhrm commented Jun 17, 2025

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.

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.

@evverx
Copy link
Contributor

evverx commented Jun 17, 2025

I don't intend to enable rustfmt

Various IDEs run rustfmt automatically when they see Rust code so I think it would be better to add .rustfmt.toml with ignore (or something like that) anyway to avoid changes where files get reformatted in unrelated places accidentally. It's also possible to skip code granularly (which I think would be a better option in terms of utilizing rustfmt in places where it makes sense). There are also options that can be set to control brace_style for example but they are in nightly only.

If your branch works with this PR, I'd be glad to see it merged in oss-fuzz

It works in the sense that dbus-broker compiles. RUSTFLAGS aren't set up properly and things like that but it should be fine for now I guess. It should be possible to remove the second commit from google/oss-fuzz#13299 to fix the build failures. I'll do it next week. @mrc0mmand OSS-Fuzz should start sending notification soon so if it's annoying it would be great if you could pick it up sooner.

@dvdhrm
Copy link
Member Author

dvdhrm commented Jun 18, 2025

I don't intend to enable rustfmt

Various IDEs run rustfmt automatically when they see Rust code so I think it would be better to add .rustfmt.toml with ignore (or something like that) anyway to avoid changes where files get reformatted in unrelated places accidentally.

An IDE that reformats an entire file when you touch it? Sounds horrid :-/

I think it would be easier to just put #![rustfmt::skip] into rlib.rs. Does that sounds ok?

It's also possible to skip code granularly (which I think would be a better option in terms of utilizing rustfmt in places where it makes sense). There are also options that can be set to control brace_style for example but they are in nightly only.

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

If your branch works with this PR, I'd be glad to see it merged in oss-fuzz

It works in the sense that dbus-broker compiles. RUSTFLAGS aren't set up properly and things like that but it should be fine for now I guess.

I see. Thanks for tracking it!

@evverx
Copy link
Contributor

evverx commented Jun 18, 2025

I think it would be easier to just put #![rustfmt::skip] into rlib.rs. Does that sounds ok?

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.

evverx added a commit to evverx/oss-fuzz that referenced this pull request Jun 20, 2025
@evverx
Copy link
Contributor

evverx commented Jun 20, 2025

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.

@evverx
Copy link
Contributor

evverx commented Jun 20, 2025

FWIW @bonzini I took a look at QEMU too and as far as I understand currently it builds because rust is skipped
https://oss-fuzz-build-logs.storage.googleapis.com/log-318a2add-5ba0-4ee3-9c28-82438ee30cba.txt

Compiler for language �[1mrust�[0m skipped: feature �[1mrust�[0m disabled

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

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

Successfully merging this pull request may close these issues.

3 participants