Skip to content

K
k3s
Commit f8ea3fab authored by k8s-merge-robot's avatar k8s-merge-robot
Browse files
Options

Merge pull request #23427 from wojtek-t/conversions_with_framework_4

Automatic merge from submit-queue Migrate to the new conversion generator - part1 This PR contains two commits: - few more fixes to the generator - migration of the pkg/api/v1 to use the new generator The second commit is big, but I reviewed the changes and they contain: - conversions between types that we didn't even generating conversion between - changes in how we handle maps/pointers/slices - previously we were explicitly referencing fields, now we are using "shadowing in, out" to make the code more generic - lack of auto-generated method for ReplicationControllerSpec (because these types are different (*int vs int for Replicas) and a preexisting conversion already exists Most of issues in the first commit (e.g. adding references to "in" and "out" for slices/maps/points) were discovered by our tests. So I'm pretty confident that this change is correct now.
parents 9e96d441 700e4594
Hide whitespace changes
Inline Side-by-side
Showing with 50 additions and 38 deletions
cmd/libs/go2idl/conversion-gen/generators/conversion.go
View file @ f8ea3fab
......@@ -149,6 +149,10 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
// (in the directory one above) and can be automatically converted to.
for _, p := range context.Universe {
path := p.Path
// TODO: Only a subset of InputDirs is actually where we would like
// to generate conversions, the rest of files are added, because either
// conversion methods are generated there or they contain types
// necessary for conversions.
if !inputs.Has(path) {
continue
}
......@@ -248,6 +252,13 @@ func isDirectlyConvertible(in, out *types.Type, preexisting conversions) bool {
// Check if there is an out member with that name.
outMember, found := findMember(out, inMember.Name)
if !found {
// Check if the member doesn't have comment:
// "+ genconversion=false"
// comment to ignore this field for conversion.
// TODO: Switch to SecondClosestCommentLines.
if types.ExtractCommentTags("+", inMember.CommentLines)["genconversion"] == "false" {
continue
}
return false
}
convertible = convertible && isConvertible(inMember.Type, outMember.Type, preexisting)
......@@ -306,7 +317,7 @@ func (g *genConversion) convertibleOnlyWithinPackage(inType, outType *types.Type
if t.Name.Package != g.targetPackage {
return false
}
if types.ExtractCommentTags("+", t.CommentLines)["genConversion"] == "false" {
if types.ExtractCommentTags("+", t.CommentLines)["genconversion"] == "false" {
return false
}
// TODO: Consider generating functions for other kinds too.
......@@ -478,14 +489,14 @@ func (g *genConversion) doBuiltin(inType, outType *types.Type, sw *generator.Sni
}
func (g *genConversion) doMap(inType, outType *types.Type, sw *generator.SnippetWriter) {
sw.Do("out = make($.|raw$, len(in))\n", outType)
sw.Do("*out = make($.|raw$, len(*in))\n", outType)
if outType.Key.IsAssignable() {
sw.Do("for key, val := range in {\n", nil)
sw.Do("for key, val := range *in {\n", nil)
if outType.Elem.IsAssignable() {
if inType.Key == outType.Key {
sw.Do("out[key] = ", nil)
sw.Do("(*out)[key] = ", nil)
} else {
sw.Do("out[$.|raw$(key)] = ", outType.Key)
sw.Do("(*out)[$.|raw$(key)] = ", outType.Key)
}
if inType.Elem == outType.Elem {
sw.Do("val\n", nil)
......@@ -506,40 +517,40 @@ func (g *genConversion) doMap(inType, outType *types.Type, sw *generator.Snippet
sw.Do("return err\n", nil)
sw.Do("}\n", nil)
if inType.Key == outType.Key {
sw.Do("out[key] = *newVal\n", nil)
sw.Do("(*out)[key] = *newVal\n", nil)
} else {
sw.Do("out[$.|raw$(key)] = *newVal\n", outType.Key)
sw.Do("(*out)[$.|raw$(key)] = *newVal\n", outType.Key)
}
}
} else {
// TODO: Implement it when necessary.
sw.Do("for range in {\n", nil)
sw.Do("for range *in {\n", nil)
sw.Do("// FIXME: Converting unassignable keys unsupported $.|raw$\n", inType.Key)
}
sw.Do("}\n", nil)
}
func (g *genConversion) doSlice(inType, outType *types.Type, sw *generator.SnippetWriter) {
sw.Do("out = make($.|raw$, len(in))\n", outType)
sw.Do("*out = make($.|raw$, len(*in))\n", outType)
if inType.Elem == outType.Elem && inType.Elem.Kind == types.Builtin {
sw.Do("copy(out, in)\n", nil)
sw.Do("copy(*out, *in)\n", nil)
} else {
sw.Do("for i := range in {\n", nil)
sw.Do("for i := range *in {\n", nil)
if outType.Elem.IsAssignable() {
if inType.Elem == outType.Elem {
sw.Do("out[i] = in[i]\n", nil)
sw.Do("(*out)[i] = (*in)[i]\n", nil)
} else {
sw.Do("out[i] = $.|raw$(in[i])\n", outType.Elem)
sw.Do("(*out)[i] = $.|raw$((*in)[i])\n", outType.Elem)
}
} else {
if function, ok := g.preexists(inType.Elem, outType.Elem); ok {
sw.Do("if err := $.|raw$(&in[i], &out[i], s); err != nil {\n", function)
sw.Do("if err := $.|raw$(&(*in)[i], &(*out)[i], s); err != nil {\n", function)
} else if g.convertibleOnlyWithinPackage(inType.Elem, outType.Elem) {
funcName := g.funcNameTmpl(inType.Elem, outType.Elem)
sw.Do(fmt.Sprintf("if err := %s(&in[i], &out[i], s); err != nil {\n", funcName), argsFromType(inType.Elem, outType.Elem))
sw.Do(fmt.Sprintf("if err := %s(&(*in)[i], &(*out)[i], s); err != nil {\n", funcName), argsFromType(inType.Elem, outType.Elem))
} else {
sw.Do("// TODO: Inefficient conversion - can we improve it?\n", nil)
sw.Do("if err := s.Convert(&in[i], &out[i], 0); err != nil {\n", nil)
sw.Do("if err := s.Convert(&(*in)[i], &(*out)[i], 0); err != nil {\n", nil)
}
sw.Do("return err\n", nil)
sw.Do("}\n", nil)
......@@ -550,7 +561,12 @@ func (g *genConversion) doSlice(inType, outType *types.Type, sw *generator.Snipp
func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.SnippetWriter) {
for _, m := range inType.Members {
outMember, _ := findMember(outType, m.Name)
outMember, isOutMember := findMember(outType, m.Name)
if !isOutMember {
// Since this object wasn't filtered out, this means that
// this field has "genconversion=false" comment to ignore it.
continue
}
args := map[string]interface{}{
"inType": m.Type,
"outType": outMember.Type,
......@@ -572,7 +588,7 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
}
case types.Map, types.Slice, types.Pointer:
sw.Do("if in.$.name$ != nil {\n", args)
sw.Do("in, out := in.$.name$, out.$.name$\n", args)
sw.Do("in, out := &in.$.name$, &out.$.name$\n", args)
g.generateFor(m.Type, outMember.Type, sw)
sw.Do("} else {\n", nil)
sw.Do("out.$.name$ = nil\n", args)
......@@ -589,7 +605,11 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
sw.Do("}\n", nil)
case types.Alias:
if outMember.Type.IsAssignable() {
sw.Do("out.$.name$ = $.outType|raw$(in.$.name$)\n", args)
if m.Type == outMember.Type {
sw.Do("out.$.name$ = in.$.name$\n", args)
} else {
sw.Do("out.$.name$ = $.outType|raw$(in.$.name$)\n", args)
}
} else {
if g.convertibleOnlyWithinPackage(m.Type, outMember.Type) {
funcName := g.funcNameTmpl(m.Type, outMember.Type)
......@@ -616,22 +636,22 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
}
func (g *genConversion) doPointer(inType, outType *types.Type, sw *generator.SnippetWriter) {
sw.Do("out = new($.Elem|raw$)\n", outType)
sw.Do("*out = new($.Elem|raw$)\n", outType)
if outType.Elem.IsAssignable() {
if inType.Elem == outType.Elem {
sw.Do("*out = *in\n", nil)
sw.Do("**out = **in\n", nil)
} else {
sw.Do("*out = $.|raw$(*in)\n", outType.Elem)
sw.Do("**out = $.|raw$(**in)\n", outType.Elem)
}
} else {
if function, ok := g.preexists(inType.Elem, outType.Elem); ok {
sw.Do("if err := $.|raw$(in, out, s); err != nil {\n", function)
sw.Do("if err := $.|raw$(*in, *out, s); err != nil {\n", function)
} else if g.convertibleOnlyWithinPackage(inType.Elem, outType.Elem) {
funcName := g.funcNameTmpl(inType.Elem, outType.Elem)
sw.Do(fmt.Sprintf("if err := %s(in, out, s); err != nil {\n", funcName), argsFromType(inType.Elem, outType.Elem))
sw.Do(fmt.Sprintf("if err := %s(*in, *out, s); err != nil {\n", funcName), argsFromType(inType.Elem, outType.Elem))
} else {
sw.Do("// TODO: Inefficient conversion - can we improve it?\n", nil)
sw.Do("if err := s.Convert(in, out, 0); err != nil {\n", nil)
sw.Do("if err := s.Convert(*in, *out, 0); err != nil {\n", nil)
}
sw.Do("return err\n", nil)
sw.Do("}\n", nil)
......
hack/after-build/run-codegen.sh
View file @ f8ea3fab
......@@ -35,8 +35,7 @@ setgen=$(kube::util::find-binary "set-gen")
# update- and verify- scripts.
${clientgen} "$@"
${clientgen} -t "$@"
# TODO: Enable when conversion generator is ready.
# ${conversiongen} "$@"
${conversiongen} "$@"
${deepcopygen} "$@"
${setgen} "$@"
......
hack/after-build/update-generated-conversions.sh
View file @ f8ea3fab
......@@ -43,7 +43,7 @@ EOF
}
# TODO(lavalamp): get this list by listing the pkg/apis/ directory?
DEFAULT_GROUP_VERSIONS="v1 authorization/v1beta1 autoscaling/v1 batch/v1 extensions/v1beta1 componentconfig/v1alpha1 metrics/v1alpha1"
DEFAULT_GROUP_VERSIONS="authorization/v1beta1 autoscaling/v1 batch/v1 extensions/v1beta1 componentconfig/v1alpha1 metrics/v1alpha1"
VERSIONS=${VERSIONS:-$DEFAULT_GROUP_VERSIONS}
for ver in $VERSIONS; do
# Ensure that the version being processed is registered by setting
......
hack/after-build/verify-generated-conversions.sh
View file @ f8ea3fab
......@@ -23,7 +23,7 @@ source "${KUBE_ROOT}/hack/lib/init.sh"
kube::golang::setup_env
APIROOTS=${APIROOTS:-pkg/api pkg/apis/authorization pkg/apis/autoscaling pkg/apis/batch pkg/apis/extensions pkg/apis/metrics}
APIROOTS=${APIROOTS:-pkg/apis/authorization pkg/apis/autoscaling pkg/apis/batch pkg/apis/extensions pkg/apis/metrics}
_tmp="${KUBE_ROOT}/_tmp"
cleanup() {
......
hack/verify-codegen.sh
View file @ f8ea3fab
......@@ -26,6 +26,7 @@ source "${KUBE_ROOT}/hack/lib/init.sh"
kube::golang::setup_env
"${KUBE_ROOT}/hack/build-go.sh" cmd/libs/go2idl/client-gen
"${KUBE_ROOT}/hack/build-go.sh" cmd/libs/go2idl/conversion-gen
"${KUBE_ROOT}/hack/build-go.sh" cmd/libs/go2idl/deepcopy-gen
"${KUBE_ROOT}/hack/build-go.sh" cmd/libs/go2idl/set-gen
......
pkg/api/unversioned/deep_copy_generated.go
View file @ f8ea3fab
......@@ -30,15 +30,6 @@ func DeepCopy_unversioned_Duration(in Duration, out *Duration, c *conversion.Clo
return nil
}
func DeepCopy_unversioned_ExportOptions(in ExportOptions, out *ExportOptions, c *conversion.Cloner) error {
if err := DeepCopy_unversioned_TypeMeta(in.TypeMeta, &out.TypeMeta, c); err != nil {
return err
}
out.Export = in.Export
out.Exact = in.Exact
return nil
}
func DeepCopy_unversioned_GroupKind(in GroupKind, out *GroupKind, c *conversion.Cloner) error {
out.Group = in.Group
out.Kind = in.Kind
......
pkg/api/v1/conversion_generated.go
View file @ f8ea3fab
This source diff could not be displayed because it is too large. You can view the blob instead.
pkg/api/v1/types.go
View file @ f8ea3fab
......@@ -1816,6 +1816,7 @@ type ServiceSpec struct {
// API for compatibility until at least 8/20/2016. It will be removed from
// any new API revisions. If both deprecatedPublicIPs *and* externalIPs are
// set, deprecatedPublicIPs is used.
// +genconversion=false
DeprecatedPublicIPs []string `json:"deprecatedPublicIPs,omitempty"`
// Supports "ClientIP" and "None". Used to maintain session affinity.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment