feat(internal/gapicgen): only update relevant gapic files (#4066)

Author: codyoss

internal/gapicgen/execv/gocmd/gocmd.go

@@ -0,0 +1,92 @@
+// Copyright 2021 Google LLC
+//
+// 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.
+
+// Package gocmd provides helers for invoking Go tooling.
+package gocmd
+
+import (
+	"errors"
+	"fmt"
+	"log"
+	"os"
+	"os/exec"
+	"strings"
+
+	"cloud.google.com/go/internal/gapicgen/execv"
+)
+
+var (
+	// ErrBuildConstraint is returned when the Go command returns this error.
+	ErrBuildConstraint error = errors.New("build constraints exclude all Go files")
+)
+
+// ModTidy tidies go.mod file in the specified directory
+func ModTidy(dir string) error {
+	log.Printf("[%s] running go mod tidy", dir)
+	c := execv.Command("go", "mod", "tidy")
+	c.Dir = dir
+	c.Env = []string{
+		fmt.Sprintf("PATH=%s", os.Getenv("PATH")), // TODO(deklerk): Why do we need to do this? Doesn't seem to be necessary in other exec.Commands.
+		fmt.Sprintf("HOME=%s", os.Getenv("HOME")), // TODO(deklerk): Why do we need to do this? Doesn't seem to be necessary in other exec.Commands.
+	}
+	return c.Run()
+}
+
+// ListModName finds a modules name for a given directory.
+func ListModName(dir string) (string, error) {
+	modC := execv.Command("go", "list", "-m")
+	modC.Dir = dir
+	mod, err := modC.Output()
+	return string(mod), err
+}
+
+// ListModDirName finds the directory in which the module resides. Returns
+// ErrBuildConstraint if all files in a module are constrained.
+func ListModDirName(dir string) (string, error) {
+	var out []byte
+	var err error
+	c := execv.Command("go", "list", "-f", "'{{.Module.Dir}}'")
+	c.Dir = dir
+	if out, err = c.Output(); err != nil {
+		if ee, ok := err.(*exec.ExitError); ok {
+			if strings.Contains(string(ee.Stderr), "build constraints exclude all Go files") {
+				return "", ErrBuildConstraint
+			}
+		}
+		return "", err
+	}
+	return strings.Trim(strings.TrimSpace(string(out)), "'"), nil
+}
+
+// Build attempts to build all packages recursively from the given directory.
+func Build(dir string) error {
+	log.Println("building generated code")
+	c := execv.Command("go", "build", "./...")
+	c.Dir = dir
+	return c.Run()
+}
+
+// Vet runs linters on all .go files recursively from the given directory.
+func Vet(dir string) error {
+	log.Println("vetting generated code")
+	c := execv.Command("goimports", "-w", ".")
+	c.Dir = dir
+	if err := c.Run(); err != nil {
+		return err
+	}
+
+	c = execv.Command("gofmt", "-s", "-d", "-w", "-l", ".")
+	c.Dir = dir
+	return c.Run()
+}

internal/gapicgen/generator/gapics.go

@@ -24,7 +24,9 @@ import (
 	"strings"
 
 	"cloud.google.com/go/internal/gapicgen/execv"
+	"cloud.google.com/go/internal/gapicgen/execv/gocmd"
 	"cloud.google.com/go/internal/gapicgen/gensnippets"
+	"cloud.google.com/go/internal/gapicgen/git"
 	"gopkg.in/yaml.v2"
 )
 
@@ -37,10 +39,11 @@ type GapicGenerator struct {
 	gapicToGenerate   string
 	regenOnly         bool
 	onlyGenerateGapic bool
+	modifiedPkgs      []string
 }
 
 // NewGapicGenerator creates a GapicGenerator.
-func NewGapicGenerator(c *Config) *GapicGenerator {
+func NewGapicGenerator(c *Config, modifiedPkgs []string) *GapicGenerator {
 	return &GapicGenerator{
 		googleapisDir:     c.GoogleapisDir,
 		protoDir:          c.ProtoDir,
@@ -49,6 +52,7 @@ func NewGapicGenerator(c *Config) *GapicGenerator {
 		gapicToGenerate:   c.GapicToGenerate,
 		regenOnly:         c.RegenOnly,
 		onlyGenerateGapic: c.OnlyGenerateGapic,
+		modifiedPkgs:      modifiedPkgs,
 	}
 }
 
@@ -72,6 +76,16 @@ func (g *GapicGenerator) Regen(ctx context.Context) error {
 		return err
 	}
 
+	// TODO(codyoss): Remove once https://github.com/googleapis/gapic-generator-go/pull/606
+	// is released.
+	if err := gocmd.Vet(g.googleCloudDir); err != nil {
+		return err
+	}
+
+	if err := g.resetUnknownVersion(); err != nil {
+		return err
+	}
+
 	if g.regenOnly {
 		return nil
 	}
@@ -94,11 +108,11 @@ func (g *GapicGenerator) Regen(ctx context.Context) error {
 		return err
 	}
 
-	if err := vet(g.googleCloudDir); err != nil {
+	if err := gocmd.Vet(g.googleCloudDir); err != nil {
 		return err
 	}
 
-	if err := build(g.googleCloudDir); err != nil {
+	if err := gocmd.Build(g.googleCloudDir); err != nil {
 		return err
 	}
 
@@ -124,33 +138,19 @@ func (g *GapicGenerator) regenSnippets(ctx context.Context) error {
 	if err := replaceAllForSnippets(g.googleCloudDir, snippetDir); err != nil {
 		return err
 	}
-	if err := goModTidy(snippetDir); err != nil {
+	if err := gocmd.ModTidy(snippetDir); err != nil {
 		return err
 	}
 	return nil
 }
 
-func goModTidy(dir string) error {
-	log.Printf("[%s] running go mod tidy", dir)
-	c := execv.Command("go", "mod", "tidy")
-	c.Dir = dir
-	c.Env = []string{
-		fmt.Sprintf("PATH=%s", os.Getenv("PATH")), // TODO(deklerk): Why do we need to do this? Doesn't seem to be necessary in other exec.Commands.
-		fmt.Sprintf("HOME=%s", os.Getenv("HOME")), // TODO(deklerk): Why do we need to do this? Doesn't seem to be necessary in other exec.Commands.
-	}
-	return c.Run()
-}
-
 func replaceAllForSnippets(googleCloudDir, snippetDir string) error {
 	return execv.ForEachMod(googleCloudDir, func(dir string) error {
 		if dir == snippetDir {
 			return nil
 		}
 
-		// Get the module name in this dir.
-		modC := execv.Command("go", "list", "-m")
-		modC.Dir = dir
-		mod, err := modC.Output()
+		mod, err := gocmd.ListModName(dir)
 		if err != nil {
 			return err
 		}
@@ -208,23 +208,60 @@ go mod edit -dropreplace "google.golang.org/genproto"
 	return c.Run()
 }
 
+// resetUnknownVersion resets doc.go files that have only had their version
+// changed to UNKNOWN by the generator.
+func (g *GapicGenerator) resetUnknownVersion() error {
+	files, err := git.FindModifiedFiles(g.googleCloudDir)
+	if err != nil {
+		return err
+	}
+
+	for _, file := range files {
+		if !strings.HasSuffix(file, "doc.go") {
+			continue
+		}
+		diff, err := git.FileDiff(g.googleCloudDir, file)
+		if err != nil {
+			return err
+		}
+		// More than one diff, don't reset.
+		if strings.Count(diff, "@@") != 2 {
+			log.Println(diff)
+			continue
+		}
+		// Not related to version, don't reset.
+		if !strings.Contains(diff, "+const versionClient = \"UNKNOWN\"") {
+			continue
+		}
+
+		if err := git.ResetFile(g.googleCloudDir, file); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // setVersion updates the versionClient constant in all .go files. It may create
 // .backup files on certain systems (darwin), and so should be followed by a
 // clean-up of .backup files.
 func (g *GapicGenerator) setVersion() error {
+	dirs, err := g.findModifiedDirs()
+	if err != nil {
+		return err
+	}
 	log.Println("updating client version")
-	// TODO(deklerk): Migrate this all to Go instead of using bash.
-
-	c := execv.Command("bash", "-c", `
+	for _, dir := range dirs {
+		c := execv.Command("bash", "-c", `
 ver=$(date +%Y%m%d)
-git ls-files -mo | while read modified; do
-	dir=${modified%/*.*}
-	find . -path "*/$dir/doc.go" -exec sed -i.backup -e "s/^const versionClient.*/const versionClient = \"$ver\"/" '{}' +;
-done
+find . -path "*/doc.go" -exec sed -i.backup -e "s/^const versionClient.*/const versionClient = \"$ver\"/" '{}' +;
 find . -name '*.backup' -delete
 `)
-	c.Dir = g.googleCloudDir
-	return c.Run()
+		c.Dir = dir
+		if err := c.Run(); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // microgen runs the microgenerator on a single microgen config.
@@ -501,3 +538,30 @@ func (g *GapicGenerator) parseAPIShortnames(confs []*microgenConfig, manualEntri
 	}
 	return shortnames, nil
 }
+
+func (g *GapicGenerator) findModifiedDirs() ([]string, error) {
+	log.Println("finding modifiled directories")
+	files, err := git.FindModifiedAndUntrackedFiles(g.googleCloudDir)
+	if err != nil {
+		return nil, err
+	}
+	dirs := map[string]bool{}
+	for _, file := range files {
+		dir := filepath.Dir(filepath.Join(g.googleCloudDir, file))
+		dirs[dir] = true
+	}
+
+	// Add modified dirs from genproto. Sometimes only a request struct will be
+	// updated, in these cases we should still make modifications the
+	// corresponding gapic directories.
+	for _, pkg := range g.modifiedPkgs {
+		dir := filepath.Join(g.googleCloudDir, pkg)
+		dirs[dir] = true
+	}
+
+	var dirList []string
+	for dir := range dirs {
+		dirList = append(dirList, dir)
+	}
+	return dirList, nil
+}

internal/gapicgen/generator/generator.go

@@ -21,11 +21,9 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
-	"log"
 	"os"
 	"path/filepath"
 
-	"cloud.google.com/go/internal/gapicgen/execv"
 	"cloud.google.com/go/internal/gapicgen/git"
 )
 
@@ -49,10 +47,6 @@ func Generate(ctx context.Context, conf *Config) ([]*git.ChangeInfo, error) {
 			return nil, fmt.Errorf("error generating genproto (may need to check logs for more errors): %v", err)
 		}
 	}
-	gapicGenerator := NewGapicGenerator(conf)
-	if err := gapicGenerator.Regen(ctx); err != nil {
-		return nil, fmt.Errorf("error generating gapics (may need to check logs for more errors): %v", err)
-	}
 
 	var changes []*git.ChangeInfo
 	if !conf.LocalMode {
@@ -65,6 +59,17 @@ func Generate(ctx context.Context, conf *Config) ([]*git.ChangeInfo, error) {
 			return nil, err
 		}
 	}
+	var modifiedPkgs []string
+	for _, v := range changes {
+		if v.Package != "" {
+			modifiedPkgs = append(modifiedPkgs, v.PackageDir)
+		}
+	}
+
+	gapicGenerator := NewGapicGenerator(conf, modifiedPkgs)
+	if err := gapicGenerator.Regen(ctx); err != nil {
+		return nil, fmt.Errorf("error generating gapics (may need to check logs for more errors): %v", err)
+	}
 
 	return changes, nil
 }
@@ -81,7 +86,7 @@ func gatherChanges(googleapisDir, genprotoDir string) ([]*git.ChangeInfo, error)
 	}
 	gapicPkgs := make(map[string]string)
 	for _, v := range microgenGapicConfigs {
-		gapicPkgs[v.inputDirectoryPath] = git.ParseConventionalCommitPkg(v.importPath)
+		gapicPkgs[v.inputDirectoryPath] = v.importPath
 	}
 	changes, err := git.ParseChangeInfo(googleapisDir, commits, gapicPkgs)
 	if err != nil {
@@ -112,25 +117,3 @@ func recordGoogleapisHash(googleapisDir, genprotoDir string) error {
 	}
 	return nil
 }
-
-// build attempts to build all packages recursively from the given directory.
-func build(dir string) error {
-	log.Println("building generated code")
-	c := execv.Command("go", "build", "./...")
-	c.Dir = dir
-	return c.Run()
-}
-
-// vet runs linters on all .go files recursively from the given directory.
-func vet(dir string) error {
-	log.Println("vetting generated code")
-	c := execv.Command("goimports", "-w", ".")
-	c.Dir = dir
-	if err := c.Run(); err != nil {
-		return err
-	}
-
-	c = execv.Command("gofmt", "-s", "-d", "-w", "-l", ".")
-	c.Dir = dir
-	return c.Run()
-}

internal/gapicgen/generator/genproto.go

@@ -26,6 +26,7 @@ import (
 	"strings"
 
 	"cloud.google.com/go/internal/gapicgen/execv"
+	"cloud.google.com/go/internal/gapicgen/execv/gocmd"
 	"cloud.google.com/go/internal/gapicgen/git"
 	"golang.org/x/sync/errgroup"
 )
@@ -131,11 +132,11 @@ func (g *GenprotoGenerator) Regen(ctx context.Context) error {
 		return err
 	}
 
-	if err := vet(g.genprotoDir); err != nil {
+	if err := gocmd.Vet(g.genprotoDir); err != nil {
 		return err
 	}
 
-	if err := build(g.genprotoDir); err != nil {
+	if err := gocmd.Build(g.genprotoDir); err != nil {
 		return err
 	}