From be481060eac4c4d2a7d96a41e39d56593aae0885 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 29 Jun 2016 02:00:55 -0700 Subject: [PATCH] Re-add constraints to deep-copy This re-institutes some of the rolled-back logic from previous commits. It bounds the scope of what the deepcopy generator is willing to do with regards to generating and calling generated functions. --- .../deepcopy-gen/generators/deepcopy.go | 56 ++++++++-- .../deepcopy-gen/generators/deepcopy_test.go | 102 ++++++++++++++++++ cmd/libs/go2idl/deepcopy-gen/main.go | 10 ++ hack/verify-flags/known-flags.txt | 1 + 4 files changed, 160 insertions(+), 9 deletions(-) create mode 100644 cmd/libs/go2idl/deepcopy-gen/generators/deepcopy_test.go diff --git a/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go b/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go index 4738dc9a153..f3072e2a971 100644 --- a/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go +++ b/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go @@ -31,6 +31,12 @@ import ( "github.com/golang/glog" ) +// CustomArgs is used tby the go2idl framework to pass args specific to this +// generator. +type CustomArgs struct { + BoundingDirs []string // Only deal with types rooted under these dirs. +} + // TODO: This is created only to reduce number of changes in a single PR. // Remove it and use PublicNamer instead. func deepCopyNamer() *namer.NameStrategy { @@ -70,10 +76,20 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat // This file was autogenerated by deepcopy-gen. Do not edit it manually! `)...) + + boundingDirs := []string{} + if customArgs, ok := arguments.CustomArgs.(*CustomArgs); ok { + for i := range customArgs.BoundingDirs { + // Strip any trailing slashes - they are not exactly "correct" but + // this is friendlier. + boundingDirs = append(boundingDirs, strings.TrimRight(customArgs.BoundingDirs[i], "/")) + } + } + for _, p := range context.Universe { copyableType := false for _, t := range p.Types { - if copyableWithinPackage(t) && inputs.Has(t.Name.Package) { + if copyableWithinPackage(t, boundingDirs) && inputs.Has(t.Name.Package) { copyableType = true } } @@ -87,7 +103,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat GeneratorFunc: func(c *generator.Context) (generators []generator.Generator) { generators = []generator.Generator{} generators = append( - generators, NewGenDeepCopy("deep_copy_generated", path, inputs.Has(path))) + generators, NewGenDeepCopy("deep_copy_generated", path, boundingDirs, inputs.Has(path))) return generators }, FilterFunc: func(c *generator.Context, t *types.Type) bool { @@ -108,6 +124,7 @@ const ( type genDeepCopy struct { generator.DefaultGen targetPackage string + boundingDirs []string imports namer.ImportTracker typesForInit []*types.Type generateInitFunc bool @@ -115,12 +132,13 @@ type genDeepCopy struct { globalVariables map[string]interface{} } -func NewGenDeepCopy(sanitizedName, targetPackage string, generateInitFunc bool) generator.Generator { +func NewGenDeepCopy(sanitizedName, targetPackage string, boundingDirs []string, generateInitFunc bool) generator.Generator { return &genDeepCopy{ DefaultGen: generator.DefaultGen{ OptionalName: sanitizedName, }, targetPackage: targetPackage, + boundingDirs: boundingDirs, imports: generator.NewImportTracker(), typesForInit: make([]*types.Type, 0), generateInitFunc: generateInitFunc, @@ -134,17 +152,37 @@ func (g *genDeepCopy) Namers(c *generator.Context) namer.NameSystems { func (g *genDeepCopy) Filter(c *generator.Context, t *types.Type) bool { // Filter out all types not copyable within the package. - copyable := copyableWithinPackage(t) + copyable := g.copyableWithinPackage(t) if copyable { g.typesForInit = append(g.typesForInit, t) } return copyable } -func copyableWithinPackage(t *types.Type) bool { +func (g *genDeepCopy) copyableWithinPackage(t *types.Type) bool { + return copyableWithinPackage(t, g.boundingDirs) +} + +func isRootedUnder(pkg string, roots []string) bool { + // Add trailing / to avoid false matches, e.g. foo/bar vs foo/barn. This + // assumes that bounding dirs do not have trailing slashes. + pkg = pkg + "/" + for _, root := range roots { + if strings.HasPrefix(pkg, root+"/") { + return true + } + } + return false +} + +func copyableWithinPackage(t *types.Type, boundingDirs []string) bool { if types.ExtractCommentTags("+", t.CommentLines)["gencopy"] == "false" { return false } + // Only packages within the restricted range can be processed. + if !isRootedUnder(t.Name.Package, boundingDirs) { + return false + } // TODO: Consider generating functions for other kinds too. if t.Kind != types.Struct { return false @@ -286,7 +324,7 @@ func (g *genDeepCopy) doMap(t *types.Type, sw *generator.SnippetWriter) { sw.Do("}\n", nil) default: sw.Do("for key, val := range in {\n", nil) - if copyableWithinPackage(t.Elem) { + if g.copyableWithinPackage(t.Elem) { sw.Do("newVal := new($.|raw$)\n", t.Elem) funcName := g.funcNameTmpl(t.Elem) sw.Do(fmt.Sprintf("if err := %s(val, newVal, c); err != nil {\n", funcName), argsFromType(t.Elem)) @@ -318,7 +356,7 @@ func (g *genDeepCopy) doSlice(t *types.Type, sw *generator.SnippetWriter) { sw.Do("for i := range in {\n", nil) if t.Elem.IsAssignable() { sw.Do("(*out)[i] = in[i]\n", nil) - } else if copyableWithinPackage(t.Elem) { + } else if g.copyableWithinPackage(t.Elem) { funcName := g.funcNameTmpl(t.Elem) sw.Do(fmt.Sprintf("if err := %s(in[i], &(*out)[i], c); err != nil {\n", funcName), argsFromType(t.Elem)) sw.Do("return err\n", nil) @@ -357,7 +395,7 @@ func (g *genDeepCopy) doStruct(t *types.Type, sw *generator.SnippetWriter) { sw.Do("out.$.name$ = nil\n", args) sw.Do("}\n", nil) case types.Struct: - if copyableWithinPackage(t) { + if g.copyableWithinPackage(t) { funcName := g.funcNameTmpl(t) sw.Do(fmt.Sprintf("if err := %s(in.$.name$, &out.$.name$, c); err != nil {\n", funcName), args) sw.Do("return err\n", nil) @@ -390,7 +428,7 @@ func (g *genDeepCopy) doPointer(t *types.Type, sw *generator.SnippetWriter) { sw.Do("*out = new($.Elem|raw$)\n", t) if t.Elem.Kind == types.Builtin { sw.Do("**out = *in", nil) - } else if copyableWithinPackage(t.Elem) { + } else if g.copyableWithinPackage(t.Elem) { funcName := g.funcNameTmpl(t.Elem) sw.Do(fmt.Sprintf("if err := %s(*in, *out, c); err != nil {\n", funcName), argsFromType(t.Elem)) sw.Do("return err\n", nil) diff --git a/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy_test.go b/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy_test.go new file mode 100644 index 00000000000..303c5e0da7c --- /dev/null +++ b/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy_test.go @@ -0,0 +1,102 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +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 generators + +import "testing" + +func Test_isRootedUnder(t *testing.T) { + testCases := []struct { + path string + roots []string + expect bool + }{ + { + path: "/foo/bar", + roots: nil, + expect: false, + }, + { + path: "/foo/bar", + roots: []string{}, + expect: false, + }, + { + path: "/foo/bar", + roots: []string{ + "/bad", + }, + expect: false, + }, + { + path: "/foo/bar", + roots: []string{ + "/foo", + }, + expect: true, + }, + { + path: "/foo/bar", + roots: []string{ + "/bad", + "/foo", + }, + expect: true, + }, + { + path: "/foo/bar/qux/zorb", + roots: []string{ + "/foo/bar/qux", + }, + expect: true, + }, + { + path: "/foo/bar", + roots: []string{ + "/foo/bar", + }, + expect: true, + }, + { + path: "/foo/barn", + roots: []string{ + "/foo/bar", + }, + expect: false, + }, + { + path: "/foo/bar", + roots: []string{ + "/foo/barn", + }, + expect: false, + }, + { + path: "/foo/bar", + roots: []string{ + "", + }, + expect: true, + }, + } + + for i, tc := range testCases { + r := isRootedUnder(tc.path, tc.roots) + if r != tc.expect { + t.Errorf("case[%d]: expected %t, got %t for %q in %q", i, tc.expect, r, tc.path, tc.roots) + } + } +} diff --git a/cmd/libs/go2idl/deepcopy-gen/main.go b/cmd/libs/go2idl/deepcopy-gen/main.go index 13553a43c2c..ae4b1d901f6 100644 --- a/cmd/libs/go2idl/deepcopy-gen/main.go +++ b/cmd/libs/go2idl/deepcopy-gen/main.go @@ -26,6 +26,7 @@ import ( "k8s.io/kubernetes/cmd/libs/go2idl/deepcopy-gen/generators" "github.com/golang/glog" + "github.com/spf13/pflag" ) func main() { @@ -60,6 +61,15 @@ func main() { "k8s.io/kubernetes/federation/apis/federation/v1beta1", } + // Custom args. + customArgs := &generators.CustomArgs{ + BoundingDirs: []string{"k8s.io/kubernetes"}, + } + pflag.CommandLine.StringSliceVar(&customArgs.BoundingDirs, "bounding-dirs", customArgs.BoundingDirs, + "Comma-separated list of import paths which bound the types for which deep-copies will be generated.") + arguments.CustomArgs = customArgs + + // Run it. if err := arguments.Execute( generators.NameSystems(), generators.DefaultNameSystem(), diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index 10e0a32a680..b6c02689b52 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -35,6 +35,7 @@ bench-workers bind-address bind-pods-burst bind-pods-qps +bounding-dirs build-only build-services build-tag