From 88f38eb9a38688c541e5d0a5a6f46f19ff7b8fed Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 13 Dec 2022 18:11:56 +0100 Subject: [PATCH 01/21] [imt] Fix missing include directories for TBB --- core/imt/test/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/imt/test/CMakeLists.txt b/core/imt/test/CMakeLists.txt index 9cfaceffbc1c6..7c2dd1d73d701 100644 --- a/core/imt/test/CMakeLists.txt +++ b/core/imt/test/CMakeLists.txt @@ -4,5 +4,5 @@ # For the licensing terms see $ROOTSYS/LICENSE. # For the list of contributors see $ROOTSYS/README/CREDITS. -ROOT_ADD_GTEST(testTaskArena testRTaskArena.cxx LIBRARIES Imt ${TBB_LIBRARIES} FAILREGEX "") -ROOT_ADD_GTEST(testTBBGlobalControl testTBBGlobalControl.cxx LIBRARIES Imt ${TBB_LIBRARIES}) +ROOT_ADD_GTEST(testTaskArena testRTaskArena.cxx LIBRARIES Imt ${TBB_LIBRARIES} FAILREGEX "" INCLUDE_DIRS ${TBB_INCLUDE_DIRS}) +ROOT_ADD_GTEST(testTBBGlobalControl testTBBGlobalControl.cxx LIBRARIES Imt ${TBB_LIBRARIES} INCLUDE_DIRS ${TBB_INCLUDE_DIRS}) From c4291ea68c5fb5e3bb9eda14596cc7b94eadf884 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 26 Dec 2023 18:40:37 +0100 Subject: [PATCH 02/21] [Tree] Remove res/ header from tests. For an unknown reason, a res/ header was included, leading to an include error. The include could be removed without problems. --- tree/tree/test/TChainParsing.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/tree/tree/test/TChainParsing.cxx b/tree/tree/test/TChainParsing.cxx index a0ec77ff66d2b..44ca68178a4c4 100644 --- a/tree/tree/test/TChainParsing.cxx +++ b/tree/tree/test/TChainParsing.cxx @@ -9,7 +9,6 @@ #include "TPluginManager.h" #include "ROOT/InternalTreeUtils.hxx" -#include #include "gtest/gtest.h" From 6d2458b7192216748c0a03cd9b8aa0455d867368 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 14 Jan 2025 15:31:29 +0100 Subject: [PATCH 03/21] Add missing includes of gtest.h gtest.h used to be parasitically included in TestSupport.hxx, which isn't using gtest at all. It's cleaner to include it where it is actually used. --- core/rint/test/TTabComTests.cxx | 2 ++ core/testsupport/src/TestSupport.cxx | 2 ++ io/io/test/TFileMergerTests.cxx | 2 ++ tree/treeplayer/test/ttreereader_friends.cxx | 2 ++ 4 files changed, 8 insertions(+) diff --git a/core/rint/test/TTabComTests.cxx b/core/rint/test/TTabComTests.cxx index 180db11e8db8b..aca5afb691b21 100644 --- a/core/rint/test/TTabComTests.cxx +++ b/core/rint/test/TTabComTests.cxx @@ -19,6 +19,8 @@ #include "TTabCom.h" +#include "gtest/gtest.h" + #include TEST(TTabComTests, Sanity) diff --git a/core/testsupport/src/TestSupport.cxx b/core/testsupport/src/TestSupport.cxx index 9889a432f408f..8c9ae56cef6ae 100644 --- a/core/testsupport/src/TestSupport.cxx +++ b/core/testsupport/src/TestSupport.cxx @@ -13,6 +13,8 @@ #include "ROOT/TestSupport.hxx" +#include "gtest/gtest.h" + #include #include #include diff --git a/io/io/test/TFileMergerTests.cxx b/io/io/test/TFileMergerTests.cxx index 66329320dc781..39b785bfe0ceb 100644 --- a/io/io/test/TFileMergerTests.cxx +++ b/io/io/test/TFileMergerTests.cxx @@ -12,6 +12,8 @@ #include +#include "gtest/gtest.h" + static void CreateATuple(TMemFile &file, const char *name, double value) { auto mytree = new TTree(name, "A tree"); diff --git a/tree/treeplayer/test/ttreereader_friends.cxx b/tree/treeplayer/test/ttreereader_friends.cxx index 12bad2a22d50a..67151c4830377 100644 --- a/tree/treeplayer/test/ttreereader_friends.cxx +++ b/tree/treeplayer/test/ttreereader_friends.cxx @@ -6,6 +6,8 @@ #include "ROOT/TestSupport.hxx" #include +#include "gtest/gtest.h" + struct TTreeReaderFriends : public ::testing::Test { constexpr static auto fMainTreeName{"tree_10entries"}; constexpr static auto fMainFileName{"tree_10entries.root"}; From 2e0fbe16f8f375f4bfcefc4c729a87059444e052 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 14 Jan 2025 15:33:29 +0100 Subject: [PATCH 04/21] Remove gtest include from TestSupport.hxx. TestSupport.hxx isn't using gtest, so it does not make sense to include it here. --- core/testsupport/inc/ROOT/TestSupport.hxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/testsupport/inc/ROOT/TestSupport.hxx b/core/testsupport/inc/ROOT/TestSupport.hxx index 0a93bad75c0b4..5051380650671 100644 --- a/core/testsupport/inc/ROOT/TestSupport.hxx +++ b/core/testsupport/inc/ROOT/TestSupport.hxx @@ -22,11 +22,10 @@ #include "TError.h" #include "TInterpreter.h" +#include #include #include -#include "gtest/gtest.h" - namespace ROOT { namespace TestSupport { From ab1c38d618e8b2dd2d454d173b588a0234a25d08 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Fri, 10 Feb 2023 14:28:05 +0100 Subject: [PATCH 05/21] [cling] Add parsing of -isystem arguments to rootcling. --- core/dictgen/src/TModuleGenerator.cxx | 46 +++++++++++++++++---------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/core/dictgen/src/TModuleGenerator.cxx b/core/dictgen/src/TModuleGenerator.cxx index 5d7717904dcf9..8736aea69a804 100644 --- a/core/dictgen/src/TModuleGenerator.cxx +++ b/core/dictgen/src/TModuleGenerator.cxx @@ -35,6 +35,7 @@ #include #endif +#include #include using namespace ROOT; @@ -179,6 +180,7 @@ std::pair SplitPPDefine(const std::string &in) void TModuleGenerator::ParseArgs(const std::vector &args) { + std::vector systemIncludePaths; for (size_t iPcmArg = 1 /*skip argv0*/, nPcmArg = args.size(); iPcmArg < nPcmArg; ++iPcmArg) { ESourceFileKind sfk = GetSourceFileKind(args[iPcmArg].c_str()); @@ -188,23 +190,36 @@ void TModuleGenerator::ParseArgs(const std::vector &args) fLinkDefFile = args[iPcmArg]; } else if (sfk == kSFKNotC && args[iPcmArg][0] == '-') { switch (args[iPcmArg][1]) { - case 'I': - if (args[iPcmArg] != "-I." && args[iPcmArg] != "-Iinclude") { - fCompI.push_back(args[iPcmArg].c_str() + 2); + case 'i': + if (args[iPcmArg].find("-isystem") != std::string::npos) { + if (args[iPcmArg].size() == 8) + systemIncludePaths.push_back(args[++iPcmArg]); + else { + auto pos = 9; // start after -isystem + while (args[iPcmArg][pos] == ' ') + pos++; + systemIncludePaths.push_back(args[iPcmArg].substr(pos)); } - break; - case 'D': - if (args[iPcmArg] != "-DTRUE=1" && args[iPcmArg] != "-DFALSE=0" - && args[iPcmArg] != "-DG__NOCINTDLL") { - fCompD.push_back(SplitPPDefine(args[iPcmArg].c_str() + 2)); - } - break; - case 'U': - fCompU.push_back(args[iPcmArg].c_str() + 2); - break; + } + break; + case 'I': + if (args[iPcmArg] != "-I." && args[iPcmArg] != "-Iinclude") { + fCompI.push_back(args[iPcmArg].c_str() + 2); + } + break; + case 'D': + if (args[iPcmArg] != "-DTRUE=1" && args[iPcmArg] != "-DFALSE=0" && args[iPcmArg] != "-DG__NOCINTDLL") { + fCompD.push_back(SplitPPDefine(args[iPcmArg].c_str() + 2)); + } + break; + case 'U': fCompU.push_back(args[iPcmArg].c_str() + 2); break; } } } + + // System includes have lowest priority, so we move them to the end: + std::move(systemIncludePaths.begin(), systemIncludePaths.end(), std::back_inserter(fCompI)); + fCompI.erase(std::unique(fCompI.begin(), fCompI.end()), fCompI.end()); } //////////////////////////////////////////////////////////////////////////////// @@ -402,9 +417,8 @@ void TModuleGenerator::WriteRegistrationSource(std::ostream &out, const std::str { if (hasCxxModule) { std::string emptyStr = "\"\""; - WriteRegistrationSourceImpl(out, GetDictionaryName(), GetDemangledDictionaryName(), {}, {}, - fwdDeclString, "{}", - emptyStr, headersClassesMapString, "0", + WriteRegistrationSourceImpl(out, GetDictionaryName(), GetDemangledDictionaryName(), {}, fCompI, fwdDeclString, + "{}", emptyStr, headersClassesMapString, "0", /*HasCxxModule*/ true); return; } From e2f1a3ff68de51e6bb40c5db2131a1c4f8f1c65a Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 12 Jul 2021 09:34:03 +0200 Subject: [PATCH 06/21] [CMake] Make VDT an IMPORTED target; convert to target-based cmake. VDT used to be configured using variables, but this creates a problem when VDT is installed in a system directory where ROOT is installed as well. The -I will lead to headers from the installed ROOT being picked up during compilation. Here, VDT is configured in a target-based way. When external, it's now an IMPORTED target, and its include directory is marked as SYSTEM, so it will not interfere with other ROOT includes. --- cmake/modules/FindVdt.cmake | 18 ++++++++++-------- cmake/modules/SearchInstalledSoftware.cmake | 15 ++++++++++++--- math/vecops/CMakeLists.txt | 5 +---- roofit/batchcompute/CMakeLists.txt | 5 +---- tmva/tmva/CMakeLists.txt | 5 +---- 5 files changed, 25 insertions(+), 23 deletions(-) diff --git a/cmake/modules/FindVdt.cmake b/cmake/modules/FindVdt.cmake index 1f8a4e87de36c..b9561085bd7ec 100644 --- a/cmake/modules/FindVdt.cmake +++ b/cmake/modules/FindVdt.cmake @@ -13,8 +13,7 @@ # Imported Targets # ^^^^^^^^^^^^^^^^ # -# This module defines :prop_tgt:`IMPORTED` target ``VDT::VDT``, -# if Vdt has been found. +# VDT::VDT if Vdt has been found. # # Result Variables # ^^^^^^^^^^^^^^^^ @@ -42,8 +41,6 @@ if(NOT VDT_LIBRARY) find_library(VDT_LIBRARY NAMES vdt) endif() -mark_as_advanced(VDT_INCLUDE_DIR VDT_LIBRARY) - if(VDT_INCLUDE_DIR) file(STRINGS "${VDT_INCLUDE_DIR}/vdt/vdtMath.h" VDT_H REGEX "^#define VDT_VERSION_[A-Z]+[ ]+[0-9]+.*$") string(REGEX REPLACE ".+VDT_VERSION_MAJOR[ ]+([0-9]+).*$" "\\1" VDT_VERSION_MAJOR "${VDT_H}") @@ -59,9 +56,14 @@ if(VDT_INCLUDE_DIR) endif() endif() +# Don't show in GUI +mark_as_advanced(VDT_FOUND VDT_VERSION VDT_INCLUDE_DIR VDT_LIBRARY) + include(FindPackageHandleStandardArgs) -find_package_handle_standard_args(Vdt FOUND_VAR VDT_FOUND - REQUIRED_VARS VDT_INCLUDE_DIR VDT_LIBRARY VERSION_VAR VDT_VERSION) +find_package_handle_standard_args(Vdt + REQUIRED_VARS VDT_INCLUDE_DIR VDT_LIBRARY + VERSION_VAR VDT_VERSION) + if(VDT_FOUND) set(VDT_INCLUDE_DIRS ${VDT_INCLUDE_DIR}) @@ -71,12 +73,12 @@ if(VDT_FOUND) endif() if(NOT TARGET VDT::VDT) - add_library(VDT::VDT UNKNOWN IMPORTED) + add_library(VDT::VDT SHARED IMPORTED) + target_include_directories(VDT::VDT SYSTEM INTERFACE ${VDT_INCLUDE_DIRS}) set_target_properties(VDT::VDT PROPERTIES IMPORTED_LOCATION "${VDT_LIBRARY}" - INTERFACE_INCLUDE_DIRECTORIES "${VDT_INCLUDE_DIRS}" ) endif() endif() diff --git a/cmake/modules/SearchInstalledSoftware.cmake b/cmake/modules/SearchInstalledSoftware.cmake index fede81d5fe8ea..67c94caece7b1 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -1575,14 +1575,16 @@ if(vdt OR builtin_vdt) set(builtin_vdt ON CACHE BOOL "Enabled because external vdt not found (${vdt_description})" FORCE) endif() endif() - endif() + else() + add_library(VDT ALIAS VDT::VDT) + endif() endif() if(builtin_vdt) set(vdt_version 0.4.6) set(VDT_FOUND True) set(VDT_LIBRARIES ${CMAKE_BINARY_DIR}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}vdt${CMAKE_SHARED_LIBRARY_SUFFIX}) ExternalProject_Add( - VDT + BUILTIN_VDT URL ${lcgpackages}/vdt-${vdt_version}.tar.gz URL_HASH SHA256=1820feae446780763ec8bbb60a0dbcf3ae1ee548bdd01415b1fb905fd4f90c54 INSTALL_DIR ${CMAKE_BINARY_DIR} @@ -1600,12 +1602,19 @@ if(vdt OR builtin_vdt) TIMEOUT 600 ) ExternalProject_Add_Step( - VDT copy2externals + BUILTIN_VDT copy2externals COMMAND ${CMAKE_COMMAND} -E copy_directory ${CMAKE_BINARY_DIR}/include/vdt ${CMAKE_BINARY_DIR}/ginclude/vdt DEPENDEES install ) set(VDT_INCLUDE_DIR ${CMAKE_BINARY_DIR}/ginclude) set(VDT_INCLUDE_DIRS ${CMAKE_BINARY_DIR}/ginclude) + if(NOT TARGET VDT) + add_library(VDT IMPORTED SHARED) + add_dependencies(VDT BUILTIN_VDT) + set_target_properties(VDT PROPERTIES IMPORTED_LOCATION "${VDT_LIBRARIES}") + target_include_directories(VDT INTERFACE $ $) + endif() + install(FILES ${CMAKE_BINARY_DIR}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}vdt${CMAKE_SHARED_LIBRARY_SUFFIX} DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT libraries) install(DIRECTORY ${CMAKE_BINARY_DIR}/include/vdt diff --git a/math/vecops/CMakeLists.txt b/math/vecops/CMakeLists.txt index cbeba1a8e8b4a..f6779026b914f 100644 --- a/math/vecops/CMakeLists.txt +++ b/math/vecops/CMakeLists.txt @@ -20,10 +20,7 @@ ROOT_STANDARD_LIBRARY_PACKAGE(ROOTVecOps ) if(builtin_vdt OR vdt) - target_link_libraries(ROOTVecOps PUBLIC VDT::VDT) -endif() -if(builtin_vdt) - add_dependencies(ROOTVecOps VDT) + target_link_libraries(ROOTVecOps PUBLIC VDT) endif() include(CheckCXXSymbolExists) diff --git a/roofit/batchcompute/CMakeLists.txt b/roofit/batchcompute/CMakeLists.txt index 11c9cc58518e5..e2b2dcafb5fa0 100644 --- a/roofit/batchcompute/CMakeLists.txt +++ b/roofit/batchcompute/CMakeLists.txt @@ -10,10 +10,7 @@ ROOT_LINKER_LIBRARY(RooBatchCompute target_include_directories(RooBatchCompute PUBLIC $) if(vdt OR builtin_vdt) - target_link_libraries(RooBatchCompute PUBLIC VDT::VDT) -endif() -if(builtin_vdt) - add_dependencies(RooBatchCompute VDT) + target_link_libraries(RooBatchCompute PUBLIC VDT) endif() diff --git a/tmva/tmva/CMakeLists.txt b/tmva/tmva/CMakeLists.txt index dfdb234b93455..eac6a78976467 100644 --- a/tmva/tmva/CMakeLists.txt +++ b/tmva/tmva/CMakeLists.txt @@ -405,10 +405,7 @@ if(MSVC) endif() if(vdt OR builtin_vdt) - target_link_libraries(TMVA PRIVATE VDT::VDT) -endif() -if(builtin_vdt) - add_dependencies(TMVA VDT) + target_link_libraries(TMVA PRIVATE VDT) endif() if(tmva-cpu) From e46f19d87c08091bb2a07e758dda7bfa0c664f70 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 12 Jul 2021 13:10:59 +0200 Subject: [PATCH 07/21] [CMake] Make XRootD config target-based. - Create the target XRootD::XrdCl for usage in ROOT Create an IMPORTED target for XRootD that we populate either with a system xrootd or with the builtin library. This also solves the problem of ROOT's build system picking up ROOT headers when it is trying to include xrootd from a system directory where both ROOT and xrootd are installed. All packages inside ROOT should use the target instead of any CMake variables. This way, an update of XRootD's CMake will only affect one single location in ROOT. - Refactor builtin XRootD. Synchronise the variables that the builtin and SearchInstalled are setting, and use those to configure the ROOT-internal target. - Add a test for XrdCl headers, since these are used in TNetXNG. If xrootd is installed in the system, the XrdCl library might be present without the corresponding headers. This would lead to a build error in ROOT, so cmake will try to find the headers at configure time to warn about a possibly missing package. - Add a CMAKE_BUILD_RPATH to all ROOT targets in order to find the libraries of a builtin XRootD. --- builtins/xrootd/CMakeLists.txt | 35 +++++++-------------- cmake/modules/SearchInstalledSoftware.cmake | 34 +++++++++----------- net/netxng/CMakeLists.txt | 4 +-- 3 files changed, 27 insertions(+), 46 deletions(-) diff --git a/builtins/xrootd/CMakeLists.txt b/builtins/xrootd/CMakeLists.txt index 1dd56b9bf1ffb..62a6b742e3c58 100644 --- a/builtins/xrootd/CMakeLists.txt +++ b/builtins/xrootd/CMakeLists.txt @@ -7,21 +7,10 @@ include(ExternalProject) set(XROOTD_VERSION "5.8.0") +set(XROOTD_PREFIX ${CMAKE_BINARY_DIR}/XROOTD-prefix) -set(XROOTD_PREFIX ${CMAKE_BINARY_DIR}) message(STATUS "Downloading and building XROOTD version ${XROOTD_VERSION}") -list(REMOVE_ITEM XROOTD_CLIENT_LIBRARIES OpenSSL::SSL) -list(REMOVE_ITEM XROOTD_UTILS_LIBRARIES OpenSSL::SSL) - -set(libname ${CMAKE_SHARED_LIBRARY_PREFIX}XrdCl${CMAKE_SHARED_LIBRARY_SUFFIX}) -list(APPEND XROOTD_CLIENT_LIBRARIES ${XROOTD_PREFIX}/lib/${libname}) -list(REMOVE_DUPLICATES XROOTD_CLIENT_LIBRARIES) - -set(libname ${CMAKE_SHARED_LIBRARY_PREFIX}XrdUtils${CMAKE_SHARED_LIBRARY_SUFFIX}) -list(APPEND XROOTD_UTILS_LIBRARIES ${XROOTD_PREFIX}/lib/${libname}) -list(REMOVE_DUPLICATES XROOTD_UTILS_LIBRARIES) - ExternalProject_Add( BUILTIN_XROOTD URL http://lcgpackages.web.cern.ch/lcgpackages/tarFiles/sources/xrootd-${XROOTD_VERSION}.tar.gz @@ -44,7 +33,7 @@ ExternalProject_Add( -DOPENSSL_ROOT_DIR=${OPENSSL_ROOT_DIR} INSTALL_COMMAND ${CMAKE_COMMAND} --build . --target install LOG_DOWNLOAD 1 LOG_CONFIGURE 1 LOG_BUILD 1 LOG_INSTALL 1 LOG_OUTPUT_ON_FAILURE 1 - BUILD_BYPRODUCTS ${XROOTD_CLIENT_LIBRARIES} ${XROOTD_UTILS_LIBRARIES} + BUILD_BYPRODUCTS XrdUtils XrdCl TIMEOUT 600 ) @@ -55,22 +44,19 @@ if(builtin_openssl) add_dependencies(BUILTIN_XROOTD OPENSSL) endif() -list(APPEND XROOTD_CLIENT_LIBRARIES OpenSSL::SSL) -list(REMOVE_DUPLICATES XROOTD_CLIENT_LIBRARIES) -list(APPEND XROOTD_UTILS_LIBRARIES OpenSSL::SSL) -list(REMOVE_DUPLICATES XROOTD_UTILS_LIBRARIES) - set(XROOTD_INCLUDE_DIRS ${XROOTD_PREFIX}/include/xrootd CACHE INTERNAL "" FORCE) -set(XROOTD_CLIENT_LIBRARIES ${XROOTD_CLIENT_LIBRARIES} CACHE INTERNAL "" FORCE) -set(XROOTD_UTILS_LIBRARIES ${XROOTD_UTILS_LIBRARIES} CACHE INTERNAL "" FORCE) +set(XRDCL_NAME ${CMAKE_SHARED_LIBRARY_PREFIX}XrdCl${CMAKE_SHARED_LIBRARY_SUFFIX}) +set(XRDUTILS_NAME ${CMAKE_SHARED_LIBRARY_PREFIX}XrdUtils${CMAKE_SHARED_LIBRARY_SUFFIX}) +set(XROOTD_CLIENT_LIBRARIES ${XROOTD_PREFIX}/lib/${XRDCL_NAME} CACHE INTERNAL "" FORCE) +set(XROOTD_UTILS_LIBRARIES ${XROOTD_PREFIX}/lib/${XRDUTILS_NAME} CACHE INTERNAL "" FORCE) +set(XROOTD_LIBRARIES ${XROOTD_PREFIX}/lib/${XRDCL_NAME} CACHE INTERNAL "" FORCE) -list(APPEND CMAKE_BUILD_RPATH ${XROOTD_PREFIX}/lib) -add_dependencies(XRootD BUILTIN_XROOTD) +install(DIRECTORY ${XROOTD_PREFIX}/lib/ DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT libraries FILES_MATCHING PATTERN "libXrd*") +install(DIRECTORY ${XROOTD_PREFIX}/include/xrootd/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/xrootd COMPONENT headers) +set(CMAKE_BUILD_RPATH ${CMAKE_BUILD_RPATH} ${XROOTD_PREFIX}/lib PARENT_SCOPE) set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS BUILTIN_XROOTD) -install(DIRECTORY ${XROOTD_PREFIX}/lib/ DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT libraries FILES_MATCHING PATTERN "libXrd*") -install(DIRECTORY ${XROOTD_PREFIX}/include/xrootd/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/xrootd COMPONENT headers) if(APPLE) # XRootD libraries on mac need the LC_RPATH variable set. The build process already takes care of setting # * BUILD_RPATH = build/XROOTD-prefix/../src @@ -81,3 +67,4 @@ if(APPLE) CODE "xrootd_libs_change_rpath(${XROOTD_PREFIX}/lib ${CMAKE_INSTALL_FULL_LIBDIR})" ) endif() + diff --git a/cmake/modules/SearchInstalledSoftware.cmake b/cmake/modules/SearchInstalledSoftware.cmake index 67c94caece7b1..faad54ec39e6f 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -985,13 +985,6 @@ foreach(suffix FOUND INCLUDE_DIR INCLUDE_DIRS LIBRARY LIBRARIES) unset(XROOTD_${suffix} CACHE) endforeach() -if(xrootd OR builtin_xrootd) - # This is the target that ROOT will use, irrespective of whether XRootD is a builtin or in the system. - # All targets should only link to ROOT::XRootD. Refrain from using XRootD variables. - add_library(XRootD INTERFACE IMPORTED GLOBAL) - add_library(ROOT::XRootD ALIAS XRootD) -endif() - if(xrootd AND NOT builtin_xrootd) message(STATUS "Looking for XROOTD") find_package(XRootD) @@ -1029,20 +1022,21 @@ if(builtin_xrootd) set(xrootd ON CACHE BOOL "Enabled because builtin_xrootd requested (${xrootd_description})" FORCE) endif() -# Finalise the XRootD target configuration -if(TARGET XRootD) - - # The XROOTD_INCLUDE_DIRS provided by XRootD is actually a list with two - # paths, like: +# Backward compatibility for XRootD ;/private - # We don't need the private headers, and we have to exclude this path from - # the build configuration if we don't want it to fail on systems were the - # private headers are not installed (most linux distributions). - list(GET XROOTD_INCLUDE_DIRS 0 XROOTD_INCLUDE_DIR_PRIMARY) - - target_include_directories(XRootD SYSTEM INTERFACE "$") - target_link_libraries(XRootD INTERFACE $) - target_link_libraries(XRootD INTERFACE $) + # The private headers are not always installed, so the configure step might fail. + # ROOT doesn't need these headers, so it's best to remove them. + list(FILTER XROOTD_INCLUDE_DIRS EXCLUDE REGEX .*/private) + + add_library(XRootD::XrdCl SHARED IMPORTED) + set_target_properties(XRootD::XrdCl PROPERTIES IMPORTED_LOCATION ${XROOTD_CLIENT_LIBRARIES}) + target_link_libraries(XRootD::XrdCl INTERFACE OpenSSL::SSL) + target_include_directories(XRootD::XrdCl SYSTEM INTERFACE $) + + add_library(XRootD::XrdUtils SHARED IMPORTED) + set_target_properties(XRootD::XrdUtils PROPERTIES IMPORTED_LOCATION ${XROOTD_UTILS_LIBRARIES}) endif() #---check if netxng can be built------------------------------- diff --git a/net/netxng/CMakeLists.txt b/net/netxng/CMakeLists.txt index 1dfaf76c9f7b8..5950eb9419636 100644 --- a/net/netxng/CMakeLists.txt +++ b/net/netxng/CMakeLists.txt @@ -26,11 +26,11 @@ ROOT_STANDARD_LIBRARY_PACKAGE(NetxNG Thread ) -target_link_libraries(NetxNG PRIVATE ROOT::XRootD) +target_link_libraries(NetxNG PRIVATE XRootD::XrdCl XRootD::XrdUtils) target_compile_options(NetxNG PRIVATE -Wno-shadow) # When linking against the XRootD target, XRootD includes become "-isystem". # By linking explicitly here, we suppress a warning during dictionary compilation. -target_link_libraries(G__NetxNG PRIVATE ROOT::XRootD) +target_link_libraries(G__NetxNG PRIVATE XRootD::XrdCl) ROOT_ADD_TEST_SUBDIRECTORY(test) From 95bed9e021d5024b3e9837cb7249c1074a4d18d0 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 3 Feb 2025 17:05:38 +0100 Subject: [PATCH 08/21] [CMake] Require OpenSSL when XRootD is a builtin. Since ROOT already has SSL support, the logic for builtin XRootD could be simplified by requiring these options to be on. --- cmake/modules/SearchInstalledSoftware.cmake | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cmake/modules/SearchInstalledSoftware.cmake b/cmake/modules/SearchInstalledSoftware.cmake index faad54ec39e6f..db1615dbfe858 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -1012,12 +1012,10 @@ if(builtin_xrootd) message(FATAL_ERROR "No internet connection. Please check your connection, or either disable the 'builtin_xrootd'" " option or the 'fail-on-missing' to automatically disable options requiring internet access") endif() + if(NOT ssl AND NOT builtin_openssl) + message(FATAL_ERROR "Building XRootD ('builtin_xrootd'=On) requires ssl support ('ssl' or 'builtin_openssl').") + endif() list(APPEND ROOT_BUILTINS BUILTIN_XROOTD) - # The builtin XRootD requires OpenSSL. - # We have to find it here, such that OpenSSL is available in this scope to - # finalize the XRootD target configuration. - # See also: https://github.com/root-project/root/issues/16374 - find_package(OpenSSL REQUIRED) add_subdirectory(builtins/xrootd) set(xrootd ON CACHE BOOL "Enabled because builtin_xrootd requested (${xrootd_description})" FORCE) endif() @@ -1032,7 +1030,6 @@ if(xrootd AND NOT TARGET XRootD::XrdCl) add_library(XRootD::XrdCl SHARED IMPORTED) set_target_properties(XRootD::XrdCl PROPERTIES IMPORTED_LOCATION ${XROOTD_CLIENT_LIBRARIES}) - target_link_libraries(XRootD::XrdCl INTERFACE OpenSSL::SSL) target_include_directories(XRootD::XrdCl SYSTEM INTERFACE $) add_library(XRootD::XrdUtils SHARED IMPORTED) From 30985f3a827cbf1fa9d63754b9f96698fbc64278 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 3 Feb 2025 10:43:40 +0100 Subject: [PATCH 09/21] [CMake] Save location of the builtin openssl. To help find the builtin openssl location, it is now saved as a cache variable, which is passed to xrootd. --- builtins/openssl/CMakeLists.txt | 1 + builtins/xrootd/CMakeLists.txt | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/builtins/openssl/CMakeLists.txt b/builtins/openssl/CMakeLists.txt index e661402f192e6..3359fafd675a3 100644 --- a/builtins/openssl/CMakeLists.txt +++ b/builtins/openssl/CMakeLists.txt @@ -56,6 +56,7 @@ set(OPENSSL_INCLUDE_DIRS ${OPENSSL_PREFIX}/include CACHE INTERNAL "" FORCE) set(OPENSSL_CRYPTO_LIBRARY ${OPENSSL_CRYPTO_LIBRARY} CACHE INTERNAL "" FORCE) set(OPENSSL_SSL_LIBRARY ${OPENSSL_SSL_LIBRARY} CACHE INTERNAL "" FORCE) set(OPENSSL_LIBRARIES ${OPENSSL_SSL_LIBRARY} ${OPENSSL_CRYPTO_LIBRARY} ${CMAKE_DL_LIBS} CACHE INTERNAL "" FORCE) +set(OPENSSL_ROOT ${OPENSSL_PREFIX} CACHE INTERNAL "Location of the builtin OpenSSL installation" FORCE) # Dependent libraries might check for the existence of the include directories file(MAKE_DIRECTORY ${OPENSSL_INCLUDE_DIR}) diff --git a/builtins/xrootd/CMakeLists.txt b/builtins/xrootd/CMakeLists.txt index 62a6b742e3c58..e9f8690950c6a 100644 --- a/builtins/xrootd/CMakeLists.txt +++ b/builtins/xrootd/CMakeLists.txt @@ -30,7 +30,8 @@ ExternalProject_Add( -DENABLE_CEPH=OFF -DXRDCL_LIB_ONLY=ON -DCMAKE_INSTALL_RPATH:STRING=${XROOTD_PREFIX}/lib - -DOPENSSL_ROOT_DIR=${OPENSSL_ROOT_DIR} + -DOpenSSL_ROOT=${OPENSSL_ROOT} #For CMake <3.27 + -DOPENSSL_ROOT=${OPENSSL_ROOT} INSTALL_COMMAND ${CMAKE_COMMAND} --build . --target install LOG_DOWNLOAD 1 LOG_CONFIGURE 1 LOG_BUILD 1 LOG_INSTALL 1 LOG_OUTPUT_ON_FAILURE 1 BUILD_BYPRODUCTS XrdUtils XrdCl From ace2d9b69d8c059ed828b899d31bad385836553b Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 13 Jul 2021 09:23:32 +0200 Subject: [PATCH 10/21] [CMake] Make nlohmann_json config target-based. - Create the target nlohmann_json that will configure all dependent libraries. - Replace all uses of json with target_link_libraries(... nlohman_json) - Remove explicit uses of the nlohmann include directory across ROOT. - Add depedency to RooFitCore, which depends on json through exposed json interface. --- builtins/nlohmann/CMakeLists.txt | 22 ++++++++++++++++++--- cmake/modules/SearchInstalledSoftware.cmake | 10 ++++++++++ graf3d/eve7/CMakeLists.txt | 7 +------ io/io/CMakeLists.txt | 7 +------ roofit/jsoninterface/CMakeLists.txt | 6 +----- roofit/multiprocess/CMakeLists.txt | 6 +----- roofit/roofitcore/CMakeLists.txt | 2 ++ tree/dataframe/CMakeLists.txt | 6 +----- tree/dataframe/test/CMakeLists.txt | 5 ----- 9 files changed, 36 insertions(+), 35 deletions(-) diff --git a/builtins/nlohmann/CMakeLists.txt b/builtins/nlohmann/CMakeLists.txt index e341a9c86096e..044381b99a0be 100644 --- a/builtins/nlohmann/CMakeLists.txt +++ b/builtins/nlohmann/CMakeLists.txt @@ -1,7 +1,12 @@ -# Install nlohmann/json.hpp include to have it +# Install nlohmann/json.hpp and json_fwd.hpp +# This file will define the target +# nlohmann_json +# and the alias +# nlohmann_json::nlohmann_json +# with proper #defines and includes. Use the alias target with the full prefix for access to JSON. -# file only used when ACLiC or ROOT macros will include REve headers, -# it is not used for ROOT compilation +# The installed files are only used when ACLiC or ROOT macros will include REve headers, +# they are not used for ROOT compilation, as this happens directly from the source directory. # extract version from existing header file file(STRINGS "json.hpp" JSON_H REGEX "^#define NLOHMANN_JSON_VERSION_[A-Z]+[ ]+[0-9]+.*$") @@ -20,4 +25,15 @@ set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS builtin_nlohmann_json_i install(FILES ${CMAKE_SOURCE_DIR}/builtins/nlohmann/json.hpp DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/nlohmann/) +# Create a target and all its configs: +add_library(nlohmann_json INTERFACE) +target_include_directories(nlohmann_json INTERFACE $ $) +target_compile_definitions(nlohmann_json INTERFACE NLOHMANN_JSON_PROVIDES_FWD_HPP) + +install(TARGETS nlohmann_json + EXPORT ROOTExports) +set_property(GLOBAL APPEND PROPERTY ROOT_EXPORTED_TARGETS nlohmann_json) + +# Alias, so can use it as drop-in replacement for system nlohmann_json. +add_library(nlohmann_json::nlohmann_json ALIAS nlohmann_json) diff --git a/cmake/modules/SearchInstalledSoftware.cmake b/cmake/modules/SearchInstalledSoftware.cmake index db1615dbfe858..de2f030686b93 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -144,6 +144,16 @@ if(NOT builtin_nlohmannjson) set(builtin_nlohmannjson ON CACHE BOOL "Enabled because nlohmann/json.hpp not found" FORCE) endif() endif() + + # ROOTEve wants to know if it comes with json_fwd.hpp: + if(TARGET nlohmann_json::nlohmann_json) + get_target_property(inc_dirs nlohmann_json::nlohmann_json INTERFACE_INCLUDE_DIRECTORIES) + foreach(dir ${inc_dirs}) + if(EXISTS "${dir}/nlohmann/json_fwd.hpp") + target_compile_definitions(nlohmann_json::nlohmann_json INTERFACE NLOHMANN_JSON_PROVIDES_FWD_HPP) + endif() + endforeach() + endif() endif() if(builtin_nlohmannjson) diff --git a/graf3d/eve7/CMakeLists.txt b/graf3d/eve7/CMakeLists.txt index 813d310110814..f6152ee616c14 100644 --- a/graf3d/eve7/CMakeLists.txt +++ b/graf3d/eve7/CMakeLists.txt @@ -141,12 +141,7 @@ ROOT_STANDARD_LIBRARY_PACKAGE(ROOTEve ${EXTRA_DICT_OPTS} ) -if(builtin_nlohmannjson) - target_include_directories(ROOTEve PRIVATE ${CMAKE_SOURCE_DIR}/builtins) -else() - target_link_libraries(ROOTEve PUBLIC nlohmann_json::nlohmann_json) -endif() - +target_link_libraries(ROOTEve PUBLIC nlohmann_json::nlohmann_json) # this is required for glew target_include_directories(ROOTEve PRIVATE ${CMAKE_SOURCE_DIR}/graf3d/eve7) diff --git a/io/io/CMakeLists.txt b/io/io/CMakeLists.txt index e50cef45ff31a..39212c2cc3c03 100644 --- a/io/io/CMakeLists.txt +++ b/io/io/CMakeLists.txt @@ -65,12 +65,7 @@ ROOT_LINKER_LIBRARY(RIO target_include_directories(RIO PRIVATE ${CMAKE_SOURCE_DIR}/core/clib/res) target_link_libraries(RIO PUBLIC ${ROOT_ATOMIC_LIBS}) - -if(builtin_nlohmannjson) - target_include_directories(RIO PRIVATE ${CMAKE_SOURCE_DIR}/builtins) -else() - target_link_libraries(RIO PRIVATE nlohmann_json::nlohmann_json) -endif() +target_link_libraries(RIO PRIVATE nlohmann_json::nlohmann_json) if(uring) target_link_libraries(RIO PUBLIC ${LIBURING_LIBRARY}) diff --git a/roofit/jsoninterface/CMakeLists.txt b/roofit/jsoninterface/CMakeLists.txt index 1f9a97dd417b7..f8de872817f38 100644 --- a/roofit/jsoninterface/CMakeLists.txt +++ b/roofit/jsoninterface/CMakeLists.txt @@ -21,10 +21,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitJSONInterface Core ) -if(builtin_nlohmannjson) - target_include_directories(RooFitJSONInterface PRIVATE ${CMAKE_SOURCE_DIR}/builtins) -else() - target_link_libraries(RooFitJSONInterface PRIVATE nlohmann_json::nlohmann_json) -endif() +target_link_libraries(RooFitJSONInterface PRIVATE nlohmann_json::nlohmann_json) ROOT_ADD_TEST_SUBDIRECTORY(test) diff --git a/roofit/multiprocess/CMakeLists.txt b/roofit/multiprocess/CMakeLists.txt index 46e35993fc014..4843cd783b06c 100644 --- a/roofit/multiprocess/CMakeLists.txt +++ b/roofit/multiprocess/CMakeLists.txt @@ -28,11 +28,7 @@ target_include_directories(RooFitMultiProcess PRIVATE ${RooFitMultiProcess_INCLUDE_DIR} INTERFACE $) -if(builtin_nlohmannjson) - target_include_directories(RooFitMultiProcess PRIVATE ${CMAKE_SOURCE_DIR}/builtins) -else() - target_link_libraries(RooFitMultiProcess PRIVATE nlohmann_json::nlohmann_json) -endif() +target_link_libraries(RooFitMultiProcess PRIVATE nlohmann_json::nlohmann_json) ROOT_ADD_TEST_SUBDIRECTORY(test) diff --git a/roofit/roofitcore/CMakeLists.txt b/roofit/roofitcore/CMakeLists.txt index e9a768a108bbe..3cfc06fc58870 100644 --- a/roofit/roofitcore/CMakeLists.txt +++ b/roofit/roofitcore/CMakeLists.txt @@ -12,6 +12,8 @@ if(roofit_multiprocess) set(RooFitMPTestStatisticsSources src/TestStatistics/LikelihoodJob.cxx src/TestStatistics/LikelihoodGradientJob.cxx) list(APPEND EXTRA_LIBRARIES RooFitMultiProcess) + #FIXME: The ProcessTimer.h exposes json in its interface: + list(APPEND EXTRA_LIBRARIES nlohmann_json::nlohmann_json) list(APPEND EXTRA_DEPENDENCIES Minuit2) endif() diff --git a/tree/dataframe/CMakeLists.txt b/tree/dataframe/CMakeLists.txt index 807879b05d3fa..e9583c6d1d78e 100644 --- a/tree/dataframe/CMakeLists.txt +++ b/tree/dataframe/CMakeLists.txt @@ -165,10 +165,6 @@ if(MSVC) target_compile_definitions(ROOTDataFrame PRIVATE _USE_MATH_DEFINES) endif() -if(builtin_nlohmannjson) - target_include_directories(ROOTDataFrame PRIVATE ${CMAKE_SOURCE_DIR}/builtins) -else() - target_link_libraries(ROOTDataFrame PRIVATE nlohmann_json::nlohmann_json) -endif() +target_link_libraries(ROOTDataFrame PRIVATE nlohmann_json::nlohmann_json) ROOT_ADD_TEST_SUBDIRECTORY(test) diff --git a/tree/dataframe/test/CMakeLists.txt b/tree/dataframe/test/CMakeLists.txt index 3c0195681a21c..7c326b7251bf7 100644 --- a/tree/dataframe/test/CMakeLists.txt +++ b/tree/dataframe/test/CMakeLists.txt @@ -59,11 +59,6 @@ ROOT_GENERATE_DICTIONARY(DummyDict ${CMAKE_CURRENT_SOURCE_DIR}/DummyHeader.hxx endif() ROOT_ADD_GTEST(dataframe_datasetspec dataframe_datasetspec.cxx LIBRARIES ROOTDataFrame) -if(builtin_nlohmannjson) - target_include_directories(dataframe_datasetspec PRIVATE ${CMAKE_SOURCE_DIR}/builtins) -else() - target_link_libraries(dataframe_datasetspec nlohmann_json::nlohmann_json) -endif() ROOT_ADD_GTEST(dataframe_display dataframe_display.cxx LIBRARIES ROOTDataFrame) ROOT_ADD_GTEST(dataframe_ranges dataframe_ranges.cxx LIBRARIES ROOTDataFrame) From d99f95e80f74b2c0d3f09c6cedab8700f271bfa0 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 27 Feb 2024 16:36:38 +0100 Subject: [PATCH 11/21] [CMake] Make lzma config target-based. - Remove the use of variables for lzma, use target_link_libraries instead. - Use the same target name as the CMake module: LibLZMA::LibLZMA --- cmake/modules/SearchInstalledSoftware.cmake | 5 +++++ core/lzma/CMakeLists.txt | 8 ++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cmake/modules/SearchInstalledSoftware.cmake b/cmake/modules/SearchInstalledSoftware.cmake index de2f030686b93..8ef7b47ad0dc4 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -352,6 +352,11 @@ if(builtin_lzma) ) set(LIBLZMA_INCLUDE_DIR ${CMAKE_BINARY_DIR}/include) endif() + + add_library(LibLZMA STATIC IMPORTED GLOBAL) + add_library(LibLZMA::LibLZMA ALIAS LibLZMA) + target_include_directories(LibLZMA INTERFACE ${LIBLZMA_INCLUDE_DIR}) + set_target_properties(LibLZMA PROPERTIES IMPORTED_LOCATION ${LIBLZMA_LIBRARIES}) endif() #---Check for xxHash----------------------------------------------------------------- diff --git a/core/lzma/CMakeLists.txt b/core/lzma/CMakeLists.txt index 9de19d2d26a83..f19eec364cb8d 100644 --- a/core/lzma/CMakeLists.txt +++ b/core/lzma/CMakeLists.txt @@ -10,12 +10,8 @@ target_sources(Core PRIVATE src/ZipLZMA.c) -target_link_libraries(Core PRIVATE ${LIBLZMA_LIBRARIES}) +target_link_libraries(Core PRIVATE LibLZMA::LibLZMA) -target_include_directories(Core - PUBLIC - $ - $ -) +target_include_directories(Core PUBLIC $) ROOT_INSTALL_HEADERS() From 46387e4d7639c6cebe90406663698063d6a0b80e Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 13 Jul 2021 11:05:02 +0200 Subject: [PATCH 12/21] [CMake] Remove manual lists of include directories. For an unknown reason, ROOT's cmake macros were reading all include directories from the targets they depend on, and doing some manual processing of those. Due to more extensive use of target-based cmake, this manual treatment should become obsolete. This makes the management of include directories simpler, and will allow for better debugging with CMake. --- cmake/modules/RootMacros.cmake | 91 ---------------------------------- 1 file changed, 91 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index a78fdcd0d9835..3acf60ca21f41 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -925,72 +925,6 @@ function(ROOT_LINKER_LIBRARY library) ROOT_ADD_INCLUDE_DIRECTORIES(${library}) - if(PROJECT_NAME STREQUAL "ROOT") - set(dep_list) - if(ARG_DEPENDENCIES) - foreach(lib ${ARG_DEPENDENCIES}) - if((TARGET ${lib}) AND NOT (${lib} STREQUAL Core)) - # Include directories property is different for INTERFACE libraries - get_target_property(_target_type ${lib} TYPE) - if(${_target_type} STREQUAL "INTERFACE_LIBRARY") - get_target_property(lib_incdirs ${lib} INTERFACE_INCLUDE_DIRECTORIES) - else() - get_target_property(lib_incdirs ${lib} INCLUDE_DIRECTORIES) - endif() - if(lib_incdirs) - foreach(dir ${lib_incdirs}) - ROOT_REPLACE_BUILD_INTERFACE(dir ${dir}) - if(NOT ${dir} MATCHES "^[$]") - list(APPEND dep_list ${dir}) - endif() - endforeach() - endif() - endif() - endforeach() - endif() - if(dep_list) - list(REMOVE_DUPLICATES dep_list) - endif() - foreach(incl ${dep_list}) - target_include_directories(${library} PRIVATE ${incl}) - endforeach() - endif() - - if(PROJECT_NAME STREQUAL "ROOT") - set(dep_inc_list) - if(ARG_LIBRARIES) - foreach(lib ${ARG_LIBRARIES}) - if(TARGET ${lib}) - get_target_property(_target_type ${lib} TYPE) - if(${_target_type} STREQUAL "INTERFACE_LIBRARY") - get_target_property(lib_incdirs ${lib} INTERFACE_INCLUDE_DIRECTORIES) - get_target_property(lib_rpath ${lib} INTERFACE_BUILD_RPATH) - else() - get_target_property(lib_incdirs ${lib} INCLUDE_DIRECTORIES) - get_target_property(lib_rpath ${lib} BUILD_RPATH) - endif() - if(lib_incdirs) - foreach(dir ${lib_incdirs}) - ROOT_REPLACE_BUILD_INTERFACE(dir ${dir}) - if(NOT ${dir} MATCHES "^[$]") - list(APPEND dep_inc_list ${dir}) - endif() - endforeach() - endif() - if(lib_rpath) - set_target_properties(${library} PROPERTIES BUILD_RPATH ${lib_rpath}) - endif() - endif() - endforeach() - endif() - if(dep_inc_list) - list(REMOVE_DUPLICATES dep_inc_list) - foreach(incl ${dep_inc_list}) - target_include_directories(${library} PRIVATE ${incl}) - endforeach() - endif() - endif() - if(TARGET G__${library}) add_dependencies(${library} G__${library}) endif() @@ -1454,31 +1388,6 @@ function(ROOT_EXECUTABLE executable) add_executable(${executable} ${_all} ${exe_srcs}) target_link_libraries(${executable} ${ARG_LIBRARIES}) - if(PROJECT_NAME STREQUAL "ROOT") - set(dep_list) - if(ARG_LIBRARIES) - foreach(lib ${ARG_LIBRARIES}) - if(TARGET ${lib}) - get_target_property(lib_incdirs ${lib} INCLUDE_DIRECTORIES) - if(lib_incdirs) - foreach(dir ${lib_incdirs}) - ROOT_REPLACE_BUILD_INTERFACE(dir ${dir}) - if(NOT ${dir} MATCHES "^[$]") - list(APPEND dep_list ${dir}) - endif() - endforeach() - endif() - endif() - endforeach() - endif() - if(dep_list) - list(REMOVE_DUPLICATES dep_list) - foreach(incl ${dep_list}) - target_include_directories(${executable} PRIVATE ${incl}) - endforeach() - endif() - endif() - if(WIN32 AND ${executable} MATCHES \\.exe) set_target_properties(${executable} PROPERTIES SUFFIX "") endif() From 65e0ec5a37f9d54b39fe6ba362c03fc5cbfcfa9d Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Wed, 21 Jul 2021 10:04:19 +0200 Subject: [PATCH 13/21] [CMake] Consolidate dependency management for ROOT_LINKER_LIBRARY. - Collect all dependency and include-related instructions in one place to make it more clear what's going on. - Improve documentation a bit. - Remove manual treatmant of include directories and link configs, as they should now be handled automatically by target_link_libraries. --- cmake/modules/RootMacros.cmake | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 3acf60ca21f41..517293fced427 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -879,9 +879,10 @@ endfunction() #--------------------------------------------------------------------------------------------------- #---ROOT_LINKER_LIBRARY( source1 source2 ...[TYPE STATIC|SHARED] [DLLEXPORT] -# [NOINSTALL] LIBRARIES library1 library2 ... -# DEPENDENCIES dep1 dep2 -# BUILTINS dep1 dep2) +# [NOINSTALL] +# LIBRARIES library1 library2 ... # PRIVATE link dependencies +# DEPENDENCIES dep1 dep2 # PUBLIC link dependencies +# BUILTINS dep1 dep2 # dependencies to builtins) #--------------------------------------------------------------------------------------------------- function(ROOT_LINKER_LIBRARY library) CMAKE_PARSE_ARGUMENTS(ARG "DLLEXPORT;CMAKENOEXPORT;TEST;NOINSTALL" "TYPE" "LIBRARIES;DEPENDENCIES;BUILTINS" ${ARGN}) @@ -894,8 +895,7 @@ function(ROOT_LINKER_LIBRARY library) endif() set(library_name ${library}) if(TARGET ${library}) - message("Target ${library} already exists. Renaming target name to ${library}_new") - set(library ${library}_new) + message(FATAL_ERROR "Target ${library} already exists.") endif() if(WIN32 AND ARG_TYPE STREQUAL SHARED AND NOT ARG_DLLEXPORT) if(MSVC) @@ -903,37 +903,37 @@ function(ROOT_LINKER_LIBRARY library) endif() #---create a shared library with the .def file------------------------ add_library(${library} ${_all} SHARED ${lib_srcs}) - target_link_libraries(${library} PUBLIC ${ARG_LIBRARIES} ${ARG_DEPENDENCIES}) set_target_properties(${library} PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE) else() add_library( ${library} ${_all} ${ARG_TYPE} ${lib_srcs}) if(ARG_TYPE STREQUAL SHARED) set_target_properties(${library} PROPERTIES ${ROOT_LIBRARY_PROPERTIES} ) endif() - target_link_libraries(${library} PUBLIC ${ARG_LIBRARIES} ${ARG_DEPENDENCIES}) endif() if(DEFINED CMAKE_CXX_STANDARD) target_compile_features(${library} INTERFACE cxx_std_${CMAKE_CXX_STANDARD}) endif() - if(PROJECT_NAME STREQUAL "ROOT") - if(NOT TARGET ROOT::${library}) - add_library(ROOT::${library} ALIAS ${library}) - endif() - endif() - - ROOT_ADD_INCLUDE_DIRECTORIES(${library}) + # Add dependencies passed via LIBRARIES or DEPENDENCIES argument: + target_link_libraries(${library} PUBLIC ${ARG_DEPENDENCIES}) + target_link_libraries(${library} PRIVATE ${ARG_LIBRARIES}) if(TARGET G__${library}) add_dependencies(${library} G__${library}) endif() - if(CMAKE_PROJECT_NAME STREQUAL ROOT) + set_property(GLOBAL APPEND PROPERTY ROOT_EXPORTED_TARGETS ${library}) + + ROOT_ADD_INCLUDE_DIRECTORIES(${library}) + + if(PROJECT_NAME STREQUAL "ROOT") add_dependencies(${library} move_headers) + if(NOT TARGET ROOT::${library}) + add_library(ROOT::${library} ALIAS ${library}) + endif() endif() - set_property(GLOBAL APPEND PROPERTY ROOT_EXPORTED_TARGETS ${library}) + set_target_properties(${library} PROPERTIES OUTPUT_NAME ${library_name}) - set_target_properties(${library} PROPERTIES INTERFACE_LINK_LIBRARIES "${ARG_DEPENDENCIES}") target_include_directories(${library} INTERFACE $) # Do not add -Dname_EXPORTS to the command-line when building files in this # target. Doing so is actively harmful for the modules build because it From 833f797eb38ac1b4dbd16f93ebe8e400ebc1ba18 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Wed, 21 Jul 2021 11:12:25 +0200 Subject: [PATCH 14/21] [CMake] Use modern target_link_library syntax for exectuables. This means that target_link_libraries has to use the PRIVATE/PUBLIC keyword. --- cmake/modules/RootMacros.cmake | 6 +++--- main/CMakeLists.txt | 2 +- roofit/multiprocess/test/CMakeLists.txt | 2 +- tree/ntupleutil/v7/test/CMakeLists.txt | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 517293fced427..b5d7665959c29 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1386,7 +1386,7 @@ function(ROOT_EXECUTABLE executable) set(exe_srcs ${exe_srcs} ${ROOT_RC_SCRIPT}) endif() add_executable(${executable} ${_all} ${exe_srcs}) - target_link_libraries(${executable} ${ARG_LIBRARIES}) + target_link_libraries(${executable} PRIVATE ${ARG_LIBRARIES}) if(WIN32 AND ${executable} MATCHES \\.exe) set_target_properties(${executable} PROPERTIES SUFFIX "") @@ -1783,9 +1783,9 @@ function(ROOT_ADD_GTEST test_suite) # against. For example, tests in Core should link only against libCore. This could be tricky # to implement because some ROOT components create more than one library. ROOT_EXECUTABLE(${test_suite} ${source_files} LIBRARIES ${ARG_LIBRARIES}) - target_link_libraries(${test_suite} gtest_main gmock gmock_main) + target_link_libraries(${test_suite} PRIVATE gtest gtest_main gmock gmock_main) if(TARGET ROOT::TestSupport) - target_link_libraries(${test_suite} ROOT::TestSupport) + target_link_libraries(${test_suite} PRIVATE ROOT::TestSupport) else() # Since we don't inherit the linkage against gtest from ROOT::TestSupport, # we need to link against gtest here. diff --git a/main/CMakeLists.txt b/main/CMakeLists.txt index 050a0c0d4144b..745166413db37 100644 --- a/main/CMakeLists.txt +++ b/main/CMakeLists.txt @@ -36,7 +36,7 @@ if(MSVC) else() ROOT_EXECUTABLE(hadd hadd.cxx LIBRARIES Core RIO Net Hist Graf Graf3d Gpad Tree Matrix MathCore MultiProc CMAKENOEXPORT) if (CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0) - target_link_libraries(hadd stdc++fs) + target_link_libraries(hadd PRIVATE stdc++fs) endif() endif() ROOT_EXECUTABLE(rootnb.exe nbmain.cxx LIBRARIES Core CMAKENOEXPORT) diff --git a/roofit/multiprocess/test/CMakeLists.txt b/roofit/multiprocess/test/CMakeLists.txt index 27a4046c940eb..9f72e5cf830cb 100644 --- a/roofit/multiprocess/test/CMakeLists.txt +++ b/roofit/multiprocess/test/CMakeLists.txt @@ -6,7 +6,7 @@ target_include_directories(RooFit_multiprocess_testing_utils INTERFACE ${RooFitM 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 RooFit_multiprocess_testing_utils) +target_link_libraries(test_RooFit_MultiProcess_Job PUBLIC RooFit_multiprocess_testing_utils) ROOT_ADD_GTEST(test_RooFit_MultiProcess_ProcessManager test_ProcessManager.cxx LIBRARIES RooFitMultiProcess) ROOT_ADD_GTEST(test_RooFit_MultiProcess_Messenger test_Messenger.cxx LIBRARIES RooFitMultiProcess) diff --git a/tree/ntupleutil/v7/test/CMakeLists.txt b/tree/ntupleutil/v7/test/CMakeLists.txt index 9dfe739fbd9ac..eb7679b874376 100644 --- a/tree/ntupleutil/v7/test/CMakeLists.txt +++ b/tree/ntupleutil/v7/test/CMakeLists.txt @@ -24,5 +24,5 @@ ROOT_ADD_GTEST(ntuple_exporter ntuple_exporter.cxx LIBRARIES ROOTNTupleUtil) ROOT_ADD_GTEST(ntuple_inspector ntuple_inspector.cxx LIBRARIES ROOTNTupleUtil) if (CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0) - target_link_libraries(ntuple_exporter stdc++fs) + target_link_libraries(ntuple_exporter PRIVATE stdc++fs) endif() From ca2f701b09ae44f2ffe80a7e903f59a45b414cac Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Wed, 21 Jul 2021 15:25:31 +0200 Subject: [PATCH 15/21] [CMake] Improve ROOT_GENERATE_DICTIONARY - In case ROOT_GENERATE_DICTIONARY is invoked with a dependency that doesn't have a dictionary itself, the for loop through dependencies now just continue()s. Before, this would raise a CMake error. - The object library with the dictionary file is now linked into the main library using target_link_libraries(). - When the list of include directories for the dictionary is generated, the INTERFACE_SYSTEM_INCLUDE_DIRECTORIES of the dependencies is now honoured. Before, system includes would decay to normal includes. Unfortunately, PRIVATE includes still decay to normal -I includes. This can lead header conflicts when ROOT is built while another ROOT is installed in system include directories, but only for the dictionary files. Since ROOT include directories are very generously prepended to all targets, I wasn't able to provoke a header conflict. --- cmake/modules/RootMacros.cmake | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index b5d7665959c29..63e84a05f4148 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -589,6 +589,11 @@ function(ROOT_GENERATE_DICTIONARY dictionary) #---Get the library and module dependencies----------------- if(ARG_DEPENDENCIES) foreach(dep ${ARG_DEPENDENCIES}) + if(NOT TARGET G__${dep}) + # This is a library that doesn't come with dictionary/pcm + continue() + endif() + set(dependent_pcm ${libprefix}${dep}_rdict.pcm) if (runtime_cxxmodules AND NOT dep IN_LIST local_no_cxxmodules) set(dependent_pcm ${dep}.pcm) @@ -698,7 +703,7 @@ function(ROOT_GENERATE_DICTIONARY dictionary) if(TARGET "${ARG_MODULE}" AND NOT "${ARG_MODULE}" STREQUAL "${dictionary}") add_library(${dictionary} OBJECT ${dictionary}.cxx) set_target_properties(${dictionary} PROPERTIES POSITION_INDEPENDENT_CODE TRUE) - target_sources(${ARG_MODULE} PRIVATE $) + target_link_libraries(${ARG_MODULE} PRIVATE ${dictionary}) target_compile_options(${dictionary} PRIVATE $) @@ -709,8 +714,16 @@ function(ROOT_GENERATE_DICTIONARY dictionary) target_compile_features(${dictionary} PRIVATE $) - target_include_directories(${dictionary} PRIVATE - ${incdirs} $) + target_include_directories(${dictionary} PRIVATE ${incdirs} $) + + # Above we are copying all include directories of the module, irrespective of whether they are system includes. + # CMake copies them as -I even when they should be -isystem. + # We can fix this for INTERFACE includes by also copying the respective property. + # For PRIVATE includes this doesn't work. In that case, one needs to link both the library as well as the dictionary explicitly: + # target_link_libraries(MODULE PRIVATE dependency) + # target_link_libraries(G__MODULE PRIVATE dependency) + set_property(TARGET ${dictionary} APPEND PROPERTY + INTERFACE_SYSTEM_INCLUDE_DIRECTORIES $) else() get_filename_component(dictionary_name ${dictionary} NAME) add_custom_target(${dictionary_name} DEPENDS ${dictionary}.cxx ${pcm_name} ${rootmap_name} ${cpp_module_file}) From 95f69b4976a521f6eb27748117ef79757a08bf32 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Thu, 23 Mar 2023 16:43:35 +0100 Subject: [PATCH 16/21] [CMake] Fix unit test config for core/thread. Most unit tests in core/thread were running twice: - Once from ROOT_ADD_UNIT_TEST_DIR, which globs all *.cxx and compiles them into a test - Once more from explicitly registering the tests Here, we register all tests explicitly. --- core/thread/test/CMakeLists.txt | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/core/thread/test/CMakeLists.txt b/core/thread/test/CMakeLists.txt index e09ff7d1f95ae..e45d3dd0811f4 100644 --- a/core/thread/test/CMakeLists.txt +++ b/core/thread/test/CMakeLists.txt @@ -4,11 +4,7 @@ # For the licensing terms see $ROOTSYS/LICENSE. # For the list of contributors see $ROOTSYS/README/CREDITS. -if (tbb OR builtin_tbb) - ROOT_ADD_UNITTEST_DIR(Core Thread Hist ${TBB_LIBRARIES}) -else() - ROOT_ADD_UNITTEST_DIR(Core Thread Hist) -endif() +ROOT_ADD_GTEST(testRWLock testRWLock.cxx LIBRARIES Core Thread Hist ${TBB_LIBRARIES}) ROOT_ADD_GTEST(testTThreadedObject testTThreadedObject.cxx LIBRARIES Hist) From aa5a152e01f64338bf0c589ec13d2206137f61e1 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Thu, 26 Oct 2023 16:52:01 +0200 Subject: [PATCH 17/21] [CMake][NFC] Clarify arguments of ROOT_STANDARD_LIBRARY_PACKAGE. --- cmake/modules/RootMacros.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 63e84a05f4148..938f587da819e 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1266,8 +1266,8 @@ endmacro() # [NO_SOURCES] : don't glob to fill SOURCES variable # [OBJECT_LIBRARY] : use ROOT_OBJECT_LIBRARY to generate object files # and then use those for linking. -# LIBRARIES lib1 lib2 : linking flags such as dl, readline -# DEPENDENCIES lib1 lib2 : dependencies such as Core, MathCore +# LIBRARIES lib1 lib2 : private arguments for target_link_library() +# DEPENDENCIES lib1 lib2 : PUBLIC arguments for target_link_library() such as Core, MathCore # BUILTINS builtin1 builtin2 : builtins like AFTERIMAGE # LINKDEF LinkDef.h : linkdef file, default value is "LinkDef.h" # DICTIONARY_OPTIONS option : options passed to rootcling From ac5b9335e768e619d3b13fd28774360e822a52d7 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 27 Dec 2022 19:05:45 +0100 Subject: [PATCH 18/21] [CMake][NFC] Remove an unused argument for ROOT_ADD_TEST. The argument COMPILEMACROS isn't used anywhere, so it can be removed. --- cmake/modules/RootMacros.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 938f587da819e..bd71d662beb43 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1483,7 +1483,7 @@ set(ROOT_TEST_DRIVER ${CMAKE_CURRENT_LIST_DIR}/RootTestDriver.cmake) function(ROOT_ADD_TEST test) CMAKE_PARSE_ARGUMENTS(ARG "DEBUG;WILLFAIL;CHECKOUT;CHECKERR;RUN_SERIAL" "TIMEOUT;BUILD;INPUT;OUTPUT;ERROR;SOURCE_DIR;BINARY_DIR;WORKING_DIR;PROJECT;PASSRC;RESOURCE_LOCK" - "COMMAND;COPY_TO_BUILDDIR;DIFFCMD;OUTCNV;OUTCNVCMD;PRECMD;POSTCMD;ENVIRONMENT;COMPILEMACROS;DEPENDS;PASSREGEX;OUTREF;ERRREF;FAILREGEX;LABELS;PYTHON_DEPS;FIXTURES_SETUP;FIXTURES_CLEANUP;FIXTURES_REQUIRED;PROPERTIES" + "COMMAND;COPY_TO_BUILDDIR;DIFFCMD;OUTCNV;OUTCNVCMD;PRECMD;POSTCMD;ENVIRONMENT;DEPENDS;PASSREGEX;OUTREF;ERRREF;FAILREGEX;LABELS;PYTHON_DEPS;FIXTURES_SETUP;FIXTURES_CLEANUP;FIXTURES_REQUIRED;PROPERTIES" ${ARGN}) #- Handle COMMAND argument From 121a7e4525e91fdbf91091b3c834ee26a5553655 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Thu, 14 Mar 2024 14:58:21 +0100 Subject: [PATCH 19/21] [CMake] Move to CMake's native FindGTest - Remove own FindGTest, use the one from CMake. Starting from CMake 3.23, the GTest libraries have canonical names. - Replace uses of legacy targets like "gtest" in ROOT with canonical target names such as GTest::gtest or GTest::gtest_main. - Create ALIAS targets like in CMake 3.23 such as GTest::gtest_main. For CMake < 3.23, this will allow using the standard FindGTest with the modern names already in cmake 3.20. --- cmake/modules/FindGTest.cmake | 81 ------------------- cmake/modules/RootMacros.cmake | 5 +- cmake/modules/SearchInstalledSoftware.cmake | 9 +++ core/testsupport/CMakeLists.txt | 2 +- .../roofit/test/vectorisedPDFs/CMakeLists.txt | 2 +- 5 files changed, 12 insertions(+), 87 deletions(-) delete mode 100644 cmake/modules/FindGTest.cmake diff --git a/cmake/modules/FindGTest.cmake b/cmake/modules/FindGTest.cmake deleted file mode 100644 index 438ec501c2c61..0000000000000 --- a/cmake/modules/FindGTest.cmake +++ /dev/null @@ -1,81 +0,0 @@ -# Find the gtest and gmock includes and library. -# -# This module defines -# GTEST_LIBRARIES -# GTEST_MAIN_LIBRARIES -# GTEST_INCLUDE_DIRS -# GMOCK_LIBRARIES -# GMOCK_MAIN_LIBRARIES -# GMOCK_INCLUDE_DIRS -# -# GTEST_FOUND true if all libraries present - -find_package(Threads QUIET) - -find_path(GTEST_INCLUDE_DIRS NAMES gtest/gtest.h) -find_library(GTEST_LIBRARIES NAMES gtest) -find_library(GTEST_MAIN_LIBRARIES NAMES gtest_main) - -find_path(GMOCK_INCLUDE_DIRS NAMES gmock/gmock.h) -find_library(GMOCK_LIBRARIES NAMES gmock) -find_library(GMOCK_MAIN_LIBRARIES NAMES gmock_main) - -# Special for EPEL 7's gmock -if(NOT GMOCK_LIBRARIES) - find_path(GMOCK_SRC_DIR NAMES gmock-all.cc PATHS /usr/src/gmock) -endif() - -if(NOT GMOCK_MAIN_LIBRARIES) - find_path(GMOCK_MAIN_SRC_DIR NAMES gmock_main.cc PATHS /usr/src/gmock) -endif() - -if (GTEST_INCLUDE_DIRS AND - GTEST_LIBRARIES AND - GTEST_MAIN_LIBRARIES AND - GMOCK_INCLUDE_DIRS AND - (GMOCK_LIBRARIES OR GMOCK_SRC_DIR) AND - (GMOCK_MAIN_LIBRARIES OR GMOCK_MAIN_SRC_DIR)) - - add_library(gtest UNKNOWN IMPORTED) - set_target_properties(gtest PROPERTIES - IMPORTED_LOCATION ${GTEST_LIBRARIES} - INTERFACE_INCLUDE_DIRECTORIES ${GTEST_INCLUDE_DIRS}) - target_link_libraries(gtest INTERFACE Threads::Threads) - - add_library(gtest_main UNKNOWN IMPORTED) - set_target_properties(gtest_main PROPERTIES - IMPORTED_LOCATION ${GTEST_MAIN_LIBRARIES}) - target_link_libraries(gtest_main INTERFACE gtest Threads::Threads) - - if(GMOCK_LIBRARIES) - add_library(gmock UNKNOWN IMPORTED) - set_target_properties(gmock PROPERTIES - IMPORTED_LOCATION ${GMOCK_LIBRARIES} - INTERFACE_INCLUDE_DIRECTORIES ${GMOCK_INCLUDE_DIRS}) - else() - add_library(gmock STATIC ${GMOCK_SRC_DIR}/gmock-all.cc) - target_include_directories(gmock PUBLIC ${GMOCK_INCLUDE_DIRS}) - set(GMOCK_LIBRARIES gmock) - endif() - target_link_libraries(gmock INTERFACE gtest Threads::Threads) - - if(GMOCK_MAIN_LIBRARIES) - add_library(gmock_main UNKNOWN IMPORTED) - set_target_properties(gmock_main PROPERTIES - IMPORTED_LOCATION ${GMOCK_MAIN_LIBRARIES}) - else() - add_library(gmock_main STATIC ${GMOCK_MAIN_SRC_DIR}/gmock_main.cc) - set(GMOCK_MAIN_LIBRARIES gmock_main) - endif() - target_link_libraries(gmock_main INTERFACE gmock Threads::Threads) - -endif() - -include(FindPackageHandleStandardArgs) -find_package_handle_standard_args(GTest DEFAULT_MSG - GTEST_LIBRARIES - GTEST_MAIN_LIBRARIES - GTEST_INCLUDE_DIRS - GMOCK_LIBRARIES - GMOCK_MAIN_LIBRARIES - GMOCK_INCLUDE_DIRS) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index bd71d662beb43..048e9b7ccd485 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1796,13 +1796,10 @@ function(ROOT_ADD_GTEST test_suite) # against. For example, tests in Core should link only against libCore. This could be tricky # to implement because some ROOT components create more than one library. ROOT_EXECUTABLE(${test_suite} ${source_files} LIBRARIES ${ARG_LIBRARIES}) - target_link_libraries(${test_suite} PRIVATE gtest gtest_main gmock gmock_main) + target_link_libraries(${test_suite} PRIVATE GTest::gtest GTest::gmock GTest::gtest_main GTest::gmock_main) if(TARGET ROOT::TestSupport) target_link_libraries(${test_suite} PRIVATE ROOT::TestSupport) else() - # Since we don't inherit the linkage against gtest from ROOT::TestSupport, - # we need to link against gtest here. - target_link_libraries(${test_suite} gtest) message(WARNING "ROOT_ADD_GTEST(${test_suite} ...): The target ROOT::TestSupport is missing. It looks like the test is declared against a ROOT build that is configured with -Dtesting=OFF. If this test sends warning or error messages, this will go unnoticed.") endif() diff --git a/cmake/modules/SearchInstalledSoftware.cmake b/cmake/modules/SearchInstalledSoftware.cmake index 8ef7b47ad0dc4..df3957d036ec4 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -2061,6 +2061,15 @@ if (builtin_gtest) endif() +# Starting from cmake 3.23, the GTest targets will have stable names. +# ROOT was updated to use those, but for older CMake versions, we have to declare the aliases: +foreach(LIBNAME gtest_main gmock_main gtest gmock) + if(NOT TARGET GTest::${LIBNAME} AND TARGET ${LIBNAME}) + add_library(GTest::${LIBNAME} ALIAS ${LIBNAME}) + endif() +endforeach() + +#------------------------------------------------------------------------------------ if(webgui AND NOT builtin_openui5) ROOT_CHECK_CONNECTION("builtin_openui5=ON") if(NO_CONNECTION) diff --git a/core/testsupport/CMakeLists.txt b/core/testsupport/CMakeLists.txt index c638ea7ad1f51..f795f6d0b5694 100644 --- a/core/testsupport/CMakeLists.txt +++ b/core/testsupport/CMakeLists.txt @@ -16,7 +16,7 @@ target_include_directories(${libname} PUBLIC $ $ ) -target_link_libraries(${libname} PUBLIC Core gtest) +target_link_libraries(${libname} PRIVATE Core GTest::gtest) # Installation of header and library: set_target_properties(${libname} PROPERTIES PUBLIC_HEADER inc/${header_dir}/TestSupport.hxx) diff --git a/roofit/roofit/test/vectorisedPDFs/CMakeLists.txt b/roofit/roofit/test/vectorisedPDFs/CMakeLists.txt index c0885ce59a4bd..6ed546a26ad67 100644 --- a/roofit/roofit/test/vectorisedPDFs/CMakeLists.txt +++ b/roofit/roofit/test/vectorisedPDFs/CMakeLists.txt @@ -10,7 +10,7 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL Intel) endif() add_library(VectorisedPDFTests STATIC VectorisedPDFTests.cxx) -target_link_libraries(VectorisedPDFTests PUBLIC gtest ROOT::Gpad ROOT::RooFitCore ROOT::RooFit) +target_link_libraries(VectorisedPDFTests PUBLIC ROOT::Gpad ROOT::RooFitCore ROOT::RooFit GTest::gtest) ROOT_ADD_GTEST(testCompatMode testCompatMode.cxx LIBRARIES VectorisedPDFTests) From 53845d61d9dd592f53c4f99d96fb32ea638ecb94 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 13 Jan 2025 14:35:53 +0100 Subject: [PATCH 20/21] [core] Add missing gmock dependencies. Explicitly list GTest::gmock in core. --- core/foundation/test/CMakeLists.txt | 2 +- core/metacling/test/CMakeLists.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/foundation/test/CMakeLists.txt b/core/foundation/test/CMakeLists.txt index 4105fcd2c38f3..4e6b7a8d8dce9 100644 --- a/core/foundation/test/CMakeLists.txt +++ b/core/foundation/test/CMakeLists.txt @@ -8,7 +8,7 @@ ROOT_ADD_GTEST(testMake_unique testMake_unique.cxx LIBRARIES Core) ROOT_ADD_GTEST(testTypeTraits testTypeTraits.cxx LIBRARIES Core) ROOT_ADD_GTEST(testNotFn testNotFn.cxx LIBRARIES Core) ROOT_ADD_GTEST(testClassEdit testClassEdit.cxx LIBRARIES Core) -ROOT_ADD_GTEST(testException testException.cxx LIBRARIES Core) +ROOT_ADD_GTEST(testException testException.cxx LIBRARIES Core GTest::gmock) ROOT_ADD_GTEST(testLogger testLogger.cxx LIBRARIES Core) ROOT_ADD_GTEST(testRRangeCast testRRangeCast.cxx LIBRARIES Core) ROOT_ADD_GTEST(testStringUtils testStringUtils.cxx LIBRARIES Core) diff --git a/core/metacling/test/CMakeLists.txt b/core/metacling/test/CMakeLists.txt index aed9784adcd27..65dab16eaa4ba 100644 --- a/core/metacling/test/CMakeLists.txt +++ b/core/metacling/test/CMakeLists.txt @@ -26,6 +26,7 @@ ROOT_ADD_GTEST( LIBRARIES Core + GTest::gmock ) add_dependencies(TClingTest Cling RIO) From 9d4fd15cf6ffe319d7e85301f5456545111543d3 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Fri, 15 Nov 2024 15:49:03 +0100 Subject: [PATCH 21/21] [RF] Add a dependency on nlohmann_json. RooFit multiprocess privately depends on nlohmann_json. The res headers use it too, but since the dependency is private, the tests cannot use it unless they explicitly request nlohmann_json, too. --- roofit/multiprocess/test/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/roofit/multiprocess/test/CMakeLists.txt b/roofit/multiprocess/test/CMakeLists.txt index 9f72e5cf830cb..e9bb53b5fd447 100644 --- a/roofit/multiprocess/test/CMakeLists.txt +++ b/roofit/multiprocess/test/CMakeLists.txt @@ -12,7 +12,8 @@ ROOT_ADD_GTEST(test_RooFit_MultiProcess_ProcessManager test_ProcessManager.cxx L ROOT_ADD_GTEST(test_RooFit_MultiProcess_Messenger test_Messenger.cxx LIBRARIES RooFitMultiProcess) ROOT_ADD_GTEST(test_RooFit_MultiProcess_Queue test_Queue.cxx LIBRARIES RooFitMultiProcess) -ROOT_ADD_GTEST(test_RooFit_MultiProcess_ProcessTimer test_ProcessTimer.cxx LIBRARIES RooFitMultiProcess) +#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_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