Generate bindings once for all platforms and architectures #176

ids1024 · 2023-10-24T21:42:53Z

The same drm headers are used for all architectures, and for the BSDs. There are only a couple typedefs that differ.

This uses linux-raw-sys to define __kernel_size_t on Linux, and libc::size_t on the BSDs.

I guess CI needs to be updated for this. Not sure if there would be a good way to automatically test that the bindings are correct for multiple architectures.

MarijnS95 · 2023-10-24T21:47:00Z

drm-ffi/drm-sys/src/lib.rs

+#[cfg(any(target_os = "android", target_os = "linux"))]
+extern crate linux_raw_sys;
+
+#[cfg(not(any(target_os = "android", target_os = "linux")))]
+extern crate libc;


Is this still needed if we were to set edition 2018?

No, the extern crate lines would be unnecessary in edition 2018 or later.

I'm not sure what would limit us from setting it to 2018, given that the MSRV is already at 1.65 and drm-sys can't be compiled with much older Rust anyway?

Yeah, with the current MSRV I don't think there's any reason not to update to the current 2021 edition.

MarijnS95 · 2023-10-24T21:51:45Z

drm-ffi/drm-sys/build.rs

+        fn blocklisted_type_implements_trait(
+            &self,
+            name: &str,
+            _derive_trait: DeriveTrait,
+        ) -> Option<ImplementsTrait> {
+            if name == "__kernel_size_t" || name == "drm_handle_t" {
+                Some(ImplementsTrait::Yes)


Had to go to the (imo rather vague) bindgen documentation to figure out that this is telling bindgen that these types implement a bunch of ("standard"??) traits, so that any struct using these types could also automatically derive() those traits (all of them?).

Perhaps a doc-comment could complement this nicely?

MarijnS95

Really like the simplification this provides 👌

ids1024 · 2023-10-25T00:52:13Z

I guess if the drm headers are the same across operating systems and architectures, but have potential differences in the system headers they include and in an #if condition, this could be checked automatically by running bindgen for multiple platforms with the same drm header. Then it should find exactly identical results when we blocklist the parts that differ here.

But this should be correct currently, I doubt more differences will be added in the future, and they should be somewhat obvious looking at changes to bindings.rs.

Drakulix · 2023-10-25T10:43:58Z

I guess if the drm headers are the same across operating systems and architectures, but have potential differences in the system headers they include and in an #if condition, this could be checked automatically by running bindgen for multiple platforms with the same drm header. Then it should find exactly identical results when we blocklist the parts that differ here.

But this should be correct currently, I doubt more differences will be added in the future, and they should be somewhat obvious looking at changes to bindings.rs.

Right, CI should probably still use one architecture to generate the bindings (e.g. x86_64, given that is the least overhead) and then run the other architectures just to compare and fail the test if they differ.

Like that the crate can theoretically support anything, but we still would have a set of well tested architectures.

Given #175 we probably would also want to add testing for riscv in this PR, no?

ids1024 · 2023-10-27T01:50:33Z

It's possible to generate bindings for headers on a specific architecture with something like:

apt-get install --yes wget linux-libc-dev-riscv64-cross bindgen libclang-dev rustfmt
wget https://github.com/torvalds/linux/raw/master/include/uapi/drm/drm.h
wget https://github.com/torvalds/linux/raw/master/include/uapi/drm/drm_mode.h
bindgen drm.h -- -D __user= -isysroot /usr/riscv64-linux-gnu

(Though that particular bindgen invocation doesn't generate exactly what we want.)

Then we can generate up-to-date bindings easily on CI, and verify that there aren't platform differences for any of the architectures Debian has compilers for.

Does it still make sense to have a use_bindgen feature flag if the pre-generated bindings work across platforms?

Drakulix · 2023-10-27T10:58:44Z

Does it still make sense to have a use_bindgen feature flag if the pre-generated bindings work across platforms?

Depends on how up-to-date our bindings are. I guess it doesn't hurt to keep it around?

ids1024 · 2023-11-02T22:48:15Z

Updated with CI changes to pull headers from Linux git repo, generate on all tested architectures, and compare to verify they are the same.

The same drm headers are used for all architectures, and for the BSDs. There are only a couple typedefs that differ. This uses `linux-raw-sys` to define `__kernel_size_t` on Linux, and `libc::size_t` on the BSDs.

This pulls bindings from the Linux kernel repository, so we do not depend on Ubuntu packages, or hacks to use packages from a different arch. The parts that potentially differ are from sysroot headers. CI compares each version of generated bindings against the x86_64 version, and fails if they do not all match. This also adds `riscv64gc-unknown-linux-gnu` as a tested target. Resolves Smithay#165.

Rust 1.65 is already required.

Drakulix

Amazing. Thanks a bunch!

Uses CI based on Smithay/drm-rs#176. Includes Smithay#29.

Uses CI based on Smithay/drm-rs#176. Includes #29.

Based on the CI in Smithay/drm-rs#176. `libinput_log_set_handler` is disabled, since it requires `va_list`. That doesn't seem practical to support until `std::ffi::VaList` is stable. I don't think BSDs should be different here, but I haven't tested it yet. CI confirms that the bindings are the same on Linux for tested architectures.

ids1024 mentioned this pull request Oct 24, 2023

Add riscv64 bindings #175

Closed

MarijnS95 reviewed Oct 24, 2023

View reviewed changes

MarijnS95 approved these changes Oct 24, 2023

View reviewed changes

panekj mentioned this pull request Oct 26, 2023

Ability to Force the Use of wgpu-only Through Feature Flag lapce/floem#123

Open

ids1024 marked this pull request as ready for review November 2, 2023 22:47

ids1024 added 3 commits November 3, 2023 08:23

Generate bindings once for all platforms and architectures

c8d7348

The same drm headers are used for all architectures, and for the BSDs. There are only a couple typedefs that differ. This uses `linux-raw-sys` to define `__kernel_size_t` on Linux, and `libc::size_t` on the BSDs.

Use 2021 edition

c7fca5d

Rust 1.65 is already required.

ids1024 force-pushed the bindings branch from 959c632 to c7fca5d Compare November 3, 2023 15:42

Drakulix approved these changes Nov 3, 2023

View reviewed changes

Drakulix merged commit ec52ef7 into Smithay:develop Nov 3, 2023
16 checks passed

ids1024 deleted the bindings branch November 3, 2023 15:54

This was referenced Nov 3, 2023

drm-sys: add and update FreeBSD bindings #172

Closed

drm-sys: generate powerpc64 and riscv64 bindings #170

Closed

ids1024 added a commit to ids1024/gbm.rs that referenced this pull request Nov 14, 2023

Generate bindings once for all platforms and architectures

5ffc400

Uses CI based on Smithay/drm-rs#176. Includes Smithay#29.

ids1024 mentioned this pull request Nov 14, 2023

Generate bindings once for all platforms and architectures Smithay/gbm.rs#30

Merged

Drakulix pushed a commit to Smithay/gbm.rs that referenced this pull request Nov 14, 2023

Generate bindings once for all platforms and architectures

6bdb1bc

Uses CI based on Smithay/drm-rs#176. Includes #29.

ids1024 mentioned this pull request Dec 4, 2023

Add OpenBSD prebuilt bindings #1

Open

ids1024 mentioned this pull request Jan 16, 2024

Generate bindings once for all platforms and architectures Smithay/input.rs#67

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate bindings once for all platforms and architectures #176

Generate bindings once for all platforms and architectures #176

ids1024 commented Oct 24, 2023

MarijnS95 Oct 24, 2023

ids1024 Oct 24, 2023

MarijnS95 Oct 24, 2023 •

edited

Loading

ids1024 Oct 24, 2023

MarijnS95 Oct 24, 2023

MarijnS95 left a comment

ids1024 commented Oct 25, 2023

Drakulix commented Oct 25, 2023

ids1024 commented Oct 27, 2023

Drakulix commented Oct 27, 2023

ids1024 commented Nov 2, 2023

Drakulix left a comment

Generate bindings once for all platforms and architectures #176

Generate bindings once for all platforms and architectures #176

Conversation

ids1024 commented Oct 24, 2023

MarijnS95 Oct 24, 2023

Choose a reason for hiding this comment

ids1024 Oct 24, 2023

Choose a reason for hiding this comment

MarijnS95 Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

ids1024 Oct 24, 2023

Choose a reason for hiding this comment

MarijnS95 Oct 24, 2023

Choose a reason for hiding this comment

MarijnS95 left a comment

Choose a reason for hiding this comment

ids1024 commented Oct 25, 2023

Drakulix commented Oct 25, 2023

ids1024 commented Oct 27, 2023

Drakulix commented Oct 27, 2023

ids1024 commented Nov 2, 2023

Drakulix left a comment

Choose a reason for hiding this comment

MarijnS95 Oct 24, 2023 •

edited

Loading