Skip to content
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

Provide in the cmake configuration the C++ standard which was used to compile ROOT #7644

Closed
andriish opened this issue Mar 22, 2021 · 14 comments · Fixed by #14148
Closed

Provide in the cmake configuration the C++ standard which was used to compile ROOT #7644

andriish opened this issue Mar 22, 2021 · 14 comments · Fixed by #14148
Assignees

Comments

@andriish
Copy link
Contributor

Hi all,

It makes sense to compile software with ROOT using exactly the same C++ standard.
However, there is no nice way to obtain this information from ROOT cmake config.
Everything that is provided is a list of C++ flags.

Describe the solution you'd like

Have a CMake variable ROOT_CXX_STANDARD in ROOT cmake config.

Describe alternatives you've considered

Parsing the ROOT_CXX_FLAGS and hoping for the best.

Best regards,

Andrii

@amadio
Copy link
Member

amadio commented May 4, 2021

The better solution would be to improve ROOTConfig.cmake such that a given standard can be required when calling find_package, like find_package(ROOT REQUIRED cxx17). That way, the extra steps of checking the variable are not needed, and ROOT won't be considered found if it uses C++11, for example (in which case if you have multiple versions installed, CMake might actually search until it finds the right one).

@andriish
Copy link
Contributor Author

andriish commented May 4, 2021

Hi @amadio ,

that is an "active requirement". What I actually needed at some point is a "passive" information, an answer to a quaestion "What standard was used?". So these are similar, but yet different things.

Best regards,

Andrii

@andriish
Copy link
Contributor Author

andriish commented May 6, 2021

Hi @amadio ,

just found another code where people would benefit from this feature

https://github.com/acts-project/acts/blob/main/CMakeLists.txt line 104

Best regards,

Andrii

@eguiraud
Copy link
Member

+1. Since ROOT requires that downstream projects be compiled with the same C++ standard that was used for ROOT, ROOT should expose that standard as ROOT_CXX_STANDARD.

Many projects have to extract that information by regex-matching, e.g. https://gitlab.cern.ch/hepmc/HepMC3/-/blob/master/CMakeLists.txt#L153-171 .

@amadio
Copy link
Member

amadio commented Sep 21, 2023

ROOT's targets already export the C++ standard they were built with, so if you just link your project with ROOT using those targets, you already get the same C++ standard used in your code. Worst case, you can call get_property on a target to check this. However, exporting this in a variable as well can make it easier for users indeed, as calling get_property is a bit cumbersome.

@eguiraud
Copy link
Member

if you just link your project with ROOT using those targets

in reality most projects set their CMAKE_CXX_STANDARD which trumps the target's property.

you can call get_property on a target to check this

ah, that's already way better than regex-matching, thanks!

exporting this in a variable as well can make it easier for users indeed

👍

@dpiparo
Copy link
Member

dpiparo commented Nov 30, 2023

hi @bellenot : would it be hard to expose the standard in a potential new ROOT_CXX_STANDARD variable in your opinion?

@bellenot
Copy link
Member

hi @bellenot : would it be hard to expose the standard in a potential new ROOT_CXX_STANDARD variable in your opinion?

No, but that requires quite some changes, also for the projects using ROOT. I'll investigate (I have plenty of changes for CMake on Windows anyway)

@amadio
Copy link
Member

amadio commented Nov 30, 2023

@bellenot If I understand correctly, what @dpiparo is suggesting is a simple

set(ROOT_CXX_STANDARD @CMAKE_CXX_STANDARD@)

somewhere in ROOTConfig.cmake.in.

@bellenot
Copy link
Member

@amadio Oh! You're right, sorry, I misunderstood the requirement... Will make a PR now. Thanks!

@eguiraud
Copy link
Member

Happy to see this moving forward!

also for the projects using ROOT

this should be sufficiently backwards-compatible. unless a downstream project was somehow setting and using a variable called like that already they should see no change.

(P.S. don't forget to update https://root.cern/manual/integrate_root_into_my_cmake_project )

@bellenot
Copy link
Member

(P.S. don't forget to update https://root.cern/manual/integrate_root_into_my_cmake_project )

Right, thanks Enrico!

@bellenot
Copy link
Member

@eguiraud Here is the PR for the doc: root-project/web#938

Copy link

github-actions bot commented Dec 1, 2023

Hi @bellenot,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@bellenot bellenot added this to Issues in Fixed in 6.32.00 via automation Dec 1, 2023
maksgraczyk pushed a commit to maksgraczyk/root that referenced this issue Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants