Skip to content

Some CMake maintenance#132

Merged
gdevenyi merged 2 commits into
BIC-MNI:masterfrom
LecrisUT:cmake/maintenance
Feb 23, 2026
Merged

Some CMake maintenance#132
gdevenyi merged 2 commits into
BIC-MNI:masterfrom
LecrisUT:cmake/maintenance

Conversation

@LecrisUT

Copy link
Copy Markdown
Contributor
  • cmake_minimum_required has to be before project
  • Add a higher-bound CMake policy
  • Use the GNUInstallDirs defaults, LIB_SUFFIX was never a standardized variable

@gdevenyi

Copy link
Copy Markdown
Contributor

Superceeded by #136

@gdevenyi gdevenyi closed this Feb 20, 2026
@LecrisUT

Copy link
Copy Markdown
Contributor Author

@gdevenyi please check again the diff of this PR, the changes are quite different, beyond the cosmetic change in #136

@gdevenyi

Copy link
Copy Markdown
Contributor

Sorry I failed to look closely. Would you please rebase to resolve the conflicts and I'll take a better look.

@gdevenyi gdevenyi reopened this Feb 20, 2026
@LecrisUT

Copy link
Copy Markdown
Contributor Author

Sorry I failed to look closely. Would you please rebase to resolve the conflicts and I'll take a better look.

Here you go. I did not look carefully into BuildNIFT when I first wrote this, but I highly suggest to change that into a FetchContent approach instead.

Comment thread CMakeLists.txt
# Vladimir S. FONOV - vladimir.fonov@gmail.com

cmake_minimum_required(VERSION 3.10)
cmake_minimum_required(VERSION 3.10...4.2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test all the up to 4.2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hoping primarily that the CI can pick up on any issues. This can easily be added with https://github.com/jwlawson/actions-setup-cmake if you are interested.

Overall though the thing that this one does is set the policies to new, and if something on the list does not catch the attention, it should mostly be fine (preferably if the CI checks it as well)

@gdevenyi

Copy link
Copy Markdown
Contributor

Would you pleas rebase one more time on master, we updated our build workflows. Thanks!

@gdevenyi

Copy link
Copy Markdown
Contributor

Thanks for the followup, it passes the current CI and I'll look at the cmake actions one you suggested.

@gdevenyi
gdevenyi merged commit 3508a98 into BIC-MNI:master Feb 23, 2026
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants