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

cmake: select static dependency targets in static link #529

Closed
wants to merge 1 commit into from

Conversation

adammoody
Copy link
Contributor

@adammoody adammoody commented Jan 20, 2023

When trying a full static build and using the new cmake package config files to specify dependencies, we hit problems where some targets are looking for non-static targets of dependencies (which are not installed when the components are build static-only). This is closely related to the problem reported in: #513

One option is to support two link options, where we have the static-only link specify the static targets of all dependencies. So if we only build libscr.a (not libscr.so) we link libscr.a to the .a builds of all components. If we build libscr.so, then we link everything to .so, including linking libscr.a to the .so versions of all components. That is the approach this PR takes. Similar changes need to be done for all components if we opt for this route. I have those ready.

As a second option, we could change this so that scr::scr points at the shared objects for all dependencies and have scr::scr-static point to the -static versions of all dependencies.

As yet a third option, the article linked below suggests a way in which we'd only provide a single scr::scr target and then to make our scrConfig.cmake script smart enough to have that target point at either libscr.so or libscr.a depending on how the user has set BUILD_SHARED_LIBS:

https://alexreinking.com/blog/building-a-dual-shared-and-static-library-with-cmake.html

That's nice since the end user only needs to link to scr::scr but they can flip between shared and static versions of libscr by setting BUILD_SHARED_LIBS during their application build.

@adammoody
Copy link
Contributor Author

adammoody commented Jan 24, 2023

@white238 and @mcfadden8 , last week I merged in support for an scrConfig.cmake file that gets installed with SCR and has references to SCR dependencies. This should enable a CMake application to just list scr::scr.

#514

However, now I have a question about shared vs static builds, which is what this PR is about. The newly added scrConfig.cmake file defines two targets, scr::scr for libscr.so and scr::scr-static for libscr.a.

But things are not well defined in all cases. In particular, it's not clear whether we should have libscr.a depend on static or shared targets for its own dependencies. Also, usability might be improved if we just define a single scr::scr target by adding more logic to our scrConfig.cmake script as described in the above article.

Do you either of you know if LLNL teams or the wider CMake community have settled on some standard practice out there for this?

@mcfadden8
Copy link
Collaborator

I've been thinking about this and have asked around a little. I really don't have a great answer to offer other than I think that there should be two entirely different build/installation directories for shared and static libraries with XXXConfig.cmake files that are different depending upon what kind (static or dynamic) of library is being built

@white238
Copy link
Member

white238 commented Jan 25, 2023

Most projects don't bother to build both static and shared libraries in the same configuration. Some do though. It is really a matter of preference. What you have is fine. Some people will want static, some will want shared, but most won't care. As long as it's documented that's fine. If you are going to build both, it is nice to have a way to grab the static vs shared library.

As a point of reference, HDF5 provides the following targets: hdf5::hdf5 (defaults to shared if both is there and you don't toggle it with a variable but will be static if that is just what is there), hdf5::hdf5-static, and hdf5::hdf5-shared

@adammoody
Copy link
Contributor Author

Replaced with: #536. I settled on the second option based on above feedback. Thanks all.

@adammoody adammoody closed this May 10, 2023
@adammoody adammoody deleted the cmake-config-static branch May 10, 2023 22:02
@mcfadden8
Copy link
Collaborator

Thanks @adammoody!

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