[RFC,cmake] A layer of smoke: avoid by using file sets for library generation

cmake/modules/RootMacros.cmake

@@ -288,11 +288,74 @@ function(ROOT_REPLACE_BUILD_INTERFACE include_dir_var include_dir)
   set(${include_dir_var} ${include_dir} PARENT_SCOPE)
 endfunction(ROOT_REPLACE_BUILD_INTERFACE)
 
+#---------------------------------------------------------------------------------------------------
+#---ROOT_GENERATE_TARGET_DICTIONARY( target OPTIONS opt1 opt2 ...)
+#
+# Creates the dictionary of a predefined CMake target defined following FILE_SET naming conventions:
+# linkDefs for Linkdef.h, HEADERS for public headers
+# <target> is the CMake target on which header file sets and link libraries have been defined
+# The output dictionary will be another CMake target called G__<target>, ie with G__ prepended.;
+# the macro creates (among other files) the dictionary source as G__<target>.cxx
+#---------------------------------------------------------------------------------------------------
+function(ROOT_GENERATE_TARGET_DICTIONARY target options)
+  # Check if OPTIONS start with a dash.
+  if (options)
+    foreach(ARG_O ${options})
+      if (NOT ARG_O MATCHES "^-*")
+        message(FATAL_ERROR "Wrong rootcling option: ${options}")
+      endif()
+    endforeach()
+  endif(options)
+
+  get_target_property(type ${target} TYPE)
+  if(NOT ${type} STREQUAL "INTERFACE_LIBRARY")
+    get_target_property(SrcList ${target} SOURCES)
+    get_target_property(SrcDir ${target} SOURCE_DIR)
+    list(TRANSFORM SrcList PREPEND ${SrcDir}/)
+  endif()
+  get_target_property(HeaderList ${target} HEADER_SET)
+  get_target_property(HeaderBaseDirs ${target} HEADER_DIRS)
+  get_target_property(LinkdefList ${target} HEADER_SET_linkDefs)
+  # get_target_property(ModuleList ${target} CXX_MODULE_SET) needs CMake 3.28
+  if("HeaderList-NOTFOUND" IN_LIST HeaderList)
+    message(SEND_ERROR "ROOT_GENERATE_TARGET_DICTIONARY: Missing header file set in ${target}")
+  endif()
+  if("LinkdefList-NOTFOUND" IN_LIST LinkdefList)
+    message(SEND_ERROR "ROOT_GENERATE_TARGET_DICTIONARY: Missing Linkdef file set in ${target}")
+  endif()
+
+  # Transform absolute header paths to relative paths based on BASE_DIRS
+  set(RelativeHeaderList "")
+  foreach(header_file ${HeaderList})
+    set(resolved_path "")
+    foreach(base_dir ${HeaderBaseDirs})
+      file(RELATIVE_PATH rel_path "${base_dir}" "${header_file}")
+      # If the path doesn't start with "../", it means it's under this base directory
+      if(NOT "${rel_path}" MATCHES "^\\.\\./")
+        set(resolved_path "${rel_path}")
+        break()
+      endif()
+    endforeach()
+
+    if(resolved_path)
+      list(APPEND RelativeHeaderList "${resolved_path}")
+    else()
+      # Fallback to absolute if it couldn't be resolved relative to any base dir
+      list(APPEND RelativeHeaderList "${header_file}")
+    endif()
+  endforeach()
+
+  get_target_property(LinkedLibraries ${target} LINK_LIBRARIES)
+  ROOT_GENERATE_DICTIONARY(G__${target} ${RelativeHeaderList} LINKDEF ${LinkdefList} MODULE ${target} DEPENDENCIES ${LinkedLibraries} OPTIONS ${options} USE_FILE_SETS)
+  # TODO: Probably one could directly call rootcling without all the shenanigan inside ROOT_GENERATE_DICTIONARY with include dirs, now that file_sets are used
+endfunction(ROOT_GENERATE_TARGET_DICTIONARY)
+
 #---------------------------------------------------------------------------------------------------
 #---ROOT_GENERATE_DICTIONARY( dictionary headerfiles NODEPHEADERS ghdr1 ghdr2 ...
 #                                                    MODULE module DEPENDENCIES dep1 dep2
+#                                                    EXTRA_DEPENDENCIES
 #                                                    BUILTINS dep1 dep2
-#                                                    STAGE1 LINKDEF linkdef OPTIONS opt1 opt2 ...)
+#                                                    STAGE1 LINKDEF linkdef OPTIONS opt1 opt2 ... NO_CXXMODULE MULTIDICT NOINSTALL)
 #
 # <dictionary> is the dictionary stem; the macro creates (among other files) the dictionary source as
 #   <dictionary>.cxx
@@ -302,7 +365,7 @@ endfunction(ROOT_REPLACE_BUILD_INTERFACE)
 #   no error is emitted. The dictionary does not depend on these headers.
 #---------------------------------------------------------------------------------------------------
 function(ROOT_GENERATE_DICTIONARY dictionary)
-  CMAKE_PARSE_ARGUMENTS(ARG "STAGE1;MULTIDICT;NOINSTALL;NO_CXXMODULE"
+  CMAKE_PARSE_ARGUMENTS(ARG "STAGE1;MULTIDICT;NOINSTALL;NO_CXXMODULE;USE_FILE_SETS"
     "MODULE;LINKDEF" "NODEPHEADERS;OPTIONS;DEPENDENCIES;EXTRA_DEPENDENCIES;BUILTINS" ${ARGN})
 
   # Check if OPTIONS start with a dash.
@@ -326,8 +389,24 @@ function(ROOT_GENERATE_DICTIONARY dictionary)
   if((CMAKE_PROJECT_NAME STREQUAL ROOT) AND (TARGET ${ARG_MODULE}))
     set(headerdirs)
 
-    get_target_property(target_incdirs ${ARG_MODULE} INCLUDE_DIRECTORIES)
+    if(ARG_USE_FILE_SETS)
+       get_target_property(target_incdirs ${ARG_MODULE} INTERFACE_INCLUDE_DIRECTORIES)
+       if(NOT target_incdirs)
+          set(target_incdirs "")
+       endif()
+       get_target_property(target_fileset_dirs ${ARG_MODULE} HEADER_DIRS)
+       if(target_fileset_dirs)
+          list(APPEND target_incdirs ${target_fileset_dirs})
+       endif()
+    else()
+       get_target_property(target_incdirs ${ARG_MODULE} INCLUDE_DIRECTORIES)
+       if(NOT target_incdirs)
+          set(target_incdirs "")
+       endif()
+    endif()
+
     if(target_incdirs)
+       list(REMOVE_DUPLICATES target_incdirs)
        foreach(dir ${target_incdirs})
           ROOT_REPLACE_BUILD_INTERFACE(dir ${dir})
           # check that dir not a empty dir like $<BUILD_INTERFACE:>
@@ -1304,6 +1383,36 @@ function(ROOT_FIND_DIRS_WITH_HEADERS result_dirs)
   set(${result_dirs} ${dirs} PARENT_SCOPE)
 endfunction()
 
+#---------------------------------------------------------------------------------------------------
+#---ROOT_INSTALL_TARGET(target)
+# Declare the install command for CMake target defined according to the following naming conventions
+# for FILE_SETS:
+# - linkDefs for LinkDef*.h files
+# - HEADERS for public headers
+# - extraHEADERS for public headers purposely excluded from dictionaries
+#---------------------------------------------------------------------------------------------------
+function(ROOT_INSTALL_TARGET target)
+  install(TARGETS ${target}
+    EXPORT ${CMAKE_PROJECT_NAME}Exports
+    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT libraries
+    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT libraries
+    ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT libraries
+
+    # with this we no longer need the moveheader target etc
+    FILE_SET HEADERS
+    DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} # add /ROOT to make Debian happy
+
+    FILE_SET extraHEADERS
+    DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} # add /ROOT to make Debian happy
+
+    FILE_SET linkDefs
+    DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} # add /ROOT to make Debian happy
+  )
+  export(TARGETS ${target} NAMESPACE ROOT::
+    APPEND FILE ${PROJECT_BINARY_DIR}/ROOTConfig-Targets.cmake
+  )
+endfunction(ROOT_INSTALL_TARGET)
+
 #---------------------------------------------------------------------------------------------------
 #---ROOT_INSTALL_HEADERS([dir1 dir2 ...] [FILTER <regex>] [HEADERS <header1> ...])
 # Declare the install command for headers and copy them into <binary_dir>/include.

roofit/multiprocess/CMakeLists.txt

@@ -3,58 +3,53 @@
 # @author Patrick Bos, Netherlands eScience Center
 ############################################################################
 
-ROOT_LINKER_LIBRARY(RooFitMultiProcess
-        src/worker.cxx
-        src/Channel.cxx
-        src/Messenger.cxx
-        src/ProcessManager.cxx
-        src/util.cxx
-        src/Queue.cxx
-        src/FIFOQueue.cxx
-        src/PriorityQueue.cxx
-        src/JobManager.cxx
-        src/Job.cxx
-        src/Config.cxx
-        src/ProcessTimer.cxx
-        src/HeatmapAnalyzer.cxx
-    LIBRARIES
-        Core
+add_library(RooFitMultiProcess SHARED)
+target_sources(RooFitMultiProcess
+PRIVATE
+  src/worker.cxx
+  src/Channel.cxx
+  src/Messenger.cxx
+  src/ProcessManager.cxx
+  src/util.cxx
+  src/Queue.cxx
+  src/FIFOQueue.cxx
+  src/PriorityQueue.cxx
+  src/JobManager.cxx
+  src/Job.cxx
+  src/Config.cxx
+  src/ProcessTimer.cxx
+  src/HeatmapAnalyzer.cxx
+PRIVATE
+  FILE_SET private_header_files TYPE HEADERS BASE_DIRS src/ FILES
+    src/FIFOQueue.h
+    src/PriorityQueue.h
+  FILE_SET friend_header_files TYPE HEADERS BASE_DIRS res/ FILES # they are private, but we allow calling them from .cxx files within other RF targets
+    res/RooFit/MultiProcess/Channel.h
+    res/RooFit/MultiProcess/JobManager.h
+    res/RooFit/MultiProcess/Job.h
+    res/RooFit/MultiProcess/Message.h
+    res/RooFit/MultiProcess/Messenger.h
+    res/RooFit/MultiProcess/Messenger_decl.h
+    res/RooFit/MultiProcess/Poller.h
+    res/RooFit/MultiProcess/ProcessManager.h
+    res/RooFit/MultiProcess/ProcessTimer.h
+    res/RooFit/MultiProcess/Queue.h
+    res/RooFit/MultiProcess/util.h
+    res/RooFit/MultiProcess/worker.h
+PUBLIC
+  FILE_SET HEADERS TYPE HEADERS BASE_DIRS inc/ FILES
+    inc/RooFit/MultiProcess/Config.h
+    inc/RooFit/MultiProcess/HeatmapAnalyzer.h
+    inc/RooFit/MultiProcess/types.h
 )
-
-target_link_libraries(RooFitMultiProcess PUBLIC Hist)
-set(RooFitMultiProcess_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/res")
-target_include_directories(RooFitMultiProcess
-        PRIVATE ${RooFitMultiProcess_INCLUDE_DIR}
-        INTERFACE $<BUILD_INTERFACE:${RooFitMultiProcess_INCLUDE_DIR}>)
-
-target_link_libraries(RooFitMultiProcess PRIVATE nlohmann_json::nlohmann_json)
-
-target_sources(
-    RooFitMultiProcess
-    PRIVATE
-    FILE_SET private_header_files
-    TYPE HEADERS
-    BASE_DIRS inc/ res/ src/
-    FILES
-      inc/RooFit/MultiProcess/Config.h
-      inc/RooFit/MultiProcess/HeatmapAnalyzer.h
-      inc/RooFit/MultiProcess/types.h
-      res/RooFit/MultiProcess/Channel.h
-      res/RooFit/MultiProcess/JobManager.h
-      res/RooFit/MultiProcess/Job.h
-      res/RooFit/MultiProcess/Message.h
-      res/RooFit/MultiProcess/Messenger.h
-      res/RooFit/MultiProcess/Messenger_decl.h
-      res/RooFit/MultiProcess/Poller.h
-      res/RooFit/MultiProcess/ProcessManager.h
-      res/RooFit/MultiProcess/ProcessTimer.h
-      res/RooFit/MultiProcess/Queue.h
-      res/RooFit/MultiProcess/util.h
-      res/RooFit/MultiProcess/worker.h
-      src/FIFOQueue.h
-      src/PriorityQueue.h
+target_link_libraries(RooFitMultiProcess
+PRIVATE
+  Core
+  nlohmann_json::nlohmann_json
+PUBLIC
+  Hist
 )
 
 ROOT_ADD_TEST_SUBDIRECTORY(test)
 
-ROOT_INSTALL_HEADERS()
+ROOT_INSTALL_TARGET(RooFitMultiProcess)

roofit/multiprocess/test/CMakeLists.txt

@@ -2,19 +2,24 @@
 
 add_library(RooFit_multiprocess_testing_utils INTERFACE)
 target_link_libraries(RooFit_multiprocess_testing_utils INTERFACE RooFitCore)
-target_include_directories(RooFit_multiprocess_testing_utils INTERFACE ${RooFitMultiProcess_INCLUDE_DIR})
+# workaround to include privately headers that are not public, from a "friend" target
+add_library(RooFit_multiprocess_friend_utils INTERFACE)
+get_target_property(RooFitMultiProcess_Friend_Dirs RooFitMultiProcess HEADER_DIRS_friend_header_files)
+if(RooFitMultiProcess_Friend_Dirs)
+  target_include_directories(RooFit_multiprocess_friend_utils INTERFACE ${RooFitMultiProcess_Friend_Dirs})
+endif()
 
 ROOT_ADD_GTEST(test_RooFit_MultiProcess_Job test_Job.cxx LIBRARIES RooFitMultiProcess Core)
 # link to the INTERFACE library separately, ROOT_EXECUTABLE cannot handle INTERFACE library properties:
-target_link_libraries(test_RooFit_MultiProcess_Job PUBLIC RooFit_multiprocess_testing_utils)
+target_link_libraries(test_RooFit_MultiProcess_Job PUBLIC RooFit_multiprocess_testing_utils RooFit_multiprocess_friend_utils)
 
-ROOT_ADD_GTEST(test_RooFit_MultiProcess_Channel test_Channel.cxx LIBRARIES RooFitMultiProcess)
-ROOT_ADD_GTEST(test_RooFit_MultiProcess_ProcessManager test_ProcessManager.cxx LIBRARIES RooFitMultiProcess)
-ROOT_ADD_GTEST(test_RooFit_MultiProcess_Messenger test_Messenger.cxx LIBRARIES RooFitMultiProcess)
+ROOT_ADD_GTEST(test_RooFit_MultiProcess_Channel test_Channel.cxx LIBRARIES RooFitMultiProcess RooFit_multiprocess_friend_utils)
+ROOT_ADD_GTEST(test_RooFit_MultiProcess_ProcessManager test_ProcessManager.cxx LIBRARIES RooFitMultiProcess RooFit_multiprocess_friend_utils)
+ROOT_ADD_GTEST(test_RooFit_MultiProcess_Messenger test_Messenger.cxx LIBRARIES RooFitMultiProcess RooFit_multiprocess_friend_utils)
 
-ROOT_ADD_GTEST(test_RooFit_MultiProcess_Queue test_Queue.cxx LIBRARIES RooFitMultiProcess)
+ROOT_ADD_GTEST(test_RooFit_MultiProcess_Queue test_Queue.cxx LIBRARIES RooFitMultiProcess RooFit_multiprocess_friend_utils)
 #nlohmann is only a private dependency of RF_MP, but this test includes a header in res/
-ROOT_ADD_GTEST(test_RooFit_MultiProcess_ProcessTimer test_ProcessTimer.cxx LIBRARIES RooFitMultiProcess nlohmann_json::nlohmann_json)
+ROOT_ADD_GTEST(test_RooFit_MultiProcess_ProcessTimer test_ProcessTimer.cxx LIBRARIES RooFitMultiProcess RooFit_multiprocess_friend_utils nlohmann_json::nlohmann_json)
 ROOT_ADD_GTEST(test_RooFit_MultiProcess_HeatmapAnalyzer test_HeatmapAnalyzer.cxx LIBRARIES RooFitMultiProcess
   COPY_TO_BUILDDIR ${CMAKE_CURRENT_SOURCE_DIR}/test_logs/p_0.json
   COPY_TO_BUILDDIR ${CMAKE_CURRENT_SOURCE_DIR}/test_logs/p_1.json

roofit/roofitcore/CMakeLists.txt

@@ -573,4 +573,12 @@ target_sources(
         res/RooUnitTest.h
 )
 
+if (NOT WIN32)
+  # workaround to include privately headers that are not public, from a "friend" target
+  get_target_property(RooFitMultiProcess_Friend_Dirs RooFitMultiProcess HEADER_DIRS_friend_header_files)
+  if(RooFitMultiProcess_Friend_Dirs)
+    target_include_directories(RooFitCore PRIVATE ${RooFitMultiProcess_Friend_Dirs})
+  endif()
+endif()
+
 ROOT_ADD_TEST_SUBDIRECTORY(test)

roofit/roofitcore/test/CMakeLists.txt

@@ -96,6 +96,11 @@ ROOT_ADD_GTEST(testRooTruthModel testRooTruthModel.cxx LIBRARIES RooFitCore RooF
   COPY_TO_BUILDDIR ${CMAKE_CURRENT_SOURCE_DIR}/rooAbsAnaConvPdf_classV3.root)
 
 if(NOT WIN32)
+  # workaround to include privately headers that are not public, from a "friend" target
+  get_target_property(RooFitMultiProcess_Friend_Dirs RooFitMultiProcess HEADER_DIRS_friend_header_files)
+  if(RooFitMultiProcess_Friend_Dirs)
+    list(APPEND RooFitCore_MultiProcess_TestStatistics_INCLUDE_DIR ${RooFitMultiProcess_Friend_Dirs})
+  endif()
   ROOT_ADD_GTEST(testTestStatisticsPlot TestStatistics/testPlot.cxx LIBRARIES RooFitMultiProcess RooFitCore RooFit)
   ROOT_ADD_GTEST(testLikelihoodGradientJob TestStatistics/testLikelihoodGradientJob.cxx LIBRARIES RooFitMultiProcess RooFitCore m ROOT::TestSupport)
   target_include_directories(testLikelihoodGradientJob PRIVATE ${RooFitCore_MultiProcess_TestStatistics_INCLUDE_DIR})