Modernise FindReadline.cmake, and fix on Solaris (#813)

The CMake build fails on Solaris out of the box.

* Fix Solaris vanilla CMake build; remove CI override that forces use of termcap.
* Move up pcre2_use_system_extensions to the top of CMakeLists.txt
* Use cmake_push_check_state() instead of manual management of state
* Move _GNU_SOURCE detection into pcre2_use_system_extensions, to match Autoconf

Author: NWilson

.github/workflows/build.yml

@@ -420,7 +420,7 @@ jobs:
             echo "== CMake, 64-bit =="
             cd ../build-cmake-64
 
-            CC="cc -m64" cmake $CMAKE_FLAGS -DNCURSES_LIBRARY=termcap -DPCRE2_BUILD_PCRE2_16=ON -DPCRE2_BUILD_PCRE2_32=ON -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON -DPCRE2_DEBUG=ON -DCMAKE_C_FLAGS="$CFLAGS_SOLARIS_CC" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DCMAKE_BUILD_TYPE=Release -B build
+            CC="cc -m64" cmake $CMAKE_FLAGS -DPCRE2_BUILD_PCRE2_16=ON -DPCRE2_BUILD_PCRE2_32=ON -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON -DPCRE2_DEBUG=ON -DCMAKE_C_FLAGS="$CFLAGS_SOLARIS_CC" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DCMAKE_BUILD_TYPE=Release -B build
             cd build
             make
             ctest -j3 --output-on-failure

CMakeLists.txt

@@ -161,6 +161,7 @@ include(CheckSymbolExists)
 include(CheckIncludeFile)
 include(CheckTypeSize)
 include(CMakePackageConfigHelpers)
+include(CMakePushCheckState)
 include(GNUInstallDirs) # for CMAKE_INSTALL_LIBDIR
 include(PCRE2CheckLinkerFlag)
 include(PCRE2UseSystemExtensions)
@@ -172,8 +173,25 @@ check_include_file(sys/types.h HAVE_SYS_TYPES_H)
 check_include_file(unistd.h HAVE_UNISTD_H)
 check_include_file(windows.h HAVE_WINDOWS_H)
 
-check_symbol_exists(memfd_create "sys/mman.h" HAVE_MEMFD_CREATE)
-check_symbol_exists(secure_getenv "stdlib.h" HAVE_SECURE_GETENV)
+# Check whether any system-wide extensions need to be enabled, in order for
+# OS functionality to be exposed.
+pcre2_use_system_extensions()
+
+cmake_push_check_state(RESET)
+# Somewhat awkward code to propagate the _GNU_SOURCE definition (added to
+# COMPILE_DEFINITIONS by pcre2_use_system_extensions()).
+get_directory_property(_pcre2_compile_definitions COMPILE_DEFINITIONS)
+if(DEFINED _pcre2_compile_definitions)
+  set(CMAKE_REQUIRED_DEFINITIONS "")
+  foreach(_def IN LISTS _pcre2_compile_definitions)
+    list(APPEND CMAKE_REQUIRED_DEFINITIONS "-D${_def}")
+  endforeach()
+endif()
+
+check_symbol_exists(mkostemp stdlib.h HAVE_MKOSTEMP) # glibc 2.7
+check_symbol_exists(memfd_create "sys/mman.h" HAVE_MEMFD_CREATE) # glibc 2.27
+check_symbol_exists(secure_getenv "stdlib.h" HAVE_SECURE_GETENV) # glibc 2.17
+cmake_pop_check_state()
 
 check_c_source_compiles(
   [=[
@@ -184,10 +202,10 @@ check_c_source_compiles(
   HAVE_REALPATH
 )
 
+cmake_push_check_state()
 if(NOT DEFINED CMAKE_REQUIRED_FLAGS)
   set(CMAKE_REQUIRED_FLAGS "")
 endif()
-set(ORIG_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 if(NOT MSVC AND NOT CMAKE_C_COMPILER_ID STREQUAL "XL" AND NOT CMAKE_C_COMPILER_ID STREQUAL "SunPro")
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror")
 endif()
@@ -197,6 +215,7 @@ check_c_source_compiles(
   HAVE_ATTRIBUTE_UNINITIALIZED
 )
 
+# TODO Consider switching to modern "GenerateExportHeader"
 check_c_source_compiles(
   [=[
   extern __attribute__ ((visibility ("default"))) int f(void);
@@ -206,14 +225,14 @@ check_c_source_compiles(
   HAVE_VISIBILITY
 )
 
+cmake_pop_check_state()
+
 if(HAVE_VISIBILITY)
   set(PCRE2_EXPORT [=[__attribute__ ((visibility ("default")))]=])
 else()
   set(PCRE2_EXPORT)
 endif()
 
-set(CMAKE_REQUIRED_FLAGS ${ORIG_CMAKE_REQUIRED_FLAGS})
-
 check_c_source_compiles("int main(void) { __assume(1); return 0; }" HAVE_BUILTIN_ASSUME)
 
 check_c_source_compiles(
@@ -271,11 +290,6 @@ if(INTEL_CET_ENABLED)
   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -mshstk")
 endif()
 
-# Check whether any system-wide extensions need to be enabled, in order for
-# OS functionality to be exposed.
-
-pcre2_use_system_extensions()
-
 # User-configurable options
 #
 # Note: CMakeSetup displays these in alphabetical order, regardless of
@@ -436,32 +450,45 @@ if(BZIP2_FOUND)
   option(PCRE2_SUPPORT_LIBBZ2 "Enable support for linking pcre2grep with libbz2." ON)
 endif()
 if(PCRE2_SUPPORT_LIBBZ2)
-  include_directories(${BZIP2_INCLUDE_DIR})
+  if(BZIP2_FOUND)
+    include_directories(${BZIP2_INCLUDE_DIR})
+  else()
+    message(
+      FATAL_ERROR
+      " libbz2 not found. Set BZIP2_INCLUDE_DIR to a compatible header\n"
+      " or set BZip2_ROOT to a full bzip2 installed tree, as needed."
+    )
+  endif()
 endif()
 
 # zlib
 if(ZLIB_FOUND)
   option(PCRE2_SUPPORT_LIBZ "Enable support for linking pcre2grep with libz." ON)
 endif()
 if(PCRE2_SUPPORT_LIBZ)
-  include_directories(${ZLIB_INCLUDE_DIR})
+  if(ZLIB_FOUND)
+    include_directories(${ZLIB_INCLUDE_DIR})
+  else()
+    message(
+      FATAL_ERROR
+      " zlib not found. Set ZLIB_INCLUDE_DIR to a compatible header\n"
+      " or set ZLIB_ROOT to a full zlib installed tree, as needed."
+    )
+  endif()
 endif()
 
 # editline lib
 if(EDITLINE_FOUND)
   option(PCRE2_SUPPORT_LIBEDIT "Enable support for linking pcre2test with libedit." OFF)
 endif()
-if(EDITLINE_FOUND)
-  if(PCRE2_SUPPORT_LIBEDIT)
+if(PCRE2_SUPPORT_LIBEDIT)
+  if(EDITLINE_FOUND)
     include_directories(${EDITLINE_INCLUDE_DIR})
-  endif()
-else()
-  if(PCRE2_SUPPORT_LIBEDIT)
+  else()
     message(
       FATAL_ERROR
-      " libedit not found, set EDITLINE_INCLUDE_DIR to a compatible header\n"
-      " or set Editline_ROOT to a full libedit installed tree, as needed\n"
-      " Might need to enable policy CMP0074 in CMakeLists.txt"
+      " libedit not found. Set EDITLINE_INCLUDE_DIR to a compatible header\n"
+      " or set Editline_ROOT to a full libedit installed tree, as needed."
     )
   endif()
 endif()
@@ -471,7 +498,15 @@ if(READLINE_FOUND)
   option(PCRE2_SUPPORT_LIBREADLINE "Enable support for linking pcre2test with libreadline." ON)
 endif()
 if(PCRE2_SUPPORT_LIBREADLINE)
-  include_directories(${READLINE_INCLUDE_DIR})
+  if(READLINE_FOUND)
+    include_directories(${READLINE_INCLUDE_DIR})
+  else()
+    message(
+      FATAL_ERROR
+      " libreadline not found. Set READLINE_INCLUDE_DIR to a compatible header\n"
+      " or set Readline_ROOT to a full libreadline installed tree, as needed."
+    )
+  endif()
 endif()
 
 # Prepare build configuration
@@ -537,19 +572,7 @@ if(PCRE2_SUPPORT_JIT)
 endif()
 
 if(PCRE2_SUPPORT_JIT_SEALLOC)
-  set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE)
-  check_symbol_exists(mkostemp stdlib.h REQUIRED)
-  unset(CMAKE_REQUIRED_DEFINITIONS)
-  if(${REQUIRED})
-    if(${CMAKE_SYSTEM_NAME} MATCHES Linux|NetBSD)
-      add_compile_definitions(_GNU_SOURCE)
-      set(SLJIT_PROT_EXECUTABLE_ALLOCATOR 1)
-    else()
-      message(FATAL_ERROR "Your configuration is not supported")
-    endif()
-  else()
-    set(PCRE2_SUPPORT_JIT_SEALLOC OFF)
-  endif()
+  set(SLJIT_PROT_EXECUTABLE_ALLOCATOR 1)
 endif()
 
 if(PCRE2GREP_SUPPORT_JIT)
@@ -1491,7 +1514,12 @@ if(PCRE2_SHOW_REPORT)
     endforeach()
   else()
     string(TOUPPER "${CMAKE_BUILD_TYPE}" buildtype)
-    message(STATUS "  C compiler flags .................. : ${CMAKE_C_FLAGS}${CFSP}${CMAKE_C_FLAGS_${buildtype}}")
+    if(buildtype STREQUAL "")
+      set(CFBLD "")
+    else()
+      set(CFBLD "${CMAKE_C_FLAGS_${buildtype}}")
+    endif()
+    message(STATUS "  C compiler flags .................. : ${CMAKE_C_FLAGS}${CFSP}${CFBLD}")
   endif()
 
   message(STATUS "")

cmake/FindEditline.cmake

@@ -1,13 +1,10 @@
 # Modified from FindReadline.cmake (PH Feb 2012)
 
-if(EDITLINE_INCLUDE_DIR AND EDITLINE_LIBRARY)
-  set(EDITLINE_FOUND TRUE)
-else()
-  find_path(EDITLINE_INCLUDE_DIR readline.h PATH_SUFFIXES editline edit/readline)
+find_path(EDITLINE_INCLUDE_DIR readline.h PATH_SUFFIXES editline edit/readline)
+mark_as_advanced(EDITLINE_INCLUDE_DIR)
 
-  find_library(EDITLINE_LIBRARY NAMES edit)
-  include(FindPackageHandleStandardArgs)
-  find_package_handle_standard_args(Editline DEFAULT_MSG EDITLINE_INCLUDE_DIR EDITLINE_LIBRARY)
+find_library(EDITLINE_LIBRARY NAMES edit)
+mark_as_advanced(EDITLINE_LIBRARY)
 
-  mark_as_advanced(EDITLINE_INCLUDE_DIR EDITLINE_LIBRARY)
-endif()
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(Editline DEFAULT_MSG EDITLINE_LIBRARY EDITLINE_INCLUDE_DIR)

cmake/FindReadline.cmake

@@ -3,25 +3,53 @@
 # --> BSD licensed
 #
 # GNU Readline library finder
-if(READLINE_INCLUDE_DIR AND READLINE_LIBRARY AND NCURSES_LIBRARY)
-  set(READLINE_FOUND TRUE)
-else()
-  find_path(READLINE_INCLUDE_DIR readline/readline.h /usr/include/readline)
-
-  # 2008-04-22 The next clause used to read like this:
-  #
-  #  FIND_LIBRARY(READLINE_LIBRARY NAMES readline)
-  #        FIND_LIBRARY(NCURSES_LIBRARY NAMES ncurses )
-  #        include(FindPackageHandleStandardArgs)
-  #        FIND_PACKAGE_HANDLE_STANDARD_ARGS(Readline DEFAULT_MSG NCURSES_LIBRARY READLINE_INCLUDE_DIR READLINE_LIBRARY )
-  #
-  # I was advised to modify it such that it will find an ncurses library if
-  # required, but not if one was explicitly given, that is, it allows the
-  # default to be overridden. PH
-
-  find_library(READLINE_LIBRARY NAMES readline)
-  include(FindPackageHandleStandardArgs)
-  find_package_handle_standard_args(Readline DEFAULT_MSG READLINE_INCLUDE_DIR READLINE_LIBRARY)
-
-  mark_as_advanced(READLINE_INCLUDE_DIR READLINE_LIBRARY)
+
+find_path(READLINE_INCLUDE_DIR readline/readline.h PATH_SUFFIXES include)
+mark_as_advanced(READLINE_INCLUDE_DIR)
+
+find_library(READLINE_LIBRARY NAMES readline)
+mark_as_advanced(READLINE_LIBRARY)
+
+if(READLINE_INCLUDE_DIR AND READLINE_LIBRARY)
+  # Check if we need to link to ncurses as well
+
+  include(CheckSymbolExists)
+  include(CMakePushCheckState)
+
+  cmake_push_check_state(RESET)
+  set(CMAKE_REQUIRED_LIBRARIES "${READLINE_LIBRARY}")
+  set(CMAKE_REQUIRED_INCLUDES "${READLINE_INCLUDE_DIR}")
+  check_symbol_exists("readline" "stdio.h;readline/readline.h" HAVE_READLINE_FUNC)
+
+  if(NOT HAVE_READLINE_FUNC)
+    foreach(
+      lib IN ITEMS
+      tinfo
+      curses
+      ncurses
+      ncursesw
+      termcap
+    )
+      find_library(NCURSES_LIBRARY_${lib} NAMES ${lib})
+      mark_as_advanced(NCURSES_LIBRARY_${lib})
+      if(NCURSES_LIBRARY_${lib})
+        cmake_reset_check_state()
+        set(CMAKE_REQUIRED_LIBRARIES "${READLINE_LIBRARY}" "${NCURSES_LIBRARY_${lib}}")
+        set(CMAKE_REQUIRED_INCLUDES "${READLINE_INCLUDE_DIR}")
+        check_symbol_exists("readline" "stdio.h;readline/readline.h" HAVE_READLINE_FUNC_${lib})
+
+        if(HAVE_READLINE_FUNC_${lib})
+          message(STATUS "Looking for readline - readline needs ${lib}")
+          set(NCURSES_LIBRARY "${NCURSES_LIBRARY_${lib}}" CACHE FILEPATH "Path to the ncurses library")
+          mark_as_advanced(NCURSES_LIBRARY)
+          break()
+        endif()
+      endif()
+    endforeach()
+  endif()
+
+  cmake_pop_check_state()
 endif()
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(Readline DEFAULT_MSG READLINE_LIBRARY READLINE_INCLUDE_DIR)

cmake/PCRE2UseSystemExtensions.cmake

@@ -10,38 +10,73 @@
 # So far, we know that we require:
 #   - _ALL_SOURCE on IBM systems (z/OS, probably AIX) in order to call
 #     getrlimit() in pcre2test.
+#   - _GNU_SOURCE on Linux in order to call mkostemp() in some (non-default)
+#     configurations of the JIT.
 #
 # Autoconf enables this unconditionally. However, our CMake script potentially
 # supports *more* platforms than Autoconf, so we use a feature check.
 
 function(pcre2_use_system_extensions)
-  if (WIN32)
+  if(WIN32)
     return()
   endif()
 
+  include(CheckSymbolExists)
   include(CheckCSourceCompiles)
+  include(CMakePushCheckState)
 
-  set(_pcre2_test_src "
-#include <sys/time.h>
-#include <sys/resource.h>
+  cmake_push_check_state(RESET)
+  set(
+    _pcre2_test_src
+    [=[
+    #include <sys/time.h>
+    #include <sys/resource.h>
 
-int main(void) {
-    struct rlimit rlim;
-    getrlimit(RLIMIT_STACK, &rlim);
-    return 0;
-}
-")
+    int main(void) {
+        struct rlimit rlim;
+        getrlimit(RLIMIT_STACK, &rlim);
+        return 0;
+    }
+    ]=]
+  )
+  set(CMAKE_REQUIRED_QUIET TRUE)
+  check_c_source_compiles("${_pcre2_test_src}" _HAVE_GETRLIMIT_NAKED)
 
-  check_c_source_compiles("${_pcre2_test_src}" HAVE_GETRLIMIT)
-
-  if (NOT HAVE_GETRLIMIT)
+  if(NOT _HAVE_GETRLIMIT_NAKED)
     # Try again with _ALL_SOURCE
+    if(NOT _HAVE_GETRLIMIT_ALLSOURCE)
+      set(_allsource_first_run TRUE)
+    endif()
     set(CMAKE_REQUIRED_DEFINITIONS "-D_ALL_SOURCE")
-    check_c_source_compiles("${_pcre2_test_src}" HAVE_GETRLIMIT_ALLSOURCE)
+    check_c_source_compiles("${_pcre2_test_src}" _HAVE_GETRLIMIT_ALLSOURCE)
     unset(CMAKE_REQUIRED_DEFINITIONS)
 
-    if (HAVE_GETRLIMIT_ALLSOURCE)
+    if(_HAVE_GETRLIMIT_ALLSOURCE)
       add_compile_definitions(_ALL_SOURCE)
+      if(_allsource_first_run)
+        message(STATUS "Detected platform feature gate _ALL_SOURCE")
+      endif()
+    endif()
+  endif()
+
+  check_symbol_exists(mkostemp stdlib.h _HAVE_MKOSTEMP_NAKED)
+
+  if(NOT _HAVE_MKOSTEMP_NAKED)
+    # Try again with _GNU_SOURCE
+    if(NOT _HAVE_MKOSTEMP_GNUSOURCE)
+      set(_gnusource_first_run TRUE)
+    endif()
+    set(CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE")
+    check_symbol_exists(mkostemp stdlib.h _HAVE_MKOSTEMP_GNUSOURCE)
+    unset(CMAKE_REQUIRED_DEFINITIONS)
+
+    if(_HAVE_MKOSTEMP_GNUSOURCE)
+      add_compile_definitions(_GNU_SOURCE)
+      if(_gnusource_first_run)
+        message(STATUS "Detected platform feature gate _GNU_SOURCE")
+      endif()
     endif()
   endif()
+
+  cmake_pop_check_state()
 endfunction()

configure.ac

@@ -677,7 +677,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([[
 AC_MSG_RESULT([no]))
 LIBS="$OLD_LIBS"
 
-# Check for the availabiity of libreadline
+# Check for the availability of libreadline
 
 if test "$enable_pcre2test_libreadline" = "yes"; then
  AC_CHECK_HEADERS([readline/readline.h], [HAVE_READLINE_H=1])