From ef5eb31ee180718f615b8b77d0fa5d770761257e 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 1f665e06d201f798bfc7a89c10840d4943c66407 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 4d34eac9e94cc870a52d086e6498385e091f465f 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 0aa14e38626e7..fc6189857060b 100644 --- a/io/io/test/TFileMergerTests.cxx +++ b/io/io/test/TFileMergerTests.cxx @@ -8,6 +8,8 @@ #include "TTree.h" #include "TH1.h" +#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 c589081e753d2f8a4ca3cd0f6cd159ece2d295df 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 9116f7a3bc12bb41ed9080c0594e0ac8b6319ff2 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 b6a551c0574811e0508654b2a1ee5761adce8406 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 6bbc199029967..126bd7f56b670 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -1533,14 +1533,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.4) 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=8b1664b45ec82042152f89d171dd962aea9bb35ac53c8eebb35df1cb9c34e498 INSTALL_DIR ${CMAKE_BINARY_DIR} @@ -1557,12 +1559,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 3b6869bc90221b8cbe21170c037a7010c8b69d08 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 c103cf95b8e4d..2906384bd7dbe 100644 --- a/builtins/xrootd/CMakeLists.txt +++ b/builtins/xrootd/CMakeLists.txt @@ -7,21 +7,10 @@ include(ExternalProject) set(XROOTD_VERSION "5.7.2") +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 126bd7f56b670..f00f7f037f06f 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -945,13 +945,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) @@ -989,20 +982,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 57787f1fe16bc19861307f43211463d7121ce27f 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 f00f7f037f06f..d31447cb4e167 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -972,12 +972,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() @@ -992,7 +990,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 0eb32275c61a7ff4221f36e1a734e031dbddb688 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 862015cf68714..6e0d6f8c5512d 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 2906384bd7dbe..23d17754ae62c 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 daed0df2db275689b88661985ee150db72a197c9 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 d31447cb4e167..d0d8b4804275a 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 7964b9fc3d54f..45bedc6ac8d75 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(root7) set(RIO_EXTRA_HEADERS ROOT/RFile.hxx) diff --git a/roofit/jsoninterface/CMakeLists.txt b/roofit/jsoninterface/CMakeLists.txt index db632cf93d6a6..9a4375fe04176 100644 --- a/roofit/jsoninterface/CMakeLists.txt +++ b/roofit/jsoninterface/CMakeLists.txt @@ -47,10 +47,6 @@ if(${RYML_FOUND}) target_link_directories(RooFitJSONInterface PRIVATE ${RYML_LIB_DIR}) endif() -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 2c524b39fbf48..9a25dc2de0210 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 b60854ea4ff7f..06eac7c030f44 100644 --- a/tree/dataframe/CMakeLists.txt +++ b/tree/dataframe/CMakeLists.txt @@ -168,10 +168,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 ba93bd7164ed9..7bb5967457556 100644 --- a/tree/dataframe/test/CMakeLists.txt +++ b/tree/dataframe/test/CMakeLists.txt @@ -51,11 +51,6 @@ if(NOT (MSVC OR (APPLE AND CMAKE_SYSTEM_PROCESSOR MATCHES arm64)) OR win_broken_ 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 884e5ea1884d3b1b8981ed09cb75a273039d28d5 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 d0d8b4804275a..ae517e81e273a 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -351,6 +351,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 e4f39883d1151fae9da5b29f0cac430ff46c10b8 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 f8f02be27b490..1c4702a986951 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -923,72 +923,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() @@ -1452,31 +1386,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 a6234d84b497eeea59d7313888816422fb25dba0 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 1c4702a986951..dcf726d05006b 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -877,9 +877,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}) @@ -892,8 +893,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) @@ -901,37 +901,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 ac1ac2a15dba9859563151d8cf53cfb22ee64fbd 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 +- tmva/pymva/test/CMakeLists.txt | 4 ++-- tree/ntupleutil/v7/test/CMakeLists.txt | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index dcf726d05006b..4056946f709cc 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1384,7 +1384,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 "") @@ -1781,9 +1781,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/tmva/pymva/test/CMakeLists.txt b/tmva/pymva/test/CMakeLists.txt index ca66ac53d08ae..a6a58f7d92884 100644 --- a/tmva/pymva/test/CMakeLists.txt +++ b/tmva/pymva/test/CMakeLists.txt @@ -93,7 +93,7 @@ if(ROOT_TORCH_FOUND) SYSTEM ${CMAKE_CURRENT_BINARY_DIR} ) - target_link_libraries(TestRModelParserPyTorch ${BLAS_LINKER_FLAGS} ${BLAS_LIBRARIES}) + target_link_libraries(TestRModelParserPyTorch PUBLIC ${BLAS_LINKER_FLAGS} ${BLAS_LIBRARIES}) endif(ROOT_TORCH_FOUND) @@ -137,6 +137,6 @@ if((ROOT_KERAS_FOUND AND ROOT_THEANO_FOUND) OR (ROOT_KERAS_FOUND AND ROOT_TENSOR SYSTEM ${CMAKE_CURRENT_BINARY_DIR} ) - target_link_libraries(TestRModelParserKeras ${BLAS_LINKER_FLAGS} ${BLAS_LIBRARIES}) + target_link_libraries(TestRModelParserKeras PUBLIC ${BLAS_LINKER_FLAGS} ${BLAS_LIBRARIES}) endif() 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 ae63b8a9cc1151b1950243c9aed2529b4ecfc6c7 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 4056946f709cc..cfd08200ebc24 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -587,6 +587,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) @@ -696,7 +701,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 $) @@ -707,8 +712,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 3e923b487be083997ef8e42fe03077a45f1b6249 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 cd30dab96b9ef63dbfc405f252f6da6c3243e1ce 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 cfd08200ebc24..7feeb5fa6596f 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1264,8 +1264,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 5646c67f321103b30e4289e9149031a49155b0eb 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 7feeb5fa6596f..1287914bf20d5 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1481,7 +1481,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 80e02d044410ee840f4a3c6410371c55e1b1163e 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 1287914bf20d5..94285ad704613 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1794,13 +1794,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 ae517e81e273a..16689d428c2af 100644 --- a/cmake/modules/SearchInstalledSoftware.cmake +++ b/cmake/modules/SearchInstalledSoftware.cmake @@ -2011,6 +2011,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 1c6f7071aaa110c39a7eb5efaee89a26270f07af 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 4026295b34cfe24421f19643fb00b99d731f65ad 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