From f00d4b978e4675ebd1e3933cb24e62153e41b554 Mon Sep 17 00:00:00 2001 From: kanarus Date: Tue, 16 Sep 2025 08:42:55 +0900 Subject: [PATCH 1/3] feat: conditional bindgen generation --- .github/workflows/CI.yml | 17 +++---- Cargo.toml | 5 +- build.rs | 101 +++++++++++++++++++++------------------ 3 files changed, 65 insertions(+), 58 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index d007141..51032ae 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -26,15 +26,15 @@ jobs: rustup component add rustfmt ### required for the build script to work ### [ "${{ matrix.arch }}" = 'aarch64' ] && sudo apt update && sudo apt install -y g++-aarch64-linux-gnu || : - - name: check fails without MUJOCO_DIR + - name: build fails without MUJOCO_DIR env: CARGO_BUILD_TARGET: ${{ matrix.arch }}-unknown-linux-gnu run: | if cargo build; then - echo 'cargo check succeeded without mujoco, which is unexpected.' + echo 'cargo build succeeded without MUJOCO_DIR, which is unexpected.' exit 1 else - echo 'cargo check failed as expected without mujoco.' + echo 'cargo build failed as expected without mujoco.' fi - name: install mujoco and set MUJOCO_DIR @@ -46,17 +46,12 @@ jobs: echo "MUJOCO_DIR=$HOME/.mujoco/mujoco-3.3.2" >> $GITHUB_ENV echo "LD_LIBRARY_PATH=$HOME/.mujoco/mujoco-3.3.2/lib:$LD_LIBRARY_PATH" >> $GITHUB_ENV - - name: check succeeds with MUJOCO_DIR + - name: build succeeds with MUJOCO_DIR env: CARGO_BUILD_TARGET: ${{ matrix.arch }}-unknown-linux-gnu run: | - if cargo build; then - echo 'cargo check succeeded with mujoco, as expected.' - else - echo 'cargo check failed with mujoco, which is unexpected.' - echo "[DEBUG] bindgen.rs content:" && cat ./src/bindgen.rs - exit 1 - fi + cargo build + cargo build --features bindgen test: strategy: diff --git a/Cargo.toml b/Cargo.toml index 77b83d7..899390b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,10 @@ keywords = ["mujoco", "rl", "ml", "physics", "robotics"] categories = ["api-bindings", "science::robotics", "simulation"] [build-dependencies] -bindgen = "0.72" +bindgen = { optional = true, version = "0.72" } [dev-dependencies] glfw = "0.60" + +[features] +bindgen = ["dep:bindgen"] # run bindgen at build time, instead of using pre-generated bindgen.rs diff --git a/build.rs b/build.rs index 93225a3..64dc741 100644 --- a/build.rs +++ b/build.rs @@ -1,37 +1,3 @@ -use std::{env, io::BufRead, path::Path, process::{Command, Stdio}}; - -#[derive(Debug)] -struct TrimUnderscoreCallbacks; -impl bindgen::callbacks::ParseCallbacks for TrimUnderscoreCallbacks { - fn item_name(&self, item_info: bindgen::callbacks::ItemInfo) -> Option { - /* - This finally oversets non-suffixed name (like `mjData`) - to/over its original name (like `mjData_`). - */ - item_info.name.strip_suffix('_').map(str::to_owned) - } -} - -#[derive(Debug)] -struct MakeMjnConstantsCallbacks; -impl bindgen::callbacks::ParseCallbacks for MakeMjnConstantsCallbacks { - fn enum_variant_behavior( - &self, - _enum_name: Option<&str>, - original_variant_name: &str, - _variant_value: bindgen::callbacks::EnumVariantValue, - ) -> Option { - /* - This generates const like: - ``` - pub const mjNTEXROLE: mjtTextureRole = mjtTextureRole::mjNTEXROLE; - ``` - at module top for `mjN*` variants. - */ - original_variant_name.starts_with("mjN").then_some(bindgen::callbacks::EnumVariantCustomBehavior::Constify) - } -} - fn main() { if option_env!("DOCS_RS").is_some() { return } @@ -42,24 +8,68 @@ fn main() { * This is a **requirement** for the build script to continue. */ assert!( - Command::new("cargo").args(["help", "fmt"]).stdout(Stdio::null()).status().is_ok_and(|s| s.success()), + std::process::Command::new("cargo") + .args(["help", "fmt"]) + .stdout(std::process::Stdio::null()) + .status() + .is_ok_and(|s| s.success()), "`cargo fmt` is not available; This build script can't continue without it." ); - let src_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); - let bindgen_h = src_dir.join("bindgen.h").to_str().unwrap().to_owned(); - let bindgen_rs = src_dir.join("bindgen.rs").to_str().unwrap().to_owned(); - - println!("cargo:rerun-if-changed={bindgen_h}"); - let mujoco_dir = std::env::var("MUJOCO_DIR").expect("MUJOCO_DIR environment variable is not set"); - let mujoco_dir = Path::new(&mujoco_dir).canonicalize().expect("MUJOCO_DIR is not a valid path"); + let mujoco_dir = std::path::Path::new(&mujoco_dir).canonicalize().expect("MUJOCO_DIR is not a valid path"); let mujoco_lib = mujoco_dir.join("lib").to_str().unwrap().to_owned(); + + println!("cargo:rustc-link-search={mujoco_lib}"); + println!("cargo:rustc-link-lib=dylib=mujoco"); + + #[cfg(feature = "bindgen")] + bindgen(mujoco_dir); +} + +#[cfg(feature = "bindgen")] +fn bindgen(mujoco_dir: impl AsRef) { + #[derive(Debug)] + struct TrimUnderscoreCallbacks; + impl bindgen::callbacks::ParseCallbacks for TrimUnderscoreCallbacks { + fn item_name(&self, item_info: bindgen::callbacks::ItemInfo) -> Option { + /* + This finally oversets non-suffixed name (like `mjData`) + to/over its original name (like `mjData_`). + */ + item_info.name.strip_suffix('_').map(str::to_owned) + } + } + + #[derive(Debug)] + struct MakeMjnConstantsCallbacks; + impl bindgen::callbacks::ParseCallbacks for MakeMjnConstantsCallbacks { + fn enum_variant_behavior( + &self, + _enum_name: Option<&str>, + original_variant_name: &str, + _variant_value: bindgen::callbacks::EnumVariantValue, + ) -> Option { + /* + This generates const like: + ``` + pub const mjNTEXROLE: mjtTextureRole = mjtTextureRole::mjNTEXROLE; + ``` + at module top for `mjN*` variants. + */ + original_variant_name.starts_with("mjN").then_some(bindgen::callbacks::EnumVariantCustomBehavior::Constify) + } + } + + let mujoco_dir = mujoco_dir.as_ref(); let mujoco_include = mujoco_dir.join("include").to_str().unwrap().to_owned(); let mujoco_include_mujoco = mujoco_dir.join("include").join("mujoco").to_str().unwrap().to_owned(); - println!("cargo:rustc-link-search={mujoco_lib}"); - println!("cargo:rustc-link-lib=dylib=mujoco"); + let src_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let bindgen_h = src_dir.join("bindgen.h").to_str().unwrap().to_owned(); + let bindgen_rs = src_dir.join("bindgen.rs").to_str().unwrap().to_owned(); + + println!("cargo:rerun-if-changed={bindgen_h}"); let mut bindings = Vec::new(); bindgen::builder() @@ -98,8 +108,7 @@ fn main() { using bindgen, so we do them manually... */ - let bindings = bindings - .lines() + let bindings = io::BufRead::lines(bindings) .map(Result::unwrap) .fold(Vec::with_capacity(bindings.len()), |mut new, line| { if line.starts_with("pub struct mjt") { From 1e46e9726920da3944474838397182f1aa13c489 Mon Sep 17 00:00:00 2001 From: kanarus Date: Tue, 16 Sep 2025 08:45:56 +0900 Subject: [PATCH 2/3] move rustfmt assertion into `fn bindgen` --- build.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/build.rs b/build.rs index 64dc741..100394c 100644 --- a/build.rs +++ b/build.rs @@ -1,21 +1,6 @@ fn main() { if option_env!("DOCS_RS").is_some() { return } - /* - * The hand-process step after `bindgen` generation assumes that - * `cargo fmt` (and then it's automatically applied to the - * bindgen's raw output, and the hand-processing correctly works). - * This is a **requirement** for the build script to continue. - */ - assert!( - std::process::Command::new("cargo") - .args(["help", "fmt"]) - .stdout(std::process::Stdio::null()) - .status() - .is_ok_and(|s| s.success()), - "`cargo fmt` is not available; This build script can't continue without it." - ); - let mujoco_dir = std::env::var("MUJOCO_DIR").expect("MUJOCO_DIR environment variable is not set"); let mujoco_dir = std::path::Path::new(&mujoco_dir).canonicalize().expect("MUJOCO_DIR is not a valid path"); let mujoco_lib = mujoco_dir.join("lib").to_str().unwrap().to_owned(); @@ -61,6 +46,21 @@ fn bindgen(mujoco_dir: impl AsRef) { } } + /* + * The hand-process step after `bindgen` generation assumes that + * `cargo fmt` (and then it's automatically applied to the + * bindgen's raw output, and the hand-processing correctly works). + * This is a **requirement** for the build script to continue. + */ + assert!( + std::process::Command::new("cargo") + .args(["help", "fmt"]) + .stdout(std::process::Stdio::null()) + .status() + .is_ok_and(|s| s.success()), + "`cargo fmt` is not available; This build script can't continue without it." + ); + let mujoco_dir = mujoco_dir.as_ref(); let mujoco_include = mujoco_dir.join("include").to_str().unwrap().to_owned(); let mujoco_include_mujoco = mujoco_dir.join("include").join("mujoco").to_str().unwrap().to_owned(); From 84f6bd8dd9308846308732f0d65da118f96fdbf3 Mon Sep 17 00:00:00 2001 From: kanarus Date: Tue, 16 Sep 2025 08:51:41 +0900 Subject: [PATCH 3/3] fix --- build.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.rs b/build.rs index 100394c..f8cff2b 100644 --- a/build.rs +++ b/build.rs @@ -47,7 +47,7 @@ fn bindgen(mujoco_dir: impl AsRef) { } /* - * The hand-process step after `bindgen` generation assumes that + * The hand-processing step after `bindgen` generation requires * `cargo fmt` (and then it's automatically applied to the * bindgen's raw output, and the hand-processing correctly works). * This is a **requirement** for the build script to continue. @@ -108,7 +108,7 @@ fn bindgen(mujoco_dir: impl AsRef) { using bindgen, so we do them manually... */ - let bindings = io::BufRead::lines(bindings) + let bindings = std::io::BufRead::lines(&*bindings) .map(Result::unwrap) .fold(Vec::with_capacity(bindings.len()), |mut new, line| { if line.starts_with("pub struct mjt") {