Make Rust bindgen build dependency optional (#382)

.gitattributes

@@ -0,0 +1 @@
+bindings/rust/src/bindings/generated.rs linguist-generated

.github/workflows/rust-tests.yml

@@ -23,6 +23,8 @@ jobs:
       - name: cargo hack
         working-directory: bindings/rust
         run: cargo hack check --feature-powerset --depth 2
+      - name: Check that bindings are up to date
+        run: git diff --exit-code bindings/rust/src/bindings/generated.rs
 
   tests:
     runs-on: ${{ matrix.host }}
@@ -58,7 +60,9 @@ jobs:
 
       - name: Build and Test
         working-directory: bindings/rust
-        run: cargo test --target ${{ matrix.target }}
+        run: cargo test --target ${{ matrix.target }} --features generate-bindings
+      - name: Check that bindings are up to date
+        run: git diff --exit-code bindings/rust/src/bindings/generated.rs
       - name: Benchmark
         working-directory: bindings/rust
         run: cargo bench --target ${{ matrix.target }}

bindings/rust/Cargo.toml

@@ -11,20 +11,21 @@ links = "ckzg"
 default = ["std", "blst/portable"]
 std = ["hex/std", "libc/std", "serde?/std"]
 serde = ["dep:serde"]
+generate-bindings = ["dep:bindgen"]
 
 # BLST Compilation:
 # Suppress multi-threading.
 # Engaged on wasm32 target architecture automatically.
 no-threads = ["blst/no-threads"]
 
 [dependencies]
+blst = { version = "0.3.11", default-features = false }
 hex = { version = "0.4.2", default-features = false, features = ["alloc"] }
 libc = { version = "0.2", default-features = false }
 serde = { version = "1.0", optional = true, default-features = false, features = [
     "alloc",
     "derive",
 ] }
-blst = { version = "0.3.11", default-features = false }
 
 [dev-dependencies]
 criterion = "0.5.1"
@@ -35,7 +36,7 @@ serde_yaml = "0.9.17"
 serde_json = "1.0.105"
 
 [build-dependencies]
-bindgen = "0.69"
+bindgen = { version = "0.69", optional = true }
 cc = "1.0"
 
 [target.'cfg(target_env = "msvc")'.build-dependencies]

bindings/rust/build.rs

@@ -34,25 +34,23 @@ fn main() {
 
     cc.try_compile("ckzg").expect("Failed to compile ckzg");
 
-    let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
-    let bindings_out_path = out_dir.join("generated.rs");
-    let header_file_path = c_src_dir.join("c_kzg_4844.h");
-    let header_file = header_file_path.to_str().expect("valid header file");
-
-    make_bindings(
-        header_file,
-        &blst_headers_dir.to_string_lossy(),
-        bindings_out_path,
-    );
+    #[cfg(feature = "generate-bindings")]
+    {
+        let header_path = c_src_dir.join("c_kzg_4844.h");
+        let bindings_out_path = concat!(env!("CARGO_MANIFEST_DIR"), "/src/bindings/generated.rs");
+        make_bindings(
+            header_path.to_str().expect("valid header path"),
+            blst_headers_dir.to_str().expect("valid blst header path"),
+            bindings_out_path.as_ref(),
+        );
+    }
 
     // Finally, tell cargo this provides ckzg/ckzg_min
     println!("cargo:rustc-link-lib=ckzg");
 }
 
-fn make_bindings<P>(header_path: &str, blst_headers_dir: &str, bindings_out_path: P)
-where
-    P: AsRef<std::path::Path>,
-{
+#[cfg(feature = "generate-bindings")]
+fn make_bindings(header_path: &str, blst_headers_dir: &str, bindings_out_path: &std::path::Path) {
     use bindgen::Builder;
 
     #[derive(Debug)]
@@ -123,7 +121,41 @@ where
         .generate()
         .unwrap();
 
+    let mut bindings = bindings.to_string();
+    bindings = replace_ckzg_ret_repr(bindings);
+    std::fs::write(bindings_out_path, bindings).expect("Failed to write bindings");
+}
+
+// Here we hardcode the C_KZG_RET enum to use C representation. Bindgen
+// will use repr(u32) on Unix and repr(i32) on Windows. We would like to
+// use the same generated bindings for all platforms. This can be removed
+// if/when bindgen fixes this or allows us to choose our own representation
+// for types. Using repr(C) is equivalent to repr(u*) for fieldless enums,
+// so this should be safe to do. The alternative was to modify C_KZG_RET in
+// C and we decided this was the lesser of two evils. There should be only
+// one instance where repr(C) isn't used: C_KZG_RET.
+// See: https://github.com/rust-lang/rust-bindgen/issues/1907
+#[cfg(feature = "generate-bindings")]
+fn replace_ckzg_ret_repr(mut bindings: String) -> String {
+    let target = env::var("TARGET").unwrap_or_default();
+    let repr_to_replace = if target.contains("windows") {
+        "#[repr(i32)]"
+    } else {
+        "#[repr(u32)]"
+    };
+
+    // Find `repr_to_replace` as an attribute of `enum C_KZG_RET`.
+    let ckzg_ret = bindings
+        .find("enum C_KZG_RET")
+        .expect("Could not find C_KZG_RET in bindings");
+    let repr_start = bindings[..ckzg_ret]
+        .rfind(repr_to_replace)
+        .expect("Could not find repr to replace in bindings");
+
+    // Sanity check that it's an attribute of `C_KZG_RET` and not another type.
+    assert!(repr_start > bindings[..ckzg_ret].rfind('}').unwrap());
+
+    bindings.replace_range(repr_start..repr_start + repr_to_replace.len(), "#[repr(C)]");
+
     bindings
-        .write_to_file(bindings_out_path)
-        .expect("Failed to write bindings");
 }

bindings/rust/src/bindings/mod.rs

@@ -1,13 +1,11 @@
-#![allow(non_upper_case_globals)]
-#![allow(non_camel_case_types)]
-#![allow(non_snake_case)]
+#![allow(non_camel_case_types, non_snake_case, non_upper_case_globals)]
 
 #[cfg(feature = "serde")]
 mod serde;
 #[cfg(test)]
 mod test_formats;
 
-include!(concat!(env!("OUT_DIR"), "/generated.rs"));
+include!("./generated.rs");
 
 use alloc::string::String;
 use alloc::vec::Vec;