Merge Fix PackageConfig

This PR fixes PackageConfig and create the corresponding test Related PR: #1109
ginkgo-project · Nov 6, 2022 · ca2a213 · ca2a213
2 parents 64c18ca + 2c9e4ac
commit ca2a213
Show file tree

Hide file tree

Showing 6 changed files with 127 additions and 77 deletions.
diff --git a/.gitlab/scripts.yml b/.gitlab/scripts.yml
@@ -52,6 +52,7 @@
         -DGINKGO_EXPORT_BUILD_DIR=${EXPORT_BUILD_DIR}
     - ninja -j${NUM_CORES} -l${CI_LOAD_LIMIT} install
     - if [ "${EXPORT_BUILD_DIR}" == "ON" ]; then ninja test_exportbuild; fi
+    - LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH ninja test_pkgconfig
   dependencies: []
 
 .build_and_test_template:
@@ -123,6 +124,7 @@
     - if [ -n "${SYCL_DEVICE_TYPE}" ]; then unset SYCL_DEVICE_TYPE; fi
     - if [ -n "${SYCL_DEVICE_FILTER}" ]; then unset SYCL_DEVICE_FILTER; fi
     - if [ "${EXPORT_BUILD_DIR}" == "ON" ]; then ninja test_exportbuild; fi
+    - LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH ninja test_pkgconfig
   dependencies: []
 
 

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -345,6 +345,15 @@ if(GINKGO_BUILD_DOC)
     add_subdirectory(doc)
 endif()
 
+
+# add escape character '\' for space
+string(REPLACE " " "\ " PKG_CMAKE_INSTALL_PREFIX "${CMAKE_INSTALL_PREFIX}")
+# add escape character '\' for space in regex mode
+list(TRANSFORM GINKGO_INTERFACE_LINK_FLAGS REPLACE " " "\\\\ ")
+list(TRANSFORM GINKGO_INTERFACE_CXX_FLAGS REPLACE " " "\\\\ ")
+# convert the list to string
+string(REPLACE ";" " " GINKGO_INTERFACE_CXX_FLAGS "${GINKGO_INTERFACE_CXX_FLAGS}")
+string(REPLACE ";" " " GINKGO_INTERFACE_LINK_FLAGS "${GINKGO_INTERFACE_LINK_FLAGS}")
 configure_file(${Ginkgo_SOURCE_DIR}/cmake/ginkgo.pc.in
     ${Ginkgo_BINARY_DIR}/ginkgo.pc.in @ONLY)
 file(GENERATE OUTPUT ${Ginkgo_BINARY_DIR}/ginkgo_$<CONFIG>.pc
@@ -361,21 +370,19 @@ set(GINKGO_TEST_INSTALL_SRC_DIR "${Ginkgo_SOURCE_DIR}/test/test_install/")
 set(GINKGO_TEST_INSTALL_BIN_DIR "${Ginkgo_BINARY_DIR}/test/test_install/")
 set(GINKGO_TEST_EXPORTBUILD_SRC_DIR "${Ginkgo_SOURCE_DIR}/test/test_exportbuild/")
 set(GINKGO_TEST_EXPORTBUILD_BIN_DIR "${Ginkgo_BINARY_DIR}/test/test_exportbuild/")
-if(MSVC)
-    set(GINKGO_TEST_INSTALL_CMD ${GINKGO_TEST_INSTALL_BIN_DIR}/$<CONFIG>/test_install)
-    set(GINKGO_TEST_EXPORTBUILD_CMD ${GINKGO_TEST_EXPORTBUILD_BIN_DIR}/$<CONFIG>/test_exportbuild)
-    if(GINKGO_BUILD_CUDA)
-        set(GINKGO_TEST_INSTALL_CUDA_CMD ${GINKGO_TEST_INSTALL_BIN_DIR}/$<CONFIG>/test_install_cuda)
-    endif()
-else()
-    set(GINKGO_TEST_INSTALL_CMD ${GINKGO_TEST_INSTALL_BIN_DIR}/test_install)
-    set(GINKGO_TEST_EXPORTBUILD_CMD ${GINKGO_TEST_EXPORTBUILD_BIN_DIR}/test_exportbuild)
-    if(GINKGO_BUILD_CUDA)
-        set(GINKGO_TEST_INSTALL_CUDA_CMD ${GINKGO_TEST_INSTALL_BIN_DIR}/test_install_cuda)
-    endif()
+set(GINKGO_TEST_PKGCONFIG_SRC_DIR "${Ginkgo_SOURCE_DIR}/test/test_pkgconfig/")
+set(GINKGO_TEST_PKGCONFIG_BIN_DIR "${Ginkgo_BINARY_DIR}/test/test_pkgconfig/")
+get_property(GINKGO_USE_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
+# GINKGO_CONFIG_PREFIX contains / in the end.
+set(GINKGO_CONFIG_PREFIX "$<$<BOOL:${GINKGO_USE_MULTI_CONFIG}>:$<CONFIG>/>")
+set(GINKGO_TEST_INSTALL_CMD ${GINKGO_TEST_INSTALL_BIN_DIR}/${GINKGO_CONFIG_PREFIX}test_install)
+set(GINKGO_TEST_EXPORTBUILD_CMD ${GINKGO_TEST_EXPORTBUILD_BIN_DIR}/${GINKGO_CONFIG_PREFIX}test_exportbuild)
+set(GINKGO_TEST_PKGCONFIG_CMD ${GINKGO_TEST_PKGCONFIG_BIN_DIR}/${GINKGO_CONFIG_PREFIX}test_pkgconfig)
+if(GINKGO_BUILD_CUDA)
+    set(GINKGO_TEST_INSTALL_CUDA_CMD ${GINKGO_TEST_INSTALL_BIN_DIR}/${GINKGO_CONFIG_PREFIX}test_install_cuda)
 endif()
 if(GINKGO_BUILD_HIP)
-    set(GINKGO_TEST_INSTALL_HIP_CMD ${GINKGO_TEST_INSTALL_BIN_DIR}/test_install_hip)
+    set(GINKGO_TEST_INSTALL_HIP_CMD ${GINKGO_TEST_INSTALL_BIN_DIR}/${GINKGO_CONFIG_PREFIX}test_install_hip)
 endif()
 
 file(MAKE_DIRECTORY "${GINKGO_TEST_INSTALL_BIN_DIR}")
@@ -425,6 +432,24 @@ add_custom_target(test_exportbuild
     COMMENT "Running a test on Ginkgo's exported build directory. "
     "This requires compiling Ginkgo with `-DGINKGO_EXPORT_BUILD_DIR=ON` first.")
 
+add_custom_target(test_pkgconfig
+    COMMAND ${CMAKE_COMMAND} -G${CMAKE_GENERATOR} ${TOOLSET}
+    -H${GINKGO_TEST_PKGCONFIG_SRC_DIR}
+    -B${GINKGO_TEST_PKGCONFIG_BIN_DIR}
+    -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
+    -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
+    -DCMAKE_CUDA_COMPILER=${CMAKE_CUDA_COMPILER}
+    -DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}
+    # `--config cfg` is ignored by single-configuration generator.
+    # `$<CONFIG>` is always be the same as `CMAKE_BUILD_TYPE` in
+    # single-configuration generator.
+    COMMAND ${CMAKE_COMMAND}
+    --build ${GINKGO_TEST_PKGCONFIG_BIN_DIR}
+    --config $<CONFIG>
+    COMMAND ${GINKGO_TEST_PKGCONFIG_CMD}
+    COMMENT "Running a test on Ginkgo's PkgConfig"
+    "This requires installing Ginkgo first")
+
 
 # Setup CPack
 set(CPACK_PACKAGE_DESCRIPTION_FILE "${Ginkgo_SOURCE_DIR}/README.md")

diff --git a/cmake/Modules/FindPAPI.cmake b/cmake/Modules/FindPAPI.cmake
@@ -118,13 +118,15 @@ if(PAPI_FOUND)
         if(EXISTS "${PAPI_LIBRARIES}")
             set_target_properties(PAPI::PAPI PROPERTIES
                 IMPORTED_LINK_INTERFACE_LANGUAGES "C"
+                INTERFACE_LINK_LIBRARIES "${PAPI_LIBRARIES}"
                 IMPORTED_LOCATION "${PAPI_LIBRARIES}")
         endif()
         if(PAPI_LIBRARY_RELEASE)
             set_property(TARGET PAPI::PAPI APPEND PROPERTY
                 IMPORTED_CONFIGURATIONS RELEASE)
             set_target_properties(PAPI::PAPI PROPERTIES
                 IMPORTED_LINK_INTERFACE_LANGUAGES "C"
+                INTERFACE_LINK_LIBRARIES_RELEASE "${PAPI_LIBRARY_RELEASE}"
                 IMPORTED_LOCATION_RELEASE "${PAPI_LIBRARY_RELEASE}")
             unset(PAPI_LIBRARY_RELEASE)
         endif()
@@ -133,6 +135,7 @@ if(PAPI_FOUND)
                 IMPORTED_CONFIGURATIONS DEBUG)
             set_target_properties(PAPI::PAPI PROPERTIES
                 IMPORTED_LINK_INTERFACE_LANGUAGES "C"
+                INTERFACE_LINK_LIBRARIES_DEBUG "${PAPI_LIBRARY_DEBUG}"
                 IMPORTED_LOCATION_DEBUG "${PAPI_LIBRARY_DEBUG}")
             unset(PAPI_LIBRARY_DEBUG)
         endif()

diff --git a/cmake/ginkgo.pc.in b/cmake/ginkgo.pc.in
@@ -1,4 +1,4 @@
-prefix=@CMAKE_INSTALL_PREFIX@
+prefix=@PKG_CMAKE_INSTALL_PREFIX@
 libdir=${prefix}/@GINKGO_INSTALL_LIBRARY_DIR@
 includedir=${prefix}/@GINKGO_INSTALL_INCLUDE_DIR@
 

diff --git a/cmake/information_helpers.cmake b/cmake/information_helpers.cmake
@@ -1,3 +1,5 @@
+# TODO: we may use file(GENERATE ... TARGET ...) to generate config file based on the target property
+#       when we bump the CMake minimum version to 3.15/3.19
 function(filter_generator_expressions INPUT OUTPUT)
     # See https://gitlab.kitware.com/cmake/cmake/-/blob/v3.22.2/Modules/FindMPI.cmake#L1218
     # and other versions of this file for what we are removing here.
@@ -10,67 +12,74 @@ function(filter_generator_expressions INPUT OUTPUT)
     string(REGEX REPLACE "[$<A-Z_:]*SHELL:(.+)>*" "\\1" TMP "${TMP}")
     string(REGEX REPLACE ".+INTERFACE:.+>" "" TMP "${TMP}")
     string(REGEX REPLACE "\$<COMMA>" "," TMP "${TMP}")
+    # Remove the left : or > 
+    string(REGEX REPLACE ":|>" "" TMP "${TMP}")
     # Ignore hwloc include if it is the internal one
     string(REGEX REPLACE "${PROJECT_BINARY_DIR}.*hwloc/src/include.*" "" TMP "${TMP}")
     set(${OUTPUT} "${TMP}" PARENT_SCOPE)
 endfunction()
 
 macro(ginkgo_interface_libraries_recursively INTERFACE_LIBS)
+    # always add the interface to the list to keep the order information 
+    # Currently, it does not support the circular dependence and MSVC.
     foreach(_libs ${INTERFACE_LIBS})
-        if (NOT "${_libs}" IN_LIST GINKGO_INTERFACE_LIBS_FOUND
-                AND NOT "-l${_libs}" IN_LIST GINKGO_INTERFACE_LIBS_FOUND)
-            if (TARGET ${_libs})
-                if (upper_CMAKE_BUILD_TYPE STREQUAL "DEBUG" AND "${_libs}" MATCHES "ginkgo.*")
-                    set(GINKGO_INTERFACE_LIB_NAME "-l${_libs}${CMAKE_DEBUG_POSTFIX}")
-                elseif("${_libs}" MATCHES "ginkgo.*") # Ginkgo libs are appended in the form -l
-                    set(GINKGO_INTERFACE_LIB_NAME "-l${_libs}")
-                endif()
-
-                # Get the link flags and treat them
-                get_target_property(GINKGO_INTERFACE_LIBS_LINK_FLAGS "${_libs}"
-                    INTERFACE_LINK_OPTIONS)
-                if (GINKGO_INTERFACE_LIBS_LINK_FLAGS)
-                    filter_generator_expressions("${GINKGO_INTERFACE_LIBS_LINK_FLAGS}"
-                        GINKGO_INTERFACE_LIB_NAME)
+        if (TARGET ${_libs})
+            if("${_libs}" MATCHES "ginkgo.*")
+                set(GINKGO_INTERFACE_LIB_NAME "-l${_libs}$<$<CONFIG:Debug>:${CMAKE_DEBUG_POSTFIX}>")
+                list(APPEND GINKGO_INTERFACE_LIBS_FOUND "${GINKGO_INTERFACE_LIB_NAME}")
+            endif()
+            # Get the link flags and treat them
+            get_target_property(GINKGO_INTERFACE_LIBS_LINK_FLAGS "${_libs}"
+                INTERFACE_LINK_OPTIONS)
+            if (GINKGO_INTERFACE_LIBS_LINK_FLAGS)
+                filter_generator_expressions("${GINKGO_INTERFACE_LIBS_LINK_FLAGS}"
+                    GINKGO_INTERFACE_LIB_NAME)
+            endif()
+            # Get the imported library
+            get_target_property(_libs_type "${_libs}" TYPE)
+            get_target_property(_libs_imported "${_libs}" IMPORTED)
+            if (_libs_imported AND NOT ${_libs_type} STREQUAL "INTERFACE_LIBRARY")
+                # only get the value from release for HIP related target
+                get_target_property(GINKGO_LIBS_IMPORTED_LIBS "${_libs}" IMPORTED_LOCATION_RELEASE)
+                if (GINKGO_LIBS_IMPORTED_LIBS)
+                    list(APPEND GINKGO_INTERFACE_LIBS_FOUND "${GINKGO_LIBS_IMPORTED_LIBS}")
                 endif()
-                if (NOT "${GINKGO_INTERFACE_LIB_NAME}" IN_LIST GINKGO_INTERFACE_LIBS_FOUND)
-                    list(APPEND GINKGO_INTERFACE_LIBS_FOUND "${GINKGO_INTERFACE_LIB_NAME}")
+            endif()
+            # Populate the include directories
+            get_target_property(GINKGO_LIBS_INTERFACE_INCS "${_libs}"
+                INTERFACE_INCLUDE_DIRECTORIES)
+            foreach(_incs ${GINKGO_LIBS_INTERFACE_INCS})
+                filter_generator_expressions("${_incs}" GINKGO_INTERFACE_INC_FILTERED)
+                if (GINKGO_INTERFACE_INC_FILTERED AND NOT
+                        "-I${GINKGO_INTERFACE_INC_FILTERED}" IN_LIST GINKGO_INTERFACE_CFLAGS_FOUND)
+                    list(APPEND GINKGO_INTERFACE_CFLAGS_FOUND "-I${GINKGO_INTERFACE_INC_FILTERED}")
                 endif()
+            endforeach()
 
-                # Populate the include directories
-                get_target_property(GINKGO_LIBS_INTERFACE_INCS "${_libs}"
-                    INTERFACE_INCLUDE_DIRECTORIES)
-                foreach(_incs ${GINKGO_LIBS_INTERFACE_INCS})
-                    filter_generator_expressions("${_incs}" GINKGO_INTERFACE_INC_FILTERED)
-                    if (GINKGO_INTERFACE_INC_FILTERED AND NOT
-                            "-I${GINKGO_INTERFACE_INC_FILTERED}" IN_LIST GINKGO_INTERFACE_CFLAGS_FOUND)
-                        list(APPEND GINKGO_INTERFACE_CFLAGS_FOUND "-I${GINKGO_INTERFACE_INC_FILTERED}")
-                    endif()
-                endforeach()
-
-                # Populate the compiler options and definitions if needed
-                get_target_property(GINKGO_LIBS_INTERFACE_DEFS "${_libs}"
-                    INTERFACE_COMPILE_DEFINITIONS)
-                if (GINKGO_LIBS_INTERFACE_DEFS)
-                    list(APPEND GINKGO_INTERFACE_CFLAGS_FOUND "${GINKGO_LIBS_INTERFACE_DEFS}")
-                endif()
-                get_target_property(GINKGO_LIBS_INTERFACE_OPTS "${_libs}"
-                    INTERFACE_COMPILE_OPTIONS)
-                filter_generator_expressions("${GINKGO_LIBS_INTERFACE_OPTS}" GINKGO_LIBS_INTERFACE_OPTS_FILTERED)
-                if (GINKGO_LIBS_INTERFACE_OPTS)
-                    list(APPEND GINKGO_INTERFACE_CFLAGS_FOUND "${GINKGO_LIBS_INTERFACE_OPTS_FILTERED}")
-                endif()
+            # Populate the compiler options and definitions if needed
+            get_target_property(GINKGO_LIBS_INTERFACE_DEFS "${_libs}"
+                INTERFACE_COMPILE_DEFINITIONS)
+            if (GINKGO_LIBS_INTERFACE_DEFS)
+                list(APPEND GINKGO_INTERFACE_CFLAGS_FOUND "${GINKGO_LIBS_INTERFACE_DEFS}")
+            endif()
+            get_target_property(GINKGO_LIBS_INTERFACE_OPTS "${_libs}"
+                INTERFACE_COMPILE_OPTIONS)
+            filter_generator_expressions("${GINKGO_LIBS_INTERFACE_OPTS}" GINKGO_LIBS_INTERFACE_OPTS_FILTERED)
+            if (GINKGO_LIBS_INTERFACE_OPTS)
+                list(APPEND GINKGO_INTERFACE_CFLAGS_FOUND "${GINKGO_LIBS_INTERFACE_OPTS_FILTERED}")
+            endif()
 
-                # Keep recursing through the libraries
-                get_target_property(GINKGO_LIBS_INTERFACE_LIBS "${_libs}"
-                    INTERFACE_LINK_LIBRARIES)
-                ginkgo_interface_libraries_recursively("${GINKGO_LIBS_INTERFACE_LIBS}")
-            elseif(EXISTS "${_libs}")
-                if ("${_libs}" MATCHES "${PROJECT_BINARY_DIR}.*hwloc.so")
-                    list(APPEND GINKGO_INTERFACE_LIBS_FOUND "${CMAKE_INSTALL_PREFIX}/${GINKGO_INSTALL_LIBRARY_DIR}/libhwloc.so")
-                else()
-                    list(APPEND GINKGO_INTERFACE_LIBS_FOUND "${_libs}")
-                endif()
+            # Keep recursing through the libraries
+            get_target_property(GINKGO_LIBS_INTERFACE_LIBS "${_libs}"
+                INTERFACE_LINK_LIBRARIES)
+            # removing $<LINK_ONLY:>
+            list(TRANSFORM GINKGO_LIBS_INTERFACE_LIBS REPLACE "\\$<LINK_ONLY:(.*)>" "\\1")
+            ginkgo_interface_libraries_recursively("${GINKGO_LIBS_INTERFACE_LIBS}")
+        elseif(EXISTS "${_libs}")
+            if ("${_libs}" MATCHES "${PROJECT_BINARY_DIR}.*hwloc.so")
+                list(APPEND GINKGO_INTERFACE_LIBS_FOUND "${CMAKE_INSTALL_PREFIX}/${GINKGO_INSTALL_LIBRARY_DIR}/libhwloc.so")
+            else()
+                list(APPEND GINKGO_INTERFACE_LIBS_FOUND "${_libs}")
             endif()
         endif()
     endforeach()
@@ -81,34 +90,29 @@ macro(ginkgo_interface_information)
     unset(GINKGO_INTERFACE_LIBS_FOUND)
     unset(GINKGO_INTERFACE_CFLAGS_FOUND)
     # Prepare recursively populated library list
-    string(TOUPPER "${CMAKE_BUILD_TYPE}" upper_CMAKE_BUILD_TYPE)
-    if (upper_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
-        list(APPEND GINKGO_INTERFACE_LIBS_FOUND "-lginkgo${CMAKE_DEBUG_POSTFIX}")
-    else()
-        list(APPEND GINKGO_INTERFACE_LIBS_FOUND "-lginkgo")
-    endif()
+    list(APPEND GINKGO_INTERFACE_LIBS_FOUND "-lginkgo$<$<CONFIG:Debug>:${CMAKE_DEBUG_POSTFIX}>")
     # Prepare recursively populated include directory list
     list(APPEND GINKGO_INTERFACE_CFLAGS_FOUND
         "-I${CMAKE_INSTALL_PREFIX}/${GINKGO_INSTALL_INCLUDE_DIR}")
 
     # Call the recursive interface libraries macro
     get_target_property(GINKGO_INTERFACE_LINK_LIBRARIES ginkgo INTERFACE_LINK_LIBRARIES)
     ginkgo_interface_libraries_recursively("${GINKGO_INTERFACE_LINK_LIBRARIES}")
-
     # Format and store the interface libraries found
+    # remove duplicates on the reversed list to keep the dependecy in the end of list.
+    list(REVERSE GINKGO_INTERFACE_LIBS_FOUND)
     list(REMOVE_DUPLICATES GINKGO_INTERFACE_LIBS_FOUND)
+    list(REVERSE GINKGO_INTERFACE_LIBS_FOUND)
     list(REMOVE_ITEM GINKGO_INTERFACE_LIBS_FOUND "")
-    string(REPLACE ";" " "
-        GINKGO_FORMATTED_INTERFACE_LIBS_FOUND "${GINKGO_INTERFACE_LIBS_FOUND}")
+    # keep it as list 
     set(GINKGO_INTERFACE_LINK_FLAGS
-        "${GINKGO_INTERFACE_LINK_FLAGS} ${GINKGO_FORMATTED_INTERFACE_LIBS_FOUND}")
+        ${GINKGO_INTERFACE_LINK_FLAGS} ${GINKGO_INTERFACE_LIBS_FOUND})
     unset(GINKGO_INTERFACE_LIBS_FOUND)
     # Format and store the interface cflags found
     list(REMOVE_DUPLICATES GINKGO_INTERFACE_CFLAGS_FOUND)
     list(REMOVE_ITEM GINKGO_INTERFACE_CFLAGS_FOUND "")
-    string(REPLACE ";" " "
-        GINKGO_FORMATTED_INTERFACE_CFLAGS_FOUND "${GINKGO_INTERFACE_CFLAGS_FOUND}")
-    set(GINKGO_INTERFACE_CXX_FLAGS "${GINKGO_FORMATTED_INTERFACE_CFLAGS_FOUND}")
+    # Keep it as list
+    set(GINKGO_INTERFACE_CXX_FLAGS ${GINKGO_INTERFACE_CFLAGS_FOUND})
     unset(GINKGO_INTERFACE_CFLAGS_FOUND)
 endmacro(ginkgo_interface_information)
 

diff --git a/test/test_pkgconfig/CMakeLists.txt b/test/test_pkgconfig/CMakeLists.txt
@@ -0,0 +1,16 @@
+cmake_minimum_required(VERSION 3.9)
+project(GinkgoExportBuildWithPkgConfigTest LANGUAGES CXX)
+
+find_package(PkgConfig REQUIRED)
+pkg_check_modules(GINKGO REQUIRED IMPORTED_TARGET ginkgo)
+
+
+# Here, we use test install without any data. We instantiate the
+# interface only.
+add_executable(test_pkgconfig ../test_install/test_install.cpp)
+target_compile_features(test_pkgconfig PUBLIC cxx_std_14)
+# CMake PkgConfig only puts the -l, -L, and -framework into link_libraries and others into link_options
+# When linking the target, the linking option will be before the compiled object to lead the linking error
+set_property(TARGET PkgConfig::GINKGO PROPERTY INTERFACE_LINK_LIBRARIES "${GINKGO_LDFLAGS}")
+set_property(TARGET PkgConfig::GINKGO PROPERTY INTERFACE_LINK_OPTIONS "")
+target_link_libraries(test_pkgconfig PRIVATE PkgConfig::GINKGO)