TriBITS: Pull in partial refactoring to modern CMake targets (TriBITSPub/TriBITS#299) #9894
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
…riBITS#299) The updated version of TriBITS using modern CMake for TriBITSPub/TriBITS#299 creates a ${PACKAGE_NAME}_all_libs library target (used to create an ALIAS ${PACKAGE_NAME}::all_libs library target) from all of the targets under a package's base CMakeLists.txt file that don't have the TRIBITS_TESTONLY_LIB property set to TRUE. The tribits_add_library( ... TESTONLY ... ) function call automatically sets this property but if a package uses raw CMake to create targets (like Sacado is doing here with GoogleTest), then one must set the TRIBITS_TESTONLY_LIB property or one must create the ${PACKAGE_NAME}_all_libs and ${PACKAGE_NAME}::all_libs targets manually. Small price to pay on the path to making TriBITS more flexible and allowing raw CMake target formation.
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = git@github.com:TriBITSPub/TriBITS.git' At commit: commit 99d3869645d0083cb499389659f1394f0b5e6a89 Author: Roscoe A. Bartlett <rabartl@sandia.gov> Date: Mon Nov 1 09:48:44 2021 -0600 Summary: Call generate and include <tplName>Config.cmake files (trilinos#299)
…targets-1 I resolved the confict in: * cmake/tribits/core/package_arch/TribitsConstants.cmake by going with what comes from the TriBITS GitHub snapshot (with the lower-case set()).
bartlettroscoe
added
type: enhancement
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
|
Just a few notes for anyone that may want to do a detailed code review of this PR:
Thanks! |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: python-3
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: python-3
Jenkins Parameters
|
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
bartlettroscoe
added
AT: WIP
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = git@github.com:TriBITSPub/TriBITS.git' At commit: commit f8f2df0b5d884c9c2a1644ebf40cff25e515ff76 Author: Roscoe A. Bartlett <rabartl@sandia.gov> Date: Wed Nov 3 09:51:04 2021 -0600 Summary: Create tribits_external_package_process_libraries_list_library_entry(), elim dup (trilinos#299)
…targets-1 This should resolve the issue with duplicate libs reported in TriBITSPub/TriBITS#427 addressed in PR TriBITSPub/TriBITS#428.
|
FYI: Looks like there is an issue with duplicate entries in This is ready to review and merge again. |
bartlettroscoe
added
AT: AUTOMERGE
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: python-3
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
e10harvey
previously approved these changes
Nov 3, 2021
cmake/tribits/ReleaseNotes.txt
Outdated
| the build directory tree have been moved from | ||
| `<buildDir>/packages/<packageDir>/` to | ||
| `<buildDir>/cmake_packages/<PackageName>/`. This makes it easy for | ||
| `find_package(<PackageName>)` to fine these files by simply adding the |
There was a problem hiding this comment.
Suggested change
| `find_package(<PackageName>)` to fine these files by simply adding the | |
| `find_package(<PackageName>)` to find these files by simply adding the |
cmake/tribits/ReleaseNotes.txt
Outdated
| has been removed along with the cache variable | ||
| <Project>_ENABLE_EXPORT_MAKEFILES. This is to allow the refactoring of | ||
| TriBITS to use modern CMake targets that propagate all information and | ||
| removing complex dependency tracking inforamtion from TriBITS (see |
There was a problem hiding this comment.
Suggested change
| removing complex dependency tracking inforamtion from TriBITS (see | |
| removing complex dependency tracking information from TriBITS (see |
|
|
||
| if (NOT TARGET ${PACKAGE_NAME}::all_libs) | ||
| # Create ALIAS ${PACKAGE_NAME}::all_libs target | ||
| add_library(${PACKAGE_NAME}::all_libs ALIAS ${PACKAGE_NAME}_all_libs) |
There was a problem hiding this comment.
What is the role of this macro if ${PACKAGE_NAME}::all_libs is already a TARGET?
There was a problem hiding this comment.
Don't understand the question.
There was a problem hiding this comment.
What is the role of this macro if NOT TARGET ${PACKAGE_NAME}::all_libs evaluates to false?
There was a problem hiding this comment.
The idea is that a package may choose to define its own <Package>::all_libs target. An example of a streamlined project where packages define all of their own targets with raw CMake commands will be coming up. Also, this logic might be needed for the installation testing use case where the tests and examples are configured that pull in pre-installed libraries and header files through installed <Package>Config.cmake files that already have this target defined. (Not sure about that last part.)
cmake/tribits/examples/TribitsExampleProject/packages/wrap_external/CMakeLists.txt
Outdated
Show resolved
Hide resolved
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: python-3
Jenkins Parameters
Console Output (last 100 lines) : Trilinos_pullrequest_gcc_8.3.0 # 5927 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_gcc_7.2.0_serial # 3472 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_gcc_7.2.0_debug # 3990 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_intel_17.0.1 # 11151 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_cuda_10.1.105 # 2636 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_cuda_10.1.105_uvm_off # 1627 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_clang_10.0.0 # 3949 (click to expand)Console Output (last 100 lines) : python-3 # 585 (click to expand) |
|
Status Flag 'Pull Request AutoTester' - Failure: Timed out waiting for job Trilinos_pullrequest_cuda_weaver_uvm_off to start: Total Wait = 3603
|
trilinos-autotester
removed
the
AT: RETEST
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_cuda_weaver
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_weaver_uvm_off
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_cuda_weaver
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_weaver_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: python-3
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_cuda_weaver
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_weaver_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: python-3
Jenkins Parameters
|
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ e10harvey ]! |
|
Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged |
|
Merge on Pull Request# 9894: IS A SUCCESS - Pull Request successfully merged |
trilinos-autotester
removed
the
AT: AUTOMERGE
This was referenced Nov 29, 2021
bartlettroscoe
added a commit
to bartlettroscoe/Trilinos
that referenced
this pull request
Nov 29, 2021
…/tribits-299-modern-cmake-targets-1" This reverts commit db3205b, reversing changes made to 110b6c4 which reverts the PR merge trilinos#9894. This is to allow reproducing and addressing the problems described in the new issues trilinos#9972 and #trilinos#9973 offline to allow the Albany and Nalu Trilinos integration process, respectively, to continue working in the meantime.
bartlettroscoe
added a commit
to bartlettroscoe/Trilinos
that referenced
this pull request
Nov 29, 2021
…Trilinos/tribits-299-modern-cmake-targets-1"" This reverts commit fd27a20. This gets us back to the state of the 'develop' branch after the PR trilinos#9894 that merged the branch 'tribits-299-modern-cmake-targets-1' was merged (as well as other PRs in the days after that). Now I can try to reproduce the errors in issues trilinos#9972 and trilinos#9973.
8 tasks
trilinos-autotester
added a commit
that referenced
this pull request
Nov 30, 2021
…299-modern-cmake-targets-1 Automatically Merged using Trilinos Pull Request AutoTester PR Title: Revert: TriBITS: Pull in partial refactoring to modern CMake targets (TriBITSPub/TriBITS#299) (#9894, #9972, #9973) PR Author: bartlettroscoe
jmgate
pushed a commit
to tcad-charon/Trilinos
that referenced
this pull request
Dec 1, 2021
…s:develop' (e953a67). * trilinos-develop: Revert "Merge Pull Request trilinos#9894 from bartlettroscoe/Trilinos/tribits-299-modern-cmake-targets-1" Panzer: fix compiler warning, add safety check to q8/q4 converter Panzer: allow for mpi subcomm in Q8 to Q4 mesh factory NOX: update solver stats Panzer: add Quad8 to Quad4 mesh factory Tempus: Fix Failing Tests Due to FPE
jmgate
pushed a commit
to tcad-charon/Trilinos
that referenced
this pull request
Dec 1, 2021
…s:develop' (e953a67). * trilinos-develop: Revert "Merge Pull Request trilinos#9894 from bartlettroscoe/Trilinos/tribits-299-modern-cmake-targets-1" Panzer: fix compiler warning, add safety check to q8/q4 converter Panzer: allow for mpi subcomm in Q8 to Q4 mesh factory NOX: update solver stats Panzer: add Quad8 to Quad4 mesh factory Tempus: Fix Failing Tests Due to FPE
bartlettroscoe
added a commit
to TriBITSPub/TriBITS
that referenced
this pull request
Dec 15, 2021
…-linkflags Fix issues with <tplName>Config.cmake generation for TriBITGS TPLs from trilinos/Trilinos#9894 (#433)
bartlettroscoe
added a commit
to bartlettroscoe/Trilinos
that referenced
this pull request
Dec 15, 2021
…Trilinos/tribits-299-modern-cmake-targets-1"" This reverts commit fd27a20. This gets us back to the state of the 'develop' branch after the PR trilinos#9894 that merged the branch 'tribits-299-modern-cmake-targets-1' was merged (as well as other PRs in the days after that). Now I can try to reproduce the errors in issues trilinos#9972 and trilinos#9973.
bartlettroscoe
added a commit
to bartlettroscoe/Trilinos
that referenced
this pull request
Dec 15, 2021
…targets-1-again (TriBITSPub/TriBITS#433) Should address all of the issues with the merge of PR trilinos#9894 listed out in TriBITSPub/TriBITS#433 (which is part of TriBITSPub/TriBITS#299). This should resolve the failures reported in trilinos#9972 and trilinos#9973.
PhilMiller
pushed a commit
to PhilMiller/Trilinos
that referenced
this pull request
Dec 22, 2021
…Trilinos:develop' (8f92205). * trilinos/develop: (35 commits) Piro: some fixes / additions relevant to Schwarz coupling in Albany-LCM. Tpetra: Fixed two errors in Tpetra::FECrsMatrix test (trilinos#9965) Revert "Merge Pull Request trilinos#9894 from bartlettroscoe/Trilinos/tribits-299-modern-cmake-targets-1" Panzer: fix compiler warning, add safety check to q8/q4 converter Panzer: allow for mpi subcomm in Q8 to Q4 mesh factory change calls for sems-[TPLname] to sems-archive-[TPLname]. Automatic snapshot commit from tribits at 1241168 NOX: update solver stats Tpetra: remove unnecessary broadcast Piro: correcting logic so code does not attempt to construct adjoint model when adjoint model passed in is null. Xpetra IO: More fixes for binary reading Panzer: add Quad8 to Quad4 mesh factory Ifpack2: rebaselining long double test. Something in Trilinos changes that changed slightly the matrices / vectors defined in this problem. Verified that precision of result given new matrix/vector is correct. Tempus: Fix Failing Tests Due to FPE Xpetra: fix bug in binary reader Tpetra: use exact nnz per row in matrix reader removed outdated information MueLu ML translation: Translate Hiptmair MueLu RefMaxwell: Use smoother factory for Hiptmair Ifpack2 Hiptmair: add subsmoothers to Hiptmair description ...
7 tasks
bartlettroscoe
added a commit
to bartlettroscoe/Trilinos
that referenced
this pull request
Jun 10, 2022
…Trilinos/tribits-299-modern-cmake-targets-1"" This reverts commit fd27a20. This gets us back to the state of the 'develop' branch after the PR trilinos#9894 that merged the branch 'tribits-299-modern-cmake-targets-1' was merged (as well as other PRs in the days after that). Now I can try to reproduce the errors in issues trilinos#9972 and trilinos#9973.
16 tasks
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.
Relates to TriBITSPub/TriBITS#299 and TriBITSPub/TriBITS#367.
This PR pulls in a partial refactoring of TriBITS to adopt modern CMake targets. This pulls in the TriBITS PRs (listed most recent to least recent):
As of this PR, TriBITS now creates all of the needed
<Package>Config.cmakeand<tplName>Config.cmakefiles in the build and install trees that have all of the needed basic modern CMake targets (TriBITSPub/TriBITS#299).What is missing is all of the proper
target_link_libraries()connections between targets and to refactor the gut of TriBITS to use all of these targets correctly and then strip out all of old complex TriBITS logic that manually deals with libraries and include directories.NOTE: I had to make one small tweak to a Sacado CMakeLists.txt file in commit b6837af (see commit log for detail). That is non-standard TriBITS code but the fact it was so easy to address gives me hope that this is right path for TriBITS to strip out the usage of variables and allow packages to use modern CMake targets if they like.
One reason for merging this partial refactoring is to see if users or developers run into any problems with the generation of these
<tplName>Config.cmakefiles as early as we can. As long as only full library files and-l<lib>and-L<dir>arguments are used withTPL_<tplName>_LIBRARIES, then there should not be any problem. But any extra special link flags will result in a configure error as described in TriBITSPub/TriBITS#299 (comment).Another reason is that we will be able to refactor the example
simpleBuildAgainstTrilinosto use these modern targets and rip out the usage of include and library variables (which I will do in a follow-on Trilinos PR).How was this tested?
I tested this with a SEMS and an ATDM Trilinos configuration.
(10/29/2021)
I tested the TriBITS branch '299-modern-cmake-targets-2' against Trilinos 'develop' with the SEMS GCC 7.2.0 env using the files:
load-env.sh
do-configure
On 'crf450' I ran:
I got some timeouts initially and a failure:
I got a single failing test after rerunning the failed tests:
The failure shows:
As shown in this query:
this test is already failing in a bunch of the ATDM Trilinos builds.
I just had to make a single patch to Trilinos with the commit:
that I just pushed to my Trilinos branch 'tribits-299-modern-cmake-targets-1'.
(11/1/2021)
Testing with an ATDM Trilinos configuration that has
-L<dir>and-l<lib>. Seems that the ATDM Trilinos cee-rhel7 configuration has this with several TPLs like BLAS, LAPACK and Netcdf.Testing on 'ceerws1113' using the files:
load-env.sh:do-configure.base:and
do-configure:Running:
That is excellent.
We already know about the failing test
Xpetra_MultiVector_UnitTests_MPI_4as per above.I looked at some of the generated
<tplName>Config.cmakefiles and I am seeing mixes of IMPORTED library types like forHDF5Config.cmake:(11/19/2021)
Charon2 inadvertently tested this from the TriBITS 'master' branch and recorded an issued in TriBITSPub/TriBITS#427. That issue was resolved by PR TriBITSPub/TriBITS#428 and TriBITSPub/TriBITS#431 which are part of this updated TriBITS snapshot.