From 087f2a5a9446216741eb4749b1f1e3e0362b71d0 Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Thu, 30 Jul 2026 18:11:31 +0800 Subject: [PATCH 1/3] fix(ffi): regenerate the drifted cbindgen header The committed header lacked the namespace_ field that CContextConfig has carried since it was added. cgo sizes its struct from this file, so Go allocated 80 bytes where the Rust side reads 88 and dereferenced cfg.namespace past the end of the allocation on every advanced-config ContextBuilder.Build(). Regenerated with the cbindgen the flake pins (0.29.3). Also restores 11 parameter-type constants, KEEP_ALL_CACHE_DEPTH, and corrected doc text. Refs #270 --- crates/hiroz-go/hiroz/hiroz_ffi.h | 56 ++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/crates/hiroz-go/hiroz/hiroz_ffi.h b/crates/hiroz-go/hiroz/hiroz_ffi.h index 662ea2823..5b8b998ea 100644 --- a/crates/hiroz-go/hiroz/hiroz_ffi.h +++ b/crates/hiroz-go/hiroz/hiroz_ffi.h @@ -11,11 +11,29 @@ #define hiroz_ZENOH_EVENT_ID_MAX 11 /** - * Default depth for KEEP_LAST when SYSTEM_DEFAULT (depth=0) is used - * This matches ROS 2 and rmw_zenoh_cpp behavior + * Default depth for `KeepLast` when SYSTEM_DEFAULT (depth=0) is used. + * Matches rclcpp's default of 10. Note this is distinct from + * [`KEEP_ALL_CACHE_DEPTH`] below, which is the rmw_zenoh-aligned cap + * applied to `KeepAll` when mapped to zenoh-ext's `cache.max_samples`. */ #define hiroz_DEFAULT_HISTORY_DEPTH 10 +/** + * Cache/history depth used when a TransientLocal endpoint carries + * `QosHistory::KeepAll` (no inherent depth) or `KeepLast(0)`. + * + * Matches rmw_zenoh_cpp's `RMW_ZENOH_DEFAULT_HISTORY_DEPTH = 42` + * (`rmw_zenoh_cpp/src/detail/qos.cpp:27`), which is the value + * rmw_zenoh's `best_available_qos` substitutes for a zero-valued + * `qos.depth` before passing it to + * `AdvancedPublisherOptions::CacheOptions::max_samples`. + * + * This is intentionally a *finite* cap that mirrors rmw_zenoh's + * pragmatic behaviour rather than the DDS KEEP_ALL spec's "keep + * everything" semantics. + */ +#define hiroz_KEEP_ALL_CACHE_DEPTH 42 + /** * Default shared memory pool size (10 MB). */ @@ -29,6 +47,31 @@ */ #define hiroz_DEFAULT_SHM_THRESHOLD 512 +/** + * Constant for ListParameters: recursively get parameters with unlimited depth. + */ +#define hiroz_DEPTH_RECURSIVE 0 + +#define hiroz_NOT_SET 0 + +#define hiroz_BOOL 1 + +#define hiroz_INTEGER 2 + +#define hiroz_DOUBLE 3 + +#define hiroz_STRING 4 + +#define hiroz_BYTE_ARRAY 5 + +#define hiroz_BOOL_ARRAY 6 + +#define hiroz_INTEGER_ARRAY 7 + +#define hiroz_DOUBLE_ARRAY 8 + +#define hiroz_STRING_ARRAY 9 + /** * Opaque action client handle for FFI */ @@ -185,6 +228,11 @@ typedef struct hiroz_context_config_t { * Whether to enable logging */ bool enable_logging; + /** + * Default namespace inherited by nodes created from this context (nullable). + * Added after all pre-existing fields to preserve ABI compatibility. + */ + const char *namespace_; } hiroz_context_config_t; /** @@ -630,8 +678,8 @@ int32_t hiroz_service_client_destroy(struct hiroz_service_client_t *client); /** * Wait until at least one matching service server is visible in the graph, - * or `timeout_ms` elapses. Returns 0 if ready, -10 (ServiceTimeout) on - * timeout, -1 (NullPointer) if `client_handle` is null. + * or `timeout_ms` elapses. Returns `Success` (0) if ready, `ServiceTimeout` + * (-10) on timeout, `NullPointer` (-1) if `client` is null. */ int32_t hiroz_service_client_wait_for_service(struct hiroz_service_client_t *client_handle, uint64_t timeout_ms); From c562383f339eef655ef4681d2e777a46edd36f0f Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Thu, 30 Jul 2026 18:13:42 +0800 Subject: [PATCH 2/3] ci(ffi): gate the generated FFI header against cbindgen output Nothing regenerated the header in CI, so its drift from the Rust structs was invisible. cbindgen was absent from every dev shell, and build.rs degrades its absence to a non-fatal cargo:warning=, so enabling the ffi feature alone would not have caught this. Adds rust-cbindgen to commonBuildInputs and a check-ffi-header check that deletes the header, regenerates it, and fails on any diff -- mirroring check-python-stubs, which gates the generated Python stubs the same way. The deletion is what makes a build that generates nothing fail instead of reporting on the committed copy; cbindgen's absence is checked explicitly so that path cannot pass vacuously either. Runs as its own nix-based job because go-tests has no nix environment and cbindgen must come from the pinned dev shell -- 0.29.4 emits a constant 0.29.3 does not, so an unpinned version would itself read as drift. Closes item 3 of #270 --- .github/workflows/ci.yml | 58 +++++++++++++++++++++++++++++++++++++++ flake.nix | 6 ++++ scripts/test-pure-rust.nu | 58 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1da4b5dac..b794536c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -769,6 +769,64 @@ jobs: source /tmp/nix-dev-env.sh nu scripts/test-pure-rust.nu check-python-stubs + ffi-header-fresh: + name: Generated FFI Header (ubuntu-latest) + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Free Disk Space (Ubuntu) + uses: jlumbroso/free-disk-space@main + with: + large-packages: true + tool-cache: true + android: true + dotnet: true + haskell: true + docker-images: true + swap-storage: false + + - name: Install Nix + uses: cachix/install-nix-action@v31 + with: + extra_nix_config: | + extra-substituters = https://hiroz.cachix.org + extra-trusted-public-keys = hiroz.cachix.org-1:wKJuqEckTG0DL3Df7Ly9OVsg5S5TGBHtvlPGs+vlqrY= + + - name: Setup Cachix + uses: cachix/cachix-action@v15 + with: + name: hiroz + authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}' + skipPush: ${{ github.event_name == 'pull_request' }} + + - name: Install Nushell + run: | + nix profile install nixpkgs#nushell + chmod +x scripts/test-pure-rust.nu + + - name: Setup nix environment + run: | + nix print-dev-env '.#pureRust-ci' --accept-flake-config > /tmp/nix-dev-env.sh + echo "export CI=true" >> /tmp/nix-dev-env.sh + + - name: Setup Rust cache + uses: Swatinem/rust-cache@v2 + with: + shared-key: ubuntu-latest-ffi-header + + # Its own job rather than a step in `go-tests`: that job has no Nix + # environment, and cbindgen has to come from the same dev shell the check + # runs in so a version skew cannot silently change the generated output. + - name: Generated FFI header is up to date + run: | + source /tmp/nix-dev-env.sh + nu scripts/test-pure-rust.nu check-ffi-header + go-tests: name: Go Tests (${{ matrix.os }}) runs-on: ${{ matrix.os }} diff --git a/flake.nix b/flake.nix index 3e3d36084..ef699b177 100644 --- a/flake.nix +++ b/flake.nix @@ -219,6 +219,12 @@ # Ensure python is available since we unwrapped the ROS env python3 go # Go toolchain (latest stable) + # Generates crates/hiroz-go/hiroz/hiroz_ffi.h from crates/hiroz/src/ffi/ + # (crates/hiroz/build.rs) under `--features ffi`. Sits beside `go` + # because that header is what cgo compiles against. Absent from every + # shell until now, so CI never regenerated it and the committed copy + # silently drifted out of sync with the Rust structs -- see #270. + rust-cbindgen ]; # Development tools diff --git a/scripts/test-pure-rust.nu b/scripts/test-pure-rust.nu index be7c3ac5e..b5fc2c90b 100755 --- a/scripts/test-pure-rust.nu +++ b/scripts/test-pure-rust.nu @@ -144,6 +144,62 @@ def check-python-stubs [] { print $"Generated Python stubs match the message assets." } +def check-ffi-header [] { + log-step "Generated FFI header is up to date" + # `crates/hiroz-go/hiroz/hiroz_ffi.h` is generated by cbindgen from + # `crates/hiroz/src/ffi/` (see crates/hiroz/build.rs) and committed, because + # cgo needs it at build time. Nothing checked that the committed copy still + # matched the generator, and it had drifted by 52 lines -- including a field + # `namespace_` added to `hiroz_context_config_t`. cgo sizes its struct from + # this header, so Go allocated 80 bytes where Rust reads 88: an + # out-of-bounds read of `cfg.namespace` on every advanced-config + # `ContextBuilder.Build()`. See #270. + # + # cbindgen is checked explicitly rather than left to build.rs, which + # degrades its absence to a non-fatal `cargo:warning=` (build.rs:55-61). + # Without this the build would succeed, write nothing, and the check would + # report on whatever was already in the tree. + if (which cbindgen | is-empty) { + error make { + msg: ("cbindgen not found, so the FFI header cannot be regenerated and this check " + + "would pass without verifying anything. Install it (`cargo install cbindgen`, " + + "or enter the `.#pureRust` dev shell) and re-run.") + } + } + + # Printed because cbindgen's output is version-dependent -- 0.29.4 emits a + # `CDR_HEADER_LE` constant that 0.29.3 does not -- so a stale-header report + # can also mean "generated by a different cbindgen". The flake pins the + # authoritative one; a developer whose PATH prefers another (e.g. an older + # `cargo install` copy in ~/.cargo/bin) sees which was used here rather + # than an unexplained diff. + print $"cbindgen: (^cbindgen --version | str trim)" + + # Deleted first for the same reason the Python-stub check empties its + # directory: a build that generates nothing then leaves the committed copy + # in place and passes. With the file absent, that failure mode surfaces as + # a ` D` entry instead. + # + # If the build below fails, the header is left deleted in the working tree; + # `git checkout --
` restores it. + let header = "crates/hiroz-go/hiroz/hiroz_ffi.h" + rm -f $header + # Forces build.rs to rerun even when cargo considers the crate fresh -- + # otherwise a warm target dir makes this a no-op that passes. + touch crates/hiroz/build.rs + run-cmd "cargo build -j4 -p hiroz --features ffi" + + let drift = (^git status --porcelain -- $header | complete) + if ($drift.stdout | str trim | is-not-empty) { + print ($drift.stdout | str trim) + print (^git diff -- $header | complete | get stdout) + error make { + msg: $"generated FFI header is stale -- run `cargo build -p hiroz --features ffi` and commit ($header)" + } + } + print $"Generated FFI header matches the Rust FFI surface." +} + def check-examples [] { log-step "Check all examples (cargo check --examples)" run-cmd "cargo check --examples" @@ -184,6 +240,7 @@ def get-test-map [] { check-examples: { check-examples } check-rustdoc-links: { check-rustdoc-links } check-python-stubs: { check-python-stubs } + check-ffi-header: { check-ffi-header } check-distro-features: { check-distro-features } clippy-hiroz-py: { clippy-hiroz-py } clippy-tests: { clippy-tests } @@ -200,6 +257,7 @@ def get-test-pipeline [] { "check-examples" "check-rustdoc-links" "check-python-stubs" + "check-ffi-header" "check-distro-features" "clippy-hiroz-py" "clippy-tests" From 26a881fbe6a7222d581a55529c7e7d008269f878 Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Thu, 10 Sep 2026 01:09:47 +0800 Subject: [PATCH 3/3] ci(ffi-header-fresh): pin actions to full commit SHAs The org policy enforced on ZettaScaleLabs/hiroz rejects any workflow step whose action is not pinned to a full-length commit SHA. This job's five actions were still tag-pinned, so it would fail CI on that gate alone. Pin each to the SHA already used for the same action and version elsewhere in the repo (see #346), keeping the tag as a trailing comment. --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b794536c3..83909f5a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -777,10 +777,10 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Free Disk Space (Ubuntu) - uses: jlumbroso/free-disk-space@main + uses: jlumbroso/free-disk-space@54081f138730dfa15788a46383842cd2f914a1be # main with: large-packages: true tool-cache: true @@ -791,14 +791,14 @@ jobs: swap-storage: false - name: Install Nix - uses: cachix/install-nix-action@v31 + uses: cachix/install-nix-action@13d8dd58da0234aa297dedd986986ccb8e7f3e24 # v31 with: extra_nix_config: | extra-substituters = https://hiroz.cachix.org extra-trusted-public-keys = hiroz.cachix.org-1:wKJuqEckTG0DL3Df7Ly9OVsg5S5TGBHtvlPGs+vlqrY= - name: Setup Cachix - uses: cachix/cachix-action@v15 + uses: cachix/cachix-action@ad2ddac53f961de1989924296a1f236fcfbaa4fc # v15 with: name: hiroz authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}' @@ -815,7 +815,7 @@ jobs: echo "export CI=true" >> /tmp/nix-dev-env.sh - name: Setup Rust cache - uses: Swatinem/rust-cache@v2 + uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 with: shared-key: ubuntu-latest-ffi-header