cgal icon indicating copy to clipboard operation
cgal copied to clipboard

CMake find_package with a minimal CGAL version number does not detect properly 6.X CGAL version

Open VincentRouvreau opened this issue 9 months ago • 4 comments

Issue Details

CMake find_package(CGAL 5.1.0) (with a minimal cgal version number) does not seem to behave as expected with master version.

Source Code

Here is a CMake file to reproduce the behaviour:

cmake_minimum_required(VERSION 3.8)
project(minimal_cgal_version)

find_package(CGAL 5.1.0)

if (TARGET CGAL::CGAL)
  message("++ CGAL version: ${CGAL_VERSION}. Includes found in ${CGAL_INCLUDE_DIRS}")
endif ()

With CGAL 5.1.5, it works fine:

++ CGAL version: 5.1.5. Includes found in /home/gailuron/workspace/contrib/CGAL-5.1.5/include

(tested with 5.5.3 also).

But, with 6.0-I-207 (master from some days ago):

CMake Warning at src/cmake/modules/GUDHI_third_party_libraries.cmake:30 (find_package):
  Could not find a configuration file for package "CGAL" that is compatible
  with requested version "5.1.0".

  The following configuration files were considered but not accepted:

    /home/gailuron/workspace/contrib/CGAL-6.0-I-207/build/CGALConfig.cmake, version: 6.0

Call Stack (most recent call first):
  CMakeLists.txt:20 (include)

Environment

  • Operating system (Windows/Mac/Linux, 32/64 bits): Linux 64 bits
  • Compiler: g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
  • Release or debug mode: Release mode
  • Specific flags used (if any):
  • CGAL version: 5.1.5, 5.5.3 and 6.0-I-207 (master from some days ago)
  • Boost version: 1.84.0
  • Other libraries versions if used (Eigen, TBB, etc.): cmake 3.29.0 hcfe8598_0 conda-forge eigen 3.4.0 h00ab1b0_0 conda-forge gmp 6.3.0 h59595ed_1 conda-forge mpfr 4.2.1 h9458935_0 conda-forge

VincentRouvreau avatar May 07 '24 16:05 VincentRouvreau

That is an history of PR #4697 and, in particular commit 252b58d39fb3a997365b584b84a43bc75d7b655f, it seems that at that time we decided that, if the CMake user-code was asking for CGAL compatible with version 5.1, then a major version change like 6.0 would not be compatible. Do you agree it was a reasonable choice?

In recent CMake versions, find_package can specify a version range. I do not know if our file Installation/lib/cmake/CGAL/CGALConfigVersion.cmake is ready for that. I am pretty sure it is not tested, and thus I would guess it is broken.

I am open for a discussion, here. @VincentRouvreau, @afabri, @sloriot, others, please comment.

lrineau avatar May 08 '24 15:05 lrineau

Will the major changes for CGAL 6.0 invalidate the CMake code above? From a CGAL user point of view, if you keep PACKAGE_VERSION_COMPATIBLE=FALSE when major version is not the same, I do not see any CMake mechanism to make find_package(CGAL 5.1.0) work as I am expecting to. For instance, multiple find_package does not work:

find_package(CGAL 5.1.0...<6.0.0)
if (NOT TARGET CGAL::CGAL)
  find_package(CGAL 6.0.0)
endif()

But I can still do:

find_package(CGAL) # No expected version
if (NOT TARGET CGAL::CGAL)
  # Do some tests to check the version number
endif()

VincentRouvreau avatar May 13 '24 08:05 VincentRouvreau

Hi @VincentRouvreau @mglisse,

After a discussion with @sloriot, we decided that we will change that file CGALConfigVersion.cmake for master/CGAL-6.0. Whatever we decide for the policy in CGALConfigVersion.cmake, CGAL tries to be reasonably backward-compatible, with a few breaking changes for any new version (except for bug-fixes versions). Regardless of the policy we adopt, it will only approximate the actual situation. Therefore, we should avoid creating unnecessary obstacles for CGAL users.

lrineau avatar May 13 '24 13:05 lrineau

Hi @VincentRouvreau @mglisse,

After a discussion with @sloriot, we decided that we will change that file CGALConfigVersion.cmake for master/CGAL-6.0. Whatever we decide for the policy in CGALConfigVersion.cmake, CGAL tries to be reasonably backward-compatible, with a few breaking changes for any new version (except for bug-fixes versions). Regardless of the policy we adopt, it will only approximate the actual situation. Therefore, we should avoid creating unnecessary obstacles for CGAL users.

Thanks for the fix proposal !

VincentRouvreau avatar May 16 '24 16:05 VincentRouvreau

@VincentRouvreau A pull-request fixing that issue is being tested: https://github.com/CGAL/cgal/pull/8221.

lrineau avatar May 24 '24 14:05 lrineau