From 67db401cabfdc80cd7f29e860f7c783f5a4be95f Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Sat, 1 May 2021 18:31:12 -0700 Subject: [PATCH] CMake: Fix iconv detection The current implementations check for iconv.h and set HAVE_ICONV_H but don't set HAVE_ICONV, which is the macro we actually depend on. In the autotools build system, HAVE_ICONV is set by the `AM_ICONV` macro, if you have gettext-devel installed (note, this will cause an autoreconf warning if you don't have gettext-devel installed). Anyways... to fix this in CMake, I've added FindICONV.cmake module which is a slightly modified variant of the official CMake iconv detection module. This one has some additional features including setting the ICONV_CONST macro as needed. This commit fixes the cabextract "encoding" test, which was the final outstanding issue with CMake support, excluding the fact that there are no cabextract feature tests for Windows. I think this is acceptable for now, since it's not like there were any before switching to CMake. --- cabextract/.gitignore | 2 + cabextract/CMakeLists.txt | 17 +++- cabextract/cmake/FindICONV.cmake | 168 +++++++++++++++++++++++++++++++ cabextract/config.h.in.cmake | 8 +- libmspack/.gitignore | 2 + libmspack/CMakeLists.txt | 1 - libmspack/config.h.in.cmake | 3 - 7 files changed, 192 insertions(+), 9 deletions(-) create mode 100644 cabextract/cmake/FindICONV.cmake diff --git a/cabextract/.gitignore b/cabextract/.gitignore index 3b8a8ef..d912544 100644 --- a/cabextract/.gitignore +++ b/cabextract/.gitignore @@ -1,3 +1,5 @@ +/build + # CabExtract specific COPYING INSTALL diff --git a/cabextract/CMakeLists.txt b/cabextract/CMakeLists.txt index 9429acf..51a3bea 100644 --- a/cabextract/CMakeLists.txt +++ b/cabextract/CMakeLists.txt @@ -53,7 +53,6 @@ check_include_file(dirent.h HAVE_DIRENT_H) check_include_file(sys/types.h HAVE_SYS_TYPES_H) check_include_file(sys/stat.h HAVE_SYS_STAT_H) check_include_file(fnmatch.h HAVE_FNMATCH_H) -check_include_file(iconv.h HAVE_ICONV_H) check_include_file(locale.h HAVE_LOCALE_H) check_include_file(stdarg.h HAVE_STDARG_H) check_include_file(stdlib.h HAVE_STDLIB_H) @@ -191,6 +190,16 @@ set(libdir ${CMAKE_INSTALL_FULL_LIBDIR}) set(includedir ${CMAKE_INSTALL_FULL_INCLUDEDIR}) set(VERSION ${PROJECT_VERSION}) +# +# External library dependencies +# +if(NOT WIN32) + find_package(ICONV) + if(ICONV_FOUND) + set(HAVE_ICONV 1) + endif() +endif() + # Generate config.h add_definitions(-DHAVE_CONFIG_H) configure_file(config.h.in.cmake config.h) @@ -219,6 +228,9 @@ else() target_include_directories(cabextract PRIVATE ${PROJECT_SOURCE_DIR} ${PROJECT_SOURCE_DIR}/win32) target_sources(cabextract PRIVATE win32/dirent.h) target_link_libraries(cabextract Shlwapi.lib) + if(HAVE_ICONV) + target_link_libraries( cabextract PRIVATE ICONV::Iconv ) + endif() endif() target_include_directories(cabextract PRIVATE ${PROJECT_SOURCE_DIR}) target_link_libraries(cabextract MSPack::mspack) @@ -252,4 +264,7 @@ message(STATUS "Summary of build options: WARNCFLAGS: ${WARNCFLAGS} Features: External mspack: ${ENABLE_EXTERNAL_MSPACK} + Text conversion: + iconv ${ICONV_INCLUDE_DIRS} + ${ICONV_LIBRARIES} ") diff --git a/cabextract/cmake/FindICONV.cmake b/cabextract/cmake/FindICONV.cmake new file mode 100644 index 0000000..4ffa0a8 --- /dev/null +++ b/cabextract/cmake/FindICONV.cmake @@ -0,0 +1,168 @@ +# From https://github.com/Kitware/CMake/blob/master/Modules/FindIconv.cmake +# +# Distributed under the OSI-approved BSD 3-Clause License. See accompanying +# file Copyright.txt or https://cmake.org/licensing for details. + +# Mods by Micah Snyder to support systems with both libc's iconv + libconv + +#[=======================================================================[.rst: +FindICONV +--------- + +.. versionadded:: 3.11 + +This module finds the ``iconv()`` POSIX.1 functions on the system. +These functions might be provided in the regular C library or externally +in the form of an additional library. + +The following variables are provided to indicate iconv support: + +.. variable:: ICONV_FOUND + + Variable indicating if the iconv support was found. + +.. variable:: ICONV_CONST + + Variable to use when declaring "in" char* variable for the iconv() function + so that it is const if the iconv implementation expects it to be const. + +.. variable:: ICONV_INCLUDE_DIRS + + The directories containing the iconv headers. + +.. variable:: ICONV_LIBRARIES + + The iconv libraries to be linked. + +.. variable:: ICONV_IS_BUILT_IN + + A variable indicating whether iconv support is stemming from the + C library or not. Even if the C library provides `iconv()`, the presence of + an external `libiconv` implementation might lead to this being false. + +Additionally, the following :prop_tgt:`IMPORTED` target is being provided: + +.. variable:: ICONV::Iconv + + Imported target for using iconv. + +The following cache variables may also be set: + +.. variable:: ICONV_INCLUDE_DIR + + The directory containing the iconv headers. + +.. variable:: ICONV_LIBRARY + + The iconv library (if not implicitly given in the C library). + +.. note:: + On POSIX platforms, iconv might be part of the C library and the cache + variables ``ICONV_INCLUDE_DIR`` and ``ICONV_LIBRARY`` might be empty. + +#]=======================================================================] + +include(CMakePushCheckState) +include(CheckCSourceCompiles) +include(CheckCXXSourceCompiles) + +# iconv can only be provided in libc on a POSIX system. +# If any cache variable is already set, we'll skip this test. +if(NOT DEFINED ICONV_IS_BUILT_IN) + # Check for iconv.h first. + # If it's not the built-in one, then ICONV_INCLUDE_DIR will + find_path(ICONV_INCLUDE_DIR + NAMES "iconv.h" + DOC "iconv include directory") + set(ICONV_LIBRARY_NAMES "iconv" "libiconv") + + if(UNIX AND ICONV_INCLUDE_DIR AND NOT DEFINED ICONV_LIBRARY) + cmake_push_check_state(RESET) + # We always suppress the message here: Otherwise on supported systems + # not having iconv in their C library (e.g. those using libiconv) + # would always display a confusing "Looking for iconv - not found" message + set(CMAKE_FIND_QUIETLY TRUE) + # The following code will not work, but it's sufficient to see if it compiles. + # Note: libiconv will define the iconv functions as macros, so CheckSymbolExists + # will not yield correct results. + set(ICONV_IMPLICIT_TEST_CODE + " + #include + #include + int main() { + char *a, *b; + size_t i, j; + iconv_t ic; + ic = iconv_open(\"to\", \"from\"); + iconv(ic, &a, &i, &b, &j); + iconv_close(ic); + } + " + ) + + # Make sure we're using the iconv.h we found above. This way we don't + # accidentally compile against libiconv's header later but link with only + # libc on systems that have both (eg FreeBSD with libiconv pkg installed). + set(CMAKE_REQUIRED_INCLUDES ${ICONV_INCLUDE_DIR}) + + if(CMAKE_C_COMPILER_LOADED) + check_c_source_compiles("${ICONV_IMPLICIT_TEST_CODE}" ICONV_IS_BUILT_IN) + else() + check_cxx_source_compiles("${ICONV_IMPLICIT_TEST_CODE}" ICONV_IS_BUILT_IN) + endif() + cmake_pop_check_state() + else() + set(ICONV_IS_BUILT_IN FALSE) + endif() +endif() + +if(ICONV_IS_BUILT_IN) + set(ICONV_INCLUDE_DIR "" CACHE FILEPATH "iconv include directory") + set(ICONV_LIBRARY_NAMES "c") +endif() + +find_library(ICONV_LIBRARY + NAMES ${ICONV_LIBRARY_NAMES} + NAMES_PER_DIR + DOC "iconv library (potentially the C library)") + +mark_as_advanced(ICONV_INCLUDE_DIR) +mark_as_advanced(ICONV_LIBRARY) + +include(FindPackageHandleStandardArgs) +if(NOT ICONV_IS_BUILT_IN) + find_package_handle_standard_args(ICONV REQUIRED_VARS ICONV_LIBRARY ICONV_INCLUDE_DIR) +else() + find_package_handle_standard_args(ICONV REQUIRED_VARS ICONV_LIBRARY) +endif() + +if(ICONV_FOUND) + # Check if the second argument for iconv() needs to be const + set(OLD_CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}") + set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror") + check_c_source_compiles(" + #include + int main() { + iconv_t conv = 0; + const char* in = 0; + size_t ilen = 0; + char* out = 0; + size_t olen = 0; + iconv(conv, &in, &ilen, &out, &olen); + return 0; + } +" ICONV_SECOND_ARGUMENT_IS_CONST ) + set(CMAKE_REQUIRED_FLAGS "${OLD_CMAKE_REQUIRED_FLAGS}") + + if(ICONV_SECOND_ARGUMENT_IS_CONST) + set(ICONV_CONST "const") + endif(ICONV_SECOND_ARGUMENT_IS_CONST) + + set(ICONV_INCLUDE_DIRS "${ICONV_INCLUDE_DIR}") + set(ICONV_LIBRARIES "${ICONV_LIBRARY}") + if(NOT TARGET ICONV::Iconv) + add_library(ICONV::Iconv INTERFACE IMPORTED) + endif() + set_property(TARGET ICONV::Iconv PROPERTY INTERFACE_INCLUDE_DIRECTORIES "${ICONV_INCLUDE_DIRS}") + set_property(TARGET ICONV::Iconv PROPERTY INTERFACE_LINK_LIBRARIES "${ICONV_LIBRARIES}") +endif() diff --git a/cabextract/config.h.in.cmake b/cabextract/config.h.in.cmake index 99aeb7a..9fecfff 100644 --- a/cabextract/config.h.in.cmake +++ b/cabextract/config.h.in.cmake @@ -35,9 +35,6 @@ /* Define to 1 if you have the header file. */ #cmakedefine HAVE_FNMATCH_H 1 -/* Define to 1 if you have the header file. */ -#cmakedefine HAVE_ICONV_H 1 - /* Define to 1 if you have the header file. */ #cmakedefine HAVE_LOCALE_H 1 @@ -79,7 +76,10 @@ /* Define to empty if `const' does not conform to ANSI C. */ -#cmakedefine ICONV_CONST "@ICONV_CONST@" +#define ICONV_CONST @ICONV_CONST@ + +/* Define to 1 if you have the iconv library. */ +#cmakedefine HAVE_ICONV 1 /* The size of `off_t', as computed by sizeof. */ diff --git a/libmspack/.gitignore b/libmspack/.gitignore index 393f115..59ca3a9 100644 --- a/libmspack/.gitignore +++ b/libmspack/.gitignore @@ -1,3 +1,5 @@ +/build + # Logs *.log diff --git a/libmspack/CMakeLists.txt b/libmspack/CMakeLists.txt index 170d27d..0306a8d 100644 --- a/libmspack/CMakeLists.txt +++ b/libmspack/CMakeLists.txt @@ -80,7 +80,6 @@ check_include_file(dirent.h HAVE_DIRENT_H) check_include_file(sys/types.h HAVE_SYS_TYPES_H) check_include_file(sys/stat.h HAVE_SYS_STAT_H) check_include_file(fnmatch.h HAVE_FNMATCH_H) -check_include_file(iconv.h HAVE_ICONV_H) check_include_file(locale.h HAVE_LOCALE_H) check_include_file(stdarg.h HAVE_STDARG_H) check_include_file(stdlib.h HAVE_STDLIB_H) diff --git a/libmspack/config.h.in.cmake b/libmspack/config.h.in.cmake index 99aeb7a..ac25bba 100644 --- a/libmspack/config.h.in.cmake +++ b/libmspack/config.h.in.cmake @@ -35,9 +35,6 @@ /* Define to 1 if you have the header file. */ #cmakedefine HAVE_FNMATCH_H 1 -/* Define to 1 if you have the header file. */ -#cmakedefine HAVE_ICONV_H 1 - /* Define to 1 if you have the header file. */ #cmakedefine HAVE_LOCALE_H 1