-
Notifications
You must be signed in to change notification settings - Fork 45
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
Update CMake min version from 3.17 to 3.23 (#411) #565
Conversation
FYI: The GHA builds shown here for this PR branch shows that this works with the code as of the large PR #560. Therefore, I will not rebase this one commit to be on top of the 'master' branch and deploy this. NOTE: Any time you change the GHA builds, you have to manually change the set of required builds to pass a PR on the GitHub settings (which is a manual process). |
512c7f8
to
010674b
Compare
Hello @KyleFromKitware, here is another smaller PR to review. This needs to be reviewed and merged before the large PR #560 because I have already changed the setting of the branch protection for 'master' to require these updated GHA jobs for updated versions of CMake. |
This is part of allowing TriBITS to take advantage of newer CMake features and replacing some older TriBITS code that now has (better or compatible) native implementations in CMake (often many years after the TriBITS support was created). This allows unconditionally using CMake features up to version 3.23 (where we were limited unconditionally using features up to CMake version 3.17). The min version of CMake 3.23 is chosen due to Trilinos changing its min version to 3.23 (see trilinos/Trilinos#10355). Some non-obvious changes made in this commit were: * For some reason, find_file() with CMake 3.23 (and using CMake 3.23 default policies) adds on an extra '/' before the file name in the found path. This messed up some of the test checks in TriBITS but I fixed that by putting '[/]*' in the regex to account for that extra '/'. * Updated the GitHub Actions testing jobs to all be CMake 3.23+. (I updated the patch versions of the different versions as well to the most recent patch releases for those minor versions as of right now. CMake 3.25 is still the most recent release of CMake as CMake 3.26 is still in release candidate testing. Therefore, CMake 3.26.2 is still the most current release of CMake.) * Added checks for setting CTEST_RESOURCE_SPEC_FILE in CTestTestfile.cmake now that CMake is >= 3.18. * Removed constant ${PROJECT_NAME}_CTEST_USE_NEW_AAO_FEATURES since it is hard-coded now that CMake is 3.17+ is required. * Removed some documentation discussing older versions of CMake (which are no longer supported by TriBITS). * install-cmake.py: Changed the default version of CMake to install from 3.17 to 3.23. * install-cmake.py: Removed patch for CMake 3.17 * Removed documentation for the Kitware fork of Ninja versions < 1.10 since Ninja 1.10 has been out for several years with Fortran support.
010674b
to
45b514c
Compare
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.
I am going to allow the merge and then set up a post-merge review.
@KyleFromKitware, can you please do a post-merge reivew? |
…SPub#411) This replaces '[/]*/' which will match one or more '/' chars in a row with '/?/' which will match one or two '/' in a row (which is what I wanted). This addreses review feedback from @KyleFromKitware in PR TriBITSPub#565.
Is 3.23 a strict requirement for the Trilinos CMake config to work now? some of the systems I work with still only go up to 3.22 unfortunately. |
Yes
Which systems, specifically? |
Parent Issue:
Description
Addresses part of #411 and is part of:
See last commit for more details.