iLCSoft / LCIO

Linear Collider I/O
BSD 3-Clause "New" or "Revised" License
17 stars 34 forks source link

Bump the minimum CMake version #175

Closed jmcarcell closed 6 months ago

jmcarcell commented 11 months ago

BEGINRELEASENOTES

ENDRELEASENOTES

Right now, when running cmake .. I get the following warning:

  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

The old behaviour of having the variables is deprecated as explained here: https://cmake.org/cmake/help/latest/policy/CMP0048.html so it may be removed in the future but for using the new behavior we need CMake 3.0 (released in 2014).

How VERSION works inside project() is explained here: https://cmake.org/cmake/help/v3.21/command/project.html

so that means that this PR will keep the same behavior as before.

andresailer commented 11 months ago

The tagging script replaces the _VERSION_XYZ lines

https://github.com/iLCSoft/iLCInstall/blob/f3a08398aed0b254cca45fadeb7bc344edcd55e8/scripts/tagging/gitinterface.py#L362-L373

Can you instead use PROJECT_VERSION_XYZ?

jmcarcell commented 11 months ago

Mm no that doesn't silence the warning, which is not that much of a problem. I would say the bigger problem is setting the version in a deprecated way:

The OLD behavior for this policy is to leave VERSION variables untouched. The NEW behavior for this policy is to set VERSION as documented by the project() command.

Note

The OLD behavior of a policy is deprecated by definition and may be removed in a future version of CMake.

Options:

  1. We bump the minimum version of cmake required to 3.0 which will get rid of the warning
  2. We do it like in this PR and change the installation script to also get the version if it's inside project().

I think since we are already dealing with this we could do 2 in some repos at least since to silence the warning some change is going to be needed anyway

tmadlener commented 10 months ago

I don't see a real problem with bumping the minimum cmake version to 3.X. X could probably be quite large already such that we at least partially catch up with cmake. (https://github.com/iLCSoft/LCIO/pull/140 would put it to 3.14)

jmcarcell commented 6 months ago

Actually I don't get the warning anymore (different cmake version? I'm not sure) so we can just leave this PR at a bump on the minimum cmake version which does generate a warning if set too low