conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
7.99k stars 953 forks source link

[bug] CMakeToolchain can no longer configure pre-3.13 CMake options #7831

Open ohanar opened 3 years ago

ohanar commented 3 years ago

https://github.com/conan-io/conan/commit/a77b97817a73fbdd2fbeb5f7038ad400aafd1803 switched from using cached variables to normal cmake variables. This means that there is no mechanism to specify cmake options with the CMake toolchain for old versions of cmake (pre 3.13), or when a cmake project is using old policies. See https://cmake.org/cmake/help/v3.13/policy/CMP0077.html.

Environment Details (include every applicable attribute)

Steps to reproduce (Include if Applicable)

CMakeLists.txt

cmake_minimum_required(VERSION 3.12)
project(my_project)
option(my_option "my option" ON)

conanfile.py

class MyClass(ConanFile):
    ...
    def toolchain(self):
        toolchain = CMakeToolchain(self)
        toolchain.variables["my_option"] = "OFF"
        toolchain.write_toolchain_files()
memsharded commented 3 years ago

This is a result of not enough testing in CI with multiple Cmake versions in a valid range. We are planning changes to start testing more thoroughly these things.

Should this apply to all generated variables? Is it for options only? Does it make sense that the toolchain explicitly model options for this purpose?

ohanar commented 3 years ago

I don't personally see a big difference between options and other cache variables, I feel like they mostly exist as a legacy artifact of CMake. It would be good to get a second opinion on this topic.

ericLemanissier commented 2 years ago

This bug has been solved for the variables set manually using CMakeToolchain.variables dict, but it persists for variables set by conan itself, for example BUILD_SHARED_LIBS: conan_toolchain.cmake contains set(BUILD_SHARED_LIBS OFF). CMake erases this variable and indicates:

CMake Warning (dev) at CMakeLists.txt:72 (option):
  Policy CMP0077 is not set: option() honors normal variables.  Run "cmake
  --help-policy CMP0077" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  For compatibility with older versions of CMake, option is clearing the
  normal variable 'BUILD_SHARED_LIBS'.
memsharded commented 2 years ago

Just to be clear, the intended support for the new build system integrations is 3.15, as approved by the Tribe 2.0.

Of course, if it is possible to fix things that will make it work in pre-3.15, without breaking things or adding too much complexity, it would be nice to do it.

So is the above saying that the CMakeToolchain conan_toolchain.cmake will not work if users are setting things like BUILD_SHARED_LIBS as options in their CMakeLists.txt? Will making them CACHE variables solved this issue (without breaking later versions)?

ericLemanissier commented 2 years ago

I think so. My use case is https://github.com/FluidSynth/fluidsynth/blob/1511b5a575944a604873853366253ada60b2212a/CMakeLists.txt#L72 Adding tc.variables["BUILD_SHARED_LIBS"] = self.options.shared to the recipe works around the problem