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

Export targets and make sure they can be used by other projects #177

Merged
merged 6 commits into from
Nov 14, 2023

Conversation

jmcarcell
Copy link
Contributor

BEGINRELEASENOTES

  • Export targets and make sure they can be used by other CMake projects

ENDRELEASENOTES

This allows other programs to use the targets without having to rely on some variables that are set like LCIO_LIBRARIES and so on but it's backwards compatible (a clean up would be nice...). This will help with key4hep/k4geo#290 and iLCSoft/LCCD#7 that will have to be adapted based in this PR.

sio/CMakeLists.txt Outdated Show resolved Hide resolved
@jmcarcell
Copy link
Contributor Author

I made a few changes not to modify INSTALL_SHARED_LIBRARY since I realised after that this is used in several repos and added a SIOTargets file in case it's builtin so that it installs it like it does when using standalone SIO. I think this should be ready now and not interfere with other packages.

@jmcarcell
Copy link
Contributor Author

Any comments about this @andresailer @tmadlener ? It's less changes than #140 that has been open since 2021 and probably will require more work

Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

For me this looks good. Just for my understanding: Effectively this PR makes the builtin SIO install an SIOTargets.cmake, which then makes it possible for LCIO to simply find it via find_package? Resp. it makes packages that depend on SIO (but maybe not LCIO) able to find SIO, even if it is builtin?

CMakeLists.txt Show resolved Hide resolved
src/cpp/CMakeLists.txt Outdated Show resolved Hide resolved
@andresailer
Copy link
Contributor

When changing key4hep/k4geo#298 to use LCIO::LCIO and building on top of this PR everything looks nice.

Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Also works in my local build of the stack.

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

Successfully merging this pull request may close these issues.

3 participants