Skip to content

Ship pregenerated bindings in ruby-prism-sys#4179

Open
sorafujitani wants to merge 4 commits into
ruby:mainfrom
sorafujitani:rust/pregenerated-bindings
Open

Ship pregenerated bindings in ruby-prism-sys#4179
sorafujitani wants to merge 4 commits into
ruby:mainfrom
sorafujitani:rust/pregenerated-bindings

Conversation

@sorafujitani

Copy link
Copy Markdown

Closes #4175.

ruby-prism-sys now ships the bindgen output as a committed file (src/bindings.rs), so building the crate no longer requires bindgen or libclang. This follows the pattern of libsqlite3-sys and zstd-sys.

Notes

  • The committed file is generated with layout_tests(false). Layout tests hardcode the generating host's struct sizes, which breaks 32-bit targets (wasm32-wasip1 fails with a const-eval overflow). Without them the output is source-portable; I verified byte-identical output on macOS arm64 and Linux x86_64.
  • The buildtime_bindgen feature restores build-time generation. It is needed when RUBY_PRISM_CFLAGS changes the API surface.
  • rake cargo:bindings regenerates the committed file. CI regenerates and diffs it, runs the layout tests via buildtime_bindgen on the 64-bit test matrix, and checks that the committed bindings compile for wasm32-wasip1.
  • One correction to the issue thread: I claimed the bindgen output had no layout tests. It does; bindgen 0.72 emits them as const assertions rather than #[test] functions. That is why the committed copy disables them.
  • Scope: this removes the libclang/bindgen requirement. Cross-compiling still needs a C cross toolchain for the cc build of the vendored sources.

Commit the bindgen output at src/bindings.rs so building the crate no
longer requires bindgen or libclang. The committed file is generated
with layout_tests(false) to stay portable across pointer widths. The
new buildtime_bindgen feature restores build-time generation, and
rake cargo:bindings regenerates the committed file.
The sys-bindings job regenerates the bindings and fails if the
committed file is stale. The test matrix also runs the layout tests
via buildtime_bindgen on 64-bit hosts and checks that the committed
bindings compile for wasm32-wasip1.

- name: Check pregenerated bindings on wasm32-wasip1
if: ${{ matrix.os == 'ubuntu-latest' }}
run: cargo check --target wasm32-wasip1 --no-default-features

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite know enough about this, but I'm assuming we want to check other bindings besides just this right? Should we check x64/a64/etc?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

64-bit targets are already tested: the matrix builds the committed bindings on x64 (ubuntu, windows) and arm64 (macos) and runs the test suite against them.
this step only exists because wasm32 is the only supported target with a different pointer width.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mark this as generated in .gitattributes

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed
d210591

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.

ruby-prism-sys: cross-compilation fails out of the box; consider pregenerated bindings

3 participants