Fix find_package() from separate subdirs for <Package>Config.cmake files (#505) #506
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TriBITSPub#505) This fixes the test added in the last commit for calling find_package() for related packages from different subdirs that was failing. I belive the problem is that the targets created by the CMake export() command don't have GLOBAL on them and only have directory scope. Therefore, we can't use include_guard(GLOBAL) because that will stop the creation of the imported targets in a different subdir where they are not defined.
…Tpl>::all_libs) (TriBITSPub#505) This change makes the IMPORTED targets produced by TriBITS for external packages/TPLs equivalent to those created by CMake export() in that they are **not** global but are just local to the directory where they are called. But to get ahead of the scalability problem, I added the guard: if (NOT TARGET <upstreamTpl>::all_libs) ... endif() around the find_dependency(<upstreamTpl> ...) calls.
rabartlett1972
approved these changes
Jul 28, 2022
There was a problem hiding this comment.
I will approve without an initial review so this can be updated to SEACAS and address but #505.
Then we can do a post-merge review.
|
Hello @KyleFromKitware, can you please do a post-merge review of this PR? Thanks! |
|
LGTM +1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should fix the use case described in #505. I produced a TriBITS test that exposed the problem and updated the code to fix it. I think what is here now works as find_package() should from different subdirs for dependent packages but still avoid the scalability problem described in https://gitlab.kitware.com/cmake/cmake/-/issues/23685. This also updates the
<tplName>Config.cmakefiles to act in a consistent way with CMake-generated imported targets (usingexport()command).See the individual commit log messages for details.