noloader / cryptopp-cmake

CMake files for Crypto++ project
BSD 3-Clause "New" or "Revised" License
92 stars 68 forks source link

Please add VERSION to cmake project() #10

Closed abdes closed 6 years ago

abdes commented 6 years ago
-- *************************************************************************
The Crypto++ library does not officially support CMake. CMake support is a
community effort, and the library works with the folks using CMake to help
improve it. If you find an issue then please fix it or report it at
https://github.com/noloader/cryptopp-cmake.
-- *************************************************************************
CMake Warning (dev) at cmake-build-debug/cryptopp-src/CMakeLists.txt:19 (project):
  Policy CMP0048 is not set: project() command manages VERSION variables.
  Run "cmake --help-policy CMP0048" for policy details.  Use the cmake_policy
  command to set the policy and suppress this warning.

  The following variable(s) would be set to empty:

    PROJECT_VERSION
    PROJECT_VERSION_MAJOR
    PROJECT_VERSION_MINOR
    PROJECT_VERSION_PATCH
This warning is for project developers.  Use -Wno-dev to suppress it.
noloader commented 6 years ago

@abdes,

Please add VERSION to cmake project...

I believe CmakeList.txt : 75 has it:

set(cryptopp_VERSION_MAJOR 7)
set(cryptopp_VERSION_MINOR 0)
set(cryptopp_VERSION_PATCH 0)

Maybe you can track down the issue and work up a patch.

abdes commented 6 years ago

The only issue bro is that make policies deprecate the setting of version numbers like above. The new way is to set the version in the project() line. The solution is very simple: set the version as following and remove the explicit setting of the version variables, which will be automatically populated by cmake:

project(cryptopp VERSION 7.0.0)

Update: You just released a completely updated CMakeLists.txt with minimum requirement of cmake version 3.7.0 - this makes the issue disappear :-) Thanks for the hard work.

noloader commented 6 years ago

You just released a completely updated CMakeLists.txt with minimum requirement of cmake version 3.7.0 - this makes the issue disappear :-)

Yeah, CMake is a mess. It looks like Ubuntu 14.05 is probably broke even though it is LTS and supported through 2019:

CMake Error at CMakeLists.txt:22 (cmake_minimum_required):
  CMake 3.7 or higher is required.  You are running version 2.8.12.2

-- Configuring incomplete, errors occurred!
cmake failed

This is why I gave up on CMake. There are too many problems with it.

abdes commented 6 years ago

IKR :-) Don't give up... This is what I do in my travis build script to get the latest cmake on Ubuntu:

#
# Build Dependencies
#
before_script:
  # Add an IPv6 config - see the corresponding Travis issue
  # https://github.com/travis-ci/travis-ci/issues/8361
  - if [ "${TRAVIS_OS_NAME}" == "linux" ]; then
      sudo sh -c 'echo 0 > /proc/sys/net/ipv6/conf/all/disable_ipv6';
    fi

  #
  # Build Folder
  #
  - mkdir _build
  - cd _build

  #
  # Update / Install CMake
  #
  - |
    if [[ "${TRAVIS_OS_NAME}" == "linux" ]]; then
      mkdir -p external/cmake
      pushd external/cmake
      wget https://cmake.org/files/v3.11/cmake-3.11.0-Linux-x86_64.sh
      chmod +x cmake-*-Linux-x86_64.sh
      ./cmake-*-Linux-x86_64.sh --exclude-subdir --skip-license
      export PATH="${PWD}/bin:$PATH"
      popd
    else
      brew update
      brew outdated cmake || brew upgrade cmake
    fi
noloader commented 6 years ago

The idea is to work with what is on a machine. Users should be able to download the files and things "just work".

Some environments I work in do not allow the changes. Other environments require a change control item and senior management sign-off when deviating from organization's standard image. For example, US Treasury requires a change control item approved by senior management in Treasury.

For what it is worth we eat our own dogfood. We still support GCC 3.1 and Visual Studio 2003. We don't want users suffering problems on older platforms. You can install Fedora 1 or Windows XP and things will "just work".

It gets even worse. Try CMake on AIX or Solaris. You can't upgrade high enough to fix all the problems on those platforms.

abdes commented 6 years ago

If you wanna downgrade the requirement to 2.7, then you know the fix for the version policy issues :-)

I've done a lot of research lately on the other alternatives to cmake, the issue is that cmake is the defacto standard as of now for C++ open source projects. It also saves a lot of time generating the build files and IDE files for windows.

noloader commented 6 years ago

I've done a lot of research lately on the other alternatives to cmake, the issue is that cmake is the defect standard os of now for C++ open source projects. It also saves a lot of time generating the build files and die files for windows.

Not for Crypto++. In essence CMake could not pass acceptance testing. Also see CMake Removal on the wiki. It details the problems we had with the program.

We do our best to help support the folks who want to use CMake, but there are no guarantees.

jcfr commented 6 years ago

Having s a dedicated project called "crypto-cmake" is great. It removes the need to argument for or against Cmake and allow the community interested to support to contribute changes and maintain the build system.

I then suggest we focus on that :)

Moving forward, I anticipate it will streamline not only compilation and integration but also cross a compilation.

The other good news is that we don't need to convince anyone about it, we can demonstrate it will work (or fail) by doing it in this project.

On Sat, Jun 30, 2018, 9:56 AM Jeffrey Walton notifications@github.com wrote:

I've done a lot of research lately on the other alternatives to cmake, the issue is that cmake is the defect standard os of now for C++ open source projects. It also saves a lot of time generating the build files and die files for windows.

Not for Crypto++. In essence CMake could not pass acceptance testing. Also see CMake Removal https://www.cryptopp.com/wiki/CMake on the wiki. It details the problems we had with the program.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/noloader/cryptopp-cmake/issues/10#issuecomment-401527996, or mute the thread https://github.com/notifications/unsubscribe-auth/AANXoytPV-Xt3TF1blb_nBp4EsEbsjyFks5uBz0mgaJpZM4UIF06 .

abdes commented 6 years ago

@noloader now that you have downgraded the requirement on cmake version, please at least set the cmake policy CMP0048 to OLD before the project() line in the CMakeLists.txt so that we don't get the warning on the VERSION variables. I know this is cosmetic, but it's part of having a clean build output and it clearly states your intent to stay at a lower version of cmake.

cmake_minimum_required(VERSION 2.8.12)
cmake_policy(SET CMP0048 OLD)

project(cryptopp)

EDIT: I'll make a pull request for this change

noloader commented 6 years ago
cmake_minimum_required(VERSION 2.8.12)
cmake_policy(SET CMP0048 OLD)

project(cryptopp)

Please be sure to test the change on older versions of Cmake, like the one found on Ubuntu 14.04 LTS.

abdes commented 6 years ago

OK.

On Jul 13, 2018, at 2:30 PM, Jeffrey Walton notifications@github.com wrote:

cmake_minimum_required(VERSION 2.8.12) cmake_policy(SET CMP0048 OLD)

project(cryptopp) Please be sure to test the change on older versions of Cmake, like the one found on Ubuntu 14.04 LTS.

noloader commented 6 years ago

@abdes,

If you have some time could you look at Issue 15.

abdes commented 6 years ago

Ok. Let me take a look...

On Jul 13, 2018, at 2:32 PM, Jeffrey Walton notifications@github.com wrote:

@abdes https://github.com/abdes,

If you have some time could you look at Issue 15 https://github.com/noloader/cryptopp-cmake/issues/15.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/noloader/cryptopp-cmake/issues/10#issuecomment-404794756, or mute the thread https://github.com/notifications/unsubscribe-auth/AAb7raMcxGJ6274Viu4FjuVlj-QM3wX4ks5uGHdJgaJpZM4UIF06.

abdes commented 6 years ago

Warning eliminated with PR #17