Rust bindings by Choochmeque · Pull Request #87 · ecmwf/gribjump

Choochmeque · 2026-03-25T18:11:04Z

Description

Contributor Declaration

By opening this pull request, I affirm the following:

All authors agree to the Contributor License Agreement.
The code follows the project's coding standards.
I have performed self-review and added comments where needed.
I have added or updated tests to verify that my changes are effective and functional.
I have run all existing tests and confirmed they pass.

codecov-commenter · 2026-03-25T18:20:31Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.89%. Comparing base (34b0ab5) to head (da0883d).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop      #87   +/-   ##
========================================
  Coverage    74.89%   74.89%           
========================================
  Files           98       98           
  Lines         5047     5047           
  Branches       445      445           
========================================
  Hits          3780     3780           
  Misses        1267     1267

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR introduces Rust bindings for the ECMWF gribjump C++ library, including a low-level gribjump-sys FFI layer and a safe gribjump wrapper crate, plus supporting tests/fixtures, examples, and benchmarks.

Changes:

Add gribjump-sys (cxx bridge + build script) to build/link gribjump (vendored or system).
Add gribjump safe wrapper API (handle, requests, iterator, results, error mapping).
Add integration/thread-safety/async tests, fixtures, examples, and Rust/C++ benchmarks.

Reviewed changes

Copilot reviewed 27 out of 32 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
rust/crates/gribjump/tests/gribjump_thread_safety.rs	Thread-safety + concurrency tests (ignored runtime cases)
rust/crates/gribjump/tests/gribjump_integration.rs	Integration tests that set up an FDB and validate wrapper behavior
rust/crates/gribjump/tests/gribjump_async.rs	Tokio-based concurrent access tests (ignored by default)
rust/crates/gribjump/tests/fixtures/template.grib	Added GRIB fixture
rust/crates/gribjump/tests/fixtures/synth11.grib	Added GRIB fixture used for tests/benches
rust/crates/gribjump/tests/fixtures/schema	Added FDB schema fixture
rust/crates/gribjump/tests/fixtures/extract_ranges.grib	Added GRIB fixture for range extraction tests
rust/crates/gribjump/tests/fixtures/axes.grib	Added GRIB fixture for axes tests
rust/crates/gribjump/src/result.rs	Zero-copy result types + views + owned conversions
rust/crates/gribjump/src/request.rs	Request/range types and conversion to FFI types
rust/crates/gribjump/src/lib.rs	Crate module wiring + public re-exports
rust/crates/gribjump/src/iterator.rs	Iterator wrapper over FFI extraction iterator
rust/crates/gribjump/src/handle.rs	Thread-safe handle wrapper around the FFI handle
rust/crates/gribjump/src/error.rs	Rust error type + mapping from `cxx::Exception`
rust/crates/gribjump/examples/gribjump_file.rs	Example: scan + extract from file offsets
rust/crates/gribjump/examples/gribjump_extract.rs	Example: build request + extract ranges
rust/crates/gribjump/examples/gribjump_basic.rs	Example: version/git-sha + handle creation
rust/crates/gribjump/examples/gribjump_axes.rs	Example: axes query
rust/crates/gribjump/benches/gribjump_bench.rs	Criterion benchmarks for Rust wrapper
rust/crates/gribjump/benches/cpp/gribjump_bench.cpp	Google Benchmark comparison using native C++ API
rust/crates/gribjump/benches/cpp/CMakeLists.txt	CMake build for C++ benchmarks
rust/crates/gribjump/README.md	Wrapper crate documentation
rust/crates/gribjump/Cargo.toml	Wrapper crate manifest
rust/crates/gribjump-sys/src/lib.rs	`cxx::bridge` definitions for FFI API surface
rust/crates/gribjump-sys/cpp/gribjump_bridge.h	C++ bridge declarations + global try/catch mapping
rust/crates/gribjump-sys/cpp/gribjump_bridge.cpp	C++ bridge implementation + conversions + validations
rust/crates/gribjump-sys/build.rs	Vendored/system build + link configuration for the bridge
rust/crates/gribjump-sys/README.md	Sys crate documentation
rust/crates/gribjump-sys/Cargo.toml	Sys crate manifest + features
rust/Cargo.toml	Rust workspace definition + shared deps
.gitignore	Ignore Rust build artifacts/lockfile under `rust/`
.github/workflows/rust.yml.example	Example CI workflow for Rust checks/tests/fmt/clippy

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mathleur

Looks good to me, I just had one question/comment about the PathExtractionRequest constructor, which we might start using in Polytope in the future

caraghbiner · 2026-03-30T15:15:17Z

Hi @Choochmeque, just so you are aware there is a PR likely to be merged soon which cleans up of the gribjump source code. This involved moving and renaming some API headers, so will likely affect your work. It should be fairly easy to update the relevant paths in your implementation, but just letting you know in advance.

Choochmeque · 2026-03-30T17:19:41Z

Hi @Choochmeque, just so you are aware there is a PR likely to be merged soon which cleans up of the gribjump source code. This involved moving and renaming some API headers, so will likely affect your work. It should be fairly easy to update the relevant paths in your implementation, but just letting you know in advance.

Hi @ChrisspyB,
Thank you for information. I will keep an eye on it.

simondsmart · 2026-03-31T13:19:32Z

+gribjump = { path = "crates/gribjump" }
+
+# FDB dependency
+fdb = { git = "ssh://git@github.com/ecmwf/fdb.git", branch = "rust-bindings", default-features = false }


Reminder note to switch this away from rust-bindings.

simondsmart · 2026-03-31T14:04:45Z

+    #[derive(Debug, Clone, Default)]
+    pub struct ExtractionRequestData {
+        /// MARS request string (e.g., "class=od,expver=0001,...")
+        pub request_str: String,


Placeholder note - this really ought to be a MarsRequest. Although it looks like gribjump is lagging there...

simondsmart · 2026-03-31T15:46:27Z

+#[cfg(feature = "system")]
+#[allow(clippy::too_many_lines)]
+fn cmake_find_package(
+    package: &str,


Is this code that is going to need to be duplicated in all packages?

Fixed via bindman-build crate.

sametd

Added some minor comments

caraghbiner · 2026-04-29T14:18:40Z

Please rebase on the latest develop, all (py)gribjump tests should now pass the CI

… gribjump_bridge.h

…xample, because it should be reusable-action)

…ount and success

…uest

…tion

…stances

This reverts commit d320a22.

…ivate repos

…ntime

Choochmeque · 2026-04-29T16:21:11Z

Please rebase on the latest develop, all (py)gribjump tests should now pass the CI

All green now. Thanks!

simondsmart

Some comments that should be addressed. But fundamentally happy.

I would like some clarification about the Range question, but otherwise ready to merge when you are (re the comments)

simondsmart · 2026-04-29T08:49:40Z

+
+    // Set FDB5_CONFIG for GribJump
+    unsafe {
+        env::set_var("FDB5_CONFIG", &config);


This still feels like it is in the wrong place relative to a function called setup_test_dir. Feels like hidden, magic, implicit behaviour. That were are (re)setting the env var needs to be made clear in the naming, or moved out to the calling function, or something.

Fixed in 9ca00fe.

simondsmart · 2026-04-29T08:52:15Z

+    assert!(results.iter().all(|(_, c)| *c == first_count));
+    println!("Concurrent axes: all tasks found {first_count} axis entries");
+
+    drop(gj);


Unclear why the explicit drop calls are needed? This seems to be the natural drop order implied by the construction order?

Fixed in 02533d2.

simondsmart · 2026-04-29T09:57:58Z

+
+/// A range of indices to extract (start and end inclusive).
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)]
+pub struct Range {


Is there a reason we can't use std::Range here? Seems to carry the same functionality.

Validation - Range::new() rejects empty ranges at construction; std::ops::Range allows 5..3 silently

Copy - our Range is Copy; std::ops::Range is not (because it implements Iterator)

No clippy conflicts - vec![Range::new(0, 100)?] is unambiguous; vec![0..100] triggers single_range_in_vec_init

Interop - From<std::ops::Range> lets users write (0..100).into() when they want standard syntax (added in da0883d)

simondsmart · 2026-04-30T13:11:53Z

+
+    /// Get the gribjump library version.
+    #[must_use]
+    pub fn version() -> String {


I'm not sure that the version/git_sha1 functions belong to the GribJump struct.

They certainly belong somewhere in the crate (they are library-level info). But Not sure that they should be part of the object.

Fixed in 62f9224.

…sts for GribJump cloning functionality.

…h exclusive end behavior

resolved

jameshawkes requested review from Copilot, mathleur and sametd March 27, 2026 11:47

Copilot started reviewing on behalf of jameshawkes March 27, 2026 11:48 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

mathleur approved these changes Mar 27, 2026

View reviewed changes

Comment thread rust/crates/gribjump/src/request.rs Outdated

simondsmart requested changes Mar 31, 2026

View reviewed changes

Choochmeque force-pushed the rust-bindings branch from 5e82044 to c836cd8 Compare April 2, 2026 20:46

sametd previously requested changes Apr 7, 2026

View reviewed changes

Choochmeque marked this pull request as ready for review April 14, 2026 15:46

Choochmeque commented Apr 16, 2026

View reviewed changes

Comment thread rust/crates/gribjump/examples/fdb_gribjump.rs

Choochmeque and others added 15 commits April 29, 2026 16:22

Initial commit

38fb58c

Update FDB dependency to use 'rust-bindings' branch in Cargo.toml

ce80d8b

Refactor include statements and formatting in gribjump_bridge.cpp and…

65730f9

… gribjump_bridge.h

Add GitHub Actions workflow for Rust project with checks and tests (e…

d148751

…xample, because it should be reusable-action)

Update test documentation and formatting

23aee18

Add thread-safety tests for GribJump with FDB configuration setup

40375fa

Update ExtractionRequest to require grid hash for validation in API

d56f283

Improve integration test for extract_from_paths to verify results c…

1cb06a8

…ount and success

Update README to include Range struct and grid_hash in extraction req…

a0c51c1

…uest

Add validation for end in Range::new to prevent overflow errors

0a52c6c

Add from_parts method to PathExtractionRequest for full customiza…

c412846

…tion

Refactor GribJumpHandle and ExtractionIteratorHandle to use direct in…

2518909

…stances

Add include directive for ExtractionData.h in gribjump_bridge.h

7d8c9c6

Update try_convert_masks method to handle exceptions

4d9106f

Replace runtime_error with eckit exceptions

a4fb9fb

Choochmeque added 20 commits April 29, 2026 16:22

Update gribjump dependency version to 0.10.3 in build script

84e342e

Refactor print_stats to return Result for better error handling

5bc68d2

Remove IntoIterator implementation for ExtractionResult and update tests

179f9cf

Add Display implementation for Range to improve formatting in output

a87b3d6

Add rust CI workflow

dc37d4c

Update bindman dependency URL to use HTTPS instead of SSH

83f83f9

Revert "Update bindman dependency URL to use HTTPS instead of SSH"

89085fb

This reverts commit d320a22.

Update CI workflow to use correct file paths and configure git for pr…

de94707

…ivate repos

Update Clippy command to include all targets

daab482

Remove rust-cache from CI workflow

0f0e581

Update test to use len() instead of count() for owned ranges

52d08d8

Add more tests to show interop between fdb and gribjump

17cc5c5

Update FDB initialization to use Fdb::open for consistency in tests

652bce4

Add bindman-utils dependency and refactor build logic in gribjump-sys

31dd8e9

Add integration testing workflow for rust-bindings branch in CI

0e59551

Add FDB + GribJump interoperability prototype example

7de509c

Add RPATH support to gribjump crate for dynamic library linking at ru…

d0eb971

…ntime

Remove branch specification for bindman-utils in Cargo.toml

0b94805

Update CI workflow and tests to run without --ignored flag

13d3829

Set version as common constant

70e3b41

Choochmeque force-pushed the rust-bindings branch from 7700a36 to 70e3b41 Compare April 29, 2026 14:42

simondsmart approved these changes Apr 30, 2026

View reviewed changes

Choochmeque added 5 commits April 30, 2026 15:49

Remove unnecessary drop calls in gribjump test cases

02533d2

Refactor version and git SHA1 retrieval to use gribjump module directly

62f9224

Add Cargo configuration for build jobs and Clippy warnings; update te…

0ae1a7d

…sts for GribJump cloning functionality.

Set FDB5_CONFIG environment variable for GribJump tests setup

9ca00fe

Update range endpoints in benchmarks and examples for consistency wit…

da0883d

…h exclusive end behavior

caraghbiner merged commit 62390b6 into develop May 4, 2026
141 checks passed

Conversation

Choochmeque commented Mar 25, 2026

Description

Contributor Declaration

Uh oh!

codecov-commenter commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mathleur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

caraghbiner commented Mar 30, 2026

Uh oh!

Choochmeque commented Mar 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sametd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

caraghbiner commented Apr 29, 2026

Uh oh!

Choochmeque commented Apr 29, 2026

Uh oh!

simondsmart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov-commenter commented Mar 25, 2026 •

edited

Loading