MantisBT - CMake
View Issue Details
0014037CMakeCMakepublic2013-03-22 12:072013-10-07 10:04
Andreas Stahl 
Stephen Kelly 
normalminoralways
closedfixed 
Visual Studio 2010Windows7 x64
CMake 2.8.11 
CMake 2.8.11CMake 2.8.11 
0014037: target_compile_definitions empty $<CONFIGURATION> generator
I came across weird behaviour in the 2.8.11rc1.

target_compile_definitions(test PUBLIC $<$<CONFIG:Debug>:TEST_DEBUG> $<CONFIG:Debug> $<CONFIGURATION>)

Only adds "0" to the compile definitions in every Visual Studio configuration, including "Debug", leading me to believe that $<CONFIGURATION> is not set correctly.
cmake_minimum_required(VERSION 2.8.11)
project(test)

file(WRITE test.cpp "#include <stdio.h>")

add_library(test test.cpp)

target_compile_definitions(test PUBLIC $<$<CONFIG:Debug>:TEST_DEBUG> $<CONFIG:Debug> $<CONFIGURATION>)
No tags attached.
Issue History
2013-03-22 12:07Andreas StahlNew Issue
2013-03-22 12:09Andreas StahlNote Added: 0032685
2013-03-22 14:57Brad KingNote Added: 0032686
2013-03-22 14:57Brad KingAssigned To => Stephen Kelly
2013-03-22 14:57Brad KingStatusnew => assigned
2013-03-22 14:57Brad KingProduct VersionCMake 2.8.10.2 => CMake 2.8.11
2013-03-22 14:57Brad KingTarget Version => CMake 2.8.11
2013-03-22 15:04Brad KingNote Added: 0032687
2013-03-22 15:10Brad KingNote Added: 0032688
2013-03-24 16:36Stephen KellyNote Added: 0032690
2013-03-25 10:52Brad KingNote Added: 0032692
2013-03-25 10:57Stephen KellyNote Added: 0032693
2013-03-27 05:20Stephen KellyStatusassigned => resolved
2013-03-27 05:20Stephen KellyFixed in Version => CMake 2.8.11
2013-03-27 05:20Stephen KellyResolutionopen => fixed
2013-10-07 10:04Robert MaynardNote Added: 0034017
2013-10-07 10:04Robert MaynardStatusresolved => closed

Notes
(0032685)
Andreas Stahl   
2013-03-22 12:09   
Could an admin please change the Product Version to 2.8.11rc1? I couldn't select it from the report form's drop-down menu. Thanks!
(0032686)
Brad King   
2013-03-22 14:57   
Confirmed. I think the bug is here:

 http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmTarget.cxx;hb=db87209b#l2941 [^]

where it reads the COMPILE_DEFINITIONS_<CONFIG> property for each config and ignores the plain-old COMPILE_DEFINITIONS property which is the one where the generator expressions are placed by the target_compile_definitions command.

Steve, please fix this and extend the CompileDefinitions test to cover some per-config definitions.
(0032687)
Brad King   
2013-03-22 15:04   
The generators call cmTarget::GetCompileDefintions both without a config and with each config. For example, the VS 10 generator does it here:

 http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmVisualStudio10TargetGenerator.cxx;hb=db87209b#l1223 [^]

That captures both the per-config and normal property but does not evaluate the plain-old COMPILE_DEFINITIONS property generator expressions with the proper configuration.
(0032688)
Brad King   
2013-03-22 15:10   
Steve, please refactor cmTarget::GetCompileDefintions so its callers only need to call it once with their config name. It should get the values of both COMPILE_DEFINITIONS and COMPILE_DEFINITIONS_<CONFIG>, append the lists, evaluate generator expressions with the proper config name, and then return a single result. Then fix up all the call sites to call it once per config.
(0032690)
Stephen Kelly   
2013-03-24 16:36   
I've pushed fix-COMPILE_DEFINITIONS-config to stage. I haven't merged it to next yet because I have only tested the makefile generator.

I'm also not certain that the approach of creating a config-specific test is the right one. Can you confirm the use of the \${CTEST_TEST_CONFIGURATION} trick?

Thanks,
(0032692)
Brad King   
2013-03-25 10:52   
Re 0014037:0032690: Thanks. I pushed your fix as:

 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=a6286e92 [^]

but re-wrote the test change as:

 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=1703b00c [^]
(0032693)
Stephen Kelly   
2013-03-25 10:57   
Thanks. It looks good to me.
(0034017)
Robert Maynard   
2013-10-07 10:04   
Closing resolved issues that have not been updated in more than 4 months.