From 7be2617b5a52529ce0ca33e6c878a0294e1e2781 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 20 Aug 2010 14:11:18 -0400 Subject: [PATCH 1/3] Split notion of node lists and edge lists --- Source/cmComputeComponentGraph.cxx | 8 ++++---- Source/cmComputeComponentGraph.h | 3 ++- Source/cmComputeLinkDepends.cxx | 20 ++++++++++---------- Source/cmComputeLinkDepends.h | 1 + Source/cmComputeTargetDepends.cxx | 16 ++++++++-------- Source/cmComputeTargetDepends.h | 1 + Source/cmGraphAdjacencyList.h | 3 ++- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/Source/cmComputeComponentGraph.cxx b/Source/cmComputeComponentGraph.cxx index 3f2a361dc..165af1042 100644 --- a/Source/cmComputeComponentGraph.cxx +++ b/Source/cmComputeComponentGraph.cxx @@ -71,8 +71,8 @@ void cmComputeComponentGraph::TarjanVisit(int i) this->TarjanStack.push(i); // Follow outgoing edges. - NodeList const& nl = this->InputGraph[i]; - for(NodeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) + EdgeList const& nl = this->InputGraph[i]; + for(EdgeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) { int j = *ni; @@ -142,8 +142,8 @@ void cmComputeComponentGraph::TransferEdges() for(int i=0; i < n; ++i) { int i_component = this->TarjanComponents[i]; - NodeList const& nl = this->InputGraph[i]; - for(NodeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) + EdgeList const& nl = this->InputGraph[i]; + for(EdgeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) { int j = *ni; int j_component = this->TarjanComponents[j]; diff --git a/Source/cmComputeComponentGraph.h b/Source/cmComputeComponentGraph.h index 855a141f4..a2ce946c1 100644 --- a/Source/cmComputeComponentGraph.h +++ b/Source/cmComputeComponentGraph.h @@ -33,6 +33,7 @@ class cmComputeComponentGraph public: // Represent the graph with an adjacency list. typedef cmGraphNodeList NodeList; + typedef cmGraphEdgeList EdgeList; typedef cmGraphAdjacencyList Graph; cmComputeComponentGraph(Graph const& input); @@ -41,7 +42,7 @@ public: /** Get the adjacency list of the component graph. */ Graph const& GetComponentGraph() const { return this->ComponentGraph; } - NodeList const& GetComponentGraphEdges(int c) const + EdgeList const& GetComponentGraphEdges(int c) const { return this->ComponentGraph[c]; } /** Get map from component index to original node indices. */ diff --git a/Source/cmComputeLinkDepends.cxx b/Source/cmComputeLinkDepends.cxx index 24410ec3a..4b0214e2f 100644 --- a/Source/cmComputeLinkDepends.cxx +++ b/Source/cmComputeLinkDepends.cxx @@ -285,7 +285,7 @@ cmComputeLinkDepends::AllocateLinkEntry(std::string const& item) lei = this->LinkEntryIndex.insert(index_entry).first; this->EntryList.push_back(LinkEntry()); this->InferredDependSets.push_back(0); - this->EntryConstraintGraph.push_back(NodeList()); + this->EntryConstraintGraph.push_back(EdgeList()); return lei; } @@ -669,7 +669,7 @@ void cmComputeLinkDepends::CleanConstraintGraph() cmsys_stl::sort(i->begin(), i->end()); // Make the edge list unique. - NodeList::iterator last = cmsys_stl::unique(i->begin(), i->end()); + EdgeList::iterator last = cmsys_stl::unique(i->begin(), i->end()); i->erase(last, i->end()); } } @@ -681,9 +681,9 @@ void cmComputeLinkDepends::DisplayConstraintGraph() cmOStringStream e; for(unsigned int i=0; i < this->EntryConstraintGraph.size(); ++i) { - NodeList const& nl = this->EntryConstraintGraph[i]; + EdgeList const& nl = this->EntryConstraintGraph[i]; e << "item " << i << " is [" << this->EntryList[i].Item << "]\n"; - for(NodeList::const_iterator j = nl.begin(); j != nl.end(); ++j) + for(EdgeList::const_iterator j = nl.begin(); j != nl.end(); ++j) { e << " item " << *j << " must follow it\n"; } @@ -758,8 +758,8 @@ cmComputeLinkDepends::DisplayComponents() fprintf(stderr, " item %d [%s]\n", i, this->EntryList[i].Item.c_str()); } - NodeList const& ol = this->CCG->GetComponentGraphEdges(c); - for(NodeList::const_iterator oi = ol.begin(); oi != ol.end(); ++oi) + EdgeList const& ol = this->CCG->GetComponentGraphEdges(c); + for(EdgeList::const_iterator oi = ol.begin(); oi != ol.end(); ++oi) { fprintf(stderr, " followed by Component (%d)\n", *oi); } @@ -784,8 +784,8 @@ void cmComputeLinkDepends::VisitComponent(unsigned int c) // Visit the neighbors of the component first. // Run in reverse order so the topological order will preserve the // original order where there are no constraints. - NodeList const& nl = this->CCG->GetComponentGraphEdges(c); - for(NodeList::const_reverse_iterator ni = nl.rbegin(); + EdgeList const& nl = this->CCG->GetComponentGraphEdges(c); + for(EdgeList::const_reverse_iterator ni = nl.rbegin(); ni != nl.rend(); ++ni) { this->VisitComponent(*ni); @@ -856,8 +856,8 @@ void cmComputeLinkDepends::VisitEntry(int index) // are now pending. if(completed) { - NodeList const& ol = this->CCG->GetComponentGraphEdges(component); - for(NodeList::const_iterator oi = ol.begin(); oi != ol.end(); ++oi) + EdgeList const& ol = this->CCG->GetComponentGraphEdges(component); + for(EdgeList::const_iterator oi = ol.begin(); oi != ol.end(); ++oi) { // This entire component is now pending no matter whether it has // been partially seen already. diff --git a/Source/cmComputeLinkDepends.h b/Source/cmComputeLinkDepends.h index a08afb69c..e196e00ca 100644 --- a/Source/cmComputeLinkDepends.h +++ b/Source/cmComputeLinkDepends.h @@ -117,6 +117,7 @@ private: // Ordering constraint graph adjacency list. typedef cmGraphNodeList NodeList; + typedef cmGraphEdgeList EdgeList; typedef cmGraphAdjacencyList Graph; Graph EntryConstraintGraph; void CleanConstraintGraph(); diff --git a/Source/cmComputeTargetDepends.cxx b/Source/cmComputeTargetDepends.cxx index 94b8527f0..e82606cab 100644 --- a/Source/cmComputeTargetDepends.cxx +++ b/Source/cmComputeTargetDepends.cxx @@ -150,8 +150,8 @@ cmComputeTargetDepends::GetTargetDirectDepends(cmTarget* t, int i = tii->second; // Get its final dependencies. - NodeList const& nl = this->FinalGraph[i]; - for(NodeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) + EdgeList const& nl = this->FinalGraph[i]; + for(EdgeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) { deps.insert(this->Targets[*ni]); } @@ -283,11 +283,11 @@ cmComputeTargetDepends::DisplayGraph(Graph const& graph, const char* name) int n = static_cast(graph.size()); for(int depender_index = 0; depender_index < n; ++depender_index) { - NodeList const& nl = graph[depender_index]; + EdgeList const& nl = graph[depender_index]; cmTarget* depender = this->Targets[depender_index]; fprintf(stderr, "target %d is [%s]\n", depender_index, depender->GetName()); - for(NodeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) + for(EdgeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) { int dependee_index = *ni; cmTarget* dependee = this->Targets[dependee_index]; @@ -383,8 +383,8 @@ cmComputeTargetDepends << cmTarget::TargetTypeNames[depender->GetType()] << "\n"; // List its dependencies that are inside the component. - NodeList const& nl = this->InitialGraph[i]; - for(NodeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) + EdgeList const& nl = this->InitialGraph[i]; + for(EdgeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) { int j = *ni; if(cmap[j] == c) @@ -425,8 +425,8 @@ cmComputeTargetDepends for(int depender_component=0; depender_component < n; ++depender_component) { int depender_component_tail = components[depender_component].back(); - NodeList const& nl = cgraph[depender_component]; - for(NodeList::const_iterator ni = nl.begin(); ni != nl.end(); ++ni) + 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(); diff --git a/Source/cmComputeTargetDepends.h b/Source/cmComputeTargetDepends.h index 68e3e47cb..b18a053dc 100644 --- a/Source/cmComputeTargetDepends.h +++ b/Source/cmComputeTargetDepends.h @@ -59,6 +59,7 @@ private: // top-level index corresponds to a depender whose dependencies are // listed. typedef cmGraphNodeList NodeList; + typedef cmGraphEdgeList EdgeList; typedef cmGraphAdjacencyList Graph; Graph InitialGraph; Graph FinalGraph; diff --git a/Source/cmGraphAdjacencyList.h b/Source/cmGraphAdjacencyList.h index 779484012..41411c42b 100644 --- a/Source/cmGraphAdjacencyList.h +++ b/Source/cmGraphAdjacencyList.h @@ -14,7 +14,8 @@ #include "cmStandardIncludes.h" +struct cmGraphEdgeList: public std::vector {}; struct cmGraphNodeList: public std::vector {}; -struct cmGraphAdjacencyList: public std::vector {}; +struct cmGraphAdjacencyList: public std::vector {}; #endif From 681cf011dde81c08c0404569289110f9585c6daf Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 25 Aug 2010 10:07:25 -0400 Subject: [PATCH 2/3] Distinguish "strong" and "weak" target dependency edges Utility dependencies are "strong" because they must be enforced to generate a working build. Link dependencies are "weak" because they can be broken in the case of a static library cycle. --- Source/cmComputeComponentGraph.cxx | 5 ++++- Source/cmComputeLinkDepends.cxx | 3 ++- Source/cmComputeTargetDepends.cxx | 26 ++++++++++++++++---------- Source/cmGraphAdjacencyList.h | 21 ++++++++++++++++++++- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/Source/cmComputeComponentGraph.cxx b/Source/cmComputeComponentGraph.cxx index 165af1042..5bec6a1ec 100644 --- a/Source/cmComputeComponentGraph.cxx +++ b/Source/cmComputeComponentGraph.cxx @@ -149,7 +149,10 @@ void cmComputeComponentGraph::TransferEdges() int j_component = this->TarjanComponents[j]; if(i_component != j_component) { - this->ComponentGraph[i_component].push_back(j_component); + // We do not attempt to combine duplicate edges, but instead + // store the inter-component edges with suitable multiplicity. + this->ComponentGraph[i_component].push_back( + cmGraphEdge(j_component, ni->IsStrong())); } } } diff --git a/Source/cmComputeLinkDepends.cxx b/Source/cmComputeLinkDepends.cxx index 4b0214e2f..342c217d8 100644 --- a/Source/cmComputeLinkDepends.cxx +++ b/Source/cmComputeLinkDepends.cxx @@ -761,7 +761,8 @@ cmComputeLinkDepends::DisplayComponents() EdgeList const& ol = this->CCG->GetComponentGraphEdges(c); for(EdgeList::const_iterator oi = ol.begin(); oi != ol.end(); ++oi) { - fprintf(stderr, " followed by Component (%d)\n", *oi); + int i = *oi; + fprintf(stderr, " followed by Component (%d)\n", i); } fprintf(stderr, " topo order index %d\n", this->ComponentOrder[c]); diff --git a/Source/cmComputeTargetDepends.cxx b/Source/cmComputeTargetDepends.cxx index e82606cab..2b6b5ecc8 100644 --- a/Source/cmComputeTargetDepends.cxx +++ b/Source/cmComputeTargetDepends.cxx @@ -195,15 +195,13 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) // Get the depender. cmTarget* depender = this->Targets[depender_index]; - // Keep track of dependencies already listed. - std::set emitted; - - // A target should not depend on itself. - emitted.insert(depender->GetName()); - // Loop over all targets linked directly. + { cmTarget::LinkLibraryVectorType const& tlibs = depender->GetOriginalLinkLibraries(); + std::set emitted; + // A target should not depend on itself. + emitted.insert(depender->GetName()); for(cmTarget::LinkLibraryVectorType::const_iterator lib = tlibs.begin(); lib != tlibs.end(); ++lib) { @@ -213,9 +211,14 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) this->AddTargetDepend(depender_index, lib->first.c_str(), true); } } + } // Loop over all utility dependencies. + { std::set const& tutils = depender->GetUtilities(); + std::set emitted; + // A target should not depend on itself. + emitted.insert(depender->GetName()); for(std::set::const_iterator util = tutils.begin(); util != tutils.end(); ++util) { @@ -225,6 +228,7 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) this->AddTargetDepend(depender_index, util->c_str(), false); } } + } } //---------------------------------------------------------------------------- @@ -272,7 +276,8 @@ void cmComputeTargetDepends::AddTargetDepend(int depender_index, int dependee_index = tii->second; // Add this entry to the dependency graph. - this->InitialGraph[depender_index].push_back(dependee_index); + this->InitialGraph[depender_index].push_back( + cmGraphEdge(dependee_index, !linking)); } //---------------------------------------------------------------------------- @@ -291,8 +296,8 @@ cmComputeTargetDepends::DisplayGraph(Graph const& graph, const char* name) { int dependee_index = *ni; cmTarget* dependee = this->Targets[dependee_index]; - fprintf(stderr, " depends on target %d [%s]\n", dependee_index, - dependee->GetName()); + fprintf(stderr, " depends on target %d [%s] (%s)\n", dependee_index, + dependee->GetName(), ni->IsStrong()? "strong" : "weak"); } } fprintf(stderr, "\n"); @@ -390,7 +395,8 @@ cmComputeTargetDepends if(cmap[j] == c) { cmTarget* dependee = this->Targets[j]; - e << " depends on \"" << dependee->GetName() << "\"\n"; + e << " depends on \"" << dependee->GetName() << "\"" + << " (" << (ni->IsStrong()? "strong" : "weak") << ")\n"; } } } diff --git a/Source/cmGraphAdjacencyList.h b/Source/cmGraphAdjacencyList.h index 41411c42b..0149d33bd 100644 --- a/Source/cmGraphAdjacencyList.h +++ b/Source/cmGraphAdjacencyList.h @@ -14,7 +14,26 @@ #include "cmStandardIncludes.h" -struct cmGraphEdgeList: public std::vector {}; +/** + * Graph edge representation. Most use cases just need the + * destination vertex, so we support conversion to/from an int. We + * also store boolean to indicate whether an edge is "strong". + */ +class cmGraphEdge +{ +public: + cmGraphEdge(): Dest(0), Strong(true) {} + cmGraphEdge(int n): Dest(n), Strong(true) {} + cmGraphEdge(int n, bool s): Dest(n), Strong(s) {} + cmGraphEdge(cmGraphEdge const& r): Dest(r.Dest), Strong(r.Strong) {} + operator int() const { return this->Dest; } + + bool IsStrong() const { return this->Strong; } +private: + int Dest; + bool Strong; +}; +struct cmGraphEdgeList: public std::vector {}; struct cmGraphNodeList: public std::vector {}; struct cmGraphAdjacencyList: public std::vector {}; From adb58d5e36e482e7cc2c0b1a37d94b21da9234df Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 25 Aug 2010 10:14:31 -0400 Subject: [PATCH 3/3] 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 | 107 +++++++++++++++++++++------ Source/cmComputeTargetDepends.h | 10 ++- Tests/Dependency/Four/CMakeLists.txt | 3 + Tests/Dependency/Four/FourSrc.c | 1 + 4 files changed, 97 insertions(+), 24 deletions(-) diff --git a/Source/cmComputeTargetDepends.cxx b/Source/cmComputeTargetDepends.cxx index 2b6b5ecc8..313c680e5 100644 --- a/Source/cmComputeTargetDepends.cxx +++ b/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 const& cmap, int c, int i, int* head, + std::set& emitted, std::set& 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 const& cmap = ccg.GetComponentMap(); + this->ComponentHead.resize(components.size()); + this->ComponentTail.resize(components.size()); + int nc = static_cast(components.size()); + for(int c=0; c < nc; ++c) + { + int head = -1; + std::set emitted; + NodeList const& nl = components[c]; + for(NodeList::const_reverse_iterator ni = nl.rbegin(); + ni != nl.rend(); ++ni) + { + std::set 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(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(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; } diff --git a/Source/cmComputeTargetDepends.h b/Source/cmComputeTargetDepends.h index b18a053dc..240de7670 100644 --- a/Source/cmComputeTargetDepends.h +++ b/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 ComponentHead; + std::vector ComponentTail; + bool IntraComponent(std::vector const& cmap, int c, int i, int* head, + std::set& emitted, std::set& visited); }; #endif diff --git a/Tests/Dependency/Four/CMakeLists.txt b/Tests/Dependency/Four/CMakeLists.txt index ba3711fe6..df0f1624e 100644 --- a/Tests/Dependency/Four/CMakeLists.txt +++ b/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) diff --git a/Tests/Dependency/Four/FourSrc.c b/Tests/Dependency/Four/FourSrc.c index e8fefcda2..23a66a49a 100644 --- a/Tests/Dependency/Four/FourSrc.c +++ b/Tests/Dependency/Four/FourSrc.c @@ -1,3 +1,4 @@ +#include /* Requires TwoCustom to be built first. */ void NoDepAFunction(); void OneFunction(); void TwoFunction();