Skip to content
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

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

Open
VincentRouvreau opened this issue May 7, 2024 · 5 comments · May be fixed by #8221
Assignees
Labels
CMake scripts Feature request rm only: release blocker For the release team only: the next release requires this issue/PR to be solved/merge
Milestone

Comments

@VincentRouvreau
Copy link
Contributor

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
@lrineau
Copy link
Member

lrineau commented May 8, 2024

That is an history of PR #4697 and, in particular commit 252b58d, 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 lrineau added this to the 6.0-beta milestone May 8, 2024
@lrineau lrineau self-assigned this May 8, 2024
@VincentRouvreau
Copy link
Contributor Author

VincentRouvreau commented May 13, 2024

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()

@lrineau
Copy link
Member

lrineau commented May 13, 2024

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 lrineau added the rm only: release blocker For the release team only: the next release requires this issue/PR to be solved/merge label May 13, 2024
@VincentRouvreau
Copy link
Contributor Author

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 !

@lrineau lrineau modified the milestones: 6.0-beta, 6.0 May 17, 2024
lrineau added a commit to lrineau/cgal that referenced this issue May 23, 2024
@lrineau lrineau linked a pull request May 23, 2024 that will close this issue
@lrineau lrineau linked a pull request May 23, 2024 that will close this issue
@lrineau
Copy link
Member

lrineau commented May 24, 2024

@VincentRouvreau A pull-request fixing that issue is being tested: #8221.

@lrineau lrineau modified the milestones: 6.0, 6.0-beta May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake scripts Feature request rm only: release blocker For the release team only: the next release requires this issue/PR to be solved/merge
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants