-
Notifications
You must be signed in to change notification settings - Fork 570
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
TriBITS: Switch to modern cmake targets (TriBITSPub/TriBITS#299) #10614
TriBITS: Switch to modern cmake targets (TriBITSPub/TriBITS#299) #10614
Conversation
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git' At commit: commit 35d82aaa31fe81ca30a619320bb71fe481e9d4c7 Author: Roscoe A. Bartlett <[email protected]> Date: Tue Dec 14 15:48:02 2021 -0700 Summary: Interpret a raw identifer in TPL_<tplName>_LIBRARIES as a lib name (trilinos#299, trilinos#433)
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git' At commit: commit b2f309b5f8422c737c2f06542582986a7a833beb Author: Roscoe A. Bartlett <[email protected]> Date: Wed Jan 19 10:51:16 2022 -0700 Summary: Add back deprecated non-namespaced library targets (trilinos#299)
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git' Git describe: Vera4.0-RC1-start-1031-ga72c0d52 At commit: commit 10d02daafd114235864cd25c594a6cd93d13d39b Author: Roscoe A. Bartlett <[email protected]> Date: Fri Mar 4 15:12:12 2022 -0700 Summary: Fix typos and other improvements (trilinos#443)
…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.
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git' Git describe: Vera4.0-RC1-start-1173-g332f7a44 At commit: commit e82c4544fbd50769b85bab45cb98ef009ca4643c Author: Roscoe A. Bartlett <[email protected]> Date: Fri Jun 10 11:08:38 2022 -0600 Summary: Remove internal quotes for git log --pretty=format:<fmnt> strings (trilinos#485)
…targets-1-again-2 I resolved merge conflicts in the files: * cmake/tribits/common_tpls/FindTPLNetcdf.cmake * cmake/tribits/doc/build_ref/TribitsBuildReferenceBody.rst by going with what is in tribits_github_snapshot. The changes were identical and really just duplicates (except for fixing some typos).
…ty with -isystem include dirs Hopefully this will be enough for Trilinos users to work through problems with changes in the handling of include directories. For more details, see TriBITSPub/TriBITS#443
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_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
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_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
|
Status Flag 'Pull Request AutoTester' - User Requested Retest - Label AT: RETEST will be reset after testing. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
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_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
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_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
|
CC: @keitat FYI, it looks like there are mass PR testing infrastructure failures that are blocking this PR for passing the builds to allow it to merge (see #3276 (comment)). |
FYI: Someone from EMPIRE may want to test this branch before it gets merged. (I no longer have access to the EMPIRE repos to test this myself.) NOTE: I had to address some issues with the SPARC CMake build system to accommodate the switch to modern CMake and I would not be surprised if EMPIRE had similar issues. To see how I fixed SPARC, see this MR. In case you can't see that MR, you just have to link against |
@keitat, you might want to test this PR branch against xSDK to see if there are any issues there. |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
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... |
@jjellio, we can't do this with CMake. Converting from a link line for a TPL into IMPORTED targets is pretty limited with CMake. See: Anything more than this would require extensions to CMake (and then upwards of a year or more before we could use it due to impediments upgrading CMake in TriBITS and Trilinos, see #10355). Defining TPL dependencies as per:
would seem to be the cleanest way to address this issue given current CMake features. All things considered, we just need to correctly define our dependencies. A lot of good things come from that. |
@keitat, @rppawlo, @jwillenbring, @ccober6 This PR is ready to merge. Please see the list of Tasks above for the testing that was done and the issues that have been resolved since this PR was first posted over a month ago. Also, see the section Notes to Reviewers above in case anyone wants to review this PR. (Again, don't bother reviewing all of the changes to TriBITS.) We need to merge this soon as the PR builds just passed early this morning and we don't want them to go stale. If we find problems with TriBITS, I would rather we fix them ASAP instead of backing out this merge. I don't think we will see too many issues going forward (but we may need to help some APP customers to fix their build systems like I did for SPARC). |
I sent the following email follow up to the one I sent back on June 14 above. From: Bartlett, Roscoe A [email protected] Hello Trilinos users and developers, After a good bit of testing by Trilinos customers and resolving a few issues that came up, we are finally ready to merge this major upgrade to the Trilinos CMake build system in the PR: I believe that all of the issues that were raised by customers have been resolved. (See the list of testing and issues resolved in the Tasks section at the top of the GitHub PR.) Please let me know if you have any lingering concerns ASAP. -Ross |
Here is a follow-up email: From: Bartlett, Roscoe A Hello Trilinos developers, A question was asked as to how this change impacts Trilinos developers. Here is the response … The only major break of backward compatibility for the Trilinos CMake build system itself is needing to define dependencies between TPLs as per: and needing call TriBITS functions to create the ::all_libs target and generate the Config.cmake wrapper files. But I have already updated those for all of the TPLs that I could find working configure and testing processes for. (But there are many other TPLs Other than that, Trilinos developers should notice almost nothing. There were just a few tweaks (backwards compatible with old TriBITS) that had to be made to Trilinos itself:
(I tried to minimize breaks in backward compatibility as much as possible initially.) For all of the changes (most that don’t directly impact a project’s CMake system), see: After the epic: is complete, we will start refactors that will break more backwards compatibility to reduce the size of TriBITS and conform and take better advantage of the features of newer versions of CMake. Please let me know if you have any other questions, -Ross |
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ rppawlo ]! |
Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged |
Merge on Pull Request# 10614: IS A SUCCESS - Pull Request successfully merged |
I just sent the following email. I will not work ASAP to reproduce and fix these failures one at a time. From: Bartlett, Roscoe A Hello Trilinos users and developers, FYI: The updated version of the Trilinos CMake build system in: was merged yesterday. Today we see a few new nightly testing build errors on some machines reported and being worked in: If you suspect there are problem caused from this upgrade of the CMake build system, please add a comment to: and will go from there. Cheers, -Ross |
This brings in a version of TriBITS 'master' that has switched to produce and use modern CMake targets (see TriBITSPub/TriBITS#229). This will open the door to many positive future refactorings and use cases (see TriBITSPub/TriBITS#367).
This PR reverts the revert commit from PR #9977 and therefore reapplies the merge of PR #9894. This PR replaces PR #9978. This PR, therefore, represents about 9 months worth of work on TriBITS 'master'!
This PR branch addresses the issues reported in #9972 and #9973 and listed in TriBITSPub/TriBITS#433 as well as many other issues found during testing with Trilinos and selected APPs.
In addition to reverting the revert commit from PR #9977, this also pulls in the additional TriBITS PRs going back 6 months or so.
This PR also addresses issues with several application codes.
NOTE: This PR breaks backward compatibility of the usage of
<Package>Config.cmake
files from Trilinos. This will require some downstream customers to change their CMake build systems to adapt (SeeRELEASE_NOTED
for Trilinos 14.0 here) This did not require any changes to Albany or Nalu but it did require changes to SPARC. This can not be helped since it is not possible to maintain perfect backward compatibility and switch to modern CMake (but we have done a pretty good job avoiding as many breaks in backward compatibility as possible).NOTE: This should not break any existing configure script for Trilinos. The external dependencies are are specified in exactly the same way and external dependencies are found in exactly the same way. (This was no small feat.)
More details can be provided on request.
Notes to Reviewers
cmake/tribits/
directory.Tasks:
*.framework
and*.tbd
) and update TriBITS snapshot into this branch ... Addressed in A few more fixes and updates to TriBITS for transition to modern CMake (#299) TriBITSPub/TriBITS#492Trilinos/cmake/TPLs/FindTPL<tplName>.cmake
files (see below)<tplName>_LIB_ENABLED_DEPENDENCIES
and enabled TPLs ... See Improve management of TPL dependencies with new TriBITS (#299, #63) TriBITSPub/TriBITS#494 and Add support for FindTPL<tplName>Dependencies.cmake and doc updates (#63, #299, #494) TriBITSPub/TriBITS#495find_package(Trilinos ...)
discovered during Xyce testing ... Fixed in Fix <Project>Config.cmake for missing OPTIONAL_COMPONENTS (#299) TriBITSPub/TriBITS#497How was this tested?
I did a few Trilinos builds and I tested against Albany, Nalu, and SPARC. SPARC required small changes to its build systems to accommodate the loss of
Trilinos_TPL_INCLUDE_DIRS
.