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

[RF][CMake] Some fixes to RooFit CMake files #15664

Merged
merged 2 commits into from
May 29, 2024

Conversation

guitargeek
Copy link
Contributor

@guitargeek guitargeek commented May 28, 2024

See the commit descriptions.

Should be also backported to 6.32.

Some of the changes are also part of #8709.

The targets are using the headers in `res` themselves when compiling:
it's not only an interface dependency when compiling against the target.

It worked before, but like this it's more precise and the configuration
is more robust. Will help in particular when building RooFit standalone.
Add some missing dependencies for test executables.
Copy link

Test Results

    12 files      12 suites   2d 16h 57m 18s ⏱️
 2 642 tests  2 642 ✅ 0 💤 0 ❌
29 977 runs  29 977 ✅ 0 💤 0 ❌

Results for commit 8f75740.

@guitargeek guitargeek merged commit 4215ae1 into root-project:master May 29, 2024
17 of 18 checks passed
@guitargeek guitargeek deleted the roofit_fixups branch May 29, 2024 11:03
Copy link

💚 All backports created successfully

Status Branch Result
v6-32-00-patches

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

1 similar comment
Copy link

💚 All backports created successfully

Status Branch Result
v6-32-00-patches

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@@ -16,7 +16,7 @@ if(cuda)
target_compile_definitions(RooBatchCompute PUBLIC ROOFIT_CUDA)
endif()

target_include_directories(RooBatchCompute INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/res>)
target_include_directories(RooBatchCompute PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/res>)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't it PRIVATE ?

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