From 732647ea97033601a962455ae4fa2b883bc6a713 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 17 Jun 2015 15:48:27 -0400 Subject: [PATCH] Improve conversion to support multiple packages OpenShift uses multiple API packages (types are split) which Kube will also eventually have as we introduce more plugins. These changes make the generators able to handle importing different API object packages into a single generator function. --- cmd/genconversion/conversion.go | 13 +- cmd/gendeepcopy/deep_copy.go | 13 +- hack/update-generated-conversions.sh | 8 -- pkg/runtime/conversion_generator.go | 177 +++++++++++++++++++++---- pkg/runtime/deep_copy_generator.go | 188 +++++++++++++++++++++------ 5 files changed, 325 insertions(+), 74 deletions(-) diff --git a/cmd/genconversion/conversion.go b/cmd/genconversion/conversion.go index c25fb0d0aad..c60b8f1dacd 100644 --- a/cmd/genconversion/conversion.go +++ b/cmd/genconversion/conversion.go @@ -17,13 +17,16 @@ limitations under the License. package main import ( + "fmt" "io" "os" + "path" "runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1" pkg_runtime "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/golang/glog" flag "github.com/spf13/pflag" @@ -50,7 +53,9 @@ func main() { funcOut = file } - generator := pkg_runtime.NewConversionGenerator(api.Scheme.Raw()) + generator := pkg_runtime.NewConversionGenerator(api.Scheme.Raw(), path.Join("github.com/GoogleCloudPlatform/kubernetes/pkg/api", *version)) + apiShort := generator.AddImport("github.com/GoogleCloudPlatform/kubernetes/pkg/api") + generator.AddImport("github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource") // TODO(wojtek-t): Change the overwrites to a flag. generator.OverwritePackage(*version, "") for _, knownType := range api.Scheme.KnownTypes(*version) { @@ -58,10 +63,14 @@ func main() { glog.Errorf("error while generating conversion functions for %v: %v", knownType, err) } } + generator.RepackImports(util.NewStringSet()) + if err := generator.WriteImports(funcOut); err != nil { + glog.Fatalf("error while writing imports: %v", err) + } if err := generator.WriteConversionFunctions(funcOut); err != nil { glog.Fatalf("Error while writing conversion functions: %v", err) } - if err := generator.RegisterConversionFunctions(funcOut); err != nil { + if err := generator.RegisterConversionFunctions(funcOut, fmt.Sprintf("%s.Scheme", apiShort)); err != nil { glog.Fatalf("Error while writing conversion functions: %v", err) } } diff --git a/cmd/gendeepcopy/deep_copy.go b/cmd/gendeepcopy/deep_copy.go index 59ac43bb72f..dcb1c33766d 100644 --- a/cmd/gendeepcopy/deep_copy.go +++ b/cmd/gendeepcopy/deep_copy.go @@ -19,12 +19,14 @@ package main import ( "io" "os" + "path" "runtime" "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1" pkg_runtime "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/golang/glog" flag "github.com/spf13/pflag" @@ -53,10 +55,14 @@ func main() { } knownVersion := *version + registerTo := "api.Scheme" if knownVersion == "api" { knownVersion = api.Scheme.Raw().InternalVersion + registerTo = "Scheme" } - generator := pkg_runtime.NewDeepCopyGenerator(api.Scheme.Raw()) + pkgPath := path.Join("github.com/GoogleCloudPlatform/kubernetes/pkg/api", knownVersion) + generator := pkg_runtime.NewDeepCopyGenerator(api.Scheme.Raw(), pkgPath, util.NewStringSet("github.com/GoogleCloudPlatform/kubernetes")) + generator.AddImport("github.com/GoogleCloudPlatform/kubernetes/pkg/api") for _, overwrite := range strings.Split(*overwrites, ",") { vals := strings.Split(overwrite, "=") @@ -67,13 +73,14 @@ func main() { glog.Errorf("error while generating deep copy functions for %v: %v", knownType, err) } } - if err := generator.WriteImports(funcOut, *version); err != nil { + generator.RepackImports() + if err := generator.WriteImports(funcOut); err != nil { glog.Fatalf("error while writing imports: %v", err) } if err := generator.WriteDeepCopyFunctions(funcOut); err != nil { glog.Fatalf("error while writing deep copy functions: %v", err) } - if err := generator.RegisterDeepCopyFunctions(funcOut, *version); err != nil { + if err := generator.RegisterDeepCopyFunctions(funcOut, registerTo); err != nil { glog.Fatalf("error while registering deep copy functions: %v", err) } } diff --git a/hack/update-generated-conversions.sh b/hack/update-generated-conversions.sh index d7b9f9fd591..7df31ca18f8 100755 --- a/hack/update-generated-conversions.sh +++ b/hack/update-generated-conversions.sh @@ -33,14 +33,6 @@ function generate_version() { cat >> $TMPFILE < 0 { + name = dirname + name + if _, ok := g.shortImports[name]; !ok { + g.imports[pkg] = name + g.shortImports[name] = pkg + return name + } + if subdirname := path.Base(path.Dir(path.Dir(pkg))); len(subdirname) > 0 { + name = subdirname + name + if _, ok := g.shortImports[name]; !ok { + g.imports[pkg] = name + g.shortImports[name] = pkg + return name + } + } + } + for i := 2; i < 100; i++ { + generatedName := fmt.Sprintf("%s%d", name, i) + if _, ok := g.shortImports[generatedName]; !ok { + g.imports[pkg] = generatedName + g.shortImports[generatedName] = pkg + return generatedName + } + } + panic(fmt.Sprintf("unable to find a unique name for the package path %q: %v", pkg, g.shortImports)) +} + func (g *conversionGenerator) typeName(inType reflect.Type) string { switch inType.Kind() { - case reflect.Map: - return fmt.Sprintf("map[%s]%s", g.typeName(inType.Key()), g.typeName(inType.Elem())) case reflect.Slice: return fmt.Sprintf("[]%s", g.typeName(inType.Elem())) case reflect.Ptr: return fmt.Sprintf("*%s", g.typeName(inType.Elem())) + case reflect.Map: + if len(inType.Name()) == 0 { + return fmt.Sprintf("map[%s]%s", g.typeName(inType.Key()), g.typeName(inType.Elem())) + } + fallthrough default: - typeWithPkg := fmt.Sprintf("%s", inType) - slices := strings.Split(typeWithPkg, ".") - if len(slices) == 1 { + pkg, name := inType.PkgPath(), inType.Name() + if len(name) == 0 && inType.Kind() == reflect.Struct { + return "struct{}" + } + if len(pkg) == 0 { // Default package. - return slices[0] + return name } - if len(slices) == 2 { - pkg := slices[0] - if val, found := g.pkgOverwrites[pkg]; found { - pkg = val - } - if pkg != "" { - pkg = pkg + "." - } - return pkg + slices[1] + if val, found := g.pkgOverwrites[pkg]; found { + pkg = val } - panic("Incorrect type name: " + typeWithPkg) + if len(pkg) == 0 { + return name + } + short := g.addImportByPath(pkg) + if len(short) > 0 { + return fmt.Sprintf("%s.%s", short, name) + } + return name } } @@ -658,6 +785,10 @@ func (g *conversionGenerator) existsDedicatedConversionFunction(inType, outType // unnamed. Thus we return false here. return false } + // TODO: no way to handle private conversions in different packages + if g.assumePrivateConversions { + return false + } return g.scheme.Converter().HasConversionFunc(inType, outType) } diff --git a/pkg/runtime/deep_copy_generator.go b/pkg/runtime/deep_copy_generator.go index 7be7af6bd86..20d931caa0a 100644 --- a/pkg/runtime/deep_copy_generator.go +++ b/pkg/runtime/deep_copy_generator.go @@ -19,6 +19,7 @@ package runtime import ( "fmt" "io" + "path" "reflect" "sort" "strings" @@ -38,9 +39,20 @@ type DeepCopyGenerator interface { // functions for this type and all nested types will be generated. AddType(inType reflect.Type) error + // ReplaceType registers a type that should be used instead of the type + // with the provided pkgPath and name. + ReplaceType(pkgPath, name string, in interface{}) + + // AddImport registers a package name with the generator and returns its + // short name. + AddImport(pkgPath string) string + + // RepackImports creates a stable ordering of import short names + RepackImports() + // Writes all imports that are necessary for deep-copy function and // their registration. - WriteImports(w io.Writer, pkg string) error + WriteImports(w io.Writer) error // Writes deel-copy functions for all types added via AddType() method // and their nested types. @@ -57,20 +69,80 @@ type DeepCopyGenerator interface { OverwritePackage(pkg, overwrite string) } -func NewDeepCopyGenerator(scheme *conversion.Scheme) DeepCopyGenerator { - return &deepCopyGenerator{ +func NewDeepCopyGenerator(scheme *conversion.Scheme, targetPkg string, include util.StringSet) DeepCopyGenerator { + g := &deepCopyGenerator{ scheme: scheme, + targetPkg: targetPkg, copyables: make(map[reflect.Type]bool), - imports: util.StringSet{}, + imports: make(map[string]string), + shortImports: make(map[string]string), pkgOverwrites: make(map[string]string), + replace: make(map[pkgPathNamePair]reflect.Type), + include: include, } + g.targetPackage(targetPkg) + g.AddImport("github.com/GoogleCloudPlatform/kubernetes/pkg/conversion") + return g +} + +type pkgPathNamePair struct { + PkgPath string + Name string } type deepCopyGenerator struct { - scheme *conversion.Scheme - copyables map[reflect.Type]bool - imports util.StringSet + scheme *conversion.Scheme + targetPkg string + copyables map[reflect.Type]bool + // map of package names to shortname + imports map[string]string + // map of short names to package names + shortImports map[string]string pkgOverwrites map[string]string + replace map[pkgPathNamePair]reflect.Type + include util.StringSet +} + +func (g *deepCopyGenerator) addImportByPath(pkg string) string { + if name, ok := g.imports[pkg]; ok { + return name + } + name := path.Base(pkg) + if _, ok := g.shortImports[name]; !ok { + g.imports[pkg] = name + g.shortImports[name] = pkg + return name + } + if dirname := path.Base(path.Dir(pkg)); len(dirname) > 0 { + name = dirname + name + if _, ok := g.shortImports[name]; !ok { + g.imports[pkg] = name + g.shortImports[name] = pkg + return name + } + if subdirname := path.Base(path.Dir(path.Dir(pkg))); len(subdirname) > 0 { + name = subdirname + name + if _, ok := g.shortImports[name]; !ok { + g.imports[pkg] = name + g.shortImports[name] = pkg + return name + } + } + } + for i := 2; i < 100; i++ { + generatedName := fmt.Sprintf("%s%d", name, i) + if _, ok := g.shortImports[generatedName]; !ok { + g.imports[pkg] = generatedName + g.shortImports[generatedName] = pkg + return generatedName + } + } + panic(fmt.Sprintf("unable to find a unique name for the package path %q: %v", pkg, g.shortImports)) +} + +func (g *deepCopyGenerator) targetPackage(pkg string) { + g.imports[pkg] = "" + g.shortImports[""] = pkg } func (g *deepCopyGenerator) addAllRecursiveTypes(inType reflect.Type) error { @@ -90,11 +162,18 @@ func (g *deepCopyGenerator) addAllRecursiveTypes(inType reflect.Type) error { return err } case reflect.Interface: - g.imports.Insert(inType.PkgPath()) + g.addImportByPath(inType.PkgPath()) return nil case reflect.Struct: - g.imports.Insert(inType.PkgPath()) - if !strings.HasPrefix(inType.PkgPath(), "github.com/GoogleCloudPlatform/kubernetes") { + g.addImportByPath(inType.PkgPath()) + found := false + for s := range g.include { + if strings.HasPrefix(inType.PkgPath(), s) { + found = true + break + } + } + if !found { return nil } for i := 0; i < inType.NumField(); i++ { @@ -110,6 +189,15 @@ func (g *deepCopyGenerator) addAllRecursiveTypes(inType reflect.Type) error { return nil } +func (g *deepCopyGenerator) AddImport(pkg string) string { + return g.addImportByPath(pkg) +} + +// ReplaceType registers a replacement type to be used instead of the named type +func (g *deepCopyGenerator) ReplaceType(pkgPath, name string, t interface{}) { + g.replace[pkgPathNamePair{pkgPath, name}] = reflect.TypeOf(t) +} + func (g *deepCopyGenerator) AddType(inType reflect.Type) error { if inType.Kind() != reflect.Struct { return fmt.Errorf("non-struct copies are not supported") @@ -117,10 +205,23 @@ func (g *deepCopyGenerator) AddType(inType reflect.Type) error { return g.addAllRecursiveTypes(inType) } -func (g *deepCopyGenerator) WriteImports(w io.Writer, pkg string) error { +func (g *deepCopyGenerator) RepackImports() { + var packages []string + for key := range g.imports { + packages = append(packages, key) + } + sort.Strings(packages) + g.imports = make(map[string]string) + g.shortImports = make(map[string]string) + + g.targetPackage(g.targetPkg) + for _, pkg := range packages { + g.addImportByPath(pkg) + } +} + +func (g *deepCopyGenerator) WriteImports(w io.Writer) error { var packages []string - packages = append(packages, "github.com/GoogleCloudPlatform/kubernetes/pkg/api") - packages = append(packages, "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion") for key := range g.imports { packages = append(packages, key) } @@ -130,10 +231,13 @@ func (g *deepCopyGenerator) WriteImports(w io.Writer, pkg string) error { indent := 0 buffer.addLine("import (\n", indent) for _, importPkg := range packages { - if strings.HasSuffix(importPkg, pkg) { + if len(importPkg) == 0 { continue } - buffer.addLine(fmt.Sprintf("\"%s\"\n", importPkg), indent+1) + if len(g.imports[importPkg]) == 0 { + continue + } + buffer.addLine(fmt.Sprintf("%s \"%s\"\n", g.imports[importPkg], importPkg), indent+1) } buffer.addLine(")\n", indent) buffer.addLine("\n", indent) @@ -159,35 +263,47 @@ func (s byPkgAndName) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (g *deepCopyGenerator) typeName(inType reflect.Type) string { +func (g *deepCopyGenerator) nameForType(inType reflect.Type) string { switch inType.Kind() { - case reflect.Map: - return fmt.Sprintf("map[%s]%s", g.typeName(inType.Key()), g.typeName(inType.Elem())) case reflect.Slice: return fmt.Sprintf("[]%s", g.typeName(inType.Elem())) case reflect.Ptr: return fmt.Sprintf("*%s", g.typeName(inType.Elem())) + case reflect.Map: + if len(inType.Name()) == 0 { + return fmt.Sprintf("map[%s]%s", g.typeName(inType.Key()), g.typeName(inType.Elem())) + } + fallthrough default: - typeWithPkg := fmt.Sprintf("%s", inType) - slices := strings.Split(typeWithPkg, ".") - if len(slices) == 1 { + pkg, name := inType.PkgPath(), inType.Name() + if len(name) == 0 && inType.Kind() == reflect.Struct { + return "struct{}" + } + if len(pkg) == 0 { // Default package. - return slices[0] + return name } - if len(slices) == 2 { - pkg := slices[0] - if val, found := g.pkgOverwrites[pkg]; found { - pkg = val - } - if pkg != "" { - pkg = pkg + "." - } - return pkg + slices[1] + if val, found := g.pkgOverwrites[pkg]; found { + pkg = val } - panic("Incorrect type name: " + typeWithPkg) + if len(pkg) == 0 { + return name + } + short := g.addImportByPath(pkg) + if len(short) > 0 { + return fmt.Sprintf("%s.%s", short, name) + } + return name } } +func (g *deepCopyGenerator) typeName(inType reflect.Type) string { + if t, ok := g.replace[pkgPathNamePair{inType.PkgPath(), inType.Name()}]; ok { + return g.nameForType(t) + } + return g.nameForType(inType) +} + func (g *deepCopyGenerator) deepCopyFunctionName(inType reflect.Type) string { funcNameFormat := "deepCopy_%s_%s" inPkg := packageForName(inType) @@ -442,12 +558,8 @@ func (g *deepCopyGenerator) writeDeepCopyForType(b *buffer, inType reflect.Type, func (g *deepCopyGenerator) writeRegisterHeader(b *buffer, pkg string, indent int) { b.addLine("func init() {\n", indent) - registerFormat := "err := %sScheme.AddGeneratedDeepCopyFuncs(\n" - if pkg == "api" { - b.addLine(fmt.Sprintf(registerFormat, ""), indent+1) - } else { - b.addLine(fmt.Sprintf(registerFormat, "api."), indent+1) - } + registerFormat := "err := %s.AddGeneratedDeepCopyFuncs(\n" + b.addLine(fmt.Sprintf(registerFormat, pkg), indent+1) } func (g *deepCopyGenerator) writeRegisterFooter(b *buffer, indent int) {