rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.4k stars 691 forks source link

ci: Add Rust for Linux #2851

Closed ojeda closed 1 week ago

ojeda commented 3 months ago

Rust for Linux, so far, has pinned the Rust compiler and bindgen versions. The kernel is looking into expanding that support to several versions, i.e. establishing a minimum supported version, so that the kernel can start to be more easily built. In particular, it should be possible to build the kernel using the tools provided directly by Linux distributions. In order to help achieve that goal, the Rust project has added the kernel to its Rust pre-merge CI.

This commit does the same for bindgen. In particular, it adds a quick, build-only test of the Rust code in the kernel as an extra step in the test workflow.

This is intended to be an end-to-end test that runs what kernel developers/users would do. In particular, it is useful to catch certain issues that go beyond the C header comparisons. For instance, it would have been able to catch an issue like the --version option unexpectedly requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1: a panic handling certain C headers with string literals containing an interior NUL [2]. While the kernel is not really a stable test, and such an issue would still require that a proper test is added, it is nevertheless a good test case of non-trivial C headers that may trigger edge cases like that.

Of course, bindgen may need to disable the test for different reasons, i.e. there is no expectation to block any urgent/important PR, and the kernel can also call bindgen differently depending on the version, i.e. we are happy to adjust on our side too. Even if it gets disabled often, we would still be in a better situation than not having the test at all.

The Linux version (hash or tag) should ideally be updated from time to time (e.g. every kernel -rc1), and each update should only contain that change.

Link: https://github.com/rust-lang/rust-bindgen/pull/2678 [1] Link: https://github.com/rust-lang/rust-bindgen/pull/2567 [2]

ojeda commented 3 months ago

In e.g. https://github.com/rust-lang/rust-bindgen/actions/runs/9568431855/job/26378640703?pr=2851 you can see at the bottom:

  RUSTC L rust/core.o
  BINDGEN rust/bindings/bindings_generated.rs
  BINDGEN rust/bindings/bindings_helpers_generated.rs
  BINDGEN rust/uapi/uapi_generated.rs
  EXPORTS rust/exports_core_generated.h
  RUSTC P rust/libmacros.so
  RUSTC L rust/compiler_builtins.o
  RUSTC L rust/alloc.o
  RUSTC L rust/bindings.o
  RUSTC L rust/build_error.o
  RUSTC L rust/uapi.o
  EXPORTS rust/exports_alloc_generated.h
  EXPORTS rust/exports_bindings_generated.h
  RUSTC L rust/kernel.o
  EXPORTS rust/exports_kernel_generated.h
  RUSTDOC TK rust/kernel/lib.rs
  RUSTC [M] samples/rust/rust_minimal.o
  RUSTC     samples/rust/rust_print.o
  RUSTC     drivers/net/phy/ax88796b_rust.o
ojeda commented 3 months ago

v2: instead of the unstable --out-dir, put both the common profiles in $PATH and use --target-dir to isolate a bit.

(Using --target-dir implies we rebuild more than needed, since some crates may be already built by the previous tests)

pvdrz commented 1 month ago

@ojeda would be reasonable to run this job on a cron, let's say, once every week?

ojeda commented 1 month ago

Up to you of course -- did something change?

If the extra couple minutes would be too much even in a new job, then a periodic post-merge CI is better than nothing. The disadvantage would be that one would find the issues after the fact, which is more painful overall (e.g. may require figuring out which commit actually broke it and whether a revert is needed and whether that breaks something else, the PR author may not be around to help, etc.).