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] conan_toolchain.cmake is shared by both build_types #12827

Closed
jellespijker opened this issue Jan 2, 2023 · 8 comments · Fixed by #12829
Closed

[bug] conan_toolchain.cmake is shared by both build_types #12827

jellespijker opened this issue Jan 2, 2023 · 8 comments · Fixed by #12829
Assignees
Labels
Milestone

Comments

@jellespijker
Copy link
Contributor

Environment details

  • Operating System+version: Linux 6.1.1-1-MANJARO
  • Compiler+version: N/A
  • Conan version: 1.56
  • Python version: 3.10.8
  • CMake 3.24.2

Steps to reproduce

  1. create a CMakeLists.txt with an option
  2. enable that option based on build_type in the conanfile
  3. run the conan install for both buildtypes conan install ... && conan install ... -s build_type=Debug
  4. Spend half an hour wondering why the cmake expression generator which is assigned to that option keeps on having the same value.
  5. Observe the CMakePresets.json and notice that the conan_toolchain.cmake is used by both ConfigurePresets.
  6. Observe the value of the option specified in the CMakeLists.txt haveing that same value.

Logs

No response

@jellespijker
Copy link
Contributor Author

Easy fix imo would be to add the toolchain file in the actual build folder instead of the shared generator folder

<off-topic>
best wishes for 2023
</off-topic>

@memsharded
Copy link
Member

Hi @jellespijker

Best wishes for 2023 for you too!

I am having a look at this and trying: #12829

However, quick question, are you using the CMakeToolchain.variables attribute? to use it and have it multi-config the right syntax will be:

 tc.variables.debug["MYVAR_CONFIG"] = "MYVAR_DEBUG"
tc.variables.release["MYVAR_CONFIG"] = "MYVAR_RELEASE"

It is true that there are a couple of other things that might depend on the build_type, so I am still trying to have a look to #12829, lets see how it goes.

@jellespijker
Copy link
Contributor Author

hi @memsharded, no, I wasn't aware of the new syntax. I'm old skool ;-)

we use it as such: https://github.com/Ultimaker/CuraEngine/blob/86be726a6c4b487dc8a358f052365298f040d549/conanfile.py#L105

I will try the new syntax tomorrow

Related issue:
I also had me some worries porting conanfile (boolean) options to CMake variables as the will be interpreted as strings "True" and "False" and the documentation for CMake states these as all capitals for generator expression. https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html

But placing not in front of the option it will be translated to OFF or ON, probably due to an implicit conversion to bool. This seems to be not an issue, but it did prompt me to investigate (and therefore also raise questions with other devs).
I can open up a new issue for this, or leave it be

@memsharded
Copy link
Member

I will try the new syntax tomorrow

Good

I also had me some worries porting conanfile (boolean) options to CMake variables as the will be interpreted as strings "True" and "False" and the documentation for CMake states these as all capitals for generator expression. https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html

I thought that it worked fine, and apparently it does, and there have been no reports. Yes, please, if you find there is something there not working as it should, open a new ticket and let us know.

@memsharded memsharded added this to the 1.57 milestone Jan 5, 2023
@memsharded
Copy link
Member

We have changed the behavior for single-config, to output files to a different directory in #12829. This might break a bit for some users, but it seems the right thing to do, and CMakeToolchain is still experimental, so it seems one of those breaking changes that it is worth.

Thanks very much for the feedback, this will be in 1.57

@jellespijker
Copy link
Contributor Author

thx, this should fix the issue I was experiencing. It also breaks some of our workflows it will be an easy enough fix.

When is 1.57 expected to be released? That would allow to plan it in our next sprint properly.

@memsharded
Copy link
Member

In theory, all the releases are by the end of the month, but this one will most likely be sooner, to prepare for Conan 2.0. I would say the week of 15th January.

@jellespijker
Copy link
Contributor Author

Thx
Looking forward to Conan 2.0.

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

Successfully merging a pull request may close this issue.

2 participants