Skip to content
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

build: automate bindings generation #41

Closed
JarvisCraft opened this issue Feb 10, 2023 · 11 comments
Closed

build: automate bindings generation #41

JarvisCraft opened this issue Feb 10, 2023 · 11 comments

Comments

@JarvisCraft
Copy link
Contributor

Description

Introduction

Bindings are currently generated using tools/ based on bindgen. This works okay but have the following cons:

  • any updates to original flipper headers require manual regeneration of the sources:
    • this also requires some maintainer (i.e. @dcoles) aware of the right generation process (including correct file and environment setup);
  • this is hardly reproducible due to non-trivial build order;
  • this process is not transparent.

The suggestion

The idea is to replace this approach with a build.rs-based generation.

I expect this to be in a form of a cross-platform build.rs in crates/sys capable of running codegeneration by itself.

Unanswered questions

Original definitions location

It's hard for me to decide what is the best way to store files with original header-definitions. I guess that it is worth looking on other projects utilizing build.rs for FFI code generation.

One way may be using git submodules, although I look at this as not the best mechanism which is hard to maintain sometimes so looking at alternative approaches is something expected.

We may also be able to utilize api_symbols.csv provided in firmware repository as it seems to list available functions, but it seems to lack any struct definitions and type aliases which is a serious limitation.

Version updates

It is also worth discussing the mechanism for performing version updates.

I would suggest following dependabot's approach of performing scheduled update checks. We could theoretically configure GitHub Actions to run on schedule checking for fresh firmware releases and, in case of them being present, run automation to create a PR with header updates.

@dcoles
Copy link
Collaborator

dcoles commented Feb 14, 2023

Hi @JarvisCraft,

I took a bit of a look into how a package like openssl-sys handle this problem.

In short, they build against the system OpenSSL headers and provide no guarantee of any API stability when linking against one specific version or another. This puts the burden on the higher level crates to bridge this gap.

They expose the currently linked version via a DEP_OPENSSL_VERSION_NUMBER build environment variable so that other crates can conditionally compile code depending on the version. Additionally there is also a vendored feature that uses the openssl-src crate to build OpenSSL, however it still requires someone to manually update a Git sub-module for each new version.

One good bit of news is that Flipper Devices now provide SDK archives for each release (e.g. flipper-z-f7-sdk-0.76.0.zip). This eliminates a dependency on https://github.com/flipperdevices/flipperzero-firmware sources and one of the reasons for needing an upstream toolchain.

The current other reason for needing an upstream toolchain is for a compatible "sysroot". Unfortunately bindgen only supports clang as a backend (rust-lang/rust-bindgen#1949), so we end up with a bit of a frankenbuild using one of the binary toolchains and doing some massaging of the SDK's CFLAGS to keep clang happy.

Note: The generate-bindings.rs script already uses api_symbols.csv to minimize the number of Rust definitions that need to be defined, but it still results in about 26,000 lines of code.

So the current trade-off has been to vendor the generated bindings to minimize build requirements (flipperzero-firmware, pre-built toolchain, etc.), but I think the situation can be improved:

  • I agree it would be better to generate the bindings at compile time (which gives more flexibility in building the crates)
  • I'd like to switch to the SDK release archives for SDK headers
  • I'd like to take another look if we can make the SDK work using our own sysroot

@georgikoemdzhiev
Copy link
Contributor

I am in new territory here but would any of this work allow us to use a more recent API version? For example, the "latest"? I think it is impossible to write Rust applications with the latest API (e.g. in the upstream's dev branch) which Unleashed uses.

@dcoles
Copy link
Collaborator

dcoles commented Feb 14, 2023

Hi @georgikoemdzhiev,

I am in new territory here but would any of this work allow us to use a more recent API version? For example, the "latest"?

Yes, that should be possible. To keep things somewhat reproducible, each version of the flipperzero-sys crate would default to a specific upstream release (e.g. 0.77.1), but you could override the upstream release or even provide your own SDK (such as for dev) via some build environment variables.

Note: I can't guarantee compatibility with forks of the official firmware. But provided you don't break the upstream APIs, things should work.

@JarvisCraft
Copy link
Contributor Author

Thanks for the detailed overview of the current implementation reasoning and problems with alternative approaches!

Since there still is a possibility for improvements using the approaches you've mentioned, I suggest that this issue is kept as a tracking-issue for such improvements in the future. Maybe its worth adding some GitHub metadata to it (i.e labels, assignees etc) so it is easier to track its status.

As for the 0.77.1 referenced previously by @georgikoemdzhiev, it may be worth updating to the upstream firmware version since it has been bumped to 0.77.1 (qFlipper already uses it as the default one). BTW, it also contains some merged improvements of mine (more const qualifiers and FFI-friendly IRQ status API) from which this project should benefit.

@dcoles
Copy link
Collaborator

dcoles commented Feb 16, 2023

I've just released flipperzero-0.7.0 which supports flipperzero-firmware@0.77.1 (SDK 14.0).

This release still uses the old mechanism of generating bindings.rs against the official SDK release, but I am working on switching things to a build.rs-based method (see updating-sdk.md).

dcoles added a commit that referenced this issue Mar 30, 2023
@DCNick3
Copy link

DCNick3 commented May 1, 2023

It may be worthwhile to consider what inkwell does with LLVM versions. Basically they have llvm-sys versions correspond to actual LLVM versions 1:1, while the higher-level inkwell has feature flags and magical macros to bridge the gap between different LLVM versions, basically allowing you to select the version you are building against with feature flags.

This allows the user to build inkwell without running bindgen during build process (which might be tedious due to libclang dependency and slow for large APIs) AND allows compatibility with practically any LLVM version

@str4d
Copy link
Contributor

str4d commented May 9, 2023

That's a very clever way of going about it! It's also probably not necessary before upstream reaches 1.0, because they are making changes quite frequently at the moment, and I'm not sure there's much benefit in spending time on supporting multiple pre-1.0 SDK APIs when we are still building out the Rust APIs. But it is an interesting idea to keep in mind as the SDK finalizes.

@DCNick3
Copy link

DCNick3 commented May 9, 2023

Oh, didn't know they planned to stabilize. With the rate they break the API, it's probably a good idea to wait for 1.0 then

@str4d
Copy link
Contributor

str4d commented Sep 10, 2024

That's a very clever way of going about it! It's also probably not necessary before upstream reaches 1.0, because they are making changes quite frequently at the moment, and I'm not sure there's much benefit in spending time on supporting multiple pre-1.0 SDK APIs when we are still building out the Rust APIs. But it is an interesting idea to keep in mind as the SDK finalizes.

Firmware 1.0 was just released, so we should now consider multi-version support.

@JarvisCraft
Copy link
Contributor Author

Since #152 is now merged, we can automatically generate bindings in the repository, but +1 on goal of multi-version support.

dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 13, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
@dcoles
Copy link
Collaborator

dcoles commented Oct 14, 2024

I'm going to close this issue now that #152 is merged, though will create a new issue for supporting multiple SDK versions.

@dcoles dcoles closed this as completed Oct 14, 2024
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
dcoles added a commit that referenced this issue Oct 14, 2024
This replaces the `generate-bindings` script with a `build.rs` script
for `flipperzero-sys` that downloads the latest SDK and uses that to
generate Rust bindings.

Fixes: #41
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

No branches or pull requests

5 participants