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

root7 is turned off by default even if the default C++ standard of the compiler is C++14 or above #6406

Closed
eguiraud opened this issue Sep 22, 2020 · 1 comment · Fixed by #6407

Comments

@eguiraud
Copy link
Member

Describe the bug

With ROOT-10692 fixed, ROOT now, by default, uses the default C++ standard of the compiler rather than always using C++11. However, due to how our cmake logic is structured, root7 is still turned off by default, even if the default C++ standard used by the compiler was detected to be C++14 or higher.

Expected behavior

With a compiler that defaults to -std=C++14 or above, a vanilla cmake path/to/root should have root7 turned on.

Additional context

I think the root cause is that, at the following lines in our main CMakeLists.txt, we first include RootBuildOptions (which sets root7 to OFF by default because it doesn't detect a high-enough C++ standard) and then we include CheckCompiler, which sets our default CMAKE_CXX_STANDARD to the compiler default.

root/CMakeLists.txt

Lines 128 to 134 in 33458dc

#---Load some basic macros which are needed later for the confiuration and build----------------
include(SearchRootCoreDeps)
include(RootBuildOptions)
include(RootMacros)
include(CheckCompiler)
include(CheckAssembler)
include(CheckIntrinsics)

Moving include(CheckCompiler) above include(RootBuildOptions) fixes this issue but breaks Windows, because some cmake variable that CheckCompiler needs in the case of windows were defined earlier.

@eguiraud
Copy link
Member Author

Looking into it a bit more, I think the right fix is indeed to move CheckCompiler above RootBuildOptions, and then fix the problem with Windows.

That problem is a circular dependency:

  1. RootBuildOptions.cmake depends on CheckCompiler.cmake, because e.g. the default value of the root7 option (set in RootBuildOptions) depends on the value of CMAKE_CXX_STANDARD (set in CheckCompiler)
  2. however, CheckCompiler.cmake includes SetUpWindows.cmake, which uses CMAKE_INSTALL_INCLUDEDIR in a couple of install commands. CMAKE_INSTALL_INCLUDEDIR is set by RootInstallDirs which is included by RootBuildOptions, completing the circle

@amadio suggested moving the install commands Windows needs from SetUpWindows to RootConfiguration.

eguiraud added a commit to eguiraud/root that referenced this issue Sep 22, 2020
Otherwise features like root7 that depend on compiler support are turned
off by default even if, later, we add the required compilation flags.
This fixes root-project#6406.
@eguiraud eguiraud added this to the 6.24/00 milestone Sep 23, 2020
Triage automation moved this from Rest to Closed Sep 24, 2020
eguiraud added a commit that referenced this issue Sep 24, 2020
Otherwise features like root7 that depend on compiler support are turned
off by default even if, later, we add the required compilation flags.
This fixes #6406.
@eguiraud eguiraud added this to In this branch in Fixed in 6.24/00 via automation Oct 12, 2020
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.

1 participant