Cleanup CMake reundancies 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

cmake/Installation.cmake

@@ -18,31 +18,22 @@ write_basic_package_version_file(
 )
 
 # Define installation targets
-set(LSLTargets lsl)
-if(LSL_BUILD_STATIC)
-    list(APPEND LSLTargets lslobj lslboost)
-endif()
+set(LSLTargets lsl lslobj lslboost)
 
 # 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::
+#    INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
+    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 +44,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

@@ -23,18 +23,11 @@ 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 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
+        lslobj
         ${PUGIXML_LIBRARIES}
 )
 

cmake/TargetLib.cmake

@@ -8,21 +8,15 @@ add_library(lslobj OBJECT
 # 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
+# Note that HEADERS is a special set name and implies its type.
+target_sources(lslobj
     PUBLIC
-        $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>
-        $<INSTALL_INTERFACE:include>
+        FILE_SET HEADERS
+        BASE_DIRS include
+        FILES ${lslheaders}
+)
+target_include_directories(lslobj
     INTERFACE
-        # Propagate include directories to consumers
         $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/src>  # for unit tests
 )
 
@@ -61,12 +55,6 @@ 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