MantisBT - CMake
View Issue Details
0015333CMakeCMakepublic2015-01-02 18:392015-11-02 09:13
arlbranch 
Ben Boeckel 
normalmajoralways
closedfixed 
CMake 3.1 
CMake 3.1.1CMake 3.1.1 
0015333: Behaviour change with 3.1 - target properies set to empty string returned as -NOTFOUND
Under the old behaviour, when a target property was set to "", get_target_property would give "". In version 3.1 it now gives prop-NOTFOUND. Needless to say, this change breaks build systems that relied on the old behaviour.
CMakeLists.txt
---------------
cmake_minimum_required(VERSION 2.8.0)

add_custom_target(tgt)
set_target_properties(tgt PROPERTIES emptyprop "")
get_target_property(val tgt emptyprop)
message("val = ${val}")
---------------

Old Behaviour
---------------
[branch@viter on /dev/pts/9] 1044 ~/tmp/cm31bug/build
$ cmake --version
cmake version 2.8.12.2
[branch@viter on /dev/pts/9] 1045 ~/tmp/cm31bug/build
$ cmake ..
val =
-- Configuring done
-- Generating done
-- Build files have been written to: /local/home/branch/tmp/cm31bug/build
----------------

New Behaviour
----------------
$ cmake --version
cmake version 3.1.0

CMake suite maintained and supported by Kitware (kitware.com/cmake).
[branch@fortuna on /dev/pts/5] 1054 ~/tmp/cm31bug/fbuild
$ cmake ..
val = val-NOTFOUND
-- Configuring done
-- Generating done
-- Build files have been written to: /home/branch/tmp/cm31bug/fbuild
----------------
No tags attached.
has duplicate 0015478closed  As of CMake 3.1 Properties cannot be set to "" 
Issue History
2015-01-02 18:39arlbranchNew Issue
2015-01-03 10:32Stephen KellyNote Added: 0037544
2015-01-03 10:32Stephen KellyAssigned To => Stephen Kelly
2015-01-03 10:32Stephen KellyStatusnew => confirmed
2015-01-03 10:32Stephen KellyAssigned ToStephen Kelly =>
2015-01-03 10:54Stephen KellyNote Added: 0037545
2015-01-03 10:55Stephen KellyNote Added: 0037546
2015-01-03 10:57Stephen KellyNote Edited: 0037546bug_revision_view_page.php?bugnote_id=37546#r1668
2015-01-08 10:47Brad KingAssigned To => Ben Boeckel
2015-01-08 10:47Brad KingStatusconfirmed => assigned
2015-01-08 10:47Brad KingTarget Version => CMake 3.1.1
2015-01-09 08:59Ben BoeckelNote Added: 0037656
2015-01-10 13:08Stephen KellyNote Added: 0037659
2015-01-11 12:01Brad KingNote Added: 0037662
2015-01-11 12:06Brad KingStatusassigned => resolved
2015-01-11 12:06Brad KingResolutionopen => fixed
2015-01-11 12:06Brad KingFixed in Version => CMake 3.1.1
2015-03-26 15:36Brad KingRelationship addedhas duplicate 0015478
2015-11-02 09:13Robert MaynardNote Added: 0039746
2015-11-02 09:13Robert MaynardStatusresolved => closed

Notes
(0037544)
Stephen Kelly   
2015-01-03 10:32   
Bisects to v3.1.0-rc1~812^2~50 (stringapi: Use strings for property names, 2013-09-02)
(0037545)
Stephen Kelly   
2015-01-03 10:54   
The workaround is to use get_property instead:

 # get_target_property(val tgt emptyprop)
 get_property(val TARGET tgt PROPERTY emptyprop)
(0037546)
Stephen Kelly   
2015-01-03 10:55   
(edited on: 2015-01-03 10:57)
Here's the fix, but I'll leave it to others to write the tests, verify whether other commands/interfaces are affected by the commit/'class of bug' etc:

  diff --git a/Source/cmGetTargetPropertyCommand.cxx b/Source/cmGetTargetPropertyCommand.cxx
  index aa6f0c1..fb59df8 100644
  --- a/Source/cmGetTargetPropertyCommand.cxx
  +++ b/Source/cmGetTargetPropertyCommand.cxx
  @@ -23,6 +23,7 @@ bool cmGetTargetPropertyCommand
    std::string var = args[0];
    const std::string& targetName = args[1];
    std::string prop;
  + bool prop_exists = false;
  
    if(args[2] == "ALIASED_TARGET")
      {
  @@ -32,6 +33,7 @@ bool cmGetTargetPropertyCommand
                            this->Makefile->FindTargetToUse(targetName))
          {
          prop = target->GetName();
  + prop_exists = true;
          }
        }
      }
  @@ -42,6 +44,7 @@ bool cmGetTargetPropertyCommand
      if(prop_cstr)
        {
        prop = prop_cstr;
  + prop_exists = true;
        }
      }
    else
  @@ -74,7 +77,7 @@ bool cmGetTargetPropertyCommand
          }
        }
      }
  - if (!prop.empty())
  + if (prop_exists)
      {
      this->Makefile->AddDefinition(var, prop.c_str());
      return true;

(0037656)
Ben Boeckel   
2015-01-09 08:59   
I've pushed this into next with tests as 'fix-empty-target-property-queries'. What I get from writing the test is that any [gs]et_*_property command is currently very inconsistent (in signature and what it does for non-existent properties) and I wouldn't be against a policy to deprecate all but get_property and set_property for 3.2 or 3.3.
(0037659)
Stephen Kelly   
2015-01-10 13:08   
Thanks, I ran the tests with a cmake 3.0 binary too as a sanity check with the correct results.
(0037662)
Brad King   
2015-01-11 12:01   
I rebased Ben's topic on 'release' since it fixes a regression:

 get_target_property: discern empty from undefined properties
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=34a99094 [^]

 get_test_property: clarify the documentation
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=28214862 [^]

 set_tests_properties: fix documentation
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=76ff92e0 [^]

 tests: add tests for querying properties
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=c6d03a10 [^]
(0039746)
Robert Maynard   
2015-11-02 09:13   
Closing resolved issues that have not been updated in more than 4 months.