Hi Gerard,

On 17-12-21, 11:29, Gerard Ryan wrote:

Hello,

I submitted https://lore.kernel.org/all/CAKycSdDMxfto6oTqt06TbJxXY=S7p_gtEXWDQv8mz0d9zt3Gvw@mail.gmail.com/ (local)
and my attention was drawn here and have a few comments.

Firstly, I was wondering why you didn't create a separate *-sys crate
for these bindings?
see https://doc.rust-lang.org/cargo/reference/build-scripts.html#-sys-packages
for more information.

I wasn't aware of it :(

I think yes this should be modified to a sys-crate, followed by wrapper crate to
contain the wrappers around it.

Secondly, I noticed when developing my aforementioned, patch that
`bindgen` adds quite a few dependencies that probably aren't needed by
the average consumer of this crate.
So I was wondering what are your thoughts about generating and
committing a bindings.rs then optionally using these dependencies via
a feature flag?

I don't have a strong preference either way, whatever works best.

Miguel, any suggestions ?

Lastly, With your `make` integration, it looks like we could also
remove the `cc` dependency by allowing `make` to build libgpiod
instead and just linking with that, instead of compiling libgpiod
twice.

I agree, that would be better. It wasn't integrated with Make earlier and so I
had to do it separately. But I may have some problem with it:

This is the vhost-device (gpio virtio) crate where I am using these bindings and
have the libgpiod as a dependency:

https://github.com/vireshk/vhost-device/blob/gpio/irq/src/gpio/Cargo.toml#L18

When I do a cargo build there (for vhost-device crate), it will try to build the
dependencies as well, i.e. libgpiod, and I need to build the libgpiod's C files
as well there. There are good chances that I need to build from source and
libgpiod isn't installed there. How do I do it with Make ?

-- 
viresh