Skip to content

Add libgpiod#130

Merged
eli-schwartz merged 1 commit into
mesonbuild:masterfrom
amboar:libgpiod
Aug 16, 2021
Merged

Add libgpiod#130
eli-schwartz merged 1 commit into
mesonbuild:masterfrom
amboar:libgpiod

Conversation

@amboar
Copy link
Copy Markdown
Contributor

@amboar amboar commented Aug 12, 2021

The libgpiod library encapsulates the ioctl calls and data structures for accessing Linux's GPIO chardevs behind a straightforward API.

Comment thread subprojects/packagefiles/libgpiod/bindings/cxx/tests/meson.build Outdated
Comment thread subprojects/packagefiles/libgpiod/bindings/python/meson.build Outdated
Comment thread subprojects/packagefiles/libgpiod/man/meson.build Outdated
Comment thread subprojects/packagefiles/libgpiod/meson.build Outdated
Comment thread subprojects/packagefiles/libgpiod/readme.txt Outdated
Comment thread subprojects/packagefiles/libgpiod/man/meson.build Outdated
@xclaesse
Copy link
Copy Markdown
Member

I should add in README/Doc that new wraps should specify if meson port has been proposed upstream first.

@amboar this is a non trivial meson port, has it been proposed upstream? If no, please do. If yes, please link to the discussion :)

Comment thread subprojects/packagefiles/libgpiod/lib/meson.build Outdated
@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 13, 2021

@amboar this is a non trivial meson port, has it been proposed upstream? If no, please do. If yes, please link to the discussion :)

I've started the discussion upstream: https://lore.kernel.org/linux-gpio/cc4926af-95bb-4178-a760-d55821dfb626@www.fastmail.com/

Comment thread subprojects/packagefiles/libgpiod/man/meson.build
Comment thread subprojects/packagefiles/libgpiod/meson.build Outdated
@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 13, 2021

@eli-schwartz regarding the docs, I'm going to leave manpages as a boolean option and keep the current behaviour of not installing the doxygen output. The Doxyfile.in is too antisocial in its current form; I tried various hacks with the doc subdirectory but couldn't get it working (output goes into build/doc rather than build/subprojects/libgpiod-1.6.3/doc, and then there's the "path element" issue with the output kwarg for custom_target().

Regarding the >=0.48.0 constraint, I'm using the .name() method of the build target object for the custom_target() output in the help2man case in order to name the man-page after the tool. .name() support was added in 0.54.0, and I don't think I can get rid of using without a lot of copy/paste.

Copy link
Copy Markdown
Member

@eli-schwartz eli-schwartz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the doxygen issue, I see that Doxyfile.in specifies OUTPUT_DIRECTORY = doc so yeah, it will unfortunately build in whichever is the working directory of doxygen, and that does not play nicely with subprojects at all.

Given this, I don't believe you can enable the doxygen target at all, whether it gets installed or not. But if upstream makes that variable configurable, then this is solvable without patching Doxyfile.in

Comment thread subprojects/packagefiles/libgpiod/bindings/cxx/meson.build Outdated
Comment thread subprojects/packagefiles/libgpiod/man/meson.build Outdated
@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 15, 2021

Given this, I don't believe you can enable the doxygen target at all, whether it gets installed or not.

Good point. I'll skip doxygen.

@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 16, 2021

Okay, in the latest push:

  • Reduce meson version requirement to >=0.48.0
  • Only require a C++ compiler for -Dbindings=cxx
  • Compare and correct shared object versioning against autotools
  • Ensure all artefacts installed by upstream configure options are installed by meson configure options, including test artefacts (see discussion about test environment requirements on the kernel above)
  • Disable and warn for library documentation build due to incompatible Doxyfile from upstream

@eli-schwartz
Copy link
Copy Markdown
Member

$ meson setup --wipe builddir . -Dwraps=libgpiod -Dlibgpiod:bindings=cxx,python
[...]
$ ninja -C builddir
[5/27] Compiling C++ object bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o
FAILED: bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o 
ccache c++ -Ibindings/cxx/examples/gpiodetectcxx.p -Ibindings/cxx/examples -I../bindings/cxx/examples -Ibindings/cxx -I../bindings/cxx -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -std=gnu++11 -O2 -g -MD -MQ bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o -MF bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o.d -o bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o -c ../bindings/cxx/examples/gpiodetectcxx.cpp
In file included from ../bindings/cxx/examples/gpiodetectcxx.cpp:10:
../bindings/cxx/gpiod.hpp:13:10: fatal error: gpiod.h: No such file or directory
   13 | #include <gpiod.h>
      |          ^~~~~~~~~
compilation terminated.

I think you should be making much heavier use of the declare_dependencies() you have set up.

description: 'C++ bindings for libgpiod')

gpiodcxx_dep = declare_dependency(link_with: gpiodcxx,
include_directories: gpiod_includes,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also add the include_directories: gpiodcxx_includes ?

gpiodcxx_sources = files('chip.cpp', 'iter.cpp', 'line.cpp', 'line_bulk.cpp')
gpiodcxx_includes = include_directories('.')
gpiodcxx = library('gpiodcxx', gpiodcxx_sources,
include_directories: [ '.', gpiod_includes ],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'.' -> gpiodcxx_includes


executable('gpiodetectcxx', 'gpiodetectcxx.cpp',
include_directories: '..',
link_with: gpiodcxx)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'.' -> gpiodcxx_includes

... or just dependencies: gpiodcxx_dep which is supposed to expose everything all together for you.

@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 16, 2021

$ meson setup --wipe builddir . -Dwraps=libgpiod -Dlibgpiod:bindings=cxx,python
[...]
$ ninja -C builddir
[5/27] Compiling C++ object bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o
FAILED: bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o 
ccache c++ -Ibindings/cxx/examples/gpiodetectcxx.p -Ibindings/cxx/examples -I../bindings/cxx/examples -Ibindings/cxx -I../bindings/cxx -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -std=gnu++11 -O2 -g -MD -MQ bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o -MF bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o.d -o bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o -c ../bindings/cxx/examples/gpiodetectcxx.cpp
In file included from ../bindings/cxx/examples/gpiodetectcxx.cpp:10:
../bindings/cxx/gpiod.hpp:13:10: fatal error: gpiod.h: No such file or directory
   13 | #include <gpiod.h>
      |          ^~~~~~~~~
compilation terminated.

Hmm, I wonder why I didn't see this in my testing.

I think you should be making much heavier use of the declare_dependencies() you have set up.

Yep.

@eli-schwartz
Copy link
Copy Markdown
Member

The cxx bindings are disabled by default, so it felt like a useful thing to poke at to see if I can break it.

... look, my IRC nick isn't "elibrokeit" for nothing.

@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 16, 2021

$ meson setup --wipe builddir . -Dwraps=libgpiod -Dlibgpiod:bindings=cxx,python
[...]
$ ninja -C builddir
[5/27] Compiling C++ object bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o
FAILED: bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o 
ccache c++ -Ibindings/cxx/examples/gpiodetectcxx.p -Ibindings/cxx/examples -I../bindings/cxx/examples -Ibindings/cxx -I../bindings/cxx -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -std=gnu++11 -O2 -g -MD -MQ bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o -MF bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o.d -o bindings/cxx/examples/gpiodetectcxx.p/gpiodetectcxx.cpp.o -c ../bindings/cxx/examples/gpiodetectcxx.cpp
In file included from ../bindings/cxx/examples/gpiodetectcxx.cpp:10:
../bindings/cxx/gpiod.hpp:13:10: fatal error: gpiod.h: No such file or directory
   13 | #include <gpiod.h>
      |          ^~~~~~~~~
compilation terminated.

Hmm, I wonder why I didn't see this in my testing.

That would be because I had libgpiod-dev installed on my system 🤦

FWIW I was testing with meson setup -Dtools=true -Dtests=true -Dbindings=cxx,python -Dmanpages=true build in the subprojects/libgpiod-1.6.3 subdirectory, so was enabling the C++ bindings.

@eli-schwartz
Copy link
Copy Markdown
Member

Yeah, I guess that would do it.

...

On the upstream note, looks like this will be a wrapdb exclusive for now?

@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 16, 2021

On the upstream note, looks like this will be a wrapdb exclusive for now?

Yep, looking that way.

@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 16, 2021

Okay, this time around:

  • Removed libgpiod-dev from my system, retested and fixed the broken includes
  • Refactored cumbersome use of link_with: / include_directories: kwargs by instead using dependencies: with existing results from declare_dependency()
  • Removed with_tests in favour of just inlining get_option('tests')
  • Renamed the manpages option to docs for forwards compatibility with generating other formats (e.g. html) whenever that becomes possible with the upstream Doxyfile

@amboar amboar requested a review from eli-schwartz August 16, 2021 03:31
Comment thread subprojects/packagefiles/libgpiod/bindings/cxx/tests/meson.build Outdated
Comment thread subprojects/packagefiles/libgpiod/bindings/cxx/tests/meson.build
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
@amboar
Copy link
Copy Markdown
Contributor Author

amboar commented Aug 16, 2021

Fixed the include path issue in the c++ binding tests.

@eli-schwartz eli-schwartz merged commit 6623079 into mesonbuild:master Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants