View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0009188CMakeCMakepublic2009-06-19 18:492014-06-02 08:38
ReporterPaul Oppenheim (Poppy Linden) 
Assigned ToNils Gladitz 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake-2-6 
Target VersionCMake 2.8.12Fixed in VersionCMake 3.0 
Summary0009188: ADD_DEPENDENCIES silently accepts invalid target names
DescriptionTry
ADD_DEPENDENCIES(somefaketarget realtarget)

it works. It should probably not!
TagsNo tags attached.
Attached Filespatch file icon checkForExistingDependencyTarget.patch [^] (682 bytes) 2012-07-26 17:42 [Show Content]
patch file icon checkForExistingDependencyTargetWithPolicy.patch [^] (3,098 bytes) 2012-07-27 06:20 [Show Content]
patch file icon checkForDependingTargetThatDoesNotExistNowButWillBeAddedLater.patch [^] (2,977 bytes) 2012-07-27 13:54 [Show Content]
patch file icon finalCheckForDependingTargetThatDoesNotExistNowButWillBeAddedLater.patch [^] (3,052 bytes) 2012-08-08 16:15 [Show Content]

 Relationships
has duplicate 0012595closedBrad King Missing dependencies for imported libraries are being ignored 

  Notes
(0017573)
Brad King (manager)
2009-09-16 15:24

In CMake 2.6 and in CMake HEAD from CVS I get this error:

CMake Error at CMakeLists.txt:6 (add_dependencies):
  add_dependencies Adding dependency to non-existent target: somefaketarget

Annotating the source tells me this error message has been there since 2001.

The other direction:

  add_dependencies(realtarget somefaketarget)

silently works because somefaketarget may or may not be created later in the configuration process. This was an intentional decision.
(0017577)
Paul Oppenheim (Poppy Linden) (reporter)
2009-09-16 17:36

It sounds like the original intention is to error on invalid targets. The rationale for not erroring out at the exact moment of adding dependency on a non-existent target is sound. However, the correct behavior would then be to error out at the end of configuration, when the list of targets would be known to expand no further.
(0017579)
Brad King (manager)
2009-09-16 17:48

Yes, of course. However I guarantee there are many projects whose build will break if we start enforcing this because it wasn't checked from the start. We'll have to introduce a CMake Policy to maintain compatibility.
(0017580)
Paul Oppenheim (Poppy Linden) (reporter)
2009-09-16 17:54

I'm going to imagine Second Life will be one of them as well, but it's much better than the surprises associated with thinking it *should* work ;)

Thank you for your consideration!
(0024857)
David Cole (manager)
2011-01-18 11:16

Looking at "older" "feedback" status bugs today... is this still an issue that we want to address moving forward? If so, please remove the "feedback" status and just set it to "assigned"... If not, please resolve it.

Thanks.
(0024872)
Brad King (manager)
2011-01-18 11:42

Someday the policy mentioned in 0009188:0017579 should be added, but I have no time now. Changing back to "assigned".
(0027864)
Brad King (manager)
2011-11-28 15:50

Moving to backlog as I have no time to work on this.

As mentioned in the discussion in this issue the change in behavior will need to be implemented as a CMake Policy:

  http://www.cmake.org/Wiki/CMake/Policies [^]

The OLD behavior will be to silently accept the dangling dependency. The NEW behavior will be an error.

The implementation will probably be in cmComputeTargetDepends::AddTargetDepend.
(0027865)
Brad King (manager)
2011-11-28 15:53

Re 0009188:0027864: There is further difficulty in providing proper context for the broken dependency. We allow targets to be added in arbitrary order so the bad dependency cannot be detected until the generate step long after all commands have finished. Currently dependencies are recorded by cmTarget::AddUtility but that does not keep any information about the context of the add_dependencies call that added a given dependency. That information will need to be added so the error detected during generation can have proper CMake-language context.
(0027866)
devurandom (reporter)
2011-11-28 16:01

Until then I could create a wrapper function where I check the existence of the target, right?

if (NOT "${target}")
  message(FATAL_ERROR "Target ${target} does not exist")
endif (NOT "${target}")
... same for dependency ...
(0027867)
Brad King (manager)
2011-11-28 16:30

Re 0009188:0027866: That could work if you enforce adding targets in dependency order. The proper test would be

 if(NOT TARGET "${dependency}")

Note that add_dependencies already rejects adding outgoing dependencies to a missing target.
(0030131)
ClausKlein (reporter)
2012-07-26 17:43

my patch was tested here:
http://open.cdash.org/viewTest.php?onlypassed&buildid=2471127 [^]
Claus
(0030132)
Brad King (manager)
2012-07-27 08:13

Re 0009188:0030131: The patch in checkForExistingDependencyTargetWithPolicy.patch is just the earlier patch with a policy added. As explained in 0009188:0017573 and 0009188:0027864 the main description is incorrect and the real problem is this:

 add_dependencies(existing_target target_that_will_never_be_added)

CMake silently ignores the dependency if the RHS target is never created. However, we *do* support

 add_dependencies(existing_target target_that_does_not_exist_now_but_will_be_added_later)

and must continue to do so. The detection, policy, and error must be implemented somewhere around cmComputeTargetDepends::AddTargetDepend when all targets defined by the project are known. As explained in 0009188:0027865 more changes are required to provide context to the error/warning messages.
(0030133)
Rolf Eike Beer (developer)
2012-07-27 11:37

I think this should really be an error, no warning. Like trying to link something to a nonexisting target, this is also an error.
(0030134)
ClausKlein (reporter)
2012-07-27 13:53

Yes, IMO too

But this would be better than none Waring:

claus-kleins-macbook-pro:build clausklein$ /Users/clausklein/Downloads/CmakeBuildDir/bin/cmake -G Ninja ..
-- Configuring done

CMake does not support this but it used to work accidentally and is being allowed for compatibility.
Policy CMP0019 is not set: add_dependencies() Dependent targets must all exist. Run "cmake --help-policy CMP0019" for policy details. Use the cmake_policy command to set the policy and suppress this warning.
ftpcpp: Adding dependency to non-existent target: target_that_does_not_exist_now_but_will_be_added_later

-- Generating done
-- Build files have been written to: /Users/clausklein/Workspace/cpp/ftplibpp-2.0.2/build
claus-kleins-macbook-pro:build clausklein$
(0030135)
Brad King (manager)
2012-07-27 14:33

Re 0009188:0030133, 0009188:0030134: I'm not sure I'm coming across here because I've seen a bunch of discussion here and on the list since I posted 0009188:0030132. The error CANNOT be diagnosed at the add_dependencies call, EVER. It is wrong to do so.

We support

 add_executable(exe ...)
 target_link_libraries(exe lib_added_later)
 add_library(lib_added_later ...)

and also support

 add_executable(exe ...)
 add_dependencies(exe lib_added_later)
 add_library(lib_added_later ...)

Both cases add a dependency, the former also links. In both cases the command that connects the targets runs BEFORE the library exists. This is okay. This is allowed and will not change.

What needs to happen is that during GENERATION CMake must recognize that the project specified a dependency on a target that was never created. At this point the problem can be diagnosed and reported as an error back in the original context. The diagnostic will be conditional on a policy, but the NEW behavior will be an error.
(0030136)
Brad King (manager)
2012-07-27 14:40

Re 0009188:0030134: Oops, I looked at the wrong patch.

The change in checkForDependingTargetThatDoesNotExistNowButWillBeAddedLater.patch looks much better. While it may not appear to break the generation, what is the process return code?
(0030138)
ClausKlein (reporter)
2012-07-27 16:37

I do not know how to set the generation to Error state at this void function:

  void AddTargetDepend(int depender_index, cmTarget* dependee, bool linking);
(0030139)
Brad King (manager)
2012-07-27 16:41

Re 0009188:0030138: Just calling cmSystemTools::Error should cause the exit code of the CMake process to be non-zero when returning to the operating system. We do not abort generation outright because we want to generate as much of the build system as possible so that things like running "make" again will still work (for example to re-run CMake again).
(0030194)
ClausKlein (reporter)
2012-08-08 16:20

claus-kleins-macbook-pro:CmakeBuildDir clausklein$ bin/cmake --help-policy CMP0019
cmake version 2.8.8.20120807-gc598d3-dirty
  CMP0019
       add_dependencies(target-name depend-target1 depend-target2 ...)
       Dependent targets must all exist.

       In CMake 2.8.9 and above this policy determines whether or not the
       case is an error. The OLD behavior for this policy is to silently
       ignore the problem. The NEW behavior for this policy is to report an
       error.

       This policy was introduced in CMake version 2.8.9. CMake version
       2.8.8.20120807-gc598d3-dirty warns when the policy is not set and uses
       OLD behavior. Use the cmake_policy command to set it to OLD or NEW
       explicitly.

That works now as expected from me.
(0030693)
David Cole (manager)
2012-08-17 16:04

I tried this today on top of today's master, with Visual Studio 9 2008, and it had 15 test failures. Unfortunately, I don't have time to investigate the test failures right now, and cannot push this change to 'next' until somebody does have time to investigate them and figure out how to prevent them.

To run just the tests that failed, use the following:

  ctest -C Release -j 8 --output-on-failure -R "^(SystemInformationNew|TryCompile|ExportImport|CrossCompile|VSMidl|CTestTest|CTestTest2|CMakeOnly.LinkInterfaceLoop|CMakeOnly.CheckSymbolExists|CMakeOnly.CheckCXXSymbolExists|CMakeOnly.CheckCXXCompilerFlag|CMakeOnly.CheckLanguage|CMakeOnly.AllFindModules|CMakeOnly.ProjectInclude|RunCMake.ObjectLibrary)$"

Also, please use "git format-patch -1" to produce patch files for attaching to bug reports. That way, your authorship information will make it into the git repo eventually, and you'll get credit for the change, and be a part of CMake history.

Sorry for the bad news... wish I could work on it more today. Maybe a little more next week.

Cheers!
(0031072)
David Cole (manager)
2012-09-20 17:17

This patch is not going to get into CMake 2.8.10 unless somebody has time to address the failing tests that occur when the patch is applied.

Is somebody working on getting the tests to work?
(0031141)
David Cole (manager)
2012-10-01 15:16

The deadline has past for getting proposed changes into CMake 2.8.10-rc1: setting target version to 2.8.11
(0031635)
David Cole (manager)
2012-11-20 17:08

Is anybody going to work on getting the tests passing with a Visual Studio build anytime soon?

If not, I'll take this one off the roadmap and put it back in the backlog.

Please speak up if you can spend some time on this in the next 6 weeks...

Thanks.
(0031784)
David Cole (manager)
2012-11-30 13:30

Since there's been no response to my note in 0009188:0031635, I'm moving this back to the backlog. If anybody has time to work out the remaining kinks in this issue, let me know, and I'll put it back on the roadmap.
(0034930)
Nils Gladitz (developer)
2014-01-13 10:35

Fixed by http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=0bf6f13b;hp=cb7af7af44bd9ce5ac11e345b1756ea0770bbc83 [^]
(0036074)
Robert Maynard (manager)
2014-06-02 08:38

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

 Issue History
Date Modified Username Field Change
2009-06-19 18:49 Paul Oppenheim (Poppy Linden) New Issue
2009-09-14 12:31 Bill Hoffman Status new => assigned
2009-09-14 12:31 Bill Hoffman Assigned To => Brad King
2009-09-16 15:24 Brad King Note Added: 0017573
2009-09-16 15:24 Brad King Status assigned => closed
2009-09-16 15:24 Brad King Resolution open => unable to reproduce
2009-09-16 17:36 Paul Oppenheim (Poppy Linden) Note Added: 0017577
2009-09-16 17:36 Paul Oppenheim (Poppy Linden) Status closed => feedback
2009-09-16 17:36 Paul Oppenheim (Poppy Linden) Resolution unable to reproduce => reopened
2009-09-16 17:48 Brad King Note Added: 0017579
2009-09-16 17:54 Paul Oppenheim (Poppy Linden) Note Added: 0017580
2011-01-18 11:16 David Cole Note Added: 0024857
2011-01-18 11:42 Brad King Note Added: 0024872
2011-01-18 11:42 Brad King Status feedback => assigned
2011-11-28 15:47 Brad King Relationship added has duplicate 0012595
2011-11-28 15:50 Brad King Note Added: 0027864
2011-11-28 15:50 Brad King Assigned To Brad King =>
2011-11-28 15:50 Brad King Status assigned => backlog
2011-11-28 15:53 Brad King Note Added: 0027865
2011-11-28 16:01 devurandom Note Added: 0027866
2011-11-28 16:30 Brad King Note Added: 0027867
2012-07-26 17:42 ClausKlein File Added: checkForExistingDependencyTarget.patch
2012-07-26 17:43 ClausKlein Note Added: 0030131
2012-07-27 06:20 ClausKlein File Added: checkForExistingDependencyTargetWithPolicy.patch
2012-07-27 08:13 Brad King Note Added: 0030132
2012-07-27 11:37 Rolf Eike Beer Note Added: 0030133
2012-07-27 13:53 ClausKlein Note Added: 0030134
2012-07-27 13:54 ClausKlein File Added: checkForDependingTargetThatDoesNotExistNowButWillBeAddedLater.patch
2012-07-27 14:33 Brad King Note Added: 0030135
2012-07-27 14:40 Brad King Note Added: 0030136
2012-07-27 16:37 ClausKlein Note Added: 0030138
2012-07-27 16:41 Brad King Note Added: 0030139
2012-08-08 16:15 ClausKlein File Added: finalCheckForDependingTargetThatDoesNotExistNowButWillBeAddedLater.patch
2012-08-08 16:20 ClausKlein Note Added: 0030194
2012-08-12 08:11 David Cole Assigned To => David Cole
2012-08-12 08:11 David Cole Status backlog => assigned
2012-08-12 08:11 David Cole Target Version => CMake 2.8.10
2012-08-17 16:04 David Cole Note Added: 0030693
2012-09-20 17:17 David Cole Note Added: 0031072
2012-10-01 15:16 David Cole Note Added: 0031141
2012-10-01 15:16 David Cole Target Version CMake 2.8.10 => CMake 2.8.11
2012-11-20 17:08 David Cole Note Added: 0031635
2012-11-30 13:30 David Cole Note Added: 0031784
2012-11-30 13:30 David Cole Assigned To David Cole =>
2012-11-30 13:30 David Cole Status assigned => backlog
2013-05-17 09:33 Robert Maynard Target Version CMake 2.8.11 => CMake 2.8.12
2014-01-13 10:35 Nils Gladitz Note Added: 0034930
2014-01-13 10:35 Nils Gladitz Status backlog => resolved
2014-01-13 10:35 Nils Gladitz Fixed in Version => CMake 3.0
2014-01-13 10:35 Nils Gladitz Resolution reopened => fixed
2014-01-13 10:35 Nils Gladitz Assigned To => Nils Gladitz
2014-06-02 08:38 Robert Maynard Note Added: 0036074
2014-06-02 08:38 Robert Maynard Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team