[CI] Post install tests #17956
Draft
[CI] Post install tests #17956
+290
−354
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hageboeck
force-pushed
the
postInstallTests
branch
6 times, most recently
from
3da1ddb to
58192ab
Compare
Test Results 10 files 10 suites 1d 15h 46m 23s ⏱️ Results for commit aa24dfc. ♻️ This comment has been updated with latest results. |
For an unknown reason, a res/ header was included, leading to an include error. The include could be removed without problems.
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.
TestSupport.hxx isn't using gtest, so it does not make sense to include it here.
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<VDT_LOCATION> 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.
- 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.
Since ROOT already has SSL support, the logic for builtin XRootD could be simplified by requiring these options to be on.
To help find the builtin openssl location, it is now saved as a cache variable, which is passed to xrootd.
- 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.
- Remove the use of variables for lzma, use target_link_libraries instead. - Use the same target name as the CMake module: LibLZMA::LibLZMA
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.
- 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.
This means that target_link_libraries has to use the PRIVATE/PUBLIC keyword.
- 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.
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.
The argument COMPILEMACROS isn't used anywhere, so it can be removed.
- 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.
Explicitly list GTest::gmock in core.
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.
Use OpenSSL_ROOT instead of OPENSSL_ROOT to help XRootD find the builtin OpenSSL.
…cmake. Ignore the VDT library in ROOT's target; use only the headers.
To test that ROOT is usable after it has been installed, a small CMake project is added that checks the outputs of hsimple.
Add a script that checks all installed headers for syntax errors. This tests if they can be included standalone.
hageboeck
force-pushed
the
postInstallTests
branch
2 times, most recently
from
1fe74bb to
3e3b4a1
Compare
hageboeck
force-pushed
the
postInstallTests
branch
from
3e3b4a1 to
c71a5ec
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note, this contains the commits from #8709, so that should be merged first.