diff --git a/internal/dag/dag.go b/internal/dag/dag.go index 5aca57944..6da10df51 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -88,9 +88,7 @@ func (g *AcyclicGraph) Root() (Vertex, error) { // same graph with only a single edge between A and B, and a single edge // between B and C. // -// The graph must be valid for this operation to behave properly. If -// Validate() returns an error, the behavior is undefined and the results -// will likely be unexpected. +// The graph must be free of cycles for this operation to behave properly. // // Complexity: O(V(V+E)), or asymptotically O(VE) func (g *AcyclicGraph) TransitiveReduction() { @@ -145,6 +143,8 @@ func (g *AcyclicGraph) Validate() error { return err } +// Cycles reports any cycles between graph nodes. +// Self-referencing nodes are not reported, and must be detected separately. func (g *AcyclicGraph) Cycles() [][]Vertex { var cycles [][]Vertex for _, cycle := range StronglyConnected(&g.Graph) { @@ -180,6 +180,8 @@ type vertexAtDepth struct { // DepthFirstWalk does a depth-first walk of the graph starting from // the vertices in start. +// The algorithm used here does not do a complete topological sort. To ensure +// correct overall ordering run TransitiveReduction first. func (g *AcyclicGraph) DepthFirstWalk(start Set, f DepthWalkFunc) error { seen := make(map[Vertex]struct{}) frontier := make([]*vertexAtDepth, 0, len(start)) @@ -219,6 +221,8 @@ func (g *AcyclicGraph) DepthFirstWalk(start Set, f DepthWalkFunc) error { // ReverseDepthFirstWalk does a depth-first walk _up_ the graph starting from // the vertices in start. +// The algorithm used here does not do a complete topological sort. To ensure +// correct overall ordering run TransitiveReduction first. func (g *AcyclicGraph) ReverseDepthFirstWalk(start Set, f DepthWalkFunc) error { seen := make(map[Vertex]struct{}) frontier := make([]*vertexAtDepth, 0, len(start)) diff --git a/internal/dag/dag_test.go b/internal/dag/dag_test.go index 9c8cdb794..75cfb86ff 100644 --- a/internal/dag/dag_test.go +++ b/internal/dag/dag_test.go @@ -99,6 +99,38 @@ func TestAyclicGraphTransReduction_more(t *testing.T) { } } +func TestAyclicGraphTransReduction_multipleRoots(t *testing.T) { + var g AcyclicGraph + g.Add(1) + g.Add(2) + g.Add(3) + g.Add(4) + g.Connect(BasicEdge(1, 2)) + g.Connect(BasicEdge(1, 3)) + g.Connect(BasicEdge(1, 4)) + g.Connect(BasicEdge(2, 3)) + g.Connect(BasicEdge(2, 4)) + g.Connect(BasicEdge(3, 4)) + + g.Add(5) + g.Add(6) + g.Add(7) + g.Add(8) + g.Connect(BasicEdge(5, 6)) + g.Connect(BasicEdge(5, 7)) + g.Connect(BasicEdge(5, 8)) + g.Connect(BasicEdge(6, 7)) + g.Connect(BasicEdge(6, 8)) + g.Connect(BasicEdge(7, 8)) + g.TransitiveReduction() + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testGraphTransReductionMultipleRootsStr) + if actual != expected { + t.Fatalf("bad: %s", actual) + } +} + // use this to simulate slow sort operations type counter struct { Name string @@ -429,3 +461,20 @@ const testGraphTransReductionMoreStr = ` 4 4 ` + +const testGraphTransReductionMultipleRootsStr = ` +1 + 2 +2 + 3 +3 + 4 +4 +5 + 6 +6 + 7 +7 + 8 +8 +`