diff --git a/cmd/genconversion/conversion.go b/cmd/genconversion/conversion.go index ae456ff4c7a..91e0b217555 100644 --- a/cmd/genconversion/conversion.go +++ b/cmd/genconversion/conversion.go @@ -118,7 +118,10 @@ func main() { b, err := imports.Process("", data.Bytes(), nil) if err != nil { - glog.Fatalf("Error while update imports: %v", err) + for i, s := range bytes.Split(data.Bytes(), []byte("\n")) { + glog.Infof("%d:\t%s", i, s) + } + glog.Fatalf("Error while update imports: %v\n", err) } if _, err := funcOut.Write(b); err != nil { glog.Fatalf("Error while writing out the resulting file: %v", err) diff --git a/pkg/api/conversion.go b/pkg/api/conversion.go index 339a3454ba0..46e33a8c183 100644 --- a/pkg/api/conversion.go +++ b/pkg/api/conversion.go @@ -23,6 +23,7 @@ import ( "k8s.io/kubernetes/pkg/fields" "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/intstr" ) // Codec is the identity codec for this package - it can only convert itself @@ -41,45 +42,75 @@ func init() { }, ) Scheme.AddConversionFuncs( - func(in *unversioned.Time, out *unversioned.Time, s conversion.Scope) error { - // Cannot deep copy these, because time.Time has unexported fields. - *out = *in - return nil - }, - func(in *string, out *labels.Selector, s conversion.Scope) error { - selector, err := labels.Parse(*in) - if err != nil { - return err - } - *out = selector - return nil - }, - func(in *string, out *fields.Selector, s conversion.Scope) error { - selector, err := fields.ParseSelector(*in) - if err != nil { - return err - } - *out = selector - return nil - }, - func(in *labels.Selector, out *string, s conversion.Scope) error { - if *in == nil { - return nil - } - *out = (*in).String() - return nil - }, - func(in *fields.Selector, out *string, s conversion.Scope) error { - if *in == nil { - return nil - } - *out = (*in).String() - return nil - }, - func(in *resource.Quantity, out *resource.Quantity, s conversion.Scope) error { - // Cannot deep copy these, because inf.Dec has unexported fields. - *out = *in.Copy() - return nil - }, + Convert_unversioned_TypeMeta_To_unversioned_TypeMeta, + Convert_unversioned_ListMeta_To_unversioned_ListMeta, + Convert_intstr_IntOrString_To_intstr_IntOrString, + Convert_unversioned_Time_To_unversioned_Time, + Convert_string_To_labels_Selector, + Convert_string_To_fields_Selector, + Convert_labels_Selector_To_string, + Convert_fields_Selector_To_string, + Convert_resource_Quantity_To_resource_Quantity, ) } + +func Convert_unversioned_TypeMeta_To_unversioned_TypeMeta(in, out *unversioned.TypeMeta, s conversion.Scope) error { + // These values are explicitly not copied + //out.APIVersion = in.APIVersion + //out.Kind = in.Kind + return nil +} + +func Convert_unversioned_ListMeta_To_unversioned_ListMeta(in, out *unversioned.ListMeta, s conversion.Scope) error { + out.ResourceVersion = in.ResourceVersion + out.SelfLink = in.SelfLink + return nil +} + +func Convert_intstr_IntOrString_To_intstr_IntOrString(in, out *intstr.IntOrString, s conversion.Scope) error { + out.Type = in.Type + out.IntVal = in.IntVal + out.StrVal = in.StrVal + return nil +} + +func Convert_unversioned_Time_To_unversioned_Time(in *unversioned.Time, out *unversioned.Time, s conversion.Scope) error { + // Cannot deep copy these, because time.Time has unexported fields. + *out = *in + return nil +} +func Convert_string_To_labels_Selector(in *string, out *labels.Selector, s conversion.Scope) error { + selector, err := labels.Parse(*in) + if err != nil { + return err + } + *out = selector + return nil +} +func Convert_string_To_fields_Selector(in *string, out *fields.Selector, s conversion.Scope) error { + selector, err := fields.ParseSelector(*in) + if err != nil { + return err + } + *out = selector + return nil +} +func Convert_labels_Selector_To_string(in *labels.Selector, out *string, s conversion.Scope) error { + if *in == nil { + return nil + } + *out = (*in).String() + return nil +} +func Convert_fields_Selector_To_string(in *fields.Selector, out *string, s conversion.Scope) error { + if *in == nil { + return nil + } + *out = (*in).String() + return nil +} +func Convert_resource_Quantity_To_resource_Quantity(in *resource.Quantity, out *resource.Quantity, s conversion.Scope) error { + // Cannot deep copy these, because inf.Dec has unexported fields. + *out = *in.Copy() + return nil +} diff --git a/pkg/conversion/converter.go b/pkg/conversion/converter.go index 8c0bdfc9b1f..8c106d52611 100644 --- a/pkg/conversion/converter.go +++ b/pkg/conversion/converter.go @@ -87,12 +87,12 @@ func NewConverter() *Converter { inputFieldMappingFuncs: map[reflect.Type]FieldMappingFunc{}, inputDefaultFlags: map[reflect.Type]FieldMatchingFlags{}, } - c.RegisterConversionFunc(byteSliceCopy) + c.RegisterConversionFunc(ByteSliceCopy) return c } -// Prevent recursing into every byte... -func byteSliceCopy(in *[]byte, out *[]byte, s Scope) error { +// ByteSliceCopy prevents recursing into every byte +func ByteSliceCopy(in *[]byte, out *[]byte, s Scope) error { *out = make([]byte, len(*in)) copy(*out, *in) return nil @@ -322,6 +322,11 @@ func (c *Converter) HasConversionFunc(inType, outType reflect.Type) bool { return found } +func (c *Converter) ConversionFuncValue(inType, outType reflect.Type) (reflect.Value, bool) { + value, found := c.conversionFuncs[typePair{inType, outType}] + return value, found +} + // SetStructFieldCopy registers a correspondence. Whenever a struct field is encountered // which has a type and name matching srcFieldType and srcFieldName, it wil be copied // into the field in the destination struct matching destFieldType & Name, if such a diff --git a/pkg/runtime/conversion_generator.go b/pkg/runtime/conversion_generator.go index 3b3d2389644..54e2f0a9cc8 100644 --- a/pkg/runtime/conversion_generator.go +++ b/pkg/runtime/conversion_generator.go @@ -19,8 +19,10 @@ package runtime import ( "fmt" "io" + "log" "path" "reflect" + goruntime "runtime" "sort" "strings" @@ -42,8 +44,13 @@ type ConversionGenerator interface { func NewConversionGenerator(scheme *conversion.Scheme, targetPkg string) ConversionGenerator { g := &conversionGenerator{ - scheme: scheme, - targetPkg: targetPkg, + scheme: scheme, + + nameFormat: "Convert_%s_%s_To_%s_%s", + generatedNamePrefix: "auto", + targetPkg: targetPkg, + + publicFuncs: make(map[typePair]string), convertibles: make(map[reflect.Type]reflect.Type), overridden: make(map[reflect.Type]bool), pkgOverwrites: make(map[string]string), @@ -59,8 +66,13 @@ func NewConversionGenerator(scheme *conversion.Scheme, targetPkg string) Convers var complexTypes []reflect.Kind = []reflect.Kind{reflect.Map, reflect.Ptr, reflect.Slice, reflect.Interface, reflect.Struct} type conversionGenerator struct { - scheme *conversion.Scheme - targetPkg string + scheme *conversion.Scheme + + nameFormat string + generatedNamePrefix string + targetPkg string + + publicFuncs map[typePair]string convertibles map[reflect.Type]reflect.Type overridden map[reflect.Type]bool // If pkgOverwrites is set for a given package name, that package name @@ -138,6 +150,14 @@ func (g *conversionGenerator) generateConversionsBetween(inType, outType reflect return nil } if inType == outType { + switch inType.Kind() { + case reflect.Ptr: + return g.generateConversionsBetween(inType.Elem(), inType.Elem()) + case reflect.Struct: + // pointers to structs invoke new(inType) + g.addImportByPath(inType.PkgPath()) + } + g.rememberConversionFunction(inType, inType, false) // Don't generate conversion methods for the same type. return nil } @@ -148,6 +168,8 @@ func (g *conversionGenerator) generateConversionsBetween(inType, outType reflect if inType.Kind() != outType.Kind() { if existingConversion { + g.rememberConversionFunction(inType, outType, false) + g.rememberConversionFunction(outType, inType, false) return nil } return fmt.Errorf("cannot convert types of different kinds: %v %v", inType, outType) @@ -194,6 +216,7 @@ func (g *conversionGenerator) generateConversionsBetween(inType, outType reflect if !existingConversion && (inErr != nil || outErr != nil) { return inErr } + g.rememberConversionFunction(inType, outType, true) if existingConversion { g.overridden[inType] = true } @@ -214,6 +237,42 @@ func isComplexType(reflection reflect.Type) bool { return false } +func (g *conversionGenerator) rememberConversionFunction(inType, outType reflect.Type, willGenerate bool) { + if _, ok := g.publicFuncs[typePair{inType, outType}]; ok { + return + } + + if v, ok := g.scheme.Converter().ConversionFuncValue(inType, outType); ok { + if fn := goruntime.FuncForPC(v.Pointer()); fn != nil { + name := fn.Name() + var p, n string + if last := strings.LastIndex(name, "."); last != -1 { + p = name[:last] + n = name[last+1:] + p = g.imports[p] + if len(p) > 0 { + p = p + "." + } + } else { + n = name + } + if isPublic(n) { + g.publicFuncs[typePair{inType, outType}] = p + n + } else { + log.Printf("WARNING: Cannot generate conversion %v -> %v, method %q is private", inType, outType, fn.Name()) + } + } else { + log.Printf("WARNING: Cannot generate conversion %v -> %v, method is not accessible", inType, outType) + } + } else if willGenerate { + g.publicFuncs[typePair{inType, outType}] = g.conversionFunctionName(inType, outType) + } +} + +func isPublic(name string) bool { + return strings.ToUpper(name[:1]) == name[:1] +} + func (g *conversionGenerator) generateConversionsForMap(inType, outType reflect.Type) error { inKey := inType.Key() outKey := outType.Key() @@ -507,15 +566,24 @@ func packageForName(inType reflect.Type) string { } func (g *conversionGenerator) conversionFunctionName(inType, outType reflect.Type) string { - funcNameFormat := "convert_%s_%s_To_%s_%s" + funcNameFormat := g.nameFormat inPkg := packageForName(inType) outPkg := packageForName(outType) funcName := fmt.Sprintf(funcNameFormat, inPkg, inType.Name(), outPkg, outType.Name()) return funcName } +func (g *conversionGenerator) conversionFunctionCall(inType, outType reflect.Type, scopeName string, args ...string) string { + if named, ok := g.publicFuncs[typePair{inType, outType}]; ok { + args[len(args)-1] = scopeName + return fmt.Sprintf("%s(%s)", named, strings.Join(args, ", ")) + } + log.Printf("WARNING: Using reflection to convert %v -> %v (no public conversion)", inType, outType) + return fmt.Sprintf("%s.Convert(%s)", scopeName, strings.Join(args, ", ")) +} + func (g *conversionGenerator) generatedFunctionName(inType, outType reflect.Type) string { - return "auto" + g.conversionFunctionName(inType, outType) + return g.generatedNamePrefix + g.conversionFunctionName(inType, outType) } func (g *conversionGenerator) writeHeader(b *buffer, name, inType, outType string, indent int) { @@ -547,7 +615,8 @@ func (g *conversionGenerator) writeConversionForMap(b *buffer, inField, outField newFormat := "newVal := %s{}\n" newStmt := fmt.Sprintf(newFormat, g.typeName(outField.Type.Elem())) b.addLine(newStmt, indent+2) - convertStmt := "if err := s.Convert(&val, &newVal, 0); err != nil {\n" + call := g.conversionFunctionCall(inField.Type.Elem(), outField.Type.Elem(), "s", "&val", "&newVal", "0") + convertStmt := fmt.Sprintf("if err := %s; err != nil {\n", call) b.addLine(convertStmt, indent+2) b.addLine("return err\n", indent+3) b.addLine("}\n", indent+2) @@ -607,15 +676,8 @@ func (g *conversionGenerator) writeConversionForSlice(b *buffer, inField, outFie } } if !assigned { - assignStmt := "" - if g.existsDedicatedConversionFunction(inField.Type.Elem(), outField.Type.Elem()) { - assignFormat := "if err := %s(&in.%s[i], &out.%s[i], s); err != nil {\n" - funcName := g.conversionFunctionName(inField.Type.Elem(), outField.Type.Elem()) - assignStmt = fmt.Sprintf(assignFormat, funcName, inField.Name, outField.Name) - } else { - assignFormat := "if err := s.Convert(&in.%s[i], &out.%s[i], 0); err != nil {\n" - assignStmt = fmt.Sprintf(assignFormat, inField.Name, outField.Name) - } + call := g.conversionFunctionCall(inField.Type.Elem(), outField.Type.Elem(), "s", "&in."+inField.Name+"[i]", "&out."+outField.Name+"[i]", "0") + assignStmt := fmt.Sprintf("if err := %s; err != nil {\n", call) b.addLine(assignStmt, indent+2) b.addLine("return err\n", indent+3) b.addLine("}\n", indent+2) @@ -664,20 +726,20 @@ func (g *conversionGenerator) writeConversionForPtr(b *buffer, inField, outField } } + b.addLine(fmt.Sprintf("// unable to generate simple pointer conversion for %v -> %v\n", inField.Type.Elem(), outField.Type.Elem()), indent) ifFormat := "if in.%s != nil {\n" ifStmt := fmt.Sprintf(ifFormat, inField.Name) b.addLine(ifStmt, indent) assignStmt := "" - if g.existsDedicatedConversionFunction(inField.Type.Elem(), outField.Type.Elem()) { + if _, ok := g.publicFuncs[typePair{inField.Type.Elem(), outField.Type.Elem()}]; ok { newFormat := "out.%s = new(%s)\n" newStmt := fmt.Sprintf(newFormat, outField.Name, g.typeName(outField.Type.Elem())) b.addLine(newStmt, indent+1) - assignFormat := "if err := %s(in.%s, out.%s, s); err != nil {\n" - funcName := g.conversionFunctionName(inField.Type.Elem(), outField.Type.Elem()) - assignStmt = fmt.Sprintf(assignFormat, funcName, inField.Name, outField.Name) + call := g.conversionFunctionCall(inField.Type.Elem(), outField.Type.Elem(), "s", "in."+inField.Name, "out."+outField.Name, "0") + assignStmt = fmt.Sprintf("if err := %s; err != nil {\n", call) } else { - assignFormat := "if err := s.Convert(&in.%s, &out.%s, 0); err != nil {\n" - assignStmt = fmt.Sprintf(assignFormat, inField.Name, outField.Name) + call := g.conversionFunctionCall(inField.Type.Elem(), outField.Type.Elem(), "s", "&in."+inField.Name, "&out."+outField.Name, "0") + assignStmt = fmt.Sprintf("if err := %s; err != nil {\n", call) } b.addLine(assignStmt, indent+1) b.addLine("return err\n", indent+2) @@ -714,10 +776,14 @@ func (g *conversionGenerator) writeConversionForStruct(b *buffer, inType, outTyp } existsConversion := g.scheme.Converter().HasConversionFunc(inField.Type, outField.Type) - if existsConversion && !g.existsDedicatedConversionFunction(inField.Type, outField.Type) { + _, hasPublicConversion := g.publicFuncs[typePair{inField.Type, outField.Type}] + // TODO: This allows a private conversion for a slice to take precedence over a public + // conversion for the field, even though that is technically slower. We should report when + // we generate an inefficient conversion. + if existsConversion || hasPublicConversion { // Use the conversion method that is already defined. - assignFormat := "if err := s.Convert(&in.%s, &out.%s, 0); err != nil {\n" - assignStmt := fmt.Sprintf(assignFormat, inField.Name, outField.Name) + call := g.conversionFunctionCall(inField.Type, outField.Type, "s", "&in."+inField.Name, "&out."+outField.Name, "0") + assignStmt := fmt.Sprintf("if err := %s; err != nil {\n", call) b.addLine(assignStmt, indent) b.addLine("return err\n", indent+1) b.addLine("}\n", indent) @@ -773,15 +839,8 @@ func (g *conversionGenerator) writeConversionForStruct(b *buffer, inType, outTyp } } - assignStmt := "" - if g.existsDedicatedConversionFunction(inField.Type, outField.Type) { - assignFormat := "if err := %s(&in.%s, &out.%s, s); err != nil {\n" - funcName := g.conversionFunctionName(inField.Type, outField.Type) - assignStmt = fmt.Sprintf(assignFormat, funcName, inField.Name, outField.Name) - } else { - assignFormat := "if err := s.Convert(&in.%s, &out.%s, 0); err != nil {\n" - assignStmt = fmt.Sprintf(assignFormat, inField.Name, outField.Name) - } + call := g.conversionFunctionCall(inField.Type, outField.Type, "s", "&in."+inField.Name, "&out."+outField.Name, "0") + assignStmt := fmt.Sprintf("if err := %s; err != nil {\n", call) b.addLine(assignStmt, indent) b.addLine("return err\n", indent+1) b.addLine("}\n", indent) @@ -843,33 +902,6 @@ var defaultConversions []typePair = []typePair{ {reflect.TypeOf(EmbeddedObject{}), reflect.TypeOf(RawExtension{})}, } -func (g *conversionGenerator) existsDedicatedConversionFunction(inType, outType reflect.Type) bool { - if inType == outType { - // Assume that conversion are not defined for "deep copies". - return false - } - - if g.existsConversionFunction(inType, outType) { - return true - } - - for _, conv := range defaultConversions { - if conv.inType == inType && conv.outType == outType { - return false - } - } - if inType.Kind() != outType.Kind() { - // TODO(wojtek-t): Currently all conversions between types of different kinds are - // 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) -} - func (g *conversionGenerator) OverwritePackage(pkg, overwrite string) { g.pkgOverwrites[pkg] = overwrite }