Ninja: Fix inter-target order-only dependencies of custom commands

Custom command dependencies are followed for each target's source files
and add their transitive closure to the corresponding target.  This
means that when a custom command in one target has a dependency on a
custom command in another target, both will appear in the dependent
target's sources.  For the Makefile, VS IDE, and Xcode generators this
is not a problem because each target gets its own independent build
system that is evaluated in target dependency order.  By the time the
dependent target is built the custom command that belongs to one of its
dependencies will already have been brought up to date.

For the Ninja generator we need to generate a monolithic build system
covering all targets so we can have only one copy of a custom command.
This means that we need to reconcile the target-level ordering
dependencies from its appearance in multiple targets to include only the
least-dependent common set.  This is done by computing the set
intersection of the dependencies of all the targets containing a custom
command.  However, we previously included only the direct dependencies
so any target-level dependency not directly added to all targets into
which a custom command propagates was discarded.

Fix this by computing the transitive closure of dependencies for each
target and then intersecting those sets.  That will get the common set
of dependencies.  Also add a test to cover a case in which the
incorrectly dropped target ordering dependencies would fail.

3 files changed, 52 insertions, 2 deletions

diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx
index 91f08e6..51175c7 100644
--- a/Source/cmGlobalNinjaGenerator.cxx
+++ b/Source/cmGlobalNinjaGenerator.cxx
@@ -493,6 +493,8 @@ void cmGlobalNinjaGenerator::Generate()
   this->OpenBuildFileStream();
   this->OpenRulesFileStream();
 
+  this->TargetDependsClosures.clear();
+
   this->InitOutputPathPrefix();
   this->TargetAll = this->NinjaOutputPath("all");
   this->CMakeCacheFile = this->NinjaOutputPath("CMakeCache.txt");
@@ -905,6 +907,42 @@ void cmGlobalNinjaGenerator::AppendTargetDepends(
   }
 }
 
+void cmGlobalNinjaGenerator::AppendTargetDependsClosure(
+  cmGeneratorTarget const* target, cmNinjaDeps& outputs)
+{
+  TargetDependsClosureMap::iterator i =
+    this->TargetDependsClosures.find(target);
+  if (i == this->TargetDependsClosures.end()) {
+    TargetDependsClosureMap::value_type e(
+      target, std::set<cmGeneratorTarget const*>());
+    i = this->TargetDependsClosures.insert(e).first;
+    this->ComputeTargetDependsClosure(target, i->second);
+  }
+  std::set<cmGeneratorTarget const*> const& targets = i->second;
+  cmNinjaDeps outs;
+  for (std::set<cmGeneratorTarget const*>::const_iterator ti = targets.begin();
+       ti != targets.end(); ++ti) {
+    this->AppendTargetOutputs(*ti, outs);
+  }
+  std::sort(outs.begin(), outs.end());
+  outputs.insert(outputs.end(), outs.begin(), outs.end());
+}
+
+void cmGlobalNinjaGenerator::ComputeTargetDependsClosure(
+  cmGeneratorTarget const* target, std::set<cmGeneratorTarget const*>& depends)
+{
+  cmTargetDependSet const& targetDeps = this->GetTargetDirectDepends(target);
+  for (cmTargetDependSet::const_iterator i = targetDeps.begin();
+       i != targetDeps.end(); ++i) {
+    if ((*i)->GetType() == cmState::INTERFACE_LIBRARY) {
+      continue;
+    }
+    if (depends.insert(*i).second) {
+      this->ComputeTargetDependsClosure(*i, depends);
+    }
+  }
+}
+
 void cmGlobalNinjaGenerator::AddTargetAlias(const std::string& alias,
                                             cmGeneratorTarget* target)
 {
diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h
index c74973e..52fa5c9 100644
--- a/Source/cmGlobalNinjaGenerator.h
+++ b/Source/cmGlobalNinjaGenerator.h
@@ -301,6 +301,8 @@ public:
                            cmNinjaDeps& outputs);
   void AppendTargetDepends(cmGeneratorTarget const* target,
                            cmNinjaDeps& outputs);
+  void AppendTargetDependsClosure(cmGeneratorTarget const* target,
+                                  cmNinjaDeps& outputs);
   void AddDependencyToAll(cmGeneratorTarget* target);
   void AddDependencyToAll(const std::string& input);
 
@@ -361,6 +363,10 @@ private:
   void WriteTargetClean(std::ostream& os);
   void WriteTargetHelp(std::ostream& os);
 
+  void ComputeTargetDependsClosure(
+    cmGeneratorTarget const* target,
+    std::set<cmGeneratorTarget const*>& depends);
+
   std::string ninjaCmd() const;
 
   /// The file containing the build statement. (the relationship of the
@@ -410,6 +416,11 @@ private:
   typedef std::map<std::string, cmGeneratorTarget*> TargetAliasMap;
   TargetAliasMap TargetAliases;
 
+  typedef std::map<cmGeneratorTarget const*,
+                   std::set<cmGeneratorTarget const*> >
+    TargetDependsClosureMap;
+  TargetDependsClosureMap TargetDependsClosures;
+
   std::string NinjaCommand;
   std::string NinjaVersion;
 
diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx
index 0f488a6..46d7e18 100644
--- a/Source/cmLocalNinjaGenerator.cxx
+++ b/Source/cmLocalNinjaGenerator.cxx
@@ -450,13 +450,14 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatements()
     std::set<cmGeneratorTarget*>::iterator j = i->second.begin();
     assert(j != i->second.end());
     std::vector<std::string> ccTargetDeps;
-    this->AppendTargetDepends(*j, ccTargetDeps);
+    this->GetGlobalNinjaGenerator()->AppendTargetDependsClosure(*j,
+                                                                ccTargetDeps);
     std::sort(ccTargetDeps.begin(), ccTargetDeps.end());
     ++j;
 
     for (; j != i->second.end(); ++j) {
       std::vector<std::string> jDeps, depsIntersection;
-      this->AppendTargetDepends(*j, jDeps);
+      this->GetGlobalNinjaGenerator()->AppendTargetDependsClosure(*j, jDeps);
       std::sort(jDeps.begin(), jDeps.end());
       std::set_intersection(ccTargetDeps.begin(), ccTargetDeps.end(),
                             jDeps.begin(), jDeps.end(),