Cleanup CMake redundancies and use FILE_SET for headers -- only solution that installed non-flat header tree by transitive dependencies.

Author: cboulay

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required (VERSION 3.16)
+cmake_minimum_required (VERSION 3.23)
 project (liblsl
 	VERSION 1.16.2
 	LANGUAGES C CXX
@@ -17,7 +17,6 @@ include(cmake/SourceFiles.cmake)  #
 include(cmake/TargetObjLib.cmake)  #
 include(cmake/TargetLib.cmake)  #
 include(cmake/Installation.cmake)  #
-include(cmake/LSLCMake.cmake)
 include(cmake/TargetOther.cmake)
 
 if(LSL_UNITTESTS)

cmake/Dependencies.cmake

@@ -19,14 +19,15 @@ endif()
 add_library(lslboost INTERFACE)
 if(LSL_BUNDLED_BOOST)
     message(STATUS "Using bundled header-only Boost")
-    target_include_directories(lslboost SYSTEM INTERFACE
-            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/lslboost>)
+    target_include_directories(lslboost
+        SYSTEM INTERFACE
+            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/lslboost>
+    )
 else()
     message(STATUS "Using system Boost")
     find_package(Boost REQUIRED)
-    target_compile_definitions(lslboost INTERFACE
-            lslboost=boost # allows the LSL code base to work with the original Boost namespace/headers
-    )
+    # Map `lslboost` namespace, which LSL code base uses, to system `boost` namespace/headers.
+    target_compile_definitions(lslboost INTERFACE lslboost=boost)
     target_link_libraries(lslboost INTERFACE Boost::boost Boost::disable_autolinking)
 endif()
 target_compile_definitions(lslboost INTERFACE BOOST_ALL_NO_LIB)

cmake/Installation.cmake

@@ -19,30 +19,20 @@ write_basic_package_version_file(
 
 # Define installation targets
 set(LSLTargets lsl)
-if(LSL_BUILD_STATIC)
-    list(APPEND LSLTargets lslobj lslboost)
-endif()
 
 # Install the targets and store configuration information.
 install(TARGETS ${LSLTargets}
-    EXPORT LSLTargets  # generates a CMake package config; TODO: Why the same name as the list of targets?
+    EXPORT LSLTargets
     COMPONENT liblsl
     RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
     LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
     ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
-    INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
-    # If we use CMake 3.23 FILE_SET, replace INCLUDES line with: FILE_SET HEADERS DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
-)
-
-# TODO: What does this do? Why do we need LSLTargets.cmake in the build dir?
-export(EXPORT LSLTargets
-        FILE "${CMAKE_CURRENT_BINARY_DIR}/LSLTargets.cmake"
-        NAMESPACE LSL::
+    FILE_SET HEADERS DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
 )
 
 # Generate the LSLConfig.cmake file and mark it for installation
 install(EXPORT LSLTargets
-        FILE LSLTargets.cmake  # TODO: I think we can use this to generate LSLConfig.cmake, no?
+        FILE LSLConfig.cmake
         COMPONENT liblsl
         NAMESPACE "LSL::"
         DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/LSL
@@ -53,28 +43,10 @@ install(EXPORT LSLTargets
 #        INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/lsl)
 # If we use this method, then we need a corresponding install(FILES ...) command to install the generated file.
 
-# Copy hardcoded CMake files to the build directory.
-# TODO: Why bother with this copy? Is it not enough to install (into cmake/LSL)?
-configure_file(cmake/LSLCMake.cmake "${CMAKE_CURRENT_BINARY_DIR}/LSLCMake.cmake" COPYONLY)
-# TODO: Why bother with this copy? Is it not enough to install (into cmake/LSL)?
-# TODO: Why use hardcoded files? We can generate the LSLConfig.cmake.
-configure_file(cmake/LSLConfig.cmake "${CMAKE_CURRENT_BINARY_DIR}/LSLConfig.cmake" COPYONLY)
-
-# Install the public headers.
-# TODO: Verify that this is necessary, given that we already installed the INCLUDES above.
-# TODO: Verify if it is still necessary to install the headers if we use FILE_SET.
-install(DIRECTORY include/
-        COMPONENT liblsl
-        DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
-)
-
 # Install the version file and the helper CMake script.
 install(
     FILES
-        # TODO: Keep this. But why does the configure_file(... COPYONLY) above exist?
         cmake/LSLCMake.cmake
-        # TODO: Next line shouldn't be necessary if install(EXPORT...) uses LSLConfig.cmake instead of LSLTargets.cmake
-        ${CMAKE_CURRENT_BINARY_DIR}/LSLConfig.cmake
         ${CMAKE_CURRENT_BINARY_DIR}/LSLConfigVersion.cmake
     COMPONENT liblsl
     DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/LSL

cmake/LSLConfig.cmake

@@ -6,6 +6,7 @@ set_source_files_properties("src/buildinfo.cpp"
         LSL_LIBRARY_INFO_STR="${LSL_VERSION_INFO}/link:${LSL_LIB_TYPE}"
 )
 add_library(lsl ${LSL_LIB_TYPE} src/buildinfo.cpp)
+add_library(LSL::lsl ALIAS lsl)
 
 # Configure main library
 
@@ -23,19 +24,19 @@ if(LSL_FORCE_FANCY_LIBNAME)
     )
 endif()
 
-# Includes. TODO: Can we not inherit these from lslobj?
-target_include_directories(lsl
-        INTERFACE
-        $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
-        $<INSTALL_INTERFACE:include>
-)
+# Link dependencies. The only dependency is lslobj, which contains the bulk of the library code and linkages.
+# Note: We link PRIVATE because lslobj exposes extra symbols that are not part of the public API
+#  but are used by the internal tests.
+target_link_libraries(lsl PRIVATE lslobj)
 
-# Link dependencies. The biggest dependency is lslobj, which contains the bulk of the library code.
-target_link_libraries(lsl
-    PRIVATE
-        lslobj  # TODO: If this is public, does that improve inheritance of includes and compile definitions?
-        lslboost  # TODO: Shouldn't be needed -- lslobj already links it
-        ${PUGIXML_LIBRARIES}
+# Set the include directories for the lsl target.
+# Note: We had to link lslobj as a PRIVATE dependency, therefore we must manually expose the include directories
+get_target_property(LSLOBJ_HEADERS lslobj HEADER_SET)
+target_sources(lsl
+    INTERFACE
+        FILE_SET HEADERS
+        BASE_DIRS include
+        FILES ${LSLOBJ_HEADERS}
 )
 
 # Set compile definitions for lsl

cmake/TargetLib.cmake

@@ -1,32 +1,22 @@
 # Create object library so all files are only compiled once
 add_library(lslobj OBJECT
         ${lslsources}
-        ${lslheaders}
+#        ${lslheaders}  # Headers are added later using FILE_SET
 )
 
 # Set the includes/headers for the lslobj target
-# Note: We cannot use the PUBLIC_HEADER property of the target, because
-#  it flattens the include directories.
-# We could use the new FILE_SET feature that comes with CMake 3.23.
-# This is how it would look. Note that HEADERS is a special set name and implies its type.
-#target_sources(lslobj
-#    PUBLIC
-#        FILE_SET HEADERS
-#        BASE_DIRS include
-#        FILES ${lslheaders}
-#)
-# We settle on the older and more common target_include_directories PUBLIC approach.
-# If we used the FILET_SET approach then we would remove the PUBLIC includes below.
-target_include_directories(lslobj
-    PUBLIC
-        $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>
-        $<INSTALL_INTERFACE:include>
+# Note: We cannot use the PUBLIC_HEADER property of the target,
+#  because it flattens the include directories.
+# Note: IME, this approach is less error prone than target_include_directories
+target_sources(lslobj
     INTERFACE
-        # Propagate include directories to consumers
-        $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/src>  # for unit tests
+        FILE_SET HEADERS  # special set name; implies TYPE.
+        BASE_DIRS include
+        FILES ${lslheaders}
 )
 
 # Link system libs
+# (boost might be bundled or system)
 target_link_libraries(lslobj PRIVATE lslboost Threads::Threads)
 if(MINGW)
     target_link_libraries(lslobj PRIVATE bcrypt)
@@ -61,17 +51,13 @@ if(WIN32)
             PUBLIC
             _WIN32_WINNT=${LSL_WINVER}
     )
-    if(BUILD_SHARED_LIBS)
-        #        set_target_properties(lslobj
-        #            PROPERTIES
-        #                WINDOWS_EXPORT_ALL_SYMBOLS ON
-        #        )
-    endif(BUILD_SHARED_LIBS)
 endif(WIN32)
 
 # Link in 3rd party dependencies
 # - loguru and asio header-only
 target_include_directories(lslobj
+    # Note: We use `SYSTEM` to suppress warnings from 3rd party headers and put these at the end of the include path.
+    # Note: We use `PUBLIC` because 'internal tests' import individual source files and link lslobj.
     SYSTEM PUBLIC
         $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/loguru>
         $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/asio>
@@ -88,6 +74,7 @@ if(NOT LSL_OPTIMIZATIONS)
 endif()
 
 # - pugixml
+# Note: We use `PUBLIC` because 'internal tests' import individual source files and link lslobj.
 if(LSL_BUNDLED_PUGIXML)
     target_sources(lslobj PRIVATE thirdparty/pugixml/pugixml.cpp)
     target_include_directories(lslobj

cmake/TargetObjLib.cmake

@@ -1,3 +1,5 @@
+include(cmake/LSLCMake.cmake)  # Needed for `installLSLApp`
+
 # Build utilities
 add_executable(lslver testing/lslver.c)
 target_link_libraries(lslver PRIVATE lsl)