| Message ID | cover.1667815011.git.viresh.kumar@linaro.org |
|---|---|
| Headers | show |
| Series | libgpiod: Add Rust bindings | expand |
On Mon, Nov 7, 2022 at 10:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Hello, > > Here is another version of the rust bindings, based of the master branch. > > Pushed here: > > https://github.com/vireshk/libgpiod v9 > > V8->V9: > - Merged the last patch (supporting Events) with the other patches. > - Events implementation is simplified and made efficient. nth() is also > implemented for the iterator. > - Unnecessary comment removed from Cargo.toml files. > - Updated categories in libgpiod's Cargo.toml. > - Updated gpio_events example to show cloned events live past another > read_edge_events(). > - Implement AsRawFd for Chip. > - Other minor changes. > Kent, Miguel: if you are ok with this version - can you add your review tags? Bart
On Thu, Nov 17, 2022 at 11:40 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 17-11-22, 11:18, Bartosz Golaszewski wrote: > > Do these problems you faced apply to libgpiod too? > > I faced them with libgpiod only :( > > > Honestly, putting > > automatically generated files in the repo just feels wrong. > > I agree, but ... > > > It defeats > > the whole purpose of code generation. If we can't reliably regenerate > > them, then it sounds like a problem with the tools, not the library. > > Maybe we don't need to worry about that just yet? > > it isn't about reliability of the generated code, but making everyone do it, > even if they don't need to. > > Also, the code generated here is Rust code wrappers and other declarations, > which let us call the C helpers eventually. It can be considered like hand > written code here, for the argument that it is automatically generated stuff. > Just that we have a tool (bindgen) here which lets us generate it automatically, > without introducing bugs. > > Anyway, I am fine with whatever you decide. > Let me propose a different solution. When preparing release tarballs with autotools, certain files are generated automatically that go into the tarball but are never part of the repository. This way developers don't have to deal with the automatically generated files polluting the repo while end-users get a tarball with less dependencies and reproducible results. It's called the dist-local hook in automake. Does it sound like something we could use here? Bart
> On Thu, Nov 17, 2022 at 10:56 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Do these problems you faced apply to libgpiod too? Honestly, putting > automatically generated files in the repo just feels wrong. It defeats > the whole purpose of code generation. If we can't reliably regenerate > them, then it sounds like a problem with the tools, not the library. > Maybe we don't need to worry about that just yet? If the library is stable enough, then removing a heavy build dependency on bindgen may be worth it. But yeah, by default I would avoid it unless one is sure it is possible. Though, if one can store it because it is fixed, at that point one may take advantage of that and manually write (or tweak) the bindings instead. In any case, I would explain the rationale one way or the other in the commit message or ideally in the `README.MD`. One thing I may have missed: why is there a `-sys` crate for one of the bindings but not the other? It may be a good idea to document the intention as well. Cheers, Miguel
On 17-11-22, 11:48, Bartosz Golaszewski wrote: > Let me propose a different solution. When preparing release tarballs > with autotools, certain files are generated automatically that go into > the tarball but are never part of the repository. This way developers > don't have to deal with the automatically generated files polluting > the repo while end-users get a tarball with less dependencies and > reproducible results. > > It's called the dist-local hook in automake. > > Does it sound like something we could use here? These auto-generated bindings are used only by the code present in bindings/rust/libgpiod/src/, and are never visible to end user code. The end user will use the new interface provided by the libgpiod Rust bindings instead and that works over the web and will be picked from libgpiod's git tree or crates.io (later). Not sure how the tarball solution will work here.
On Thu, Nov 17, 2022 at 11:55 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 17-11-22, 11:48, Bartosz Golaszewski wrote: > > Let me propose a different solution. When preparing release tarballs > > with autotools, certain files are generated automatically that go into > > the tarball but are never part of the repository. This way developers > > don't have to deal with the automatically generated files polluting > > the repo while end-users get a tarball with less dependencies and > > reproducible results. > > > > It's called the dist-local hook in automake. > > > > Does it sound like something we could use here? > > These auto-generated bindings are used only by the code present in > bindings/rust/libgpiod/src/, and are never visible to end user code. > > The end user will use the new interface provided by the libgpiod Rust bindings > instead and that works over the web and will be picked from libgpiod's git tree > or crates.io (later). > > Not sure how the tarball solution will work here. > So it already only impacts developers exclusively and not users who'd for example want to install libgpiod from crates.io? If so then I don't really see a reason to keep those files in the repo really. I'm not familiar with rust-vmm containers but the fact that bindgen support is missing or causes problems sounds like a problem with rust-vmm and not users of bindgen, right? Bart
On 17-11-22, 12:05, Bartosz Golaszewski wrote: > So it already only impacts developers exclusively and not users who'd > for example want to install libgpiod from crates.io? If so then I > don't really see a reason to keep those files in the repo really. Users are impacted in the sense that they will be forced to build the bindings using bindgen now, automatically of course. It is an extra, time consuming, operation which can be avoided. For rust-vmm projects, every pull request results in fresh rebuild of the entire crate, which would mean two additional bindgen builds too, just for gpio now. It isn't a huge problem, but it is time that could have been saved. > I'm not familiar with rust-vmm containers but the fact that bindgen > support is missing or causes problems sounds like a problem with > rust-vmm and not users of bindgen, right? Yeah it can be, but IIUC, in the Rust world, more often than not such bindings are pre-generated, as this basically is Rust code only. These bindings will only change if we make a change to the gpiod API. Perhaps that's why I was asked to avoid generating the bindings there, but I can ask again and try fixing the rust-vmm containers if it doesn't work.
On 17-11-22, 12:15, Bartosz Golaszewski wrote:
> So I'd say - just use CC0-1.0 license in Cargo.toml?
The Cargo.toml files already have following currently:
license = "Apache-2.0 OR BSD-3-Clause"
On Thu, Nov 17, 2022 at 12:25 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 17-11-22, 12:15, Bartosz Golaszewski wrote: > > So I'd say - just use CC0-1.0 license in Cargo.toml? > > The Cargo.toml files already have following currently: > > license = "Apache-2.0 OR BSD-3-Clause" > Then let's just add the SPDX identifier on top to make reuse happy. Bart
On Fri, Nov 18, 2022 at 10:36 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 17-11-22, 13:18, Bartosz Golaszewski wrote: > > Then let's just add the SPDX identifier on top to make reuse happy. > > This looks fine ? > > diff --git a/bindings/rust/Cargo.toml b/bindings/rust/Cargo.toml > index 4fdf4e06ff90..85abe70b4aa5 100644 > --- a/bindings/rust/Cargo.toml > +++ b/bindings/rust/Cargo.toml > @@ -1,7 +1,12 @@ > +# SPDX-License-Identifier: CC0-1.0 > +# > +# Copyright 2022 Linaro Ltd. All Rights Reserved. > +# Viresh Kumar <viresh.kumar@linaro.org> > + Should be: # SPDX-License-Identifier: CC0-1.0 # SPDX-FileCopyrightText: 2022 Linaro Ltd. # SPDX-FileCopyrightTest: 2022 Viresh Kumar <viresh.kumar@linaro.org> > [workspace] > > > diff --git a/bindings/rust/libgpiod-sys/README.md b/bindings/rust/libgpiod-sys/README.md > index ecf75b31c41e..567891a30868 100644 > --- a/bindings/rust/libgpiod-sys/README.md > +++ b/bindings/rust/libgpiod-sys/README.md > @@ -1,11 +1,15 @@ > +# SPDX-License-Identifier: CC0-1.0 > +# > +# Copyright 2022 Linaro Ltd. All Rights Reserved. > +# Viresh Kumar <viresh.kumar@linaro.org> > + > # Generated libgpiod-sys Rust FFI bindings > > -- > viresh
On Fri, Nov 18, 2022 at 10:41 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 18-11-22, 10:38, Bartosz Golaszewski wrote: > > Should be: > > > > # SPDX-License-Identifier: CC0-1.0 > > # SPDX-FileCopyrightText: 2022 Linaro Ltd. > > # SPDX-FileCopyrightTest: 2022 Viresh Kumar <viresh.kumar@linaro.org> > > For all files ? Or just Cargo.toml and README.md files ? > All files. We're using unified SPFX-FileCopyrightText instead of custom notices. Bart
Hello, Here is another version of the rust bindings, based of the master branch. Pushed here: https://github.com/vireshk/libgpiod v9 V8->V9: - Merged the last patch (supporting Events) with the other patches. - Events implementation is simplified and made efficient. nth() is also implemented for the iterator. - Unnecessary comment removed from Cargo.toml files. - Updated categories in libgpiod's Cargo.toml. - Updated gpio_events example to show cloned events live past another read_edge_events(). - Implement AsRawFd for Chip. - Other minor changes. V7->V8: - Several updates to cargo.toml files, like license, version, etc. - Removed Sync support for chip and gpiosim. - Implemented, in a separate patch, iterator support for Events. - Fixed missing SAFETY comments. - Fixed build for 32 bit systems. - Use errno::Errno. - Removed Clone derive for many structures, that store raw pointers. - line setting helpers return the object back, so another helper can be called directly on them. Also made all helpers public and used the same in tests and example for single configurations. - Enums for gpiosim constants. - New examples to demonstrate parallelism and event handling. - Separated out HTE tests and marked as #[ignore] now. - Updated commit subjects. - Other minor changes. V6->V7: - Don't let buffer read new events if the earlier events are still referenced. - BufferIntenal is gone now, to make the above work. - Update example and tests too for the same. V5->V6: - Updates according to the new line-settings interface. - New file, line_settings.rs. - Renamed 'enum Setting' as 'SettingVal' to avoid conflicting names, as we also have 'struct Settings' now. - Support for HTE clock type. - Implement 'Eq' for public structure/enums (reported by build). - Remove 'SettingKindMap' and 'SettingMap' as they aren't required anymore. - Updated tests based on new interface. V4->V5: - Arrange as workspace with crates for libgpiod-sys, libgpiod, gpiosim. - Use static libgpiod and libgpiosim libraries instead of rebuilding again. - Arrange in modules instead of flattened approach. - New enums like Setting and SettingKind and new types based on them SettingMap and SettingKindMap. - New property independent helpers for line_config, like set_prop_default(). - Improved tests/examples, new example for gpiowatch. - Add pre-built bindings for gpiosim too. - Many other changes. V3->V4: - Rebased on top of new changes, and made changes accordingly. - Added rust integration tests with gpiosim. - Found a kernel bug with tests, sent a patch for that to LKML. V2->V3: - Remove naming redundancy, users just need to do this now use libgpiod:{Chip, Direction, LineConfig} now (Bartosz); - Fix lifetime issues between event-buffer and edge-event modules, the event buffer is released after the last edge-event reference is dropped (Bartosz). - Allow edge-event to be copied, and freed later (Bartosz). - Add two separate rust crates, sys and wrapper (Gerard). - Null-terminate the strings passed to libgpiod (Wedson). - Drop unnecessary checks to validate string returned from chip:name/label/path. - Fix SAFETY comments (Wedson). - Drop unnecessary clone() instances (Bartosz). V1->V2: - Added examples (I tested everything except gpiomon.rs, didn't have right hardware/mock device to test). - Build rust bindings as part of Make, update documentation. Thanks. -- Viresh Viresh Kumar (8): bindings: rust: Add libgpiod-sys rust crate bindings: rust: Add pre generated bindings for libgpiod-sys bindings: rust: Add gpiosim crate bindings: rust: Add pre generated bindings for gpiosim bindings: rust: Add libgpiod crate bindings: rust: Add examples to libgpiod crate bindings: rust: Add tests for libgpiod crate bindings: rust: Integrate building of bindings with make .gitignore | 5 + README | 8 +- TODO | 8 - bindings/Makefile.am | 6 + bindings/rust/Cargo.toml | 7 + bindings/rust/Makefile.am | 18 + bindings/rust/gpiosim/Cargo.toml | 22 + bindings/rust/gpiosim/README.md | 11 + bindings/rust/gpiosim/build.rs | 43 + bindings/rust/gpiosim/src/bindings.rs | 180 +++ bindings/rust/gpiosim/src/lib.rs | 79 ++ bindings/rust/gpiosim/src/sim.rs | 331 +++++ bindings/rust/libgpiod-sys/Cargo.toml | 20 + bindings/rust/libgpiod-sys/README.md | 11 + bindings/rust/libgpiod-sys/build.rs | 41 + bindings/rust/libgpiod-sys/src/bindings.rs | 1173 +++++++++++++++++ bindings/rust/libgpiod-sys/src/lib.rs | 13 + bindings/rust/libgpiod/Cargo.toml | 21 + .../rust/libgpiod/examples/gpio_events.rs | 89 ++ .../examples/gpio_threaded_info_events.rs | 133 ++ bindings/rust/libgpiod/examples/gpiodetect.rs | 31 + bindings/rust/libgpiod/examples/gpiofind.rs | 37 + bindings/rust/libgpiod/examples/gpioget.rs | 46 + bindings/rust/libgpiod/examples/gpioinfo.rs | 98 ++ bindings/rust/libgpiod/examples/gpiomon.rs | 75 ++ bindings/rust/libgpiod/examples/gpioset.rs | 64 + bindings/rust/libgpiod/examples/gpiowatch.rs | 54 + bindings/rust/libgpiod/src/chip.rs | 310 +++++ bindings/rust/libgpiod/src/edge_event.rs | 93 ++ bindings/rust/libgpiod/src/event_buffer.rs | 167 +++ bindings/rust/libgpiod/src/info_event.rs | 69 + bindings/rust/libgpiod/src/lib.rs | 479 +++++++ bindings/rust/libgpiod/src/line_config.rs | 135 ++ bindings/rust/libgpiod/src/line_info.rs | 162 +++ bindings/rust/libgpiod/src/line_request.rs | 227 ++++ bindings/rust/libgpiod/src/line_settings.rs | 297 +++++ bindings/rust/libgpiod/src/request_config.rs | 95 ++ bindings/rust/libgpiod/tests/chip.rs | 98 ++ bindings/rust/libgpiod/tests/common/config.rs | 143 ++ bindings/rust/libgpiod/tests/common/mod.rs | 10 + bindings/rust/libgpiod/tests/edge_event.rs | 298 +++++ bindings/rust/libgpiod/tests/info_event.rs | 167 +++ bindings/rust/libgpiod/tests/line_config.rs | 96 ++ bindings/rust/libgpiod/tests/line_info.rs | 276 ++++ bindings/rust/libgpiod/tests/line_request.rs | 510 +++++++ bindings/rust/libgpiod/tests/line_settings.rs | 204 +++ .../rust/libgpiod/tests/request_config.rs | 39 + configure.ac | 16 + 48 files changed, 6504 insertions(+), 11 deletions(-) create mode 100644 bindings/rust/Cargo.toml create mode 100644 bindings/rust/Makefile.am create mode 100644 bindings/rust/gpiosim/Cargo.toml create mode 100644 bindings/rust/gpiosim/README.md create mode 100644 bindings/rust/gpiosim/build.rs create mode 100644 bindings/rust/gpiosim/src/bindings.rs create mode 100644 bindings/rust/gpiosim/src/lib.rs create mode 100644 bindings/rust/gpiosim/src/sim.rs create mode 100644 bindings/rust/libgpiod-sys/Cargo.toml create mode 100644 bindings/rust/libgpiod-sys/README.md create mode 100644 bindings/rust/libgpiod-sys/build.rs create mode 100644 bindings/rust/libgpiod-sys/src/bindings.rs create mode 100644 bindings/rust/libgpiod-sys/src/lib.rs create mode 100644 bindings/rust/libgpiod/Cargo.toml create mode 100644 bindings/rust/libgpiod/examples/gpio_events.rs create mode 100644 bindings/rust/libgpiod/examples/gpio_threaded_info_events.rs create mode 100644 bindings/rust/libgpiod/examples/gpiodetect.rs create mode 100644 bindings/rust/libgpiod/examples/gpiofind.rs create mode 100644 bindings/rust/libgpiod/examples/gpioget.rs create mode 100644 bindings/rust/libgpiod/examples/gpioinfo.rs create mode 100644 bindings/rust/libgpiod/examples/gpiomon.rs create mode 100644 bindings/rust/libgpiod/examples/gpioset.rs create mode 100644 bindings/rust/libgpiod/examples/gpiowatch.rs create mode 100644 bindings/rust/libgpiod/src/chip.rs create mode 100644 bindings/rust/libgpiod/src/edge_event.rs create mode 100644 bindings/rust/libgpiod/src/event_buffer.rs create mode 100644 bindings/rust/libgpiod/src/info_event.rs create mode 100644 bindings/rust/libgpiod/src/lib.rs create mode 100644 bindings/rust/libgpiod/src/line_config.rs create mode 100644 bindings/rust/libgpiod/src/line_info.rs create mode 100644 bindings/rust/libgpiod/src/line_request.rs create mode 100644 bindings/rust/libgpiod/src/line_settings.rs create mode 100644 bindings/rust/libgpiod/src/request_config.rs create mode 100644 bindings/rust/libgpiod/tests/chip.rs create mode 100644 bindings/rust/libgpiod/tests/common/config.rs create mode 100644 bindings/rust/libgpiod/tests/common/mod.rs create mode 100644 bindings/rust/libgpiod/tests/edge_event.rs create mode 100644 bindings/rust/libgpiod/tests/info_event.rs create mode 100644 bindings/rust/libgpiod/tests/line_config.rs create mode 100644 bindings/rust/libgpiod/tests/line_info.rs create mode 100644 bindings/rust/libgpiod/tests/line_request.rs create mode 100644 bindings/rust/libgpiod/tests/line_settings.rs create mode 100644 bindings/rust/libgpiod/tests/request_config.rs