Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure Iconv is found when provided via CFLAGS/LDFLAGS #430

Merged
merged 4 commits into from
Jun 21, 2022
Merged

Conversation

Youw
Copy link
Member

@Youw Youw commented Jun 18, 2022

  • by default find_file/find_library doesn't respect CFLAGS/LDFLAGS, and FindIconv fails to find Iconv;
  • by explicitly trying to link against -liconv - we're checking if library is available in such way;
  • additionally: if Iconv is detected as BUILT_IN, no need to explicitly depend on Iconv::Iconv;

Fixes: #429

- by default find_file/find_library doesn't respect CFLAGS/LDFLAGS,
and FindIconv fails to find Iconv;
- by explicitly trying to link against `-liconv` - we're checking if library is available in such way;
- additionally: if Iconv is detected as BUILT_IN, no need to explicitly depend on `Iconv::Iconv`;
@Youw
Copy link
Member Author

Youw commented Jun 18, 2022

Right now I don't have the environment to check this properly (except what is setup on CI).

@neheb can you check this as part of the openwrt build?

@neheb
Copy link

neheb commented Jun 18, 2022

As soon as I get home.

@neheb
Copy link

neheb commented Jun 20, 2022

nope:

-- The C compiler identification is GNU 12.1.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /home/mangix/devstuff/openwrt/staging_dir/toolchain-mips_24kc_gcc-12.1.0_musl/bin/mips-openwrt-linux-musl-gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- hidapi: v0.12.0
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found PkgConfig: /home/mangix/devstuff/openwrt/staging_dir/host/bin/pkg-config (found version "1.8.0")
-- Checking for module 'libudev'
--   Found libudev, version 243
-- Checking for module 'libusb-1.0>=1.0.9'
--   Found libusb-1.0, version 1.0.24
-- Performing Test Iconv_IS_BUILT_IN
-- Performing Test Iconv_IS_BUILT_IN - Failed
-- Could NOT find Iconv (missing: Iconv_LIBRARY)
-- Could NOT find Iconv (missing: Iconv_LIBRARY)
CMake Error at libusb/CMakeLists.txt:41 (message):
  Iconv is not found, make sure your build environment is right


-- Configuring incomplete, errors occurred!
See also "/home/mangix/devstuff/openwrt/build_dir/target-mips_24kc_musl/hidapi-hidapi-0.12.0/CMakeFiles/CMakeOutput.log".
See also "/home/mangix/devstuff/openwrt/build_dir/target-mips_24kc_musl/hidapi-hidapi-0.12.0/CMakeFiles/CMakeError.log".

@Youw
Copy link
Member Author

Youw commented Jun 20, 2022

Updated. This time I was able to test it partially.

@neheb
Copy link

neheb commented Jun 20, 2022

Seems to have done it. A lot of work as compared to

dependency('iconv')

though.

@Youw
Copy link
Member Author

Youw commented Jun 20, 2022

Not a lot of work, compared to uncompleted #426 :)

Besides, I'll ask CMake community, if this can be improved by CMake itself, even though openwrt CMake support is too incomplete as of yet.


Thanks for testing it.

libusb/CMakeLists.txt Outdated Show resolved Hide resolved
@neheb
Copy link

neheb commented Jun 20, 2022

I do agree CMake's Find_Iconv is not that great, Fixing it upstream would be good.

The OpenWrt comment is interesting. I don't really know what the "proper" way of handling cross compilation is like this. Although cmake.mk probably predates whatever "proper" is.

@Youw
Copy link
Member Author

Youw commented Jun 20, 2022

I don't really know what the "proper" way of handling cross compilation is like this.

Let me share with you a few links, as a general reference:

  • The best way to cross-compile with CMake is to prepare a CMake toolchain file. Here is the official documentation: https://cmake.org/cmake/help/book/mastering-cmake/chapter/Cross%20Compiling%20With%20CMake.html
    So instead of specifying so many CMake arguments and environment varibales - a single additional CMake argument should be passed: -DCMAKE_TOOLCHAIN_FILE=<toolchain for cross-compilation>;
  • a very good example of CMake toolchain file is the one provided by Android NDK. The one I used recently is located at: ndk/22.1.7171670/build/cmake/android.toolchain.cmake;
  • some while ago I've been using Yocto builds for one of the development boards; Yocto has mature CMake support - that I remember, even though cannot really share a specific link to check out;

BTW:

the one provided by Android

I'm successfully using with HIDAPI (and not only) in several of my projects.

@Youw Youw merged commit 81dd62d into master Jun 21, 2022
@Youw Youw deleted the find_iconv branch June 21, 2022 22:30
@mcuee mcuee added the build system/CI Anything related to building the project or running on CI label Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system/CI Anything related to building the project or running on CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

find_package(Iconv) fails when external iconv implementations is provided as CFLAGS/LDFLAGS
3 participants