Skip to content

added cmake 3.7 as max cmake required version #1291

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

Closed
wants to merge 2 commits into from

Conversation

DvirDukhan
Copy link

@DvirDukhan DvirDukhan commented Apr 1, 2025

following https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html#command:cmake_minimum_required
we need to add version 3.5 as the optional max value

edit:
following PR discussion, I changed the max value from 3.5 to 3.7

@michael-grunder
Copy link
Collaborator

Not opposed to adding it but do we expect the build to break with higher versions? I can build just fine with 3.22.1.

@bjosv
Copy link
Contributor

bjosv commented Apr 2, 2025

One thing to note is that CMake < 3.7 will not work when building with -DENABLE_SSL=ON

CMake Error at /opt/hostedtoolcache/cmake/3.6.0/x64/cmake-3.6.0-Linux-i386/share/cmake-3.6/Modules/FindOpenSSL.cmake:345 (list):
  list GET given empty list
Call Stack (most recent call first):
  CMakeLists.txt:159 (FIND_PACKAGE)

In libvalkey we set the minimum version to 3.7 which builds.
Using CMAKE_MINIMUM_REQUIRED(VERSION 3.0.0...3.7.0) could be an alternative.

@DvirDukhan
Copy link
Author

@michael-grunder the problem is building the current code base with cmake 4. Building with Cmake 4 yields

CMake Error at CMakeLists.txt:1 (CMAKE_MINIMUM_REQUIRED):
  Compatibility with CMake < 3.5 has been removed from CMake.

  Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
  to tell CMake that the project requires at least <min> but has been updated
  to work with policies introduced by <max> or earlier.

  Or, add -DCMAKE_POLICY_VERSION_MINIMUM=3.5 to try configuring anyway.

@DvirDukhan DvirDukhan changed the title added cmake 3.5 as max cmake required version added cmake 3.7 as max cmake required version Apr 2, 2025
@uglide
Copy link
Contributor

uglide commented Apr 2, 2025

@bjosv Are you sure it's not a build env problem on your side?

I've just tested with CMake 3.5 and 3.6 and it builds just fine:

$ cmake --version
cmake version 3.6.0

CMake suite maintained and supported by Kitware (kitware.com/cmake).

$ cmake -DENABLE_SSL=ON
Detected version: 1.2.0
-- The C compiler identification is GNU 9.4.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Configuring done
-- Generating done
-- Build files have been written to: /playground/hiredis

uglide
uglide previously approved these changes Apr 2, 2025
@uglide
Copy link
Contributor

uglide commented Apr 2, 2025

I reproduced the problem with CMake 3.5 on GitHub Actions image and agree with @michael-grunder that we shouldn't limit the maximum version. I'm closing the PR now. For further discussion, please use #1292

@uglide uglide closed this Apr 2, 2025
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.

4 participants