[bug] tools.build:defines is broken in multiconfig setting

[stevenwdv]

Describe the bug

tools.build:defines is broken in multiconfig setting, e.g. with MSVC. The quoting in the generator expression is not correct in blocks.py.

How to reproduce it

• Set tools.build:defines=['A=1', 'B=2']
• Observe how the generated toolchain file or afterwards generated vxproj by CMake does not contain the right definitions.

EDIT: See PR #15924 for more info

[stevenwdv]

I'm working on a PR

[memsharded]

Thanks for the report and the offer to fix it @stevenwdv

This was tested in test_cmake_toolchain_cxxflags_multi_config, maybe it is because the single quotes vs double quotes?

[stevenwdv]

maybe it is because the single quotes vs double quotes

No it's the generator expressions, but I'll explain more in the PR and probably create a follow-up issue.

Btw all contributing docs mention the develop branch instead of develop2, which I thought was weird.

[memsharded]

#15922 will fix that, thanks for raising it too!

[memsharded]

The PR in #15924 will be released in 2.2.2 today, as it could be considered a regression. Thanks again for reporting.

[stevenwdv]

Maybe you can re-open this for now?

[memsharded]

Sure re-opening.

It would be great to have a PR with a test similar to test_cmake_toolchain_cxxflags_multi_config(), as seen in https://github.com/conan-io/conan/pull/15924/files that fails. Probably the best would be a new test if it can add just new checks, or a full new test if changing something else.

[stevenwdv]

@memsharded What I don't understand yet is why the current test is passing (see #15924 (comment))

[DenizThatMenace]

This is still broken in Conan 2.5.

But the fix is quite simple, as I already commented in #15924 (comment):

--- a/conan/tools/cmake/toolchain/blocks.py
+++ b/conan/tools/cmake/toolchain/blocks.py
@@ -724,7 +724,7 @@ class ExtraFlagsBlock(Block):
         {% if defines %}
         {% if config %}
         {% for define in defines %}
-        add_compile_definitions($<$<CONFIG:{{config}}>:"{{ define }}">)
+        add_compile_definitions("$<$<CONFIG:{{config}}>:{{ define }}>")
         {% endfor %}
         {% else %}
         add_compile_definitions({% for define in defines %} "{{ define }}"{% endfor %})

[memsharded]

To consider these changes, we would need first a failing test that reproduces the issues.

I am trying to reproduce, and added tools.cmake.cmaketoolchain:generator=Ninja Multi-Config in the existing test, but it is still green and passing.

Maybe you want to try to do a change or a new test like

conan/test/functional/toolchains/cmake/test_cmake_toolchain.py

Line 1831 in dd28339

def test_cmake_toolchain_cxxflags_multi_config():

, and submit a PR with it that showcases the error? That would help to clarify the issues.

[DenizThatMenace]

I have created a fix and test for this issue in PR #16637.