Add verification to code gen

nikhiljindal · Dec 2, 2015 · ad925dd · ad925dd
1 parent d524bd8
commit ad925dd
Show file tree

Hide file tree

Showing 16 changed files with 193 additions and 22 deletions.
diff --git a/cmd/libs/go2idl/args/args.go b/cmd/libs/go2idl/args/args.go
@@ -56,13 +56,17 @@ 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) {
 	fs.StringSliceVarP(&g.InputDirs, "input-dirs", "i", g.InputDirs, "Comma-separated list of import paths to get input types from.")
 	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
@@ -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,15 +74,53 @@ 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)
 		return err
 	}
 }
 
+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,15 +258,26 @@ 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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