Skip to content
Snippets Groups Projects

ACTSFW-27 add cmake build options

Merged Moritz Kiehn requested to merge ACTSFW-27-cmake_options into master
All threads resolved!

This adds build options to activate various optional components that are deactivate by default. This includes a refactorization of the cmake configuration.

  • Build options and configuration are now always explicit, i.e. if you request to build w/ Geant4 via USE_GEANT4=on Geant4 becomes a requirement. At no point are components build depending on the available software. It is always the other way round: you define what you want and CMake makes sure that all the required software is available.
  • Searching for software via find_package(...) is done centrally in the main CMakeLists.txt file instead of multiple times in every subdirectory.
  • Optional components are activated by wrapping the add_subdirectory call in an small if() ... block instead of wrapping the full content of the CMakeLists.txt file in multiple ifs.

Fixes ACTSFW-27 and is required to get !45 (merged) working.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Hadrien Benjamin Grasland
  • Hadrien Benjamin Grasland
  • Hadrien Benjamin Grasland
  • Overall, I really, really like this MR, and can't wait to see it merged either, but I also think that it was merged in a bit too early and that some improvements would have been warranted before doing so. Perhaps we can discuss these here, and later address them through a separate MR?

  • Hi @hgraslan, right - I merged it in, such that we can do the next one, which is the CI for the framework, we really need that coming up.

  • @msmk can make the few adaptions in a separate MR, though again, if that helps us getting the CI working, I rather would like to have that in earlier than later.

  • Author Maintainer

    I will prepare a follow up MR quickly.

  • @asalzbur The framework CI merge request does not work yet due to an issue with the LCG Geant4 configuration, so merging it in quicker is not entirely up to us. There was thus no benefit in rushing this merge request in.

    More generally speaking, I would greatly appreciate it if you left other people a bit more time to review merge requests before merging them in (say, a day or two, we could agree on a specific duration at the next meeting). Careful collective code review is one of the strongest points of the merge request-based git workflow, and it would be sad if we lost this...

    Edited by Hadrien Benjamin Grasland
  • @hgraslan agreed, and I will do better from now on :-)

  • Moritz Kiehn mentioned in merge request !48 (merged)

    mentioned in merge request !48 (merged)

  • Moritz Kiehn resolved all discussions

    resolved all discussions

  • Author Maintainer

    I closed the discussions on this merge request. Please leave further comments here !48 (merged).

  • Please register or sign in to reply
    Loading