| Message ID | 20230522-crates-io-v1-0-42eeee775eb6@linaro.org |
|---|---|
| Headers | show |
| Series | bindings: rust: allow packaging of libgpiod-sys | expand |
On 23-05-23, 13:25, Erik Schilling wrote: > This is a relict from old Rust standards and no longer requires [1]. > > [1] https://doc.rust-lang.org/edition-guide/rust-2018/path-changes.html#no-more-extern-crate > > Signed-off-by: Erik Schilling <erik.schilling@linaro.org> > --- > bindings/rust/libgpiod-sys/build.rs | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/bindings/rust/libgpiod-sys/build.rs b/bindings/rust/libgpiod-sys/build.rs > index e3ed04a..b1333f1 100644 > --- a/bindings/rust/libgpiod-sys/build.rs > +++ b/bindings/rust/libgpiod-sys/build.rs > @@ -2,8 +2,6 @@ > // SPDX-FileCopyrightText: 2022 Linaro Ltd. > // SPDX-FileCopyrightTest: 2022 Viresh Kumar <viresh.kumar@linaro.org> > > -extern crate bindgen; > - > use std::env; > use std::path::PathBuf; Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On 5/23/23 13:25, Erik Schilling 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. > > 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(-) Viresh told me on IRC that I forgot the wrapper.h: 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>
On 23-05-23, 13:25, 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 (hopefully) cleanup and then proposes a change > in how the Rust bindings are built and linked. > > Since the changes may require people hacking on the bindings to set some > additional environment variables (at least if they want to avoid running > make install), I am sending this as an RFC in order > to hear opinions. > > For gpiosim-sys the situation is slightly more complex. Right now, > libgpiosim installs without a pkg-config. If it is desireable to add > one, that could be done and the same mechanism could be used. Otherwise, > if packaging that lib is desirable (it looks like it?), we could either > still query for libgpiod (and hope that the linker and include paths are > matching) or need some other way to acquire the linker and include paths > (and flags). > > So... The open questions: > - Is this OK at all? Are people depending on this building against > relative paths? Not sure if the build of the libgpiod git repo depends on that relative path, did you try to do `make` in the libgpiod directory ? Like: $ ./autogen.sh --enable-tools=yes --enable-bindings-rust --enable-examples --enable-tests; make > - What to do with gpiosim-sys (see above)? The only user for the cargo tests for libgpiod stuff is for the vhost-device crate with the help of rust-vmm containers. I am installing libgpiod there currently and so it works, not sure of how it may be required to be used later on though. > - Is there interest into getting this published on crates.io after > packaging is fixed? Yes.
On Fri, 26 May 2023 at 10:30, Erik Schilling <erik.schilling@linaro.org> wrote: > > > > >>> I am not exactly sure if I understood the above comment correctly. But > >>> if we want to eventually be able to consume gpiosim-sys via crates.io > >>> (or any packaging mechanism that relies on cargo package), then we will > >>> need to decouple the header and .so file referencing in a similar way. > >>> The easiest solution for me seems to be to just add a pkg-config file > >>> for gpiosim and use the same mechanism that I sketched for libgpiod-sys > >>> here. > >> > >> Yes we would like to get gpiosim via crates.io as well. > > > > Would simply adding a pkg-config for the gpiosim C lib be desirable? > > Then we can use the same mechanism. There is none existing at the > > moment. I am not sure whether that is intentional or just not done yet. > > @Bartosz: Viresh said on IRC that I should ask you about this :). Shall > I just add a pkg-config for gpiosim? Or is that not desired for some reason? > > - Erik libgpiosim was not meant to be installed for development, that's why there's no pkg-config for it. We don't install the header and only install the shared object if tests are enabled but with the expectation that the tests were built in-tree. I also don't quite get why we'd want to get gpiosim via crates.io - I would prefer to use the one in the tree. Or am I not understanding something here right? Bart
On 5/26/23 10:36, Bartosz Golaszewski wrote: > On Fri, 26 May 2023 at 10:30, Erik Schilling <erik.schilling@linaro.org> wrote: >> >> >> >>>>> I am not exactly sure if I understood the above comment correctly. But >>>>> if we want to eventually be able to consume gpiosim-sys via crates.io >>>>> (or any packaging mechanism that relies on cargo package), then we will >>>>> need to decouple the header and .so file referencing in a similar way. >>>>> The easiest solution for me seems to be to just add a pkg-config file >>>>> for gpiosim and use the same mechanism that I sketched for libgpiod-sys >>>>> here. >>>> >>>> Yes we would like to get gpiosim via crates.io as well. >>> >>> Would simply adding a pkg-config for the gpiosim C lib be desirable? >>> Then we can use the same mechanism. There is none existing at the >>> moment. I am not sure whether that is intentional or just not done yet. >> >> @Bartosz: Viresh said on IRC that I should ask you about this :). Shall >> I just add a pkg-config for gpiosim? Or is that not desired for some reason? > > libgpiosim was not meant to be installed for development, that's why > there's no pkg-config for it. We don't install the header and only > install the shared object if tests are enabled but with the > expectation that the tests were built in-tree. Ah. Did not catch that the header is not installed. > I also don't quite get why we'd want to get gpiosim via crates.io - I > would prefer to use the one in the tree. Or am I not understanding > something here right? I am also only seeing the libgpiod package using it as dev-dependency. If that is the only consumer and we do not want to expose it to out-of-tree things then that should be fine and we can keep building that one against the "in-tree" artifacts for it. @Viresh: Could you comment on the use-case that you had in mind with gpiosim being on crates.io? I am not seeing any good options to package it if it is only intended as an testing tool for in-tree things. - Erik
On 26-05-23, 10:59, Erik Schilling wrote: > @Viresh: Could you comment on the use-case that you had in mind with gpiosim > being on crates.io? I am not seeing any good options to package it if it is > only intended as an testing tool for in-tree things. I thought vhost-device is already using it for testing, I was wrong :( So we don't need it via crates.io for now at least.
On 5/24/23 11:17, Bartosz Golaszewski wrote:
> I applied the first two patches, I'll wait for the respon of 3/3.
Thanks!
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 (hopefully) cleanup and then proposes a change in how the Rust bindings are built and linked. Since the changes may require people hacking on the bindings to set some additional environment variables (at least if they want to avoid running make install), I am sending this as an RFC in order to hear opinions. For gpiosim-sys the situation is slightly more complex. Right now, libgpiosim installs without a pkg-config. If it is desireable to add one, that could be done and the same mechanism could be used. Otherwise, if packaging that lib is desirable (it looks like it?), we could either still query for libgpiod (and hope that the linker and include paths are matching) or need some other way to acquire the linker and include paths (and flags). So... The open questions: - Is this OK at all? Are people depending on this building against relative paths? - What to do with gpiosim-sys (see above)? - Is there interest into getting this published on crates.io after packaging is fixed? Signed-off-by: Erik Schilling <erik.schilling@linaro.org> --- Erik Schilling (3): bindings: rust: drop legacy extern crate syntax bindings: rust: remove unneeded cc dependency bindings: rust: build against pkg-config info README | 13 ++++++++++- bindings/rust/libgpiod-sys/Cargo.toml | 5 ++++- bindings/rust/libgpiod-sys/build.rs | 42 ++++++++++++++++++++++------------- 3 files changed, 42 insertions(+), 18 deletions(-) --- base-commit: 0a51d62f060dbc1b036dfd45e52d4d90f0ce3eeb change-id: 20230522-crates-io-773a0b6b423d Best regards,