Re: [PATCH V4 3/8] libgpiod-sys: Add support to generate gpiosim bindings

[Kent Gibson <warthog618@xxxxxxxxx>]

On Fri, Jul 08, 2022 at 05:04:56PM +0530, Viresh Kumar wrote:
> Add support to generate gpiosim bindings.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> ---
>  bindings/rust/libgpiod-sys/Cargo.toml        |  1 +
>  bindings/rust/libgpiod-sys/build.rs          | 19 +++++++++++++++++--
>  bindings/rust/libgpiod-sys/gpiosim_wrapper.h |  1 +
>  3 files changed, 19 insertions(+), 2 deletions(-)
>  create mode 100644 bindings/rust/libgpiod-sys/gpiosim_wrapper.h
> 
> diff --git a/bindings/rust/libgpiod-sys/Cargo.toml b/bindings/rust/libgpiod-sys/Cargo.toml
> index 77f82719d269..73b6761d16dd 100644
> --- a/bindings/rust/libgpiod-sys/Cargo.toml
> +++ b/bindings/rust/libgpiod-sys/Cargo.toml
> @@ -9,6 +9,7 @@ edition = "2018"
>  
>  [features]
>  generate = [ "bindgen" ]
> +gpiosim = [ "generate", "bindgen" ]
>  

Should gpiosim be a feature or a separate library/crate?
I would expect it to be a separate crate and for libgpiod to have a
dev-dependency on it (it is only required for tests, right?).

>  [build-dependencies]
>  bindgen = { version = "0.59.1", optional = true }
> diff --git a/bindings/rust/libgpiod-sys/build.rs b/bindings/rust/libgpiod-sys/build.rs
> index bbcd30f79d23..147daaf6b1da 100644
> --- a/bindings/rust/libgpiod-sys/build.rs
> +++ b/bindings/rust/libgpiod-sys/build.rs
> @@ -14,13 +14,25 @@ fn generate_bindings(files: &Vec<&str>) {
>          println!("cargo:rerun-if-changed={}", file);
>      }
>  
> +    if cfg!(feature = "gpiosim") {
> +        println!("cargo:rerun-if-changed=gpiosim_wrapper.h");
> +    }
> +
>      // The bindgen::Builder is the main entry point
>      // to bindgen, and lets you build up options for
>      // the resulting bindings.
> -    let bindings = bindgen::Builder::default()
> +    let mut builder = bindgen::Builder::default()
>          // The input header we would like to generate
>          // bindings for.
> -        .header("wrapper.h")
> +        .header("wrapper.h");
> +
> +    if cfg!(feature = "gpiosim") {
> +        builder = builder.header("gpiosim_wrapper.h");
> +        println!("cargo:rustc-link-lib=kmod");
> +        println!("cargo:rustc-link-lib=mount");
> +    }
> +
> +    let bindings = builder
>          // Tell cargo to invalidate the built crate whenever any of the
>          // included header files changed.
>          .parse_callbacks(Box::new(bindgen::CargoCallbacks))
> @@ -46,6 +58,7 @@ fn build_gpiod(files: Vec<&str>) {
>          .define("_GNU_SOURCE", None)
>          .define("GPIOD_VERSION_STR", "\"libgpio-sys\"")
>          .include("../../../include")
> +        .include("/usr/include/libmount")
>          .compile("gpiod");
>  }
>  
> @@ -61,6 +74,8 @@ fn main() {
>          "../../../lib/line-request.c",
>          "../../../lib/misc.c",
>          "../../../lib/request-config.c",
> +        #[cfg(feature = "gpiosim")]
> +        "../../../tests/gpiosim/gpiosim.c",
>      ];
>  
>      #[cfg(feature = "generate")]
> diff --git a/bindings/rust/libgpiod-sys/gpiosim_wrapper.h b/bindings/rust/libgpiod-sys/gpiosim_wrapper.h
> new file mode 100644
> index 000000000000..47dc12a87917
> --- /dev/null
> +++ b/bindings/rust/libgpiod-sys/gpiosim_wrapper.h
> @@ -0,0 +1 @@
> +#include "../../../tests/gpiosim/gpiosim.h"
> -- 

Why bother with this wrapper - just bindgen that header directly?

Cheers,
Kent.