cmake: Switch ROOT to config mode#1050
Merged
dennisklein merged 5 commits intoFairRootGroup:devfrom Apr 13, 2021
Merged
Conversation
dennisklein
reviewed
Apr 8, 2021
dennisklein
reviewed
Apr 8, 2021
| PATHS ${ROOT_BINDIR} | ||
| NO_DEFAULT_PATH | ||
| ) | ||
| include(ROOTMacros) |
Member
There was a problem hiding this comment.
Suggested change
| include(ROOTMacros) |
Nothing in this module should be in use in FairRoot any more, why include it? (The file may be used by experiment repos however)
Member
There was a problem hiding this comment.
Ah, I see, it was included in the deleted FindROOT.cmake
Member
Author
There was a problem hiding this comment.
And without this, some things don't work. Don't ask me what. I forgot about it.
So maybe experiments will need to add this include somewhere? Because the find_package wont load it any more.
dennisklein
reviewed
Apr 8, 2021
5b27a78 to
6738926
Compare
Member
Author
|
Okay, this should be ready for a first real review. I think, I have already adressed all the raised issues above. That said:
|
694b7fc to
909718e
Compare
All the ROOT library targets that we use, should use the ROOT:: namespace. So let's switch them over. For this iteration, let's add all the needed ones to our own discovery code.
As we now should use targets to link to all ROOT libraries, there shouldn't be any need to use target_link_directories to tell cmake about the directory for those libs.
In preparation for switching ROOT to config mode, let's rename some variables and move things around into newer places. * Move `include(ROOTMacros)` out of FindROOT. * Move setting ROOT_CINT_EXECUTABLE out of FindROOT into ROOTMacros. * Rename variables: ROOT_HAS_OPENGL -> ROOT_opengl_FOUND ROOT_GDML_FOUND -> ROOT_gdml_FOUND (and move setting of it into FindROOT)
Instead of using the internal find module, let's use ROOT's own cmake config mode system.
* We should use ROOT:: targets that give us the include directories. * ROOT_INCLUDE_DIR doesn't work anyway, it's now ROOT_INCLUDE_DIRS, if we want it at all. Also replace ROOT_BINARY_DIR by ROOT_BINDIR.
909718e to
d605425
Compare
dennisklein
approved these changes
Apr 13, 2021
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Instead of using the internal find module, let's use ROOT's own cmake config mode system.
This commit does the basic switch over.
It turns all used library targetd over to their ROOT:: equivalent and does some basic cleanup.
It should compile and hopefully run the test suite.
There is more stuff to cleanup afterwards.
Checklist:
devbranch