From ad925dd2e819978dd79aeb25b2a71ee0ddbad5f8 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Wed, 4 Nov 2015 13:56:38 -0800 Subject: [PATCH] Add verification to code gen --- cmd/libs/go2idl/args/args.go | 7 ++ cmd/libs/go2idl/generator/execute.go | 74 ++++++++++++++++++++-- cmd/libs/go2idl/generator/generator.go | 5 ++ cmd/libs/go2idl/set-gen/generators/sets.go | 6 +- cmd/libs/go2idl/set-gen/main.go | 6 +- hack/after-build/run-codegen.sh | 35 ++++++++++ hack/update-all.sh | 9 +-- hack/update-codegen.sh | 30 +++++++++ hack/verify-codegen.sh | 30 +++++++++ hack/verify-flags/known-flags.txt | 1 + pkg/util/sets/byte.go | 2 +- pkg/util/sets/doc.go | 2 +- pkg/util/sets/empty.go | 2 +- pkg/util/sets/int.go | 2 +- pkg/util/sets/int64.go | 2 +- pkg/util/sets/string.go | 2 +- 16 files changed, 193 insertions(+), 22 deletions(-) create mode 100755 hack/after-build/run-codegen.sh create mode 100755 hack/update-codegen.sh create mode 100755 hack/verify-codegen.sh diff --git a/cmd/libs/go2idl/args/args.go b/cmd/libs/go2idl/args/args.go index 2b589ca2f17..4fd0d87c605 100644 --- a/cmd/libs/go2idl/args/args.go +++ b/cmd/libs/go2idl/args/args.go @@ -56,6 +56,9 @@ type GeneratorArgs struct { // Where to get copyright header text. GoHeaderFilePath string + + // If true, only verify, don't write anything. + VerifyOnly bool } func (g *GeneratorArgs) AddFlags(fs *pflag.FlagSet) { @@ -63,6 +66,7 @@ func (g *GeneratorArgs) AddFlags(fs *pflag.FlagSet) { fs.StringVarP(&g.OutputBase, "output-base", "o", g.OutputBase, "Output base; defaults to $GOPATH/src/ or ./ if $GOPATH is not set.") fs.StringVarP(&g.OutputPackagePath, "output-package", "p", g.OutputPackagePath, "Base package path.") fs.StringVarP(&g.GoHeaderFilePath, "go-header-file", "h", g.GoHeaderFilePath, "File containing boilerplate header text. The string YEAR will be replaced with the current 4-digit year.") + fs.BoolVar(&g.VerifyOnly, "verify-only", g.VerifyOnly, "If true, only verify existing output, do not write anything.") } // LoadGoBoilerplate loads the boilerplate file passed to --go-header-file. @@ -75,6 +79,8 @@ func (g *GeneratorArgs) LoadGoBoilerplate() ([]byte, error) { return b, nil } +// NewBuilder makes a new parser.Builder and populates it with the input +// directories. func (g *GeneratorArgs) NewBuilder() (*parser.Builder, error) { b := parser.New() for _, d := range g.InputDirs { @@ -112,6 +118,7 @@ func (g *GeneratorArgs) Execute(nameSystems namer.NameSystems, defaultSystem str return fmt.Errorf("Failed making a context: %v", err) } + c.Verify = g.VerifyOnly packages := pkgs(c, g) if err := c.ExecutePackages(g.OutputBase, packages); err != nil { return fmt.Errorf("Failed executing generator: %v", err) diff --git a/cmd/libs/go2idl/generator/execute.go b/cmd/libs/go2idl/generator/execute.go index 7aa333c1917..2e6a868bb1b 100644 --- a/cmd/libs/go2idl/generator/execute.go +++ b/cmd/libs/go2idl/generator/execute.go @@ -21,6 +21,7 @@ import ( "fmt" "go/format" "io" + "io/ioutil" "log" "os" "path/filepath" @@ -30,17 +31,29 @@ import ( "k8s.io/kubernetes/cmd/libs/go2idl/types" ) +func errs2strings(errors []error) []string { + strs := make([]string, len(errors)) + for i := range errors { + strs[i] = errors[i].Error() + } + return strs +} + // ExecutePackages runs the generators for every package in 'packages'. 'outDir' // is the base directory in which to place all the generated packages; it // should be a physical path on disk, not an import path. e.g.: // /path/to/home/path/to/gopath/src/ // Each package has its import path already, this will be appended to 'outDir'. func (c *Context) ExecutePackages(outDir string, packages Packages) error { + var errors []error for _, p := range packages { if err := c.ExecutePackage(outDir, p); err != nil { - return err + errors = append(errors, err) } } + if len(errors) > 0 { + return fmt.Errorf("some packages had errors:\n%v\n", strings.Join(errs2strings(errors), "\n")) + } return nil } @@ -61,8 +74,11 @@ func (ft golangFileType) AssembleFile(f *File, pathname string) error { return et.Error() } if formatted, err := format.Source(b.Bytes()); err != nil { - log.Printf("Warning: unable to run gofmt on %q (%v).", pathname, err) - _, err = destFile.Write(b.Bytes()) + err = fmt.Errorf("unable to run gofmt on %q (%v).", pathname, err) + // Write the file anyway, so they can see what's going wrong and fix the generator. + if _, err2 := destFile.Write(b.Bytes()); err2 != nil { + return err2 + } return err } else { _, err = destFile.Write(formatted) @@ -70,6 +86,41 @@ func (ft golangFileType) AssembleFile(f *File, pathname string) error { } } +func (ft golangFileType) VerifyFile(f *File, pathname string) error { + log.Printf("Verifying file %q", pathname) + friendlyName := filepath.Join(f.PackageName, f.Name) + b := &bytes.Buffer{} + et := NewErrorTracker(b) + ft.assemble(et, f) + if et.Error() != nil { + return et.Error() + } + formatted, err := format.Source(b.Bytes()) + if err != nil { + return fmt.Errorf("unable to gofmt the output for %q: %v", friendlyName, err) + } + existing, err := ioutil.ReadFile(pathname) + if err != nil { + return fmt.Errorf("unable to read file %q for comparison: %v", friendlyName, err) + } + if bytes.Compare(formatted, existing) == 0 { + return nil + } + // Be nice and find the first place where they differ + i := 0 + for i < len(formatted) && i < len(existing) && formatted[i] == existing[i] { + i++ + } + eDiff, fDiff := existing[i:], formatted[i:] + if len(eDiff) > 100 { + eDiff = eDiff[:100] + } + if len(fDiff) > 100 { + fDiff = fDiff[:100] + } + return fmt.Errorf("output for %q differs; first existing/expected diff: \n %q\n %q", friendlyName, string(eDiff), string(fDiff)) +} + func (ft golangFileType) assemble(w io.Writer, f *File) { w.Write(f.Header) fmt.Fprintf(w, "package %v\n\n", f.PackageName) @@ -149,7 +200,7 @@ func (c *Context) addNameSystems(namers namer.NameSystems) *Context { // import path already, this will be appended to 'outDir'. func (c *Context) ExecutePackage(outDir string, p Package) error { path := filepath.Join(outDir, p.Path()) - log.Printf("Executing package %v into %v", p.Name(), path) + log.Printf("Processing package %q, disk location %q", p.Name(), path) // Filter out any types the *package* doesn't care about. packageContext := c.filteredBy(p.Filter) os.MkdirAll(path, 0755) @@ -207,14 +258,25 @@ func (c *Context) ExecutePackage(outDir string, p Package) error { } } + var errors []error for _, f := range files { + finalPath := filepath.Join(path, f.Name) assembler, ok := c.FileTypes[f.FileType] if !ok { return fmt.Errorf("the file type %q registered for file %q does not exist in the context", f.FileType, f.Name) } - if err := assembler.AssembleFile(f, filepath.Join(path, f.Name)); err != nil { - return err + var err error + if c.Verify { + err = assembler.VerifyFile(f, finalPath) + } else { + err = assembler.AssembleFile(f, finalPath) } + if err != nil { + errors = append(errors, err) + } + } + if len(errors) > 0 { + return fmt.Errorf("errors in package %q:\n%v\n", p.Name(), strings.Join(errs2strings(errors), "\n")) } return nil } diff --git a/cmd/libs/go2idl/generator/generator.go b/cmd/libs/go2idl/generator/generator.go index 6edd5ee781b..21910890a32 100644 --- a/cmd/libs/go2idl/generator/generator.go +++ b/cmd/libs/go2idl/generator/generator.go @@ -62,6 +62,7 @@ type File struct { type FileType interface { AssembleFile(f *File, path string) error + VerifyFile(f *File, path string) error } // Packages is a list of packages to generate. @@ -158,6 +159,10 @@ type Context struct { // A set of types this context can process. If this is empty or nil, // the default "golang" filetype will be provided. FileTypes map[string]FileType + + // If true, Execute* calls will just verify that the existing output is + // correct. (You may set this after calling NewContext.) + Verify bool } // NewContext generates a context from the given builder, naming systems, and diff --git a/cmd/libs/go2idl/set-gen/generators/sets.go b/cmd/libs/go2idl/set-gen/generators/sets.go index f25c4cb1068..bb7f8a0a1e5 100644 --- a/cmd/libs/go2idl/set-gen/generators/sets.go +++ b/cmd/libs/go2idl/set-gen/generators/sets.go @@ -19,8 +19,6 @@ package generators import ( "io" - "os" - "strings" "k8s.io/kubernetes/cmd/libs/go2idl/args" "k8s.io/kubernetes/cmd/libs/go2idl/generator" @@ -57,9 +55,7 @@ func Packages(_ *generator.Context, arguments *args.GeneratorArgs) generator.Pac PackagePath: arguments.OutputPackagePath, HeaderText: append(boilerplate, []byte( ` -// This file was autogenerated by the command: -// $ `+strings.Join(os.Args, " ")+` -// Do not edit it manually! +// This file was autogenerated by set-gen. Do not edit it manually! `)...), PackageDocumentation: []byte( diff --git a/cmd/libs/go2idl/set-gen/main.go b/cmd/libs/go2idl/set-gen/main.go index 9e2dd4bec54..41ff07eae29 100644 --- a/cmd/libs/go2idl/set-gen/main.go +++ b/cmd/libs/go2idl/set-gen/main.go @@ -25,6 +25,8 @@ limitations under the License. package main import ( + "os" + "k8s.io/kubernetes/cmd/libs/go2idl/args" "k8s.io/kubernetes/cmd/libs/go2idl/set-gen/generators" @@ -44,6 +46,8 @@ func main() { generators.DefaultNameSystem(), generators.Packages, ); err != nil { - glog.Fatalf("Error: %v", err) + glog.Errorf("Error: %v", err) + os.Exit(1) } + glog.Info("Completed successfully.") } diff --git a/hack/after-build/run-codegen.sh b/hack/after-build/run-codegen.sh new file mode 100755 index 00000000000..a2cbf3e4a1e --- /dev/null +++ b/hack/after-build/run-codegen.sh @@ -0,0 +1,35 @@ +#!/bin/bash + +# Copyright 2015 The Kubernetes Authors All rights reserved. +# +# 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. + +set -o errexit +set -o nounset +set -o pipefail + +KUBE_ROOT=$(dirname "${BASH_SOURCE}")/../.. +source "${KUBE_ROOT}/hack/lib/init.sh" + +kube::golang::setup_env + +setgen=$(kube::util::find-binary "set-gen") + +# Please do not add any logic to this shell script. Add logic to the go code +# that generates the set-gen program. +# +# This can be called with one flag, --verify-only, so it works for both the +# update- and verify- scripts. +${setgen} "$@" + +# You may add additional calls of code generators like set-gen below. diff --git a/hack/update-all.sh b/hack/update-all.sh index ff8a58fbdbe..f58f41c651c 100755 --- a/hack/update-all.sh +++ b/hack/update-all.sh @@ -42,11 +42,12 @@ fi BASH_TARGETS="codecgen generated-conversions - generated-deep-copies - generated-docs - generated-swagger-docs + generated-deep-copies + generated-docs + generated-swagger-docs swagger-spec - api-reference-docs" + api-reference-docs + codegen" for t in $BASH_TARGETS diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh new file mode 100755 index 00000000000..9d9da661d35 --- /dev/null +++ b/hack/update-codegen.sh @@ -0,0 +1,30 @@ +#!/bin/bash + +# Copyright 2014 The Kubernetes Authors All rights reserved. +# +# 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. + +set -o errexit +set -o nounset +set -o pipefail + +KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. + +KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. +source "${KUBE_ROOT}/hack/lib/init.sh" + +kube::golang::setup_env + +"${KUBE_ROOT}/hack/build-go.sh" cmd/libs/go2idl/set-gen + +"${KUBE_ROOT}/hack/after-build/run-codegen.sh" "$@" diff --git a/hack/verify-codegen.sh b/hack/verify-codegen.sh new file mode 100755 index 00000000000..ae93e36efb7 --- /dev/null +++ b/hack/verify-codegen.sh @@ -0,0 +1,30 @@ +#!/bin/bash + +# Copyright 2014 The Kubernetes Authors All rights reserved. +# +# 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. + +set -o errexit +set -o nounset +set -o pipefail + +KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. + +KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. +source "${KUBE_ROOT}/hack/lib/init.sh" + +kube::golang::setup_env + +"${KUBE_ROOT}/hack/build-go.sh" cmd/libs/go2idl/set-gen + +"${KUBE_ROOT}/hack/after-build/run-codegen.sh" --verify-only diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index 8a3920e2924..00b49175798 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -323,6 +323,7 @@ update-period upgrade-target use-kubernetes-cluster-service user-whitelist +verify-only watch-cache watch-only whitelist-override-label diff --git a/pkg/util/sets/byte.go b/pkg/util/sets/byte.go index 2a65c4c6efe..1c945c165f1 100644 --- a/pkg/util/sets/byte.go +++ b/pkg/util/sets/byte.go @@ -15,7 +15,7 @@ limitations under the License. */ // This file was autogenerated by the command: -// $ cmd/libs/go2idl/set-gen/set-gen +// $ ./set-gen // Do not edit it manually! package sets diff --git a/pkg/util/sets/doc.go b/pkg/util/sets/doc.go index 63cf30f8cad..4cfd063d5a0 100644 --- a/pkg/util/sets/doc.go +++ b/pkg/util/sets/doc.go @@ -15,7 +15,7 @@ limitations under the License. */ // This file was autogenerated by the command: -// $ cmd/libs/go2idl/set-gen/set-gen +// $ ./set-gen // Do not edit it manually! // Package sets has auto-generated set types. diff --git a/pkg/util/sets/empty.go b/pkg/util/sets/empty.go index 5fd63e18761..774df2f42da 100644 --- a/pkg/util/sets/empty.go +++ b/pkg/util/sets/empty.go @@ -15,7 +15,7 @@ limitations under the License. */ // This file was autogenerated by the command: -// $ cmd/libs/go2idl/set-gen/set-gen +// $ ./set-gen // Do not edit it manually! package sets diff --git a/pkg/util/sets/int.go b/pkg/util/sets/int.go index d34c54b890e..65a3651e83c 100644 --- a/pkg/util/sets/int.go +++ b/pkg/util/sets/int.go @@ -15,7 +15,7 @@ limitations under the License. */ // This file was autogenerated by the command: -// $ cmd/libs/go2idl/set-gen/set-gen +// $ ./set-gen // Do not edit it manually! package sets diff --git a/pkg/util/sets/int64.go b/pkg/util/sets/int64.go index 89d47d2c607..29417526e35 100644 --- a/pkg/util/sets/int64.go +++ b/pkg/util/sets/int64.go @@ -15,7 +15,7 @@ limitations under the License. */ // This file was autogenerated by the command: -// $ cmd/libs/go2idl/set-gen/set-gen +// $ ./set-gen // Do not edit it manually! package sets diff --git a/pkg/util/sets/string.go b/pkg/util/sets/string.go index 71e0cb57468..b31eba29b1b 100644 --- a/pkg/util/sets/string.go +++ b/pkg/util/sets/string.go @@ -15,7 +15,7 @@ limitations under the License. */ // This file was autogenerated by the command: -// $ cmd/libs/go2idl/set-gen/set-gen +// $ ./set-gen // Do not edit it manually! package sets