DeepCopy should only generate types in k8s.io/kubernetes

Also make generation more selective (require opt in) to prevent lots of
unnecessary generated deep copies.
This commit is contained in:
Clayton Coleman 2016-05-24 12:03:05 -04:00
parent 5c41b93d8b
commit 0181ac61da
No known key found for this signature in database
GPG Key ID: 3D16906B4F1C5CB3
6 changed files with 121 additions and 19 deletions

View File

@ -97,6 +97,7 @@ func (g *GeneratorArgs) NewBuilder() (*parser.Builder, error) {
b.AddBuildTags("ignore_autogenerated")
for _, d := range g.InputDirs {
d = strings.TrimLeft(d, "+-*")
if g.Recursive {
if err := b.AddDirRecursive(d); err != nil {
return nil, fmt.Errorf("unable to add directory %q: %v", d, err)

View File

@ -19,6 +19,7 @@ package generators
import (
"fmt"
"io"
"path"
"path/filepath"
"strings"
@ -31,6 +32,15 @@ import (
"github.com/golang/glog"
)
// Constraints is a set of optional limitations on what deep copy will generate.
type Constraints struct {
// PackageConstraints is an optional set of package prefixes that constrain which types
// will have inline deep copy methods generated for. Any type outside of these packages
// (if specified) will not have a function generated and will result in a call to the
// cloner.DeepCopy method.
PackageConstraints []string
}
// TODO: This is created only to reduce number of changes in a single PR.
// Remove it and use PublicNamer instead.
func deepCopyNamer() *namer.NameStrategy {
@ -62,7 +72,32 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
glog.Fatalf("Failed loading boilerplate: %v", err)
}
inputs := sets.NewString(arguments.InputDirs...)
initInputs := sets.NewString()
explicitInputs := sets.NewString()
inputs := sets.NewString()
for _, s := range arguments.InputDirs {
switch {
case strings.HasPrefix(s, "+"):
// packages with '+' prefix get functions generated for everything except gencopy=false, but
// no init function
s = strings.TrimPrefix(s, "+")
inputs.Insert(s)
case strings.HasPrefix(s, "-"):
// packages with '-' prefix only get functions generated for those with gencopy=true
s = strings.TrimPrefix(s, "-")
inputs.Insert(s)
explicitInputs.Insert(s)
default:
inputs.Insert(s)
initInputs.Insert(s)
}
}
var restrictRange []string
if c, ok := arguments.CustomArgs.(Constraints); ok {
restrictRange = c.PackageConstraints
}
packages := generator.Packages{}
header := append([]byte(
`// +build !ignore_autogenerated
@ -76,11 +111,29 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
for _, p := range context.Universe {
copyableType := false
for _, t := range p.Types {
if copyableWithinPackage(t) && inputs.Has(t.Name.Package) {
if copyableWithinPackage(t, explicitInputs.Has(t.Name.Package)) && inputs.Has(t.Name.Package) {
copyableType = true
}
}
if copyableType {
// TODO: replace this with a more sophisticated algorithm that generates private copy methods
// (like auto_DeepCopy_...) for any type that is outside of the PackageConstraints. That would
// avoid having to make a reflection call.
canInlineTypeFn := func(c *generator.Context, t *types.Type) bool {
// types must be public structs or have a custom DeepCopy_<method> already defined
if !copyableWithinPackage(t, explicitInputs.Has(t.Name.Package)) && !publicCopyFunctionDefined(c, t) {
return false
}
// only packages within the restricted range can be inlined
for _, s := range restrictRange {
if strings.HasPrefix(t.Name.Package, s) {
return true
}
}
return false
}
path := p.Path
packages = append(packages,
&generator.DefaultPackage{
@ -90,7 +143,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, initInputs.Has(path), explicitInputs.Has(path), canInlineTypeFn))
return generators
},
FilterFunc: func(c *generator.Context, t *types.Type) bool {
@ -102,6 +155,9 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
return packages
}
// CanInlineTypeFunc should return true if the provided type can be converted to a function call
type CanInlineTypeFunc func(*generator.Context, *types.Type) bool
const (
apiPackagePath = "k8s.io/kubernetes/pkg/api"
conversionPackagePath = "k8s.io/kubernetes/pkg/conversion"
@ -110,23 +166,30 @@ const (
// genDeepCopy produces a file with autogenerated deep-copy functions.
type genDeepCopy struct {
generator.DefaultGen
targetPackage string
imports namer.ImportTracker
typesForInit []*types.Type
generateInitFunc bool
targetPackage string
imports namer.ImportTracker
typesForInit []*types.Type
generateInitFunc bool
requireExplicitTag bool
canInlineTypeFn CanInlineTypeFunc
context *generator.Context
globalVariables map[string]interface{}
}
func NewGenDeepCopy(sanitizedName, targetPackage string, generateInitFunc bool) generator.Generator {
func NewGenDeepCopy(sanitizedName, targetPackage string, generateInitFunc, requireExplicitTag bool, canInlineTypeFn CanInlineTypeFunc) generator.Generator {
return &genDeepCopy{
DefaultGen: generator.DefaultGen{
OptionalName: sanitizedName,
},
targetPackage: targetPackage,
imports: generator.NewImportTracker(),
typesForInit: make([]*types.Type, 0),
generateInitFunc: generateInitFunc,
targetPackage: targetPackage,
imports: generator.NewImportTracker(),
typesForInit: make([]*types.Type, 0),
generateInitFunc: generateInitFunc,
requireExplicitTag: requireExplicitTag,
canInlineTypeFn: canInlineTypeFn,
}
}
@ -137,15 +200,29 @@ 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 := copyableWithinPackage(t, g.requireExplicitTag)
if copyable {
g.typesForInit = append(g.typesForInit, t)
}
return copyable
}
func copyableWithinPackage(t *types.Type) bool {
if types.ExtractCommentTags("+", t.CommentLines)["gencopy"] == "false" {
// publicCopyFunctionDefined returns true if a DeepCopy function has already been defined in a given
// package, which allows more efficient deep copy implementations to be defined by the caller.
func publicCopyFunctionDefined(c *generator.Context, t *types.Type) bool {
p, ok := c.Universe[t.Name.Package]
if !ok {
return false
}
return p.Functions["DeepCopy_"+path.Base(t.Name.Package)+"_"+t.Name.Name] != nil
}
func copyableWithinPackage(t *types.Type, explicitCopyRequired bool) bool {
tag := types.ExtractCommentTags("+", t.CommentLines)["gencopy"]
if tag == "false" {
return false
}
if explicitCopyRequired && tag != "true" {
return false
}
// TODO: Consider generating functions for other kinds too.
@ -204,6 +281,7 @@ func (g *genDeepCopy) funcNameTmpl(t *types.Type) string {
}
func (g *genDeepCopy) Init(c *generator.Context, w io.Writer) error {
g.context = c
cloner := c.Universe.Type(types.Name{Package: conversionPackagePath, Name: "Cloner"})
g.imports.AddType(cloner)
g.globalVariables = map[string]interface{}{
@ -282,7 +360,7 @@ func (g *genDeepCopy) doMap(t *types.Type, sw *generator.SnippetWriter) {
if t.Elem.IsAssignable() {
sw.Do("(*out)[key] = val\n", nil)
} else {
if copyableWithinPackage(t.Elem) {
if g.canInlineTypeFn(g.context, 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))
@ -313,7 +391,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.canInlineTypeFn(g.context, 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)
@ -346,7 +424,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(m.Type) {
if g.canInlineTypeFn(g.context, m.Type) {
funcName := g.funcNameTmpl(m.Type)
sw.Do(fmt.Sprintf("if err := %s(in.$.name$, &out.$.name$, c); err != nil {\n", funcName), args)
sw.Do("return err\n", nil)
@ -383,7 +461,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.canInlineTypeFn(g.context, 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)

View File

@ -31,6 +31,11 @@ import (
func main() {
arguments := args.Default()
arguments.CustomArgs = generators.Constraints{
// Types outside of this package will be inlined.
PackageConstraints: []string{"k8s.io/kubernetes/"},
}
// Override defaults. These are Kubernetes specific input locations.
arguments.InputDirs = []string{
"k8s.io/kubernetes/pkg/api",
@ -56,6 +61,19 @@ func main() {
"k8s.io/kubernetes/pkg/apis/rbac/v1alpha1",
"k8s.io/kubernetes/federation/apis/federation",
"k8s.io/kubernetes/federation/apis/federation/v1alpha1",
// generate all types, but do not register them
"+k8s.io/kubernetes/pkg/api/unversioned",
"-k8s.io/kubernetes/pkg/api/meta",
"-k8s.io/kubernetes/pkg/api/meta/metatypes",
"-k8s.io/kubernetes/pkg/api/resource",
"-k8s.io/kubernetes/pkg/conversion",
"-k8s.io/kubernetes/pkg/labels",
"-k8s.io/kubernetes/pkg/runtime",
"-k8s.io/kubernetes/pkg/runtime/serializer",
"-k8s.io/kubernetes/pkg/util/intstr",
"-k8s.io/kubernetes/pkg/util/sets",
}
if err := arguments.Execute(

View File

@ -87,6 +87,7 @@ import (
// writing some sort of special handling code in the hopes that that will
// cause implementors to also use a fixed point implementation.
//
// +gencopy=false
// +protobuf=true
// +protobuf.embed=string
// +protobuf.options.marshal=false

View File

@ -40,6 +40,7 @@ import (
// TypeMeta is provided here for convenience. You may use it directly from this package or define
// your own with the same fields.
//
// +gencopy=true
// +protobuf=true
type TypeMeta struct {
APIVersion string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty" protobuf:"bytes,1,opt,name=apiVersion"`
@ -92,6 +93,7 @@ const (
// in the Object. (TODO: In the case where the object is of an unknown type, a
// runtime.Unknown object will be created and stored.)
//
// +gencopy=true
// +protobuf=true
type RawExtension struct {
// Raw is the underlying serialization of this object.
@ -109,6 +111,7 @@ type RawExtension struct {
// TODO: Make this object have easy access to field based accessors and settors for
// metadata and field mutatation.
//
// +gencopy=true
// +protobuf=true
type Unknown struct {
TypeMeta `json:",inline" protobuf:"bytes,1,opt,name=typeMeta"`

View File

@ -32,6 +32,7 @@ import (
// accept a name or number.
// TODO: Rename to Int32OrString
//
// +gencopy=true
// +protobuf=true
// +protobuf.options.(gogoproto.goproto_stringer)=false
type IntOrString struct {