diff mbox series

[libgpiod,RFC,3/3] bindings: rust: build against pkg-config info

Message ID 20230522-crates-io-v1-3-42eeee775eb6@linaro.org
State New
Headers show
Series bindings: rust: allow packaging of libgpiod-sys | expand

Commit Message

Erik Schilling May 23, 2023, 11:25 a.m. UTC
This change replaces building against "bundled" headers by always
building agains system headers (while following standard conventions to
allow users to specify the version to build against).

Reasoning:

Previously, the code generated the bindings based on the headers, but
then links against `-lgpiod` without further specifying where that is
coming from.

This results in some challenges and problems:

1. Packaging a Rust crate with `cargo package` requires the folder
   containing the Cargo.toml to be self-contained. Essentially, a tar
   ball with all the sources of that folder is created. Building against
   that tar ball fails, since the headers files passed to bindgen are
   a relative path pointing outside of that folder.

2. While, for example, the cxx bindings are built AND linked against
   the build results, the packaging situation for C++ libraries is a
   bit different compared to Rust libs. The C++ libs will likely get
   built as part of the larger libgpiod build and published together
   with the C variant.

   In Rust, the vast majority of people will want to build the glue-code
   during the compilation of the applications that consume this lib.

   This may lead to inconsistencies between the bundled headers and the
   libraries shipped by the user's distro. While ABI should hopefully
   be forward-compatible within the same MAJOR number of the .so,
   using too new headers will likely quickly lead to mismatches with
   symbols defined in the lib.

3. Trying to build the core lib as part of the Rust build quickly runs
   into similar packaging issues as the existing solution. The source
   code of the C lib would need to become part of some package
   (often people opt to pull it in as a submodule under their -sys crate
   or even create a separate -src package [1]). This clearly does not
   work well with the current setup...

Since building against system libs is probably? what 90%+ of the people
want, this change hopefully addresses the problems above. The
system-deps dependency honors pkg-config conventions, but also allows
users flexible ways to override the defaults [2]. Overall, this keeps
things simple while still allowing maximum flexibility.

Since the pkg-config interface is just telling us which include paths to
use, we switch back to a wrapper.h file that includes the real gpiod.h.

Once Rust bindings require a lower version floor, the version metadata
can also be updated to help telling users that their system library is
too old.

Drawback:

People hacking on the Rust bindings, need to either have a reasonably
up-to-date system lib, previously install the lib to some folder and
specify PKG_CONFIG_PATH or set the relevant SYSTEM_DEPS_* environment
variables. Instructions for developers are documented in the README.

[1] https://github.com/alexcrichton/openssl-src-rs
[2] https://docs.rs/system-deps/latest/system_deps/#overriding-build-flags

Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
---
 README                                | 13 +++++++++++-
 bindings/rust/libgpiod-sys/Cargo.toml |  4 ++++
 bindings/rust/libgpiod-sys/build.rs   | 40 +++++++++++++++++++++++------------
 3 files changed, 42 insertions(+), 15 deletions(-)
diff mbox series

Patch

diff --git a/README b/README
index b71739e..8780b09 100644
--- a/README
+++ b/README
@@ -218,7 +218,18 @@  the PYTHON_CPPFLAGS and PYTHON_LIBS variables in order to point the build
 system to the correct locations. During native builds, the configure script
 can auto-detect the location of the development files.
 
-Rust bindings require cargo support.
+Rust bindings require cargo support. Additionally, a compatible variant of the C
+library needs to detectable using pkg-config. Alternatively, one can inform the
+build system about the location of the libs and headers by setting environment
+variables. For example, after building the C lib, one should be able to build
+the bindings using:
+
+    cd bindings/rust/libgpiod-sys/
+    env SYSTEM_DEPS_LIBGPIOD_NO_PKG_CONFIG=1 \
+        SYSTEM_DEPS_LIBGPIOD_SEARCH_NATIVE="../../../.libs/" \
+        SYSTEM_DEPS_LIBGPIOD_LIB=gpiod \
+        SYSTEM_DEPS_LIBGPIOD_INCLUDE="../../../include/" \
+        cargo build
 
 TESTING
 -------
diff --git a/bindings/rust/libgpiod-sys/Cargo.toml b/bindings/rust/libgpiod-sys/Cargo.toml
index cb8dc70..d945e02 100644
--- a/bindings/rust/libgpiod-sys/Cargo.toml
+++ b/bindings/rust/libgpiod-sys/Cargo.toml
@@ -18,3 +18,7 @@  edition = "2021"
 
 [build-dependencies]
 bindgen = "0.63"
+system-deps = "2.0"
+
+[package.metadata.system-deps]
+libgpiod = "2"
diff --git a/bindings/rust/libgpiod-sys/build.rs b/bindings/rust/libgpiod-sys/build.rs
index b1333f1..189a12a 100644
--- a/bindings/rust/libgpiod-sys/build.rs
+++ b/bindings/rust/libgpiod-sys/build.rs
@@ -1,25 +1,44 @@ 
 // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause
-// SPDX-FileCopyrightText: 2022 Linaro Ltd.
+// SPDX-FileCopyrightText: 2022-2023 Linaro Ltd.
 // SPDX-FileCopyrightTest: 2022 Viresh Kumar <viresh.kumar@linaro.org>
+// SPDX-FileCopyrightText: 2023 Erik Schilling <erik.schilling@linaro.org>
 
 use std::env;
 use std::path::PathBuf;
 
-fn generate_bindings() {
+fn main() {
+    // Probe dependency info based on the metadata from Cargo.toml
+    // (and potentially other sources like environment, pkg-config, ...)
+    // https://docs.rs/system-deps/latest/system_deps/#overriding-build-flags
+    let libs = system_deps::Config::new().probe().unwrap();
+
     // Tell cargo to invalidate the built crate whenever following files change
-    println!("cargo:rerun-if-changed=../../../include/gpiod.h");
+    println!("cargo:rerun-if-changed=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("../../../include/gpiod.h")
+        .header("wrapper.h")
         // Tell cargo to invalidate the built crate whenever any of the
         // included header files changed.
-        .parse_callbacks(Box::new(bindgen::CargoCallbacks))
-        // Finish the builder and generate the bindings.
+        .parse_callbacks(Box::new(bindgen::CargoCallbacks));
+
+    // Inform bindgen about the include paths identified by system_deps.
+    for (_name, lib) in libs {
+        for include_path in lib.include_paths {
+            builder = builder.clang_arg("-I").clang_arg(
+                include_path
+                    .to_str()
+                    .expect("Failed to convert include_path to &str!"),
+            );
+        }
+    }
+
+    // Finish the builder and generate the bindings.
+    let bindings = builder
         .generate()
         // Unwrap the Result and panic on failure.
         .expect("Unable to generate bindings");
@@ -30,10 +49,3 @@  fn generate_bindings() {
         .write_to_file(out_path.join("bindings.rs"))
         .expect("Couldn't write bindings!");
 }
-
-fn main() {
-    generate_bindings();
-
-    println!("cargo:rustc-link-search=./../../lib/.libs/");
-    println!("cargo:rustc-link-lib=gpiod");
-}