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

cmake: bump minimum required to version 3.5.0 #859

Merged
merged 1 commit into from
Jul 22, 2023
Merged

cmake: bump minimum required to version 3.5.0 #859

merged 1 commit into from
Jul 22, 2023

Conversation

scribam
Copy link
Contributor

@scribam scribam commented Jul 12, 2023

This fixes the following warning when configuring with CMake 3.27+:

CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.5 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.

@Cyan4973
Copy link
Owner

Moving the minimum version to v3.5 is fine by me,
as it's the version supported by Ubuntu Xenial, which is the oldest linux distribution worth supporting in 2023 imho.

But we don't update a minimum version just for the sake of it, since it mechanically reduces the addressable scope of the cmake script. We must "gain" something from this change.
It seems the gain in this case is a higher guarantee to remain supported in some future versions of cmake, which may drop support for versions older than v3.5 in some unspecified future.
That's a good enough reason I believe.

The PR is also well done, updating the cmake min version test introduced by @t-mat, and exploiting the increased minimum version to slightly reduce complexity of the script.

The only remaining question is if it's worth updating it "right now", or can it wait after the imminent release.

I would tend to lean towards the latter, as there is no immediate risk (the only downside is a warning message for some really recent versions of cmake) but increasing the minimum version may prevent some older systems from building xxhash with cmake. While it's probably not major, it still is a negative.

@Cyan4973 Cyan4973 added this to the v0.8.3 milestone Jul 16, 2023
@Cyan4973 Cyan4973 merged commit ea3bd57 into Cyan4973:dev Jul 22, 2023
57 checks passed
@scribam scribam deleted the cmake-minimum-required branch July 22, 2023 16:40
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