Skip to content
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

Merged
merged 1 commit into from
Feb 25, 2023

Conversation

bartlettroscoe
Copy link
Member

@bartlettroscoe bartlettroscoe commented Feb 25, 2023

@bartlettroscoe
Copy link
Member Author

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).

@bartlettroscoe bartlettroscoe changed the title WIP: Update CMake min version from 3.17 to 3.23 (#411) Update CMake min version from 3.17 to 3.23 (#411) Feb 25, 2023
@bartlettroscoe
Copy link
Member Author

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.
Copy link
Collaborator

@rabartlett1972 rabartlett1972 left a 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.

@bartlettroscoe bartlettroscoe merged commit ec4532b into master Feb 25, 2023
@bartlettroscoe
Copy link
Member Author

@KyleFromKitware, can you please do a post-merge reivew?

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this pull request Feb 28, 2023
…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.
@Adrian-Diaz
Copy link

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.

@bartlettroscoe
Copy link
Member Author

Is 3.23 a strict requirement for the Trilinos CMake config to work now?

Yes

some of the systems I work with still only go up to 3.22 unfortunately.

Which systems, specifically?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants