Allow TPLs to be listed in LIB_[REQUIRED|OPTIONAL]_PACKAGES (#63) #583
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
This required just a minor change in the function tribits_is_pkg_defined() and removed the argument pkgsOrTpls because it does not matter. (This will fix the SEACAS case described in sandialabs/seacas#377.) I changed the Dependencies.cmake files in the TriBITS projects TribitsExampleProject, TribitsExampleProject2, and ReducedMockTrilinos to use the XXX_PACKAGES lists and not the XXX_TPLS list but I left the Dependencies.cmake in the larger MockTrilinos project alone to make sure we maintain backward compatibility (even if these are not the best examples for now). But I did updaste the code to actually append the XXX_TPLS lists to the XXX_PACKAGES lists internally so the testing of this should be pretty solid. (I should likely change all of the example projects to use only the XXX_PACKAGES list to be a good example.) See the new entry in the `CHANGLOG.md` file and updated documentation in this commit for details. NOTE: I was also able to remove the howto section "how to add a new tribits external package/tpl dependency" since there is no difference between adding a dependency on an existing defined upstream internal or external package. NOTE: This changed the order of internal vs. external packages in the `<Package>_[TEST|LIB]_DEFINED_DEPENDENCIES` lists which impacted the tests but does not otherwise impact behavior (because the order if packages in these lists does not matter).
|
@gsjaardema, this should fix the issue you reported in https://github.com/sandialabs/seacas/pull/377/files/75807545278f32397bbf6e4d0a1119f165a8a3a0#r1182980716. If you have a few minutes, would you mind doing a review of the update documentation link to above under Instructions for reviewers? |
|
@KyleFromKitware, if you have a few minutes, would you mind doing a review? See Instructions for reviewers above. |
KyleFromKitware
requested changes
Jun 6, 2023
| will refer to the downstream package as ``<packageName>`` with base directory | ||
| ``<packageDir>`` and will refer to the upstream package as | ||
| ``<upstreamPackageName>``. | ||
| existing `downstream`_ package to an existing `upstream`_ (intenral or |
There was a problem hiding this comment.
Thanks! This is fixed in the new commit 2dd9509. @KyleFromKitware, can you please update your review and if this looks good, approve the PR so it can merge (I have set up auto-merge).
KyleFromKitware
approved these changes
Jun 7, 2023
bartlettroscoe
added a commit
to bartlettroscoe/Trilinos
that referenced
this pull request
Jul 7, 2023
…ribits-update This pulls in the new TriBITS PRs: * TriBITSPub/TriBITS#583 * TriBITSPub/TriBITS#585 * TriBITSPub/TriBITS#586 * TriBITSPub/TriBITS#587 * TriBITSPub/TriBITS#588 * TriBITSPub/TriBITS#590
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.
Description
See the commit log and updated documentation for details.
For example, this should fix the SEACAS case described in sandialabs/seacas#377 (comment). I.e. this will eliminate the need for some of the logic in:
https://github.com/sandialabs/seacas/blob/master/packages/seacas/libraries/ioss/cmake/Dependencies.cmake
Instructions for reviewers
tribits_is_pkg_defined()tribits_package_define_dependencies()(see changes here)