Skip to content

Generate and verify Go FFI bindings - #374

Open
fuchsnj wants to merge 4 commits into
mainfrom
fuchsnj/sdsp-488-go-binding-ci
Open

Generate and verify Go FFI bindings#374
fuchsnj wants to merge 4 commits into
mainfrom
fuchsnj/sdsp-488-go-binding-ci

Conversation

@fuchsnj

@fuchsnj fuchsnj commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Jira

The checked-in C header can drift from the Rust FFI while the Go bindings still compile, leaving runtime failures for downstream consumers. This generates the header from Rust with cbindgen and adds one CI check that verifies the header, builds the shared library, and runs the uncached Go binding suite.

This should help prevent issues such as ones fixed by this PR

@fuchsnj
fuchsnj marked this pull request as ready for review July 24, 2026 19:26
@fuchsnj
fuchsnj requested a review from a team as a code owner July 24, 2026 19:26
Comment thread README.md
changes, install `cbindgen` 0.29.2 and regenerate the header:

```shell
cargo install cbindgen --version 0.29.2 --locked

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To make this need self documented in the command if it's missing, would it be possible to add a pre-check before re-generating the bindings:

.PHONY: check-cbindgen
check-cbindgen: ## Ensure cbindgen is installed for Go binding header generation.
	@if ! command -v cbindgen >/dev/null 2>&1; then \
		echo "error: cbindgen is not installed (required for Go binding header generation)." >&2; \
		echo "Install with: cargo install cbindgen --version 0.29.2" >&2; \
		exit 1; \
	fi

...

and then calling the check in update-sds-go-header

update-sds-go-header: check-cbindgen

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@vinckama vinckama left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I added a suggestion

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.

2 participants