Skip to content

add meson#426

Closed
neheb wants to merge 1 commit intolibusb:masterfrom
neheb:meson
Closed

add meson#426
neheb wants to merge 1 commit intolibusb:masterfrom
neheb:meson

Conversation

@neheb
Copy link
Copy Markdown

@neheb neheb commented Jun 12, 2022

Helps to avoid iconv problems that the CMake build has.

Signed-off-by: Rosen Penev rosenp@gmail.com

Comment thread meson.build Outdated
Comment thread meson.build
if libusb_dep.found() and iconv_dep.found()
libhidapi = library('hidapi-libusb',
'libusb/hid.c',
dependencies: [ iconv_dep, libusb_dep ],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This should depend on threads, I think.

Also, it is only called "-libusb" on linux.

And on Windows or macOS it uses the sources from windows/ or mac/ instead. See e.g.

hidapi/Makefile.am

Lines 18 to 40 in 59e84ca

if OS_LINUX
SUBDIRS += linux libusb
endif
if OS_DARWIN
SUBDIRS += mac
endif
if OS_FREEBSD
SUBDIRS += libusb
endif
if OS_KFREEBSD
SUBDIRS += libusb
endif
if OS_HAIKU
SUBDIRS += libusb
endif
if OS_WINDOWS
SUBDIRS += windows
endif

Copy link
Copy Markdown
Author

@neheb neheb Jun 12, 2022

Choose a reason for hiding this comment

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

threads probably implicitly gets included with libusb.

edit: as for the others, I'd probably have to use wrapdb's CI for that. Need libusb first though.

Comment thread meson.build
Comment thread meson.build Outdated
Helps to avoid iconv problems that the CMake build has.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Comment thread meson_options.txt
@@ -0,0 +1,7 @@
option('hidraw', type : 'feature',
description : 'Build hidraw. Requires udev.',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
description : 'Build hidraw. Requires udev.',
description : 'Build hidapi with hidraw backend. Requires udev.',

Comment thread meson_options.txt
)

option('libusb', type : 'feature',
description : 'Build hidapi. Requires libusb.',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
description : 'Build hidapi. Requires libusb.',
description : 'Build hidapi with libusb backend. Requires libusb.',

@Youw
Copy link
Copy Markdown
Member

Youw commented Jun 12, 2022

What does it solve compared to #410?

avoid iconv problems that the CMake build has

There is 0 known issues related to iconv: https://github.com/libusb/hidapi/issues?q=is%3Aissue+is%3Aopen+iconv

Can you elaborate?

@Youw Youw added the don't_merge Don't merge this PR as is label Jun 12, 2022
@neheb
Copy link
Copy Markdown
Author

neheb commented Jun 13, 2022

What does it solve compared to #410?

avoid iconv problems that the CMake build has

There is 0 known issues related to iconv: https://github.com/libusb/hidapi/issues?q=is%3Aissue+is%3Aopen+iconv

Can you elaborate?

didn't know there was another PR.

Anyway, issue is that when both external and internal iconv implementations exist, CMake's find_package(Iconv) fails to find iconv and terminates the build. More info here:

https://github.com/openwrt/packages/actions/runs/2477509891
openwrt/packages#18738

solved by using dependency('iconv') which just works.

@Youw
Copy link
Copy Markdown
Member

Youw commented Jun 13, 2022

I'm pretty sure, we can find a solution to CMake's find_package(Iconv).


I do not support having yet another alternative build system for HIDAPI when it can be avoided - you'd have to duplicate the entire build logic we already have in CMake.
Lets focuse on #410 and #429 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

don't_merge Don't merge this PR as is

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants