From 00706b8fe9e6f96f2a792684b02dc123b5b65c52 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 14 Jan 2020 21:55:26 -0500 Subject: [PATCH] Remove gonum.org/v1 dependency in code-generator --- .../cmd/go-to-protobuf/protobuf/BUILD | 8 +- .../cmd/go-to-protobuf/protobuf/cmd.go | 88 +++++++++++++------ .../cmd/go-to-protobuf/protobuf/cmd_test.go | 65 ++++++++++++++ 3 files changed, 129 insertions(+), 32 deletions(-) create mode 100644 staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd_test.go diff --git a/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/BUILD b/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/BUILD index 9f6018afa80..529dfd022a7 100644 --- a/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/BUILD +++ b/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/BUILD @@ -23,9 +23,6 @@ go_library( "//staging/src/k8s.io/code-generator/pkg/util:go_default_library", "//staging/src/k8s.io/code-generator/third_party/forked/golang/reflect:go_default_library", "//vendor/github.com/spf13/pflag:go_default_library", - "//vendor/gonum.org/v1/gonum/graph:go_default_library", - "//vendor/gonum.org/v1/gonum/graph/simple:go_default_library", - "//vendor/gonum.org/v1/gonum/graph/topo:go_default_library", "//vendor/k8s.io/gengo/args:go_default_library", "//vendor/k8s.io/gengo/generator:go_default_library", "//vendor/k8s.io/gengo/namer:go_default_library", @@ -37,7 +34,10 @@ go_library( go_test( name = "go_default_test", - srcs = ["namer_test.go"], + srcs = [ + "cmd_test.go", + "namer_test.go", + ], embed = [":go_default_library"], ) diff --git a/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd.go b/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd.go index 8a7be5260c7..de782e98f75 100644 --- a/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd.go +++ b/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd.go @@ -29,9 +29,6 @@ import ( "strings" flag "github.com/spf13/pflag" - "gonum.org/v1/gonum/graph" - "gonum.org/v1/gonum/graph/simple" - "gonum.org/v1/gonum/graph/topo" "k8s.io/code-generator/pkg/util" "k8s.io/gengo/args" @@ -374,38 +371,73 @@ func deps(c *generator.Context, pkgs []*protobufPackage) map[string][]string { return ret } +// given a set of pkg->[]deps, return the order that ensures all deps are processed before the things that depend on them func importOrder(deps map[string][]string) ([]string, error) { - nodes := map[string]graph.Node{} - names := map[int64]string{} - g := simple.NewDirectedGraph() - for pkg, imports := range deps { - for _, imp := range imports { - if _, found := nodes[pkg]; !found { - n := g.NewNode() - g.AddNode(n) - nodes[pkg] = n - names[n.ID()] = pkg - } - if _, found := nodes[imp]; !found { - n := g.NewNode() - g.AddNode(n) - nodes[imp] = n - names[n.ID()] = imp - } - g.SetEdge(g.NewEdge(nodes[imp], nodes[pkg])) + // add all nodes and edges + var remainingNodes = map[string]struct{}{} + var graph = map[edge]struct{}{} + for to, froms := range deps { + remainingNodes[to] = struct{}{} + for _, from := range froms { + remainingNodes[from] = struct{}{} + graph[edge{from: from, to: to}] = struct{}{} } } - ret := []string{} - sorted, err := topo.Sort(g) - if err != nil { - return nil, err + // find initial nodes without any dependencies + sorted := findAndRemoveNodesWithoutDependencies(remainingNodes, graph) + for i := 0; i < len(sorted); i++ { + node := sorted[i] + removeEdgesFrom(node, graph) + sorted = append(sorted, findAndRemoveNodesWithoutDependencies(remainingNodes, graph)...) + } + if len(remainingNodes) > 0 { + return nil, fmt.Errorf("cycle: remaining nodes: %#v, remaining edges: %#v", remainingNodes, graph) } for _, n := range sorted { - ret = append(ret, names[n.ID()]) - fmt.Println("topological order", names[n.ID()]) + fmt.Println("topological order", n) + } + return sorted, nil +} + +// edge describes a from->to relationship in a graph +type edge struct { + from string + to string +} + +// findAndRemoveNodesWithoutDependencies finds nodes in the given set which are not pointed to by any edges in the graph, +// removes them from the set of nodes, and returns them in sorted order +func findAndRemoveNodesWithoutDependencies(nodes map[string]struct{}, graph map[edge]struct{}) []string { + roots := []string{} + // iterate over all nodes as potential "to" nodes + for node := range nodes { + incoming := false + // iterate over all remaining edges + for edge := range graph { + // if there's any edge to the node we care about, it's not a root + if edge.to == node { + incoming = true + break + } + } + // if there are no incoming edges, remove from the set of remaining nodes and add to our results + if !incoming { + delete(nodes, node) + roots = append(roots, node) + } + } + sort.Strings(roots) + return roots +} + +// removeEdgesFrom removes any edges from the graph where edge.from == node +func removeEdgesFrom(node string, graph map[edge]struct{}) { + for edge := range graph { + if edge.from == node { + delete(graph, edge) + } } - return ret, nil } type positionOrder struct { diff --git a/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd_test.go b/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd_test.go new file mode 100644 index 00000000000..c6b67176386 --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd_test.go @@ -0,0 +1,65 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package protobuf + +import ( + "reflect" + "testing" +) + +func TestImportOrder(t *testing.T) { + testcases := []struct { + Name string + Input map[string][]string + Expect []string + ExpectErr bool + }{ + { + Name: "empty", + Input: nil, + Expect: []string{}, + }, + { + Name: "simple", + Input: map[string][]string{"apps": {"core", "extensions", "meta"}, "extensions": {"core", "meta"}, "core": {"meta"}}, + Expect: []string{"meta", "core", "extensions", "apps"}, + }, + { + Name: "cycle", + Input: map[string][]string{"apps": {"core", "extensions", "meta"}, "extensions": {"core", "meta"}, "core": {"meta", "apps"}}, + ExpectErr: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.Name, func(t *testing.T) { + order, err := importOrder(tc.Input) + if err != nil { + if !tc.ExpectErr { + t.Fatalf("unexpected error: %v", err) + } + return + } + if tc.ExpectErr { + t.Fatalf("expected error, got none") + } + if !reflect.DeepEqual(order, tc.Expect) { + t.Fatalf("expected %v, got %v", tc.Expect, order) + } + }) + } +}