* [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys
@ 2023-05-26 15:27 Erik Schilling
2023-05-26 15:27 ` [PATCH libgpiod v2 1/2] rust: bindings: turn SPDX tags into comments Erik Schilling
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Erik Schilling @ 2023-05-26 15:27 UTC (permalink / raw)
To: Linux-GPIO
Cc: Bartosz Golaszewski, Viresh Kumar, Manos Pitsidianakis,
Alex Bennée, Erik Schilling
As of now, the Rust bindings are only consumable as git dependencies
(and even then come with some restrictions when wanting to control
the build and linkage behaviour).
This series does some cleanup and then proposes a change in how the Rust
bindings are built and linked in order to prepare libgpiod-sys (and thus
also libgpiod) for being packageable via `cargo package` (which is a
prerequisite for eventually publishing to crates.io).
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
---
Changes in v2:
- Added wrapper.h that I forgot in v1 (Thanks Viresh!)
- Rebased on top of the commits that already got merged as part of v1
- Automatically set the right flags when using `make`
- Tweaked the docs (setting the flags is now done automatically, so it
is not as important anymore)
- Link to v1: https://lore.kernel.org/r/20230522-crates-io-v1-0-42eeee775eb6@linaro.org
---
Erik Schilling (2):
rust: bindings: turn SPDX tags into comments
bindings: rust: build against pkg-config info
README | 4 +++-
bindings/rust/gpiosim-sys/README.md | 8 ++++---
bindings/rust/libgpiod-sys/Cargo.toml | 4 ++++
bindings/rust/libgpiod-sys/README.md | 16 +++++++++++---
bindings/rust/libgpiod-sys/build.rs | 40 +++++++++++++++++++++++------------
bindings/rust/libgpiod-sys/wrapper.h | 1 +
bindings/rust/libgpiod/Makefile.am | 8 ++++++-
7 files changed, 59 insertions(+), 22 deletions(-)
---
base-commit: 4687bcc4f48a9894469ee240e0c67c42d56169c3
change-id: 20230522-crates-io-773a0b6b423d
Best regards,
--
Erik Schilling <erik.schilling@linaro.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH libgpiod v2 1/2] rust: bindings: turn SPDX tags into comments
2023-05-26 15:27 [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys Erik Schilling
@ 2023-05-26 15:27 ` Erik Schilling
2023-05-30 16:05 ` Bartosz Golaszewski
2023-05-26 15:27 ` [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info Erik Schilling
2023-05-29 5:01 ` [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys Viresh Kumar
2 siblings, 1 reply; 9+ messages in thread
From: Erik Schilling @ 2023-05-26 15:27 UTC (permalink / raw)
To: Linux-GPIO
Cc: Bartosz Golaszewski, Viresh Kumar, Manos Pitsidianakis,
Alex Bennée, Erik Schilling
In markdown `# Foo` generates a top-level heading. This leads to to some
weird, huge SPDX tags being rendered before the start of the actual
content.
Lacking good examples, I just took the syntax that `reuse addheader`[1]
outputs.
[1] https://github.com/fsfe/reuse-tool
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
---
bindings/rust/gpiosim-sys/README.md | 8 +++++---
bindings/rust/libgpiod-sys/README.md | 8 +++++---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/bindings/rust/gpiosim-sys/README.md b/bindings/rust/gpiosim-sys/README.md
index 6cd24d9..b13f09a 100644
--- a/bindings/rust/gpiosim-sys/README.md
+++ b/bindings/rust/gpiosim-sys/README.md
@@ -1,6 +1,8 @@
-# SPDX-License-Identifier: CC0-1.0
-# SPDX-FileCopyrightText: 2022 Linaro Ltd.
-# SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
+<!--
+SPDX-License-Identifier: CC0-1.0
+SPDX-FileCopyrightText: 2022 Linaro Ltd.
+SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
+-->
# Generated gpiosim Rust FFI bindings
Automatically generated Rust FFI bindings via
diff --git a/bindings/rust/libgpiod-sys/README.md b/bindings/rust/libgpiod-sys/README.md
index 3eb5c9d..1cb3b0a 100644
--- a/bindings/rust/libgpiod-sys/README.md
+++ b/bindings/rust/libgpiod-sys/README.md
@@ -1,6 +1,8 @@
-# SPDX-License-Identifier: CC0-1.0
-# SPDX-FileCopyrightText: 2022 Linaro Ltd.
-# SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
+<!--
+SPDX-License-Identifier: CC0-1.0
+SPDX-FileCopyrightText: 2022 Linaro Ltd.
+SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
+-->
# Generated libgpiod-sys Rust FFI bindings
Automatically generated Rust FFI bindings via
--
2.40.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info
2023-05-26 15:27 [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys Erik Schilling
2023-05-26 15:27 ` [PATCH libgpiod v2 1/2] rust: bindings: turn SPDX tags into comments Erik Schilling
@ 2023-05-26 15:27 ` Erik Schilling
2023-05-30 16:04 ` Bartosz Golaszewski
2023-05-29 5:01 ` [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys Viresh Kumar
2 siblings, 1 reply; 9+ messages in thread
From: Erik Schilling @ 2023-05-26 15:27 UTC (permalink / raw)
To: Linux-GPIO
Cc: Bartosz Golaszewski, Viresh Kumar, Manos Pitsidianakis,
Alex Bennée, Erik Schilling
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys
2023-05-26 15:27 [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys Erik Schilling
2023-05-26 15:27 ` [PATCH libgpiod v2 1/2] rust: bindings: turn SPDX tags into comments Erik Schilling
2023-05-26 15:27 ` [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info Erik Schilling
@ 2023-05-29 5:01 ` Viresh Kumar
2 siblings, 0 replies; 9+ messages in thread
From: Viresh Kumar @ 2023-05-29 5:01 UTC (permalink / raw)
To: Erik Schilling
Cc: Linux-GPIO, Bartosz Golaszewski, Manos Pitsidianakis, Alex Bennée
On 26-05-23, 17:27, Erik Schilling wrote:
> As of now, the Rust bindings are only consumable as git dependencies
> (and even then come with some restrictions when wanting to control
> the build and linkage behaviour).
>
> This series does some cleanup and then proposes a change in how the Rust
> bindings are built and linked in order to prepare libgpiod-sys (and thus
> also libgpiod) for being packageable via `cargo package` (which is a
> prerequisite for eventually publishing to crates.io).
>
> Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
> ---
> Changes in v2:
> - Added wrapper.h that I forgot in v1 (Thanks Viresh!)
> - Rebased on top of the commits that already got merged as part of v1
> - Automatically set the right flags when using `make`
> - Tweaked the docs (setting the flags is now done automatically, so it
> is not as important anymore)
> - Link to v1: https://lore.kernel.org/r/20230522-crates-io-v1-0-42eeee775eb6@linaro.org
>
> ---
> Erik Schilling (2):
> rust: bindings: turn SPDX tags into comments
> bindings: rust: build against pkg-config info
>
> README | 4 +++-
> bindings/rust/gpiosim-sys/README.md | 8 ++++---
> bindings/rust/libgpiod-sys/Cargo.toml | 4 ++++
> bindings/rust/libgpiod-sys/README.md | 16 +++++++++++---
> bindings/rust/libgpiod-sys/build.rs | 40 +++++++++++++++++++++++------------
> bindings/rust/libgpiod-sys/wrapper.h | 1 +
> bindings/rust/libgpiod/Makefile.am | 8 ++++++-
> 7 files changed, 59 insertions(+), 22 deletions(-)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info
2023-05-26 15:27 ` [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info Erik Schilling
@ 2023-05-30 16:04 ` Bartosz Golaszewski
2023-05-30 16:27 ` Manos Pitsidianakis
0 siblings, 1 reply; 9+ messages in thread
From: Bartosz Golaszewski @ 2023-05-30 16:04 UTC (permalink / raw)
To: Erik Schilling
Cc: Linux-GPIO, Bartosz Golaszewski, Viresh Kumar,
Manos Pitsidianakis, Alex Bennée
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH libgpiod v2 1/2] rust: bindings: turn SPDX tags into comments
2023-05-26 15:27 ` [PATCH libgpiod v2 1/2] rust: bindings: turn SPDX tags into comments Erik Schilling
@ 2023-05-30 16:05 ` Bartosz Golaszewski
0 siblings, 0 replies; 9+ messages in thread
From: Bartosz Golaszewski @ 2023-05-30 16:05 UTC (permalink / raw)
To: Erik Schilling
Cc: Linux-GPIO, Bartosz Golaszewski, Viresh Kumar,
Manos Pitsidianakis, Alex Bennée
On Fri, May 26, 2023 at 5:28 PM Erik Schilling
<erik.schilling@linaro.org> wrote:
>
> In markdown `# Foo` generates a top-level heading. This leads to to some
> weird, huge SPDX tags being rendered before the start of the actual
> content.
>
> Lacking good examples, I just took the syntax that `reuse addheader`[1]
> outputs.
>
> [1] https://github.com/fsfe/reuse-tool
>
> Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
> ---
> bindings/rust/gpiosim-sys/README.md | 8 +++++---
> bindings/rust/libgpiod-sys/README.md | 8 +++++---
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/bindings/rust/gpiosim-sys/README.md b/bindings/rust/gpiosim-sys/README.md
> index 6cd24d9..b13f09a 100644
> --- a/bindings/rust/gpiosim-sys/README.md
> +++ b/bindings/rust/gpiosim-sys/README.md
> @@ -1,6 +1,8 @@
> -# SPDX-License-Identifier: CC0-1.0
> -# SPDX-FileCopyrightText: 2022 Linaro Ltd.
> -# SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
> +<!--
> +SPDX-License-Identifier: CC0-1.0
> +SPDX-FileCopyrightText: 2022 Linaro Ltd.
> +SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
> +-->
>
> # Generated gpiosim Rust FFI bindings
> Automatically generated Rust FFI bindings via
> diff --git a/bindings/rust/libgpiod-sys/README.md b/bindings/rust/libgpiod-sys/README.md
> index 3eb5c9d..1cb3b0a 100644
> --- a/bindings/rust/libgpiod-sys/README.md
> +++ b/bindings/rust/libgpiod-sys/README.md
> @@ -1,6 +1,8 @@
> -# SPDX-License-Identifier: CC0-1.0
> -# SPDX-FileCopyrightText: 2022 Linaro Ltd.
> -# SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
> +<!--
> +SPDX-License-Identifier: CC0-1.0
> +SPDX-FileCopyrightText: 2022 Linaro Ltd.
> +SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@linaro.org>
> +-->
>
> # Generated libgpiod-sys Rust FFI bindings
> Automatically generated Rust FFI bindings via
>
> --
> 2.40.0
>
Applied. And this made me think I should convert other txt files to markdown...
Bart
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info
2023-05-30 16:04 ` Bartosz Golaszewski
@ 2023-05-30 16:27 ` Manos Pitsidianakis
2023-05-30 19:04 ` Bartosz Golaszewski
0 siblings, 1 reply; 9+ messages in thread
From: Manos Pitsidianakis @ 2023-05-30 16:27 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Erik Schilling, Linux-GPIO, Bartosz Golaszewski, Viresh Kumar,
Alex Bennée
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info
2023-05-30 16:27 ` Manos Pitsidianakis
@ 2023-05-30 19:04 ` Bartosz Golaszewski
2023-05-31 6:17 ` Erik Schilling
0 siblings, 1 reply; 9+ messages in thread
From: Bartosz Golaszewski @ 2023-05-30 19:04 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Erik Schilling, Linux-GPIO, Bartosz Golaszewski, Viresh Kumar,
Alex Bennée
On Tue, May 30, 2023 at 6:27 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> 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.
>
Ah, got it. Ok, now applied.
Bart
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info
2023-05-30 19:04 ` Bartosz Golaszewski
@ 2023-05-31 6:17 ` Erik Schilling
0 siblings, 0 replies; 9+ messages in thread
From: Erik Schilling @ 2023-05-31 6:17 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linux-GPIO, Bartosz Golaszewski, Viresh Kumar, Alex Bennée,
Manos Pitsidianakis
On 5/30/23 21:04, Bartosz Golaszewski wrote:
> Ah, got it. Ok, now applied.
Thanks for merging! I will start a separate thread on how to get this
onto crates.io :).
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-05-31 6:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-26 15:27 [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys Erik Schilling
2023-05-26 15:27 ` [PATCH libgpiod v2 1/2] rust: bindings: turn SPDX tags into comments Erik Schilling
2023-05-30 16:05 ` Bartosz Golaszewski
2023-05-26 15:27 ` [PATCH libgpiod v2 2/2] bindings: rust: build against pkg-config info Erik Schilling
2023-05-30 16:04 ` Bartosz Golaszewski
2023-05-30 16:27 ` Manos Pitsidianakis
2023-05-30 19:04 ` Bartosz Golaszewski
2023-05-31 6:17 ` Erik Schilling
2023-05-29 5:01 ` [PATCH libgpiod v2 0/2] bindings: rust: allow packaging of libgpiod-sys Viresh Kumar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.