Honor strong intra-component target dependencies

Strong dependencies (created by add_dependencies) must be honored when
linearizing a strongly-connected component of the target dependency
graph.  The initial graph edges have strong/weak labels and can contain
cycles that do not consist exclusively of strong edges.  The final graph
never contains cycles so all edges can be strong.

Source/cmComputeTargetDepends.cxx

@@ -129,7 +129,10 @@ bool cmComputeTargetDepends::Compute()
     }
 
   // Compute the final dependency graph.
-  this->ComputeFinalDepends(ccg);
+  if(!this->ComputeFinalDepends(ccg))
+    {
+    return false;
+    }
   if(this->DebugMode)
     {
     this->DisplayGraph(this->FinalGraph, "final");
@@ -368,7 +371,8 @@ cmComputeTargetDepends
 //----------------------------------------------------------------------------
 void
 cmComputeTargetDepends
-::ComplainAboutBadComponent(cmComputeComponentGraph const& ccg, int c)
+::ComplainAboutBadComponent(cmComputeComponentGraph const& ccg, int c,
+                            bool strong)
 {
   // Construct the error message.
   cmOStringStream e;
@@ -400,7 +404,15 @@ cmComputeTargetDepends
         }
       }
     }
-  if(this->NoCycles)
+  if(strong)
+    {
+    // Custom command executable dependencies cannot occur within a
+    // component of static libraries.  The cycle must appear in calls
+    // to add_dependencies.
+    e << "The component contains at least one cycle consisting of strong "
+      << "dependencies (created by add_dependencies) that cannot be broken.";
+    }
+  else if(this->NoCycles)
     {
     e << "The GLOBAL_DEPENDS_NO_CYCLES global property is enabled, so "
       << "cyclic dependencies are not allowed even among static libraries.";
@@ -414,7 +426,49 @@ cmComputeTargetDepends
 }
 
 //----------------------------------------------------------------------------
-void
+bool
+cmComputeTargetDepends
+::IntraComponent(std::vector<int> const& cmap, int c, int i, int* head,
+                 std::set<int>& emitted, std::set<int>& visited)
+{
+  if(!visited.insert(i).second)
+    {
+    // Cycle in utility depends!
+    return false;
+    }
+  if(emitted.insert(i).second)
+    {
+    // Honor strong intra-component edges in the final order.
+    EdgeList const& el = this->InitialGraph[i];
+    for(EdgeList::const_iterator ei = el.begin(); ei != el.end(); ++ei)
+      {
+      int j = *ei;
+      if(cmap[j] == c && ei->IsStrong())
+        {
+        this->FinalGraph[i].push_back(j);
+        if(!this->IntraComponent(cmap, c, j, head, emitted, visited))
+          {
+          return false;
+          }
+        }
+      }
+
+    // Prepend to a linear linked-list of intra-component edges.
+    if(*head >= 0)
+      {
+      this->FinalGraph[i].push_back(*head);
+      }
+    else
+      {
+      this->ComponentTail[c] = i;
+      }
+    *head = i;
+    }
+  return true;
+}
+
+//----------------------------------------------------------------------------
+bool
 cmComputeTargetDepends
 ::ComputeFinalDepends(cmComputeComponentGraph const& ccg)
 {
@@ -426,34 +480,43 @@ cmComputeTargetDepends
   this->FinalGraph.resize(0);
   this->FinalGraph.resize(this->InitialGraph.size());
 
+  // Choose intra-component edges to linearize dependencies.
+  std::vector<int> const& cmap = ccg.GetComponentMap();
+  this->ComponentHead.resize(components.size());
+  this->ComponentTail.resize(components.size());
+  int nc = static_cast<int>(components.size());
+  for(int c=0; c < nc; ++c)
+    {
+    int head = -1;
+    std::set<int> emitted;
+    NodeList const& nl = components[c];
+    for(NodeList::const_reverse_iterator ni = nl.rbegin();
+        ni != nl.rend(); ++ni)
+      {
+      std::set<int> visited;
+      if(!this->IntraComponent(cmap, c, *ni, &head, emitted, visited))
+        {
+        // Cycle in add_dependencies within component!
+        this->ComplainAboutBadComponent(ccg, c, true);
+        return false;
+        }
+      }
+    this->ComponentHead[c] = head;
+    }
+
   // Convert inter-component edges to connect component tails to heads.
   int n = static_cast<int>(cgraph.size());
   for(int depender_component=0; depender_component < n; ++depender_component)
     {
-    int depender_component_tail = components[depender_component].back();
+    int depender_component_tail = this->ComponentTail[depender_component];
     EdgeList const& nl = cgraph[depender_component];
     for(EdgeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni)
       {
       int dependee_component = *ni;
-      int dependee_component_head = components[dependee_component].front();
+      int dependee_component_head = this->ComponentHead[dependee_component];
       this->FinalGraph[depender_component_tail]
         .push_back(dependee_component_head);
       }
     }
-
-  // Compute intra-component edges.
-  int nc = static_cast<int>(components.size());
-  for(int c=0; c < nc; ++c)
-    {
-    // Within the component each target depends on that following it.
-    NodeList const& nl = components[c];
-    NodeList::const_iterator ni = nl.begin();
-    int last_i = *ni;
-    for(++ni; ni != nl.end(); ++ni)
-      {
-      int i = *ni;
-      this->FinalGraph[last_i].push_back(i);
-      last_i = i;
-      }
-    }
+  return true;
 }

Source/cmComputeTargetDepends.h

@@ -45,7 +45,7 @@ private:
   void CollectTargetDepends(int depender_index);
   void AddTargetDepend(int depender_index, const char* dependee_name,
                        bool linking);
-  void ComputeFinalDepends(cmComputeComponentGraph const& ccg);
+  bool ComputeFinalDepends(cmComputeComponentGraph const& ccg);
 
   cmGlobalGenerator* GlobalGenerator;
   bool DebugMode;
@@ -68,7 +68,13 @@ private:
   // Deal with connected components.
   void DisplayComponents(cmComputeComponentGraph const& ccg);
   bool CheckComponents(cmComputeComponentGraph const& ccg);
-  void ComplainAboutBadComponent(cmComputeComponentGraph const& ccg, int c);
+  void ComplainAboutBadComponent(cmComputeComponentGraph const& ccg, int c,
+                                 bool strong = false);
+
+  std::vector<int> ComponentHead;
+  std::vector<int> ComponentTail;
+  bool IntraComponent(std::vector<int> const& cmap, int c, int i, int* head,
+                      std::set<int>& emitted, std::set<int>& visited);
 };
 
 #endif

Tests/Dependency/Four/CMakeLists.txt

@@ -1,3 +1,6 @@
+INCLUDE_DIRECTORIES(${Dependency_BINARY_DIR}/Two)
 ADD_LIBRARY( Four FourSrc.c )
 TARGET_LINK_LIBRARIES( Four One Two NoDepA )
 
+# TwoCustom must build before Four.
+ADD_DEPENDENCIES(Four TwoCustom)

Tests/Dependency/Four/FourSrc.c

@@ -1,3 +1,4 @@
+#include <two-test.h> /* Requires TwoCustom to be built first.  */
 void NoDepAFunction();
 void OneFunction();
 void TwoFunction();