Allow specifying FIND_SHARED_LIBS per-TPL #624
Conversation
|
@bartlettroscoe no particular rush on this, but I'd appreciate a review whenever you have the time. |
There was a problem hiding this comment.
@sebrowne, thanks for the PR. This is a very reasonable extension. And thanks for adding the test case! (If you revert the change to the *.cmake file, does the test fail? Hopefully yes but it is worth checking.)
Just a coupe of things should be addressed before merging:
- The logic for how the var
${TPL_NAME}_FIND_SHARED_LIBSis set should be tweaked (see below) - This needs to be documented somehow. I might suggest adding a mention of this variable to the section Building static libraries and executables? (Or it could be a new subsection?)
tribits/core/package_arch/TribitsTplFindIncludeDirsAndLibraries.cmake
Outdated
Show resolved
Hide resolved
Signed-off-by: Samuel E. Browne <[email protected]>
Signed-off-by: Samuel E. Browne <[email protected]>
|
@bartlettroscoe applied your suggestion for 1., and added a blurb of documentation for 2. |
|
FYI: The GHA checks are fixed in the PR #626. I will pull those checks off into their own PR and merge it so that it will allow this and other tests to pass. |
Waiting is a-okay with me, no rush. I'll start using it once it's in, but I can manage perfectly fine until then. |
Currently we (Trilinos) wish to be able to find all TPLs statically...except for some which we must find shared, because static installations do not exist in the environment we're using. For example, we want to use "all" static TPLs for our CUDA build, but the installation of METIS is only shared. Currently, this is done by setting
TPL_FIND_SHARED_LIBStoOFF, and specifyingMETIS_LIBRARIESto be the explicit full path tolibmetis.so. This unfortunately bypasses the 'finding' of METIS. It would be nicer if we could pass-DTPL_FIND_SHARED_LIBS=OFF -DMETIS_FIND_SHARED_LIBS=ONand have TriBITS continue to do the same thing for each TPL, by finding METIS as normal (eliminates some brittleness in our configuration management).I looked at doing this, and the code change seems fairly trivial unless I'm missing something. I added a test as well.