Skip to content

[cmake] Allow to populate ROOT using FetchContent #15105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

flagarde
Copy link

@flagarde flagarde commented Apr 2, 2024

This Pull request:

Allow to populate ROOT using FetchContent. Only compilation has been tested and further work should be done to allow to used the fetched ROOT.

snippet:

cmake_minimum_required(VERSION 3.16)

project(test)

set(FETCHCONTENT_QUIET false)
include(FetchContent)
FetchContent_Declare(MyRoot GIT_REPOSITORY https://github.com/flagarde/root.git GIT_TAG FetchContent)
#FetchContent_MakeAvailable(MyRoot)
# Check if population has already been performed
FetchContent_GetProperties(MyRoot)
if(NOT myroot_POPULATED)
  # Fetch the content using previously declared details
  FetchContent_Populate(MyRoot)

  # Set custom variables, policies, etc.
  # ...

  # Bring the populated content into the build
  add_subdirectory(${myroot_SOURCE_DIR} ${myroot_BINARY_DIR})
endif()

Changes or fixes:

Mainly some CMake variables.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes :

It's a rework on #9516 . This PR should not change anything for users compiling ROOT as standalone project. However until now, it is not possible to directly include ROOT in an other project using FetchContent. This PR is focusing on the CMake configuration, generation steps and in the compilation process. No work has been done yet to let upstream project to find ROOT and to use it. I made a try on the last point but I think it is better to split the task in more steps.

Usage of FetchContent is more and more used to create "standalone" project downloading and compiling all its dependencies. In the other PR tries some people and official maintainers have raised their interest on this feature.

@phsft-bot
Copy link

Can one of the admins verify this patch?

@dpiparo
Copy link
Member

dpiparo commented Apr 2, 2024

Thanks for this code. Could you please add in the description of the PR why you are doing this, and especially what problem you are solving? A good way to tackle this would be to try to convince the reader that thanks to the code, ROOT will be like before (e.g. nothing breaks for the experiments, users, in the development process of ROOT iteself) but better (e.g. there is a reduction in the cost of maintenance, development, testing, or an increase of runtime performance).

@flagarde
Copy link
Author

flagarde commented Apr 3, 2024

Sure,I tried to explain a bit better this PR

@dpiparo
Copy link
Member

dpiparo commented Apr 3, 2024

Thanks. Could you point the reader to projects in need of this upgrade in the PR description?

@flagarde
Copy link
Author

flagarde commented Apr 7, 2024

@dpiparo Well, we had some programs we used internally in my university group and I did some work to allow to FetchContent ROOT directly for this project. Then when I deleted my own ROOT repo and the last PR disappered people told me they could be still interested (#9516). This is the reason I submited a PR again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants