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

[bug] CMakeToolchain now requires double configure to set cache variables #7832

Closed
ohanar opened this issue Oct 6, 2020 · 4 comments · Fixed by #8124
Closed

[bug] CMakeToolchain now requires double configure to set cache variables #7832

ohanar opened this issue Oct 6, 2020 · 4 comments · Fixed by #8124
Assignees
Milestone

Comments

@ohanar
Copy link
Contributor

ohanar commented Oct 6, 2020

a77b978 switched from using cached variables to normal cmake variables. This means that cmake configuration has to be run twice to override standard configurable settings. This is because on the first processing, the cache variable does not exist in the cache, so when the set function call that initializes the cache variable is reached, it overrides the variable that is set by toolchain. On a second configuration, the same set function call is a no-op, since the cache variable has already been initialized.

Environment Details (include every applicable attribute)

  • Conan version: 1.30.0
  • Python version: 3.6, 3.7, 3.8

Steps to reproduce (Include if Applicable)

CMakeLists.txt

cmake_minimum_required(VERSION 3.12)
project(my_project)
set(my_configurable_string "default value" CACHE STRING "my configurable string")

conanfile.py

class MyClass(ConanFile):
    ...
    def toolchain(self):
        toolchain = CMakeToolchain(self)
        toolchain.variables["my_configurable_string"] = "my value"
        toolchain.write_toolchain_files()
@memsharded
Copy link
Member

Related to #7831. Does this mean that all variables set by the toolchain should be cache variables? Does this apply for generator expressions variables as well? Variables conditioned to the configuration Debug/Release were never cache variables. It would be weird to have regular variables as cache variables, and very similar variables per-configuration not cache-variables.

@ohanar
Copy link
Contributor Author

ohanar commented Oct 6, 2020

I think that probably all variables should be cache variables. Right now conan primarily uses toolchain files to pass a bunch of configuration info onto CMake. A user doing the same thing by hand would either pass that to CMake via the command line with the -D flag or via cmake-gui, both of which set cache variables. It would be good to get a second opinion though.

@memsharded
Copy link
Member

The Tribe 2.0 is almost ready for launch. This will be probably be the first question, which is the minimum cmake version (there is already some issue here, but we will ask for feedback and take a final decision, then implement testing in our CI with that minimum cmake version (we are already doing changes to our test suite, moving to pytest, to be able to do this with more control).
I am moving this issue to next release, where we will have more information.

@memsharded
Copy link
Member

#8124 added back CACHE, this will be released in 1.32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants