Zstd: VERSION_LESS_EQUAL in CMake < 3.7 - Unknown arguments specified

[erikwebb]

I am working on building mcrouter and am unable to build the zstd dependency due to the following error -

+ git clone https://github.com/facebook/zstd
Cloning into 'zstd'...
+ cd /usr/local/mcrouter/pkgs/zstd
+ cmake -DCMAKE_INSTALL_PREFIX=/usr/local/mcrouter/install build/cmake/
CMake Error at CMakeLists.txt:18 (if):
  if given arguments:

    "3.5.1" "VERSION_LESS_EQUAL" "3.13.2"

  Unknown arguments specified


-- Configuring incomplete, errors occurred!

According to release notes, VERSION_LESS_EQUAL was not added to CMake until 3.7. This change was recently made by @hjmjohnson in 97d1de3d22c5ad53a465d76a5708a4ba90d0ed8b.

Before I move forward with a fix, is the right path forward to change the CMakeLists.txt conditional to check for ${CMAKE_MINOR_VERSION} as well? I'm not very familiar with CMake.

[Cyan4973]

I'm not familiar with cmake either,
but your suggestion to use ${CMAKE_MINOR_VERSION} looks good to me.

[Cyan4973]

Also : minor detail :
I would recommend using the master branch for dependency build,
instead of default dev branch.

master is very stable, basically a mirror of latest release,
while dev changes all the time, so it can contain transcient errors/problems.

[erikwebb]

@Cyan4973 Interesting. I am just using the default mcrouter build. Maybe I should update their build as well.

[vitaut]

Looks like the fix in https://github.com/facebook/zstd/commit/9448a3790dec147ac05a3ba02bcbb52a5b9b090a#diff-2f38feedf9af0f5288a86bd6f203d7ea introduces an error for CMake versions [3.0, 3.6) because ZSTD_CMAKE_POLICY_VERSION is no longer set:

CMake Error at CMakeLists.txt:24 (cmake_policy):
  cmake_policy VERSION not given an argument

[erikwebb]

So I guess the first conditional should be if("${CMAKE_MAJOR_VERSION}" LESS 3 OR ("${CMAKE_MAJOR_VERSION}" EQUAL 3 AND "${CMAKE_MINOR_VERSION}" LESS 7))?

[vitaut]

Either that or just set set(ZSTD_CMAKE_POLICY_VERSION "${CMAKE_VERSION}") uncoditionally and then override it if needed:

set(ZSTD_CMAKE_POLICY_VERSION "${CMAKE_VERSION}") 
if(("${CMAKE_MAJOR_VERSION}" EQUAL 3 AND "${CMAKE_MINOR_VERSION}" GREATER 6) OR ("${CMAKE_MAJOR_VERSION}" GREATER 3))
  if("${CMAKE_VERSION}" VERSION_GREATER "${ZSTD_MAX_VALIDATED_CMAKE_VERSION}") 
    set(ZSTD_CMAKE_POLICY_VERSION "${ZSTD_MAX_VALIDATED_CMAKE_VERSION}") 
  endif() 
endif()

(Can be even combined in one hairy if statement.)

[vitaut]

It is kinda ironic to test CMake version to check if VERSION_* is supported =). Maybe just replace ZSTD_MAX_VALIDATED_CMAKE_VERSION with major and minor counterparts?

[kornrunner]

I believe these changes now break things for Ubuntu 16.04 (for which mcrouter supplies build script).

I've created a generic/ubuntu1604 Vagrant box and here's the result on it:

root@ubuntu1604:~# git clone https://github.com/facebook/zstd
Cloning into 'zstd'...
remote: Enumerating objects: 36, done.
remote: Counting objects: 100% (36/36), done.
remote: Compressing objects: 100% (29/29), done.
remote: Total 35506 (delta 13), reused 18 (delta 7), pack-reused 35470
Receiving objects: 100% (35506/35506), 16.94 MiB | 5.12 MiB/s, done.
Resolving deltas: 100% (26281/26281), done.
Checking connectivity... done.
root@ubuntu1604:~# cd zstd/
root@ubuntu1604:~/zstd# cmake --version
cmake version 3.5.1

CMake suite maintained and supported by Kitware (kitware.com/cmake).
root@ubuntu1604:~/zstd# cmake build/cmake/
CMake Error at CMakeLists.txt:24 (cmake_policy):
  cmake_policy VERSION not given an argument


CMake Error at CMakeLists.txt:43 (project):
  VERSION not allowed unless CMP0048 is set to NEW


-- Configuring incomplete, errors occurred!

That's on dev branch, but that's the branch fbthrift.sh uses anyway (doesn't checkout master).

[hjmjohnson]

@kornrunner Is your comment fixed by the PR #1496 submitted 2 days ago?

[kornrunner]

@hjmjohnson yeah, it is, thank you!

[hjmjohnson]

@kornrunner No problem. Sorry for the initial failure.