MantisBT - CMake
View Issue Details
0015454CMakeCMakepublic2015-03-17 20:192015-11-02 09:13
Daniel Dunbar 
Brad King 
normalminoralways
closedfixed 
CMake 3.2.1 
CMake 3.3CMake 3.3 
0015454: Ninja generator should not generate phony commands with cyclic dependency
The Ninja generator frequently will generate commands of the form:
--
build testCCompiler.c: phony testCCompiler.c
--
(taken from the build.ninja generated for the initial "-- Check for working C compiler using: Ninja" step).

This command specifies a cyclic dependency, even though Ninja itself does not generally error on it:
  https://github.com/martine/ninja/issues/935 [^]
Here is a self contained example:
--
$ cat CMakeLists.txt
cmake_minimum_required(VERSION 3.0)
add_custom_command(OUTPUT foo.tmp DEPENDS foo.c COMMAND true)
add_custom_target(foo DEPENDS foo.tmp)
$ cmake -GNinja .
-- Configuring done
-- Generating done
-- Build files have been written to: /tmp/foo
$ grep "foo.c:" build.ninja
build foo.c: phony foo.c
--
No tags attached.
related to 0014963closed Brad King Add explicit specification of custom command side effect outputs 
Issue History
2015-03-17 20:19Daniel DunbarNew Issue
2015-03-18 08:37Brad KingNote Added: 0038234
2015-03-18 08:37Brad KingRelationship addedrelated to 0014963
2015-03-18 11:23Daniel DunbarNote Added: 0038236
2015-03-18 13:05Brad KingNote Added: 0038237
2015-03-18 13:05Brad KingAssigned To => Brad King
2015-03-18 13:05Brad KingStatusnew => resolved
2015-03-18 13:05Brad KingResolutionopen => fixed
2015-03-18 13:05Brad KingFixed in Version => CMake 3.3
2015-03-18 13:05Brad KingTarget Version => CMake 3.3
2015-11-02 09:13Robert MaynardNote Added: 0039736
2015-11-02 09:13Robert MaynardStatusresolved => closed

Notes
(0038234)
Brad King   
2015-03-18 08:37   
Those rules are necessary to generate working Ninja build files implementing CMake build system semantics so long as this Ninja issue remains unfixed:

 https://github.com/martine/ninja/issues/760 [^]
 https://github.com/martine/ninja/issues/760#issuecomment-46540858 [^]

See discussion in 0014963, 0014747, and 0014972 also.
(0038236)
Daniel Dunbar   
2015-03-18 11:23   
I understand why the phony rule is present (I think), but it wasn't clear to me that it needs to be cyclic, i.e.:
--
build out: phony out
--
as opposed to simply:
--
build out: phony
--

Are you saying that the workaround only works if the output node is also listed as an input? From my understanding of the Ninja source, I am having trouble seeing how that can make a difference, because it seems like in the situations where Ninja would actually recognize that it would also recognize and error out on the cyclic dependency.
(0038237)
Brad King   
2015-03-18 13:05   
Re 0015454:0038236: Okay, I see. It looks like the circular dependency has been there since these phony rules were first added:

 Ninja: Custom Command file depends don't need to exist before building
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=539356f1 [^]

Fixed:

 Ninja: Do not generate circular phony rules
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=80afe28a [^]
(0039736)
Robert Maynard   
2015-11-02 09:13   
Closing resolved issues that have not been updated in more than 4 months.