-
Notifications
You must be signed in to change notification settings - Fork 46
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
Remove unused/unneeded list vars and logic for packages and TPLs (#299, #63) #531
Conversation
Unfortunately, I could not remove the computation of the full set of direct and indirect dependencies for each pacakge <Package>_FULL_ENABLED_DEP_PACKAGES because the function tribits_find_most_recent_file_timestamp() still needs this. The only customer that still needs that function is CASL VERA (to compute when it is needed to rebuild MOOSE). Therefore, for now, I can't remove this code (which is more complex and can be an expensive computation if there are a **lot** of packages).
#63) This commit gets rid of several list variables that are not needed with the move to modern CMake and some fairly complex code that was needed to compute these list variables: * <Project>_REVERSE_DEFINED_INTERNAL_PACKAGES * <Project>_REVERSE_DEFINED_TPLS * <Project>_TPL_LIST * <Package>_PACKAGE_LIST * <Package>_TPL_LIST * <Package>_INCLUDE_DIR * <Package>_LIBRARY_DIRS * <Package>_TPL_INCLUDE_DIRS * <Package>_TPL_LIBRARIES * <Package>_TPL_LIBRARY_DIRS Some of these changes impact only internal code but some impact even customers of the installed <Project>Config.cmake and <Package>Config.cmake files (see the new entries in tribits/CHANGELOG.md). This required some minor updates to the example projects: * tribits/examples/TribitsExampleApp/ * tribits/examples/TribitsOldSimpleExampleApp/ * tribits/examples/TribitsSimpleExampleApp/ for some logic that used <Project>_TPL_LIST. (There is no need to use that with updated TriBITS. See the diffs in this commit for those projects to see how they were adjusted.)
Hello @KyleFromKitware, can you please give this PR a review when you get some time? I will build my next branch off of this branch to continue work in the meantime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, requested a few changes.
tribits/core/package_arch/TribitsSetUpEnabledOnlyDependencies.cmake
Outdated
Show resolved
Hide resolved
tribits/core/package_arch/TribitsSetUpEnabledOnlyDependencies.cmake
Outdated
Show resolved
Hide resolved
tribits/core/package_arch/TribitsSetUpEnabledOnlyDependencies.cmake
Outdated
Show resolved
Hide resolved
tribits/core/package_arch/TribitsSetUpEnabledOnlyDependencies.cmake
Outdated
Show resolved
Hide resolved
tribits/core/package_arch/TribitsSetUpEnabledOnlyDependencies.cmake
Outdated
Show resolved
Hide resolved
tribits/core/package_arch/TribitsSetUpEnabledOnlyDependencies.cmake
Outdated
Show resolved
Hide resolved
Addresses review commits from PR #531.
Hello @KyleFromKitware, all of the issues you raised in your review above should be addressed in the new commit |
Description
This is a cleanup PR to allow the next step in working towards #63 (and removing some stuff not needed after the completion of #299).
See the individual commit messages for what was changed and why.
Instructions for Reviewers