Re: [PATCH V4 2/8] libgpiod: Add pre generated rust bindings

From: Viresh Kumar <viresh.kumar@linaro.org>
To: Kent Gibson <warthog618@gmail.com>
Cc: "Linus Walleij" <linus.walleij@linaro.org>,
	"Bartosz Golaszewski" <brgl@bgdev.pl>,
	"Vincent Guittot" <vincent.guittot@linaro.org>,
	linux-gpio@vger.kernel.org,
	"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@google.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	stratos-dev@op-lists.linaro.org,
	"Gerard Ryan" <g.m0n3y.2503@gmail.com>
Subject: Re: [PATCH V4 2/8] libgpiod: Add pre generated rust bindings
Date: Wed, 27 Jul 2022 10:45:29 +0530	[thread overview]
Message-ID: <20220727051529.becrq3qecbild2iq@vireshk-i7> (raw)
In-Reply-To: <20220727025723.GB88787@sol>

On 27-07-22, 10:57, Kent Gibson wrote:
> On Fri, Jul 08, 2022 at 05:04:55PM +0530, Viresh Kumar wrote:
> > +## Updating bindings
> > +1. Clone the source from
> > +	<https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/>
> > +2. run `cd libgpiod/bindings/rust/libgpiod-sys/`
> > +2. run `cargo build --features generate`
> > +3. Commit changes in `src/bindings.rs`
> 
> Those instructions do not force the regeneration of the bindings.

It does, just that the new file that got generated lands somewhere like this:

target/debug/build/libgpiod-sys-769f98853e1c0550/out/bindings.rs

and the end user crate will use this instead of the one in src/.

> I assume the rust build system is being clever and determining nothing has
> changed and short circuiting, but I would like to regnerate them to be sure.
> How do I do that?

If you use the "generate" feature, you will end up building it again.

> How is the --features approach better than just
> bindgen -o src/bindings.rs ../../../include/gpiod.h
> ?

This lets the user crates control it from their Cargo.toml file, libgpiod
doesn't do it right now but it can like:

libgpiod-sys = { path = "libgpiod-sys", features = ["generate"] }

> Why not build this all the time rather than hiding it behind a feature?

That's what I did initially but there are environments, like rust-vmm
containers [1], where all tools aren't available to run a bindgen for each
architecture/platform type. I ran into a lot of issues there and the maintainers
suggested to do this instead as the bindings won't change a lot later on.

FWIW, in my use case vhost-device crate [2] is using libgpiod, which uses
libgpiod-sys and the vhost-device updates get tested with [1] automatically by
bots.

> > diff --git a/bindings/rust/libgpiod-sys/src/bindings.rs b/bindings/rust/libgpiod-sys/src/bindings.rs
> > new file mode 100644
> > index 000000000000..930eb894f66f
> > --- /dev/null
> > +++ b/bindings/rust/libgpiod-sys/src/bindings.rs
> > @@ -0,0 +1,1920 @@
> > +/* automatically generated by rust-bindgen 0.59.2 */
> > +
> 
> Yet the Cargo.toml specifies 0.59.1.
> So you didn't generate this with the instructions above then?

I did.

The dependency like: bindgen = "0.59.1", doesn't force a specific version but
actually a range. An update is allowed if the new version number does not modify
the left-most non-zero digit in the major, minor, patch grouping [3].

> Having said that, I can confirm that 
> 
> bindgen -o src/bindings.rs wrapper.h
> 
> with the appropriate bindgen, regenerates the pre-generated file.

-- 
viresh

[1] https://github.com/rust-vmm/rust-vmm-container
[2] https://github.com/rust-vmm/vhost-device/tree/main/gpio
[3] https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html