Re: [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

Hello Bart,

this error means pkg-config could not find libgpiod. Erik changed the
linking logic, so now instead of linking a local copy of the library
it lets pkg-config find it. But you haven't installed it locally.

Running with these flags from a makefile in the commit:

command = SYSTEM_DEPS_LIBGPIOD_NO_PKG_CO
NFIG=1 \
+
SYSTEM_DEPS_LIBGPIOD_SEARCH_NATIVE="${PWD}/../../../lib/.libs/" \
+               SYSTEM_DEPS_LIBGPIOD_LIB=gpiod \
+               SYSTEM_DEPS_LIBGPIOD_INCLUDE="${PWD}/../../../include/"  \
+               cargo build --release --lib

Should work.

On Tue, 30 May 2023 at 19:04, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Fri, May 26, 2023 at 5:28 PM Erik Schilling
> <erik.schilling@linaro.org> wrote:
> >
> > 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.
> >
> > In order to support people hacking on the Rust bindings, building with
> > make will automatically set the right set of environment variables.
> > In case people want to customize it when building without `make`, a
> > reference to the system_deps documentation is given in the README.md.
> >
> > [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                                |  4 +++-
> >  bindings/rust/libgpiod-sys/Cargo.toml |  4 ++++
> >  bindings/rust/libgpiod-sys/README.md  |  8 +++++++
> >  bindings/rust/libgpiod-sys/build.rs   | 40 +++++++++++++++++++++++------------
> >  bindings/rust/libgpiod-sys/wrapper.h  |  1 +
> >  bindings/rust/libgpiod/Makefile.am    |  8 ++++++-
> >  6 files changed, 49 insertions(+), 16 deletions(-)
> >
> > diff --git a/README b/README
> > index 85b6300..5764eee 100644
> > --- a/README
> > +++ b/README
> > @@ -218,7 +218,9 @@ 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. When building the Rust bindings along the
> > +C library using make, they will be automatically configured to build against the
> > +build results of the C library.
> >
> >  TESTING
> >  -------
> > diff --git a/bindings/rust/libgpiod-sys/Cargo.toml b/bindings/rust/libgpiod-sys/Cargo.toml
> > index 2b20fa6..8b17039 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/README.md b/bindings/rust/libgpiod-sys/README.md
> > index 1cb3b0a..90198d8 100644
> > --- a/bindings/rust/libgpiod-sys/README.md
> > +++ b/bindings/rust/libgpiod-sys/README.md
> > @@ -8,6 +8,14 @@ SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
> >  Automatically generated Rust FFI bindings via
> >         [bindgen](https://github.com/rust-lang/rust-bindgen).
> >
> > +## Build requirements
> > +
> > +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. The mechanism for that is
> > +documented in the
> > +[system_deps crate documentation](https://docs.rs/system-deps/6.1.0/system_deps/#overriding-build-flags).
> > +
> >  ## License
> >
> >  This project is licensed under either of
> > diff --git a/bindings/rust/libgpiod-sys/build.rs b/bindings/rust/libgpiod-sys/build.rs
> > index 0ac2730..9e6a93c 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-FileCopyrightText: 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");
> > -}
> > diff --git a/bindings/rust/libgpiod-sys/wrapper.h b/bindings/rust/libgpiod-sys/wrapper.h
> > new file mode 100644
> > index 0000000..8a8bd41
> > --- /dev/null
> > +++ b/bindings/rust/libgpiod-sys/wrapper.h
> > @@ -0,0 +1 @@
> > +#include <gpiod.h>
> > diff --git a/bindings/rust/libgpiod/Makefile.am b/bindings/rust/libgpiod/Makefile.am
> > index e9a10c1..92edbfc 100644
> > --- a/bindings/rust/libgpiod/Makefile.am
> > +++ b/bindings/rust/libgpiod/Makefile.am
> > @@ -2,7 +2,13 @@
> >  # SPDX-FileCopyrightText: 2022 Linaro Ltd.
> >  # SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > -command = cargo build --release --lib
> > +# We do not want to build against the system libs when building with make. So we
> > +# specify the paths to the build directory of the C lib.
> > +command = SYSTEM_DEPS_LIBGPIOD_NO_PKG_CONFIG=1 \
> > +               SYSTEM_DEPS_LIBGPIOD_SEARCH_NATIVE="${PWD}/../../../lib/.libs/" \
> > +               SYSTEM_DEPS_LIBGPIOD_LIB=gpiod \
> > +               SYSTEM_DEPS_LIBGPIOD_INCLUDE="${PWD}/../../../include/"  \
> > +               cargo build --release --lib
> >
> >  if WITH_TESTS
> >  command += --tests
> >
> > --
> > 2.40.0
> >
>
> When I build the rust bindings in-tree and run them like:
>
> sudo LD_LIBRARY_PATH=<snip>/libgpiod/lib/.libs/
> CARGO_TARGET_DIR=/tmp/libgpiod-rust /root/.cargo/bin/cargo test
>
> I get
>
> error: failed to run custom build command for `libgpiod-sys v0.1.0
> (<snip>/libgpiod/bindings/rust/libgpiod-sys)`
>
> Caused by:
>   process didn't exit successfully:
> `/tmp/libgpiod-rust/debug/build/libgpiod-sys-1d9e7999a2f148d2/build-script-build`
> (exit status: 101)
>   --- stdout
>   cargo:rerun-if-env-changed=LIBGPIOD_NO_PKG_CONFIG
>   cargo:rerun-if-env-changed=PKG_CONFIG_x86_64-unknown-linux-gnu
>   cargo:rerun-if-env-changed=PKG_CONFIG_x86_64_unknown_linux_gnu
>   cargo:rerun-if-env-changed=HOST_PKG_CONFIG
>   cargo:rerun-if-env-changed=PKG_CONFIG
>   cargo:rerun-if-env-changed=LIBGPIOD_STATIC
>   cargo:rerun-if-env-changed=LIBGPIOD_DYNAMIC
>   cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC
>   cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC
>   cargo:rerun-if-env-changed=PKG_CONFIG_PATH_x86_64-unknown-linux-gnu
>   cargo:rerun-if-env-changed=PKG_CONFIG_PATH_x86_64_unknown_linux_gnu
>   cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH
>   cargo:rerun-if-env-changed=PKG_CONFIG_PATH
>   cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_x86_64-unknown-linux-gnu
>   cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_x86_64_unknown_linux_gnu
>   cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR
>   cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR
>   cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_x86_64-unknown-linux-gnu
>   cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_x86_64_unknown_linux_gnu
>   cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR
>   cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR
>
>   --- stderr
>   thread 'main' panicked at 'called `Result::unwrap()` on an `Err`
> value: PkgConfig(`"pkg-config" "--libs" "--cflags" "libgpiod"
> "libgpiod >= 2"` did not exit successfully: exit status: 1
>   error: could not find system library 'libgpiod' required by the
> 'libgpiod-sys' crate
>
>   --- stderr
>   Package libgpiod was not found in the pkg-config search path.
>   Perhaps you should add the directory containing `libgpiod.pc'
>   to the PKG_CONFIG_PATH environment variable
>   Package 'libgpiod', required by 'virtual:world', not found
>   Package 'libgpiod', required by 'virtual:world', not found
>   )', libgpiod-sys/build.rs:13:51
>   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
>
> What am I doing wrong?
>
> Bart