Skip to content

Commit

Permalink
CMake: Fix iconv detection
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
micahsnyder committed May 2, 2021
1 parent 6572fa7 commit 67db401
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 9 deletions.
2 changes: 2 additions & 0 deletions cabextract/.gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/build

# CabExtract specific
COPYING
INSTALL
Expand Down
17 changes: 16 additions & 1 deletion cabextract/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}
")
168 changes: 168 additions & 0 deletions cabextract/cmake/FindICONV.cmake
Original file line number Diff line number Diff line change
@@ -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 <stddef.h>
#include <iconv.h>
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 <iconv.h>
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()
8 changes: 4 additions & 4 deletions cabextract/config.h.in.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
/* Define to 1 if you have the <fnmatch.h> header file. */
#cmakedefine HAVE_FNMATCH_H 1

/* Define to 1 if you have the <iconv.h> header file. */
#cmakedefine HAVE_ICONV_H 1

/* Define to 1 if you have the <locale.h> header file. */
#cmakedefine HAVE_LOCALE_H 1

Expand Down Expand Up @@ -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. */
Expand Down
2 changes: 2 additions & 0 deletions libmspack/.gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/build

# Logs
*.log

Expand Down
1 change: 0 additions & 1 deletion libmspack/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions libmspack/config.h.in.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
/* Define to 1 if you have the <fnmatch.h> header file. */
#cmakedefine HAVE_FNMATCH_H 1

/* Define to 1 if you have the <iconv.h> header file. */
#cmakedefine HAVE_ICONV_H 1

/* Define to 1 if you have the <locale.h> header file. */
#cmakedefine HAVE_LOCALE_H 1

Expand Down

0 comments on commit 67db401

Please sign in to comment.