Recreate the opt-in/opt-out logic for deepcopy

This is the last piece of Clayton's #26179 to be implemented with file tags.
All diffs are accounted for.  Followup will use this to streamline some
packages.

Also add some V(5) debugging - it was helpful in diagnosing various issues, it
may be helpful again.
This commit is contained in:
Tim Hockin 2016-06-15 23:43:13 -07:00
parent 28af54138d
commit dc10f10e48
34 changed files with 266 additions and 97 deletions

View File

@ -31,6 +31,8 @@ import (
"k8s.io/kubernetes/cmd/libs/go2idl/namer"
"k8s.io/kubernetes/cmd/libs/go2idl/parser"
"k8s.io/kubernetes/cmd/libs/go2idl/types"
"k8s.io/kubernetes/pkg/util"
utilflag "k8s.io/kubernetes/pkg/util/flag"
"github.com/spf13/pflag"
)
@ -147,7 +149,8 @@ func DefaultSourceTree() string {
// If you don't need any non-default behavior, use as:
// args.Default().Execute(...)
func (g *GeneratorArgs) Execute(nameSystems namer.NameSystems, defaultSystem string, pkgs func(*generator.Context, *GeneratorArgs) generator.Packages) error {
pflag.Parse()
utilflag.InitFlags()
util.InitLogs()
b, err := g.NewBuilder()
if err != nil {

View File

@ -37,6 +37,59 @@ type CustomArgs struct {
BoundingDirs []string // Only deal with types rooted under these dirs.
}
// This is the comment tag that carries parameters for deep-copy generation.
const tagName = "k8s:deepcopy-gen"
// Known values for the comment tag.
const tagValuePackage = "package"
// tagValue holds parameters from a tagName tag.
type tagValue struct {
value string
register bool
}
func extractTag(comments []string) *tagValue {
tagVals := types.ExtractCommentTags("+", comments)[tagName]
if tagVals == nil {
// No match for the tag.
return nil
}
// If there are multiple values, abort.
if len(tagVals) > 1 {
glog.Fatalf("Found %d %s tags: %q", len(tagVals), tagName, tagVals)
}
// If we got here we are returning something.
tag := &tagValue{}
// Get the primary value.
parts := strings.Split(tagVals[0], ",")
if len(parts) >= 1 {
tag.value = parts[0]
}
// Parse extra arguments.
parts = parts[1:]
for i := range parts {
kv := strings.SplitN(parts[i], "=", 2)
k := kv[0]
v := ""
if len(kv) == 2 {
v = kv[1]
}
switch k {
case "register":
if v != "false" {
tag.register = true
}
default:
glog.Fatalf("Unsupported %s param: %q", tagName, parts[i])
}
}
return tag
}
// TODO: This is created only to reduce number of changes in a single PR.
// Remove it and use PublicNamer instead.
func deepCopyNamer() *namer.NameStrategy {
@ -86,38 +139,61 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
}
}
for _, p := range context.Universe {
// Short-circuit if this package has not opted in.
if tagvals := types.ExtractCommentTags("+", p.Comments)["k8s:deepcopy-gen"]; tagvals == nil {
// No tag for this package.
continue
} else if tagvals[0] != "generate" && tagvals[0] != "register" {
// Unknown tag.
glog.Errorf("Uknown tag value '+k8s:deepcopy-gen=%s'", tagvals[0])
for i := range inputs {
glog.V(5).Infof("considering pkg %q", i)
pkg := context.Universe[i]
if pkg == nil {
// If the input had no Go files, for example.
continue
}
needsGeneration := false
for _, t := range p.Types {
if copyableWithinPackage(t, boundingDirs) && inputs.Has(t.Name.Package) {
needsGeneration = true
break
ptag := extractTag(pkg.Comments)
ptagValue := ""
ptagRegister := false
if ptag != nil {
ptagValue = ptag.value
if ptagValue != tagValuePackage {
glog.Fatalf("Package %v: unsupported %s value: %q", i, tagName, ptagValue)
}
ptagRegister = ptag.register
glog.V(5).Infof(" tag.value: %q, tag.register: %t", ptagValue, ptagRegister)
} else {
glog.V(5).Infof(" no tag")
}
// If the pkg-scoped tag says to generate, we can skip scanning types.
pkgNeedsGeneration := (ptagValue == tagValuePackage)
if !pkgNeedsGeneration {
// If the pkg-scoped tag did not exist, scan all types for one that
// explicitly wants generation.
for _, t := range pkg.Types {
glog.V(5).Infof(" considering type %q", t.Name.String())
ttag := extractTag(t.CommentLines)
if ttag != nil && ttag.value == "true" {
glog.V(5).Infof(" tag=true")
if !copyableWithinPackage(t, boundingDirs) {
glog.Fatalf("Type %v requests deepcopy generation but is not copyable", t)
}
pkgNeedsGeneration = true
break
}
}
}
if needsGeneration {
path := p.Path
if pkgNeedsGeneration {
packages = append(packages,
&generator.DefaultPackage{
PackageName: strings.Split(filepath.Base(path), ".")[0],
PackagePath: path,
PackageName: strings.Split(filepath.Base(pkg.Path), ".")[0],
PackagePath: pkg.Path,
HeaderText: header,
GeneratorFunc: func(c *generator.Context) (generators []generator.Generator) {
generators = []generator.Generator{}
generators = append(
generators, NewGenDeepCopy(arguments.OutputFileBaseName, path, boundingDirs))
generators, NewGenDeepCopy(arguments.OutputFileBaseName, pkg.Path, boundingDirs, (ptagValue == tagValuePackage), ptagRegister))
return generators
},
FilterFunc: func(c *generator.Context, t *types.Type) bool {
return t.Name.Package == path
return t.Name.Package == pkg.Path
},
})
}
@ -135,19 +211,23 @@ type genDeepCopy struct {
generator.DefaultGen
targetPackage string
boundingDirs []string
allTypes bool
registerTypes bool
imports namer.ImportTracker
typesForInit []*types.Type
globalVariables map[string]interface{}
}
func NewGenDeepCopy(sanitizedName, targetPackage string, boundingDirs []string) generator.Generator {
func NewGenDeepCopy(sanitizedName, targetPackage string, boundingDirs []string, allTypes, registerTypes bool) generator.Generator {
return &genDeepCopy{
DefaultGen: generator.DefaultGen{
OptionalName: sanitizedName,
},
targetPackage: targetPackage,
boundingDirs: boundingDirs,
allTypes: allTypes,
registerTypes: registerTypes,
imports: generator.NewImportTracker(),
typesForInit: make([]*types.Type, 0),
}
@ -159,8 +239,15 @@ 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 := g.copyableWithinPackage(t)
// Filter out types not being processed or not copyable within the package.
enabled := g.allTypes
if !enabled {
ttag := extractTag(t.CommentLines)
if ttag != nil && ttag.value == "true" {
enabled = true
}
}
copyable := enabled && g.copyableWithinPackage(t)
if copyable {
g.typesForInit = append(g.typesForInit, t)
}
@ -208,7 +295,8 @@ func isRootedUnder(pkg string, roots []string) bool {
func copyableWithinPackage(t *types.Type, boundingDirs []string) bool {
// If the type opts out of copy-generation, stop.
if extractBoolTagOrDie("k8s:deepcopy-gen", true, t.CommentLines) == false {
ttag := extractTag(t.CommentLines)
if ttag != nil && ttag.value == "false" {
return false
}
// Only packages within the restricted range can be processed.
@ -276,12 +364,14 @@ func (g *genDeepCopy) Init(c *generator.Context, w io.Writer) error {
g.globalVariables = map[string]interface{}{
"Cloner": cloner,
}
if tagvals := types.ExtractCommentTags("+", c.Universe[g.targetPackage].Comments)["k8s:deepcopy-gen"]; tagvals != nil && tagvals[0] != "register" {
if !g.registerTypes {
// TODO: We should come up with a solution to register all generated
// deep-copy functions. However, for now, to avoid import cycles
// we register only those explicitly requested.
return nil
}
glog.V(5).Infof("registering types in pkg %q", g.targetPackage)
scheme := c.Universe.Variable(types.Name{Package: apiPackagePath, Name: "Scheme"})
g.imports.AddType(scheme)
g.globalVariables["scheme"] = scheme
@ -302,7 +392,34 @@ func (g *genDeepCopy) Init(c *generator.Context, w io.Writer) error {
return sw.Error()
}
func (g *genDeepCopy) needsGeneration(t *types.Type) bool {
tag := extractTag(t.CommentLines)
tv := ""
if tag != nil {
tv = tag.value
if tv != "true" && tv != "false" {
glog.Fatalf("Type %v: unsupported %s value: %q", t, tagName, tag.value)
}
}
if g.allTypes && tv == "false" {
// The whole package is being generated, but this type has opted out.
glog.V(5).Infof("not generating for type %v because type opted out", t)
return false
}
if !g.allTypes && tv != "true" {
// The whole package is NOT being generated, and this type has NOT opted in.
glog.V(5).Infof("not generating for type %v because type did not opt in", t)
return false
}
return true
}
func (g *genDeepCopy) GenerateType(c *generator.Context, t *types.Type, w io.Writer) error {
if !g.needsGeneration(t) {
return nil
}
glog.V(5).Infof("generating for type %v", t)
sw := generator.NewSnippetWriter(w, c, "$", "$")
funcName := g.funcNameTmpl(t)
sw.Do(fmt.Sprintf("func %s(in $.type|raw$, out *$.type|raw$, c *$.Cloner|raw$) error {\n", funcName), g.withGlobals(argsFromType(t)))

View File

@ -265,3 +265,80 @@ func Test_hasDeepCopyMethod(t *testing.T) {
}
}
}
func Test_extractTagParams(t *testing.T) {
testCases := []struct {
comments []string
expect *tagValue
}{
{
comments: []string{
"Human comment",
},
expect: nil,
},
{
comments: []string{
"Human comment",
"+k8s:deepcopy-gen",
},
expect: &tagValue{
value: "",
register: false,
},
},
{
comments: []string{
"Human comment",
"+k8s:deepcopy-gen=package",
},
expect: &tagValue{
value: "package",
register: false,
},
},
{
comments: []string{
"Human comment",
"+k8s:deepcopy-gen=package,register",
},
expect: &tagValue{
value: "package",
register: true,
},
},
{
comments: []string{
"Human comment",
"+k8s:deepcopy-gen=package,register=true",
},
expect: &tagValue{
value: "package",
register: true,
},
},
{
comments: []string{
"Human comment",
"+k8s:deepcopy-gen=package,register=false",
},
expect: &tagValue{
value: "package",
register: false,
},
},
}
for i, tc := range testCases {
r := extractTag(tc.comments)
if r == nil && tc.expect != nil {
t.Errorf("case[%d]: expected non-nil", i)
}
if r != nil && tc.expect == nil {
t.Errorf("case[%d]: expected nil, got %v", i, *r)
}
if r != nil && *r != *tc.expect {
t.Errorf("case[%d]: expected %v, got %v", i, *tc.expect, *r)
}
}
}

View File

@ -1,33 +0,0 @@
/*
Copyright 2016 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 generators
import (
"github.com/golang/glog"
"k8s.io/kubernetes/cmd/libs/go2idl/types"
)
// extractBoolTagOrDie gets the comment-tags for the key and asserts that, if
// it exists, the value is boolean. If the tag did not exist, it returns
// defaultVal.
func extractBoolTagOrDie(key string, defaultVal bool, lines []string) bool {
val, err := types.ExtractSingleBoolCommentTag("+", key, defaultVal, lines)
if err != nil {
glog.Fatalf(err.Error())
}
return val
}

View File

@ -28,18 +28,23 @@ limitations under the License.
// Generation is governed by comment tags in the source. Any package may
// request DeepCopy generation by including a comment in the file-comments of
// one file, of the form:
// // +k8s:deepcopy-gen=generate
// or:
// // +k8s:deepcopy-gen=register
// // +k8s:deepcopy-gen=package
//
// Packages which specify `=generate` will have DeepCopy functions generated
// into them. Packages which specify `=register` will have DeepCopy functions
// generated and registered with in `init()` function call to
// `Scheme.AddGeneratedDeepCopyFuncs()`.
// Packages can request that the generated DeepCopy functions be registered
// with an `init()` function call to `Scheme.AddGeneratedDeepCopyFuncs()` by
// changing the tag to:
// // +k8s:deepcopy-gen=package,register
//
// Individual types may opt out of DeepCopy generation by specifying a comment
// of the form:
// DeepCopy functions can be generated for individual types, rather than the
// entire package by specifying a comment on the type definion of the form:
// // +k8s:deepcopy-gen=true
//
// When generating for a whole package, individual types may opt out of
// DeepCopy generation by specifying a comment on the of the form:
// // +k8s:deepcopy-gen=false
//
// Note that registration is a whole-package option, and is not available for
// individual types.
package main
import (

View File

@ -77,8 +77,8 @@ cmd/libs/go2idl/ tool.
1. Add your "group/" or "group/version" into
cmd/libs/go2idl/conversion-gen/main.go;
2. Make sure your pkg/apis/`<group>`/`<version>` directory has a doc.go file
with the comment `// +k8s:deepcopy-gen=register`, to catch the attention
of our generation tools.
with the comment `// +k8s:deepcopy-gen=package,register`, to catch the
attention of our generation tools.
3. Make sure your pkg/apis/`<group>`/`<version>` directory has a doc.go file
with the comment `// +genconversion=true`, to catch the attention of our
gen-conversion script.

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package federation

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1beta1

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// Package api contains the latest (or "internal") version of the
// Kubernetes API objects. This is the API objects as represented in memory.

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=generate
// +k8s:deepcopy-gen=package
package unversioned

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// Package v1 is the v1 version of the API.
// +genconversion=true

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package apps

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1alpha1

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package authentication

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1beta1

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package authorization

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1beta1

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package autoscaling

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package batch

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v2alpha1

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package certificates

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1alpha1

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package componentconfig

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1alpha1

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package extensions

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1beta1

View File

@ -14,6 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
package policy

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// Package policy is for any kind of policy object. Suitable examples, even if
// they aren't all here, are PodDisruptionBudget, PodSecurityPolicy,

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +groupName=rbac.authorization.k8s.io
package rbac

View File

@ -15,7 +15,7 @@ limitations under the License.
*/
// +groupName=rbac.authorization.k8s.io
// +k8s:deepcopy-gen=register
// +k8s:deepcopy-gen=package,register
// +genconversion=true
package v1alpha1

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// +k8s:deepcopy-gen=generate
// +k8s:deepcopy-gen=package
// Package conversion provides go object versioning.
//

View File

@ -42,6 +42,6 @@ limitations under the License.
// As a bonus, a few common types useful from all api objects and versions
// are provided in types.go.
// +k8s:deepcopy-gen=generate
// +k8s:deepcopy-gen=package
package runtime