refactor(internal/gapicgen): moved things around for future reuse (#3995

)

- move git operations to its own package
- fix compile error in gensnippets
- move ForEachMod into execv
- fix a couple of lint errors

internal/gapicgen/cmd/genbot/bot.go

@@ -22,6 +22,8 @@ import (
 	"fmt"
 	"log"
 	"time"
+
+	"cloud.google.com/go/internal/gapicgen/git"
 )
 
 func genBot(ctx context.Context, githubAccessToken, githubUsername, githubName, githubEmail string) error {
@@ -38,7 +40,7 @@ func genBot(ctx context.Context, githubAccessToken, githubUsername, githubName,
 	}
 
 	// Setup the client and git environment.
-	githubClient, err := NewGithubClient(ctx, githubUsername, githubName, githubEmail, githubAccessToken)
+	githubClient, err := git.NewGithubClient(ctx, githubUsername, githubName, githubEmail, githubAccessToken)
 	if err != nil {
 		return err
 	}

internal/gapicgen/cmd/genbot/generate.go

@@ -17,24 +17,21 @@
 package main
 
 import (
-	"bytes"
 	"context"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"log"
 	"os"
 	"path/filepath"
 
-	"cloud.google.com/go/internal/gapicgen/execv"
 	"cloud.google.com/go/internal/gapicgen/generator"
+	"cloud.google.com/go/internal/gapicgen/git"
 	"golang.org/x/sync/errgroup"
-	"gopkg.in/src-d/go-git.v4"
 )
 
 // generate downloads sources and generates pull requests for go-genproto and
 // google-cloud-go if needed.
-func generate(ctx context.Context, githubClient *GithubClient) error {
+func generate(ctx context.Context, githubClient *git.GithubClient) error {
 	log.Println("creating temp dir")
 	tmpDir, err := ioutil.TempDir("", "update-genproto")
 	if err != nil {
@@ -53,16 +50,16 @@ func generate(ctx context.Context, githubClient *GithubClient) error {
 
 	grp, _ := errgroup.WithContext(ctx)
 	grp.Go(func() error {
-		return gitDeepClone("https://github.com/googleapis/googleapis", googleapisDir)
+		return git.DeepClone("https://github.com/googleapis/googleapis", googleapisDir)
 	})
 	grp.Go(func() error {
-		return gitDeepClone("https://github.com/googleapis/go-genproto", genprotoDir)
+		return git.DeepClone("https://github.com/googleapis/go-genproto", genprotoDir)
 	})
 	grp.Go(func() error {
-		return gitDeepClone("https://github.com/googleapis/google-cloud-go", gocloudDir)
+		return git.DeepClone("https://github.com/googleapis/google-cloud-go", gocloudDir)
 	})
 	grp.Go(func() error {
-		return gitDeepClone("https://github.com/protocolbuffers/protobuf", protoDir)
+		return git.DeepClone("https://github.com/protocolbuffers/protobuf", protoDir)
 	})
 	if err := grp.Wait(); err != nil {
 		log.Println(err)
@@ -81,12 +78,12 @@ func generate(ctx context.Context, githubClient *GithubClient) error {
 	}
 
 	// Create PRs.
-	genprotoHasChanges, err := hasChanges(genprotoDir)
+	genprotoHasChanges, err := git.HasChanges(genprotoDir)
 	if err != nil {
 		return err
 	}
 
-	gocloudHasChanges, err := hasChanges(gocloudDir)
+	gocloudHasChanges, err := git.HasChanges(gocloudDir)
 	if err != nil {
 		return err
 	}
@@ -138,28 +135,3 @@ func generate(ctx context.Context, githubClient *GithubClient) error {
 	}
 	return nil
 }
-
-// gitClone clones a repository in the given directory.
-func gitDeepClone(repo, dir string) error {
-	log.Printf("cloning %s\n", repo)
-
-	_, err := git.PlainClone(dir, false, &git.CloneOptions{
-		URL:      repo,
-		Progress: os.Stdout,
-	})
-	return err
-}
-
-// hasChanges reports whether the given directory has uncommitted git changes.
-func hasChanges(dir string) (bool, error) {
-	// Write command output to both os.Stderr and local, so that we can check
-	// whether there are modified files.
-	inmem := &bytes.Buffer{}
-	w := io.MultiWriter(os.Stderr, inmem)
-
-	c := execv.Command("bash", "-c", "git status --short")
-	c.Dir = dir
-	c.Stdout = w
-	err := c.Run()
-	return inmem.Len() > 0, err
-}

internal/gapicgen/cmd/genbot/update.go

@@ -19,20 +19,18 @@ package main
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"log"
-	"os"
 
-	"cloud.google.com/go/internal/gapicgen/execv"
+	"cloud.google.com/go/internal/gapicgen/git"
 )
 
-func updateGocloudPR(ctx context.Context, githubClient *GithubClient, pr *PullRequest) error {
+func updateGocloudPR(ctx context.Context, githubClient *git.GithubClient, pr *git.PullRequest) error {
 	if pr.Author != githubClient.Username {
 		return fmt.Errorf("pull request author %q does not match authenticated user %q", pr.Author, githubClient.Username)
 	}
 
 	// Checkout PR and update go.mod
-	if err := updateGocloudGoMod(pr); err != nil {
+	if err := githubClient.UpdateGocloudGoMod(pr); err != nil {
 		return err
 	}
 
@@ -46,51 +44,3 @@ func updateGocloudPR(ctx context.Context, githubClient *GithubClient, pr *PullRe
 	log.Printf("done updating google-cloud-go PR: %s\n", pr.URL)
 	return nil
 }
-
-// updateGocloudGoMod updates the go.mod to include latest version of genproto
-// for the given gocloud ref.
-func updateGocloudGoMod(pr *PullRequest) error {
-	tmpDir, err := ioutil.TempDir("", "finalize-gerrit-cl")
-	if err != nil {
-		return err
-	}
-	defer os.RemoveAll(tmpDir)
-
-	c := execv.Command("/bin/bash", "-c", `
-set -ex
-
-git init
-git remote add origin https://github.com/googleapis/google-cloud-go
-git fetch --all
-git checkout $BRANCH_NAME
-
-# tidyall
-go mod tidy
-for i in $(find . -name go.mod); do
-	pushd $(dirname $i);
-		# Update genproto and api to latest for every module (latest version is
-		# always correct version). tidy will remove the dependencies if they're not
-		# actually used by the client.
-		go get -d google.golang.org/api | true # We don't care that there's no files at root.
-		go get -d google.golang.org/genproto | true # We don't care that there's no files at root.
-		go mod tidy;
-	popd;
-done
-
-git add -A
-filesUpdated=$( git status --short | wc -l )
-if [ $filesUpdated -gt 0 ];
-then
-    git config credential.helper store # cache creds from ~/.git-credentials
-   	git commit --amend --no-edit
-	git push -f origin $BRANCH_NAME
-fi
-`)
-	c.Env = []string{
-		fmt.Sprintf("BRANCH_NAME=%s", gocloudBranchName),
-		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.
-	}
-	c.Dir = tmpDir
-	return c.Run()
-}

internal/gapicgen/cmd/gensnippets/main.go

@@ -35,8 +35,8 @@ func main() {
 	if flag.NArg() > 0 {
 		rootDir = flag.Arg(0)
 	}
-
-	if err := gensnippets.Generate(rootDir, *outDir); err != nil {
+	// TODO(tbp): route proper api short names
+	if err := gensnippets.Generate(rootDir, *outDir, map[string]string{}); err != nil {
 		log.Fatal(err)
 	}
 }

internal/gapicgen/execv/command.go

@@ -16,9 +16,11 @@
 package execv
 
 import (
+	"io/fs"
 	"log"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 )
 
@@ -42,3 +44,22 @@ func (c *CmdWrapper) Run() error {
 	log.Printf("[%s] >>>> %v <<<<", c.Dir, strings.Join(c.Args, " ")) // NOTE: we have some multi-line commands, make it clear where the command starts and ends
 	return c.Cmd.Run()
 }
+
+// ForEachMod runs the given function with the directory of
+// every non-internal module.
+func ForEachMod(rootDir string, fn func(dir string) error) error {
+	return filepath.WalkDir(rootDir, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if strings.Contains(path, "internal") {
+			return filepath.SkipDir
+		}
+		if d.Name() == "go.mod" {
+			if err := fn(filepath.Dir(path)); err != nil {
+				return err
+			}
+		}
+		return nil
+	})
+}

internal/gapicgen/generator/gapics.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/fs"
 	"log"
 	"os"
 	"path/filepath"
@@ -77,7 +76,7 @@ func (g *GapicGenerator) Regen(ctx context.Context) error {
 		return err
 	}
 
-	if err := forEachMod(g.googleCloudDir, g.addModReplaceGenproto); err != nil {
+	if err := execv.ForEachMod(g.googleCloudDir, g.addModReplaceGenproto); err != nil {
 		return err
 	}
 
@@ -104,32 +103,13 @@ func (g *GapicGenerator) Regen(ctx context.Context) error {
 		return err
 	}
 
-	if err := forEachMod(g.googleCloudDir, g.dropModReplaceGenproto); err != nil {
+	if err := execv.ForEachMod(g.googleCloudDir, g.dropModReplaceGenproto); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-// forEachMod runs the given function with the directory of
-// every non-internal module.
-func forEachMod(rootDir string, fn func(dir string) error) error {
-	return filepath.WalkDir(rootDir, func(path string, d fs.DirEntry, err error) error {
-		if err != nil {
-			return err
-		}
-		if strings.Contains(path, "internal") {
-			return filepath.SkipDir
-		}
-		if d.Name() == "go.mod" {
-			if err := fn(filepath.Dir(path)); 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")
@@ -142,7 +122,7 @@ func goModTidy(dir string) error {
 }
 
 func replaceAllForSnippets(googleCloudDir, snippetDir string) error {
-	return forEachMod(googleCloudDir, func(dir string) error {
+	return execv.ForEachMod(googleCloudDir, func(dir string) error {
 		if dir == snippetDir {
 			return nil
 		}
@@ -439,7 +419,7 @@ func (g *GapicGenerator) manifest(confs []*microgenConfig) error {
 			Title string `yaml:"title"` // We only need the title field.
 		}{}
 		if err := yaml.NewDecoder(yamlFile).Decode(&yamlConfig); err != nil {
-			return fmt.Errorf("Decode: %v", err)
+			return fmt.Errorf("decode: %v", err)
 		}
 		entry := manifestEntry{
 			DistributionName:  conf.importPath,
@@ -483,7 +463,7 @@ func (g *GapicGenerator) parseAPIShortnames(confs []*microgenConfig, manualEntri
 			Name string `yaml:"name"`
 		}{}
 		if err := yaml.NewDecoder(yamlFile).Decode(&config); err != nil {
-			return nil, fmt.Errorf("Decode: %v", err)
+			return nil, fmt.Errorf("decode: %v", err)
 		}
 		shortname := strings.TrimSuffix(config.Name, ".googleapis.com")
 		shortnames[conf.importPath] = shortname

internal/gapicgen/generator/generator.go

@@ -26,6 +26,7 @@ import (
 	"path/filepath"
 
 	"cloud.google.com/go/internal/gapicgen/execv"
+	"cloud.google.com/go/internal/gapicgen/git"
 )
 
 // Config contains inputs needed to generate sources.
@@ -40,7 +41,7 @@ type Config struct {
 }
 
 // Generate generates genproto and gapics.
-func Generate(ctx context.Context, conf *Config) ([]*ChangeInfo, error) {
+func Generate(ctx context.Context, conf *Config) ([]*git.ChangeInfo, error) {
 	if !conf.OnlyGenerateGapic {
 		protoGenerator := NewGenprotoGenerator(conf.GenprotoDir, conf.GoogleapisDir, conf.ProtoDir)
 		if err := protoGenerator.Regen(ctx); err != nil {
@@ -52,7 +53,7 @@ func Generate(ctx context.Context, conf *Config) ([]*ChangeInfo, error) {
 		return nil, fmt.Errorf("error generating gapics (may need to check logs for more errors): %v", err)
 	}
 
-	var changes []*ChangeInfo
+	var changes []*git.ChangeInfo
 	if !conf.LocalMode {
 		var err error
 		changes, err = gatherChanges(conf.GoogleapisDir, conf.GenprotoDir)
@@ -67,17 +68,21 @@ func Generate(ctx context.Context, conf *Config) ([]*ChangeInfo, error) {
 	return changes, nil
 }
 
-func gatherChanges(googleapisDir, genprotoDir string) ([]*ChangeInfo, error) {
+func gatherChanges(googleapisDir, genprotoDir string) ([]*git.ChangeInfo, error) {
 	// Get the last processed googleapis hash.
 	lastHash, err := ioutil.ReadFile(filepath.Join(genprotoDir, "regen.txt"))
 	if err != nil {
 		return nil, err
 	}
-	commits, err := CommitsSinceHash(googleapisDir, string(lastHash), false)
+	commits, err := git.CommitsSinceHash(googleapisDir, string(lastHash), false)
 	if err != nil {
 		return nil, err
 	}
-	changes, err := ParseChangeInfo(googleapisDir, commits)
+	gapicPkgs := make(map[string]string)
+	for _, v := range microgenGapicConfigs {
+		gapicPkgs[v.inputDirectoryPath] = git.ParseConventionalCommitPkg(v.importPath)
+	}
+	changes, err := git.ParseChangeInfo(googleapisDir, commits, gapicPkgs)
 	if err != nil {
 		return nil, err
 	}
@@ -88,7 +93,7 @@ func gatherChanges(googleapisDir, genprotoDir string) ([]*ChangeInfo, error) {
 // recordGoogleapisHash parses the latest commit in googleapis and records it to
 // regen.txt in go-genproto.
 func recordGoogleapisHash(googleapisDir, genprotoDir string) error {
-	commits, err := CommitsSinceHash(googleapisDir, "HEAD", true)
+	commits, err := git.CommitsSinceHash(googleapisDir, "HEAD", true)
 	if err != nil {
 		return err
 	}

internal/gapicgen/generator/genproto.go

@@ -26,6 +26,7 @@ import (
 	"strings"
 
 	"cloud.google.com/go/internal/gapicgen/execv"
+	"cloud.google.com/go/internal/gapicgen/git"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -176,9 +177,9 @@ func (g *GenprotoGenerator) protoc(fileNames []string) error {
 // getUpdatedPackages parses all of the new commits to find what packages need
 // to be regenerated.
 func (g *GenprotoGenerator) getUpdatedPackages(googleapisHash string) (map[string][]string, error) {
-	files, err := UpdateFilesSinceHash(g.googleapisDir, googleapisHash)
+	files, err := git.UpdateFilesSinceHash(g.googleapisDir, googleapisHash)
 	if err != nil {
-
+		return nil, err
 	}
 	pkgFiles := make(map[string][]string)
 	for _, v := range files {