View Issue Details [ Jump to Notes ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0015333CMakeCMakepublic2015-01-02 18:392015-11-02 09:13
Reporterarlbranch 
Assigned ToBen Boeckel 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake 3.1 
Target VersionCMake 3.1.1Fixed in VersionCMake 3.1.1 
Summary0015333: Behaviour change with 3.1 - target properies set to empty string returned as -NOTFOUND
DescriptionUnder 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.
Steps To ReproduceCMakeLists.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
----------------
TagsNo tags attached.
Attached Files

 Relationships
has duplicate 0015478closed As of CMake 3.1 Properties cannot be set to "" 

  Notes
(0037544)
Stephen Kelly (developer)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (manager)
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 (manager)
2015-11-02 09:13

 Closing resolved issues that have not been updated in more than 4 months.

 Issue History
Date Modified Username Field Change
2015-01-02 18:39 arlbranch New Issue
2015-01-03 10:32 Stephen Kelly Note Added: 0037544
2015-01-03 10:32 Stephen Kelly Assigned To => Stephen Kelly
2015-01-03 10:32 Stephen Kelly Status new => confirmed
2015-01-03 10:32 Stephen Kelly Assigned To Stephen Kelly =>
2015-01-03 10:54 Stephen Kelly Note Added: 0037545
2015-01-03 10:55 Stephen Kelly Note Added: 0037546
2015-01-03 10:57 Stephen Kelly Note Edited: 0037546
2015-01-08 10:47 Brad King Assigned To => Ben Boeckel
2015-01-08 10:47 Brad King Status confirmed => assigned
2015-01-08 10:47 Brad King Target Version => CMake 3.1.1
2015-01-09 08:59 Ben Boeckel Note Added: 0037656
2015-01-10 13:08 Stephen Kelly Note Added: 0037659
2015-01-11 12:01 Brad King Note Added: 0037662
2015-01-11 12:06 Brad King Status assigned => resolved
2015-01-11 12:06 Brad King Resolution open => fixed
2015-01-11 12:06 Brad King Fixed in Version => CMake 3.1.1
2015-03-26 15:36 Brad King Relationship added has duplicate 0015478
2015-11-02 09:13 Robert Maynard Note Added: 0039746
2015-11-02 09:13 Robert Maynard Status resolved => closed

Copyright © 2000 - 2018 MantisBT Team