From 941ab029ba6f7f33e8b2e31e3818aeb68312a999 Mon Sep 17 00:00:00 2001 From: Cody Oss <6331106+codyoss@users.noreply.github.com> Date: Thu, 3 Dec 2020 14:37:53 -0700 Subject: [PATCH] feat(internal/gapicgen): support adding context to regen (#3174) Now all regen PRs will include a block that mentions all of the changes that happened in googleapis/googleapis that caused the files to be generated. This information will eventually be fed into our automated releases via release-please. There these changes will be parsed out to add extra context to our release PRs and changelogs. No longer will we have to have a vague message about many auto-generated changes. - Adds context to regen PRs. - Refactored generators so every method does not need so many parameters. - Updated goolge/protobuf to its new migrated repo protocolbuffers/protobuf. - Added some extra logging to the build. - Only run protoc on dirs that have changes. --- internal/gapicgen/README.md | 2 +- internal/gapicgen/cmd/genbot/README.md | 9 +- internal/gapicgen/cmd/genbot/generate.go | 17 +-- internal/gapicgen/cmd/genbot/github.go | 68 +++++++--- internal/gapicgen/cmd/genlocal/main.go | 19 ++- internal/gapicgen/generator/gapics.go | 95 ++++++++------ internal/gapicgen/generator/generator.go | 64 +++++---- internal/gapicgen/generator/genproto.go | 154 +++++++++++----------- internal/gapicgen/generator/git.go | 157 +++++++++++++++++++++++ internal/gapicgen/generator/git_test.go | 36 ++++++ 10 files changed, 452 insertions(+), 169 deletions(-) create mode 100644 internal/gapicgen/generator/git.go create mode 100644 internal/gapicgen/generator/git_test.go diff --git a/internal/gapicgen/README.md b/internal/gapicgen/README.md index 1e696fa08e9..fdf9f2b9ed1 100644 --- a/internal/gapicgen/README.md +++ b/internal/gapicgen/README.md @@ -13,4 +13,4 @@ gapicgen contains three binaries: gapic regen CL that needs to have reviewers added and go.mod update, and then does so. Intended to be run periodically as a bot, but humans can use it too. -See the README.md in each folder for more specific instructions. \ No newline at end of file +See the README.md in each folder for more specific instructions. diff --git a/internal/gapicgen/cmd/genbot/README.md b/internal/gapicgen/cmd/genbot/README.md index 1fa64ea63bf..5bdafc8d213 100644 --- a/internal/gapicgen/cmd/genbot/README.md +++ b/internal/gapicgen/cmd/genbot/README.md @@ -7,8 +7,9 @@ It is intended to be used as a bot, though it can be run locally too. ### Github -For Github, you need to generate/supply a Personal Access Token. More information on how that's done is here: -https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line +For Github, you need to generate/supply a Personal Access Token. More +information on how that's done can be found here: +[creating a personal access token](https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line). ## Running locally @@ -48,9 +49,9 @@ docker run -t --rm --privileged \ ## FAQ -#### How do I bump to a later version of the microgenerator? +### How to bump to a later version of the microgenerator -``` +```shell cd /path/to/internal/gapicgen go get -u github.com/googleapis/gapic-generator-go/cmd/protoc-gen-go_gapic ``` diff --git a/internal/gapicgen/cmd/genbot/generate.go b/internal/gapicgen/cmd/genbot/generate.go index bdb96f17d3c..aa0725003e7 100644 --- a/internal/gapicgen/cmd/genbot/generate.go +++ b/internal/gapicgen/cmd/genbot/generate.go @@ -60,14 +60,15 @@ func generate(ctx context.Context, githubClient *GithubClient) error { return gitClone("https://github.com/googleapis/google-cloud-go", gocloudDir) }) grp.Go(func() error { - return gitClone("https://github.com/google/protobuf", protoDir) + return gitClone("https://github.com/protocolbuffers/protobuf", protoDir) }) if err := grp.Wait(); err != nil { log.Println(err) } // Regen. - if err := generator.Generate(ctx, googleapisDir, genprotoDir, gocloudDir, protoDir, ""); err != nil { + changes, err := generator.Generate(ctx, googleapisDir, genprotoDir, gocloudDir, protoDir, "") + if err != nil { return err } @@ -85,17 +86,17 @@ func generate(ctx context.Context, githubClient *GithubClient) error { switch { case genprotoHasChanges && gocloudHasChanges: // Both have changes. - genprotoPRNum, err := githubClient.CreateGenprotoPR(ctx, genprotoDir, true) + genprotoPRNum, err := githubClient.CreateGenprotoPR(ctx, genprotoDir, true, changes) if err != nil { return fmt.Errorf("error creating PR for genproto (may need to check logs for more errors): %v", err) } - gocloudPRNum, err := githubClient.CreateGocloudPR(ctx, gocloudDir, genprotoPRNum) + gocloudPRNum, err := githubClient.CreateGocloudPR(ctx, gocloudDir, genprotoPRNum, changes) if err != nil { return fmt.Errorf("error creating CL for veneers (may need to check logs for more errors): %v", err) } - if err := githubClient.AmendWithPRURL(ctx, genprotoPRNum, genprotoDir, gocloudPRNum); err != nil { + if err := githubClient.AmendGenprotoPR(ctx, genprotoPRNum, genprotoDir, gocloudPRNum, changes); err != nil { return fmt.Errorf("error amending genproto PR: %v", err) } @@ -105,7 +106,7 @@ func generate(ctx context.Context, githubClient *GithubClient) error { log.Println(gocloudPRURL) case genprotoHasChanges: // Only genproto has changes. - genprotoPRNum, err := githubClient.CreateGenprotoPR(ctx, genprotoDir, false) + genprotoPRNum, err := githubClient.CreateGenprotoPR(ctx, genprotoDir, false, changes) if err != nil { return fmt.Errorf("error creating PR for genproto (may need to check logs for more errors): %v", err) } @@ -115,7 +116,7 @@ func generate(ctx context.Context, githubClient *GithubClient) error { log.Println("gocloud had no changes") case gocloudHasChanges: // Only gocloud has changes. - gocloudPRNum, err := githubClient.CreateGocloudPR(ctx, gocloudDir, -1) + gocloudPRNum, err := githubClient.CreateGocloudPR(ctx, gocloudDir, -1, changes) if err != nil { return fmt.Errorf("error creating CL for veneers (may need to check logs for more errors): %v", err) } @@ -145,7 +146,7 @@ func gitClone(repo, dir string) error { 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.NewBuffer([]byte{}) // TODO(deklerk): Try `var inmem bytes.Buffer`. + inmem := &bytes.Buffer{} w := io.MultiWriter(os.Stderr, inmem) c := exec.Command("bash", "-c", "git status --short") diff --git a/internal/gapicgen/cmd/genbot/github.go b/internal/gapicgen/cmd/genbot/github.go index d9bede8c481..b8c2141bc88 100644 --- a/internal/gapicgen/cmd/genbot/github.go +++ b/internal/gapicgen/cmd/genbot/github.go @@ -26,6 +26,7 @@ import ( "strings" "time" + "cloud.google.com/go/internal/gapicgen/generator" "github.com/google/go-github/v32/github" "github.com/shurcooL/githubv4" "golang.org/x/oauth2" @@ -178,13 +179,15 @@ func (gc *GithubClient) GetRegenPR(ctx context.Context, repo string, status stri // CreateGenprotoPR creates a PR for a given genproto change. // // hasCorrespondingPR indicates that there is a corresponding google-cloud-go PR. -func (gc *GithubClient) CreateGenprotoPR(ctx context.Context, genprotoDir string, hasCorrespondingPR bool) (prNumber int, _ error) { +func (gc *GithubClient) CreateGenprotoPR(ctx context.Context, genprotoDir string, hasCorrespondingPR bool, changes []*generator.ChangeInfo) (prNumber int, _ error) { log.Println("creating genproto PR") - - body := genprotoCommitBody + var sb strings.Builder + sb.WriteString(genprotoCommitBody) if !hasCorrespondingPR { - body += "\n\nThere is no corresponding google-cloud-go PR.\n" + sb.WriteString("\n\nThere is no corresponding google-cloud-go PR.\n") + sb.WriteString(formatChanges(changes, false)) } + body := sb.String() c := exec.Command("/bin/bash", "-c", ` set -ex @@ -222,6 +225,7 @@ git push origin $BRANCH_NAME Body: &body, Head: &head, Base: &base, + Draft: github.Bool(true), }) if err != nil { return 0, err @@ -246,18 +250,21 @@ git push origin $BRANCH_NAME return pr.GetNumber(), nil } -// CreateGocloudPR creats a PR for a given google-cloud-go change. -func (gc *GithubClient) CreateGocloudPR(ctx context.Context, gocloudDir string, genprotoPRNum int) (prNumber int, _ error) { +// CreateGocloudPR creates a PR for a given google-cloud-go change. +func (gc *GithubClient) CreateGocloudPR(ctx context.Context, gocloudDir string, genprotoPRNum int, changes []*generator.ChangeInfo) (prNumber int, _ error) { log.Println("creating google-cloud-go PR") - var body string + var sb strings.Builder var draft bool + sb.WriteString(gocloudCommitBody) if genprotoPRNum > 0 { - body = gocloudCommitBody + fmt.Sprintf("\n\nCorresponding genproto PR: https://github.com/googleapis/go-genproto/pull/%d\n", genprotoPRNum) + sb.WriteString(fmt.Sprintf("\n\nCorresponding genproto PR: https://github.com/googleapis/go-genproto/pull/%d\n", genprotoPRNum)) draft = true } else { - body = gocloudCommitBody + "\n\nThere is no corresponding genproto PR.\n" + sb.WriteString("\n\nThere is no corresponding genproto PR.\n") } + sb.WriteString(formatChanges(changes, true)) + body := sb.String() c := exec.Command("/bin/bash", "-c", ` set -ex @@ -304,11 +311,14 @@ git push origin $BRANCH_NAME return pr.GetNumber(), nil } -// AmendWithPRURL amends the given genproto PR with a link to the given +// AmendGenprotoPR amends the given genproto PR with a link to the given // google-cloud-go PR. -func (gc *GithubClient) AmendWithPRURL(ctx context.Context, genprotoPRNum int, genprotoDir string, gocloudPRNum int) error { - newBody := genprotoCommitBody + fmt.Sprintf("\n\nCorresponding google-cloud-go PR: googleapis/google-cloud-go#%d\n", gocloudPRNum) - +func (gc *GithubClient) AmendGenprotoPR(ctx context.Context, genprotoPRNum int, genprotoDir string, gocloudPRNum int, changes []*generator.ChangeInfo) error { + var body strings.Builder + body.WriteString(genprotoCommitBody) + body.WriteString(fmt.Sprintf("\n\nCorresponding google-cloud-go PR: googleapis/google-cloud-go#%d\n", gocloudPRNum)) + body.WriteString(formatChanges(changes, false)) + sBody := body.String() c := exec.Command("/bin/bash", "-c", ` set -ex @@ -325,7 +335,7 @@ git push -f origin $BRANCH_NAME 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. fmt.Sprintf("COMMIT_TITLE=%s", genprotoCommitTitle), - fmt.Sprintf("COMMIT_BODY=%s", newBody), + fmt.Sprintf("COMMIT_BODY=%s", sBody), fmt.Sprintf("BRANCH_NAME=%s", genprotoBranchName), } c.Dir = genprotoDir @@ -333,7 +343,7 @@ git push -f origin $BRANCH_NAME return err } _, _, err := gc.cV3.PullRequests.Edit(ctx, "googleapis", "go-genproto", genprotoPRNum, &github.PullRequest{ - Body: &newBody, + Body: &sBody, }) return err } @@ -356,3 +366,31 @@ func (gc *GithubClient) MarkPRReadyForReview(ctx context.Context, repo string, n } return nil } + +func formatChanges(changes []*generator.ChangeInfo, onlyGapicChanges bool) string { + if len(changes) == 0 { + return "" + } + var sb strings.Builder + sb.WriteString("\nChanges:\n") + for _, c := range changes { + if onlyGapicChanges && !c.HasGapicChanges { + continue + } + sb.WriteString("- ") + ss := strings.Split(c.Body, "\n") + for i, s := range ss { + if i == 0 { + sb.WriteString(fmt.Sprintf("%s\n", s)) + continue + } + if s == "" { + sb.WriteString("\n") + continue + } + sb.WriteString(fmt.Sprintf(" %s\n", s)) + } + sb.WriteString("\n") + } + return sb.String() +} diff --git a/internal/gapicgen/cmd/genlocal/main.go b/internal/gapicgen/cmd/genlocal/main.go index dfd97f3102d..98cfd3fa557 100644 --- a/internal/gapicgen/cmd/genlocal/main.go +++ b/internal/gapicgen/cmd/genlocal/main.go @@ -20,6 +20,7 @@ package main import ( "context" "flag" + "fmt" "io/ioutil" "log" "os" @@ -54,32 +55,40 @@ func main() { genprotoDir := flag.String("genproto-dir", filepath.Join(tmpDir, "genproto"), "Directory where sources of googleapis/go-genproto resides. If unset the sources will be cloned to a temporary directory that is not cleaned up.") protoDir := flag.String("proto-dir", filepath.Join(tmpDir, "proto"), "Directory where sources of google/protobuf resides. If unset the sources will be cloned to a temporary directory that is not cleaned up.") gapicToGenerate := flag.String("gapic", "", `Specifies which gapic to generate. The value should be in the form of an import path (Ex: cloud.google.com/go/pubsub/apiv1). The default "" generates all gapics.`) + verbose := flag.Bool("verbose", false, "Enables verbose logging.") flag.Parse() ctx := context.Background() // Clone repositories if needed. - grp, _ := errgroup.WithContext(ctx) gitClone(grp, "https://github.com/googleapis/googleapis.git", *googleapisDir, tmpDir) gitClone(grp, "https://github.com/googleapis/go-genproto", *genprotoDir, tmpDir) gitClone(grp, "https://github.com/googleapis/google-cloud-go", *gocloudDir, tmpDir) - gitClone(grp, "https://github.com/google/protobuf", *protoDir, tmpDir) + gitClone(grp, "https://github.com/protocolbuffers/protobuf", *protoDir, tmpDir) if err := grp.Wait(); err != nil { log.Println(err) } // Regen. - - if err := generator.Generate(ctx, *googleapisDir, *genprotoDir, *gocloudDir, *protoDir, *gapicToGenerate); err != nil { + changes, err := generator.Generate(ctx, *googleapisDir, *genprotoDir, *gocloudDir, *protoDir, *gapicToGenerate) + if err != nil { log.Printf("Generator ran (and failed) in %s\n", tmpDir) log.Fatal(err) } // Log results. - log.Println(genprotoDir) log.Println(gocloudDir) + + if *verbose { + log.Println("Changes:") + fmt.Println() + for _, v := range changes { + fmt.Println("********************************************") + fmt.Println(v.Body) + } + } } // gitClone clones a repository in the given directory if dir is not in tmpDir. diff --git a/internal/gapicgen/generator/gapics.go b/internal/gapicgen/generator/gapics.go index ebf57aa9522..4d960b0bd70 100644 --- a/internal/gapicgen/generator/gapics.go +++ b/internal/gapicgen/generator/gapics.go @@ -26,8 +26,27 @@ import ( "gopkg.in/yaml.v2" ) -// generateGapics generates gapics. -func generateGapics(ctx context.Context, googleapisDir, protoDir, gocloudDir, genprotoDir string, gapicToGenerate string) error { +// GapicGenerator is used to regenerate gapic libraries. +type GapicGenerator struct { + googleapisDir string + protoDir string + googleCloudDir string + genprotoDir string +} + +// NewGapicGenerator creates a GapicGenerator. +func NewGapicGenerator(googleapisDir, protoDir, googleCloudDir, genprotoDir string) *GapicGenerator { + return &GapicGenerator{ + googleapisDir: googleapisDir, + protoDir: protoDir, + googleCloudDir: googleCloudDir, + genprotoDir: genprotoDir, + } +} + +// Regen generates gapics. +func (g *GapicGenerator) Regen(ctx context.Context, gapicToGenerate string) error { + log.Println("regenerating gapics") for _, c := range microgenGapicConfigs { // Skip generation if generating all of the gapics and the associated // config has a block on it. Or if generating a single gapic and it does @@ -36,36 +55,36 @@ func generateGapics(ctx context.Context, googleapisDir, protoDir, gocloudDir, ge (gapicToGenerate != "" && gapicToGenerate != c.importPath) { continue } - if err := microgen(c, googleapisDir, protoDir, gocloudDir); err != nil { + if err := g.microgen(c); err != nil { return err } } - if err := copyMicrogenFiles(gocloudDir); err != nil { + if err := g.copyMicrogenFiles(); err != nil { return err } - if err := manifest(microgenGapicConfigs, googleapisDir, gocloudDir); err != nil { + if err := g.manifest(microgenGapicConfigs); err != nil { return err } - if err := setVersion(gocloudDir); err != nil { + if err := g.setVersion(); err != nil { return err } - if err := addModReplaceGenproto(gocloudDir, genprotoDir); err != nil { + if err := g.addModReplaceGenproto(); err != nil { return err } - if err := vet(gocloudDir); err != nil { + if err := vet(g.googleCloudDir); err != nil { return err } - if err := build(gocloudDir); err != nil { + if err := build(g.googleCloudDir); err != nil { return err } - if err := dropModReplaceGenproto(gocloudDir); err != nil { + if err := g.dropModReplaceGenproto(); err != nil { return err } @@ -75,18 +94,17 @@ func generateGapics(ctx context.Context, googleapisDir, protoDir, gocloudDir, ge // addModReplaceGenproto adds a genproto replace statement that points genproto // to the local copy. This is necessary since the remote genproto may not have // changes that are necessary for the in-flight regen. -func addModReplaceGenproto(gocloudDir, genprotoDir string) error { +func (g *GapicGenerator) addModReplaceGenproto() error { + log.Println("adding temporary genproto replace statement") c := command("bash", "-c", ` set -ex GENPROTO_VERSION=$(cat go.mod | cat go.mod | grep genproto | awk '{print $2}') go mod edit -replace "google.golang.org/genproto@$GENPROTO_VERSION=$GENPROTO_DIR" `) - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = gocloudDir + c.Dir = g.googleCloudDir c.Env = []string{ - "GENPROTO_DIR=" + genprotoDir, + "GENPROTO_DIR=" + g.genprotoDir, 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. } @@ -95,16 +113,15 @@ go mod edit -replace "google.golang.org/genproto@$GENPROTO_VERSION=$GENPROTO_DIR // dropModReplaceGenproto drops the genproto replace statement. It is intended // to be run after addModReplaceGenproto. -func dropModReplaceGenproto(gocloudDir string) error { +func (g *GapicGenerator) dropModReplaceGenproto() error { + log.Println("removing genproto replace statement") c := command("bash", "-c", ` set -ex GENPROTO_VERSION=$(cat go.mod | cat go.mod | grep genproto | grep -v replace | awk '{print $2}') go mod edit -dropreplace "google.golang.org/genproto@$GENPROTO_VERSION" `) - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = gocloudDir + c.Dir = g.googleCloudDir 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. @@ -115,7 +132,8 @@ go mod edit -dropreplace "google.golang.org/genproto@$GENPROTO_VERSION" // 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 setVersion(gocloudDir string) error { +func (g *GapicGenerator) setVersion() error { + log.Println("updating client version") // TODO(deklerk): Migrate this all to Go instead of using bash. c := command("bash", "-c", ` @@ -126,18 +144,16 @@ git ls-files -mo | while read modified; do done find . -name '*.backup' -delete `) - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = gocloudDir + c.Dir = g.googleCloudDir return c.Run() } // microgen runs the microgenerator on a single microgen config. -func microgen(conf *microgenConfig, googleapisDir, protoDir, gocloudDir string) error { +func (g *GapicGenerator) microgen(conf *microgenConfig) error { log.Println("microgen generating", conf.pkg) var protoFiles []string - if err := filepath.Walk(googleapisDir+"/"+conf.inputDirectoryPath, func(path string, info os.FileInfo, err error) error { + if err := filepath.Walk(g.googleapisDir+"/"+conf.inputDirectoryPath, func(path string, info os.FileInfo, err error) error { if err != nil { return err } @@ -149,19 +165,17 @@ func microgen(conf *microgenConfig, googleapisDir, protoDir, gocloudDir string) return err } - args := []string{"-I", googleapisDir, + args := []string{"-I", g.googleapisDir, "--experimental_allow_proto3_optional", - "-I", protoDir, - "--go_gapic_out", gocloudDir, + "-I", g.protoDir, + "--go_gapic_out", g.googleCloudDir, "--go_gapic_opt", fmt.Sprintf("go-gapic-package=%s;%s", conf.importPath, conf.pkg), "--go_gapic_opt", fmt.Sprintf("grpc-service-config=%s", conf.gRPCServiceConfigPath), "--go_gapic_opt", fmt.Sprintf("gapic-service-config=%s", conf.apiServiceConfigPath), "--go_gapic_opt", fmt.Sprintf("release-level=%s", conf.releaseLevel)} args = append(args, protoFiles...) c := command("protoc", args...) - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = googleapisDir + c.Dir = g.googleapisDir return c.Run() } @@ -286,9 +300,10 @@ var manualEntries = []manifestEntry{ } // manifest writes a manifest file with info about all of the confs. -func manifest(confs []*microgenConfig, googleapisDir, gocloudDir string) error { +func (g *GapicGenerator) manifest(confs []*microgenConfig) error { + log.Println("updating gapic manifest") entries := map[string]manifestEntry{} // Key is the package name. - f, err := os.Create(filepath.Join(gocloudDir, "internal", ".repo-metadata-full.json")) + f, err := os.Create(filepath.Join(g.googleCloudDir, "internal", ".repo-metadata-full.json")) if err != nil { return err } @@ -297,7 +312,7 @@ func manifest(confs []*microgenConfig, googleapisDir, gocloudDir string) error { entries[manual.DistributionName] = manual } for _, conf := range confs { - yamlPath := filepath.Join(googleapisDir, conf.apiServiceConfigPath) + yamlPath := filepath.Join(g.googleapisDir, conf.apiServiceConfigPath) yamlFile, err := os.Open(yamlPath) if err != nil { return err @@ -325,19 +340,15 @@ func manifest(confs []*microgenConfig, googleapisDir, gocloudDir string) error { // copyMicrogenFiles takes microgen files from gocloudDir/cloud.google.com/go // and places them in gocloudDir. -func copyMicrogenFiles(gocloudDir string) error { +func (g *GapicGenerator) copyMicrogenFiles() error { // The period at the end is analagous to * (copy everything in this dir). - c := command("cp", "-R", gocloudDir+"/cloud.google.com/go/.", ".") - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = gocloudDir + c := command("cp", "-R", g.googleCloudDir+"/cloud.google.com/go/.", ".") + c.Dir = g.googleCloudDir if err := c.Run(); err != nil { return err } c = command("rm", "-rf", "cloud.google.com") - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = gocloudDir + c.Dir = g.googleCloudDir return c.Run() } diff --git a/internal/gapicgen/generator/generator.go b/internal/gapicgen/generator/generator.go index f1bab52f587..407c5cf5302 100644 --- a/internal/gapicgen/generator/generator.go +++ b/internal/gapicgen/generator/generator.go @@ -16,9 +16,9 @@ package generator import ( - "bytes" "context" "fmt" + "io/ioutil" "log" "os" "os/exec" @@ -27,34 +27,53 @@ import ( ) // Generate generates genproto and gapics. -func Generate(ctx context.Context, googleapisDir, genprotoDir, gocloudDir, protoDir string, gapicToGenerate string) error { - if err := regenGenproto(ctx, genprotoDir, googleapisDir, protoDir); err != nil { - return fmt.Errorf("error generating genproto (may need to check logs for more errors): %v", err) +func Generate(ctx context.Context, googleapisDir, genprotoDir, gocloudDir, protoDir string, gapicToGenerate string) ([]*ChangeInfo, error) { + protoGenerator := NewGenprotoGenerator(genprotoDir, googleapisDir, protoDir) + gapicGenerator := NewGapicGenerator(googleapisDir, protoDir, gocloudDir, genprotoDir) + if err := protoGenerator.Regen(ctx); err != nil { + return nil, fmt.Errorf("error generating genproto (may need to check logs for more errors): %v", err) + } + if err := gapicGenerator.Regen(ctx, gapicToGenerate); err != nil { + return nil, fmt.Errorf("error generating gapics (may need to check logs for more errors): %v", err) } - if err := generateGapics(ctx, googleapisDir, protoDir, gocloudDir, genprotoDir, gapicToGenerate); err != nil { - return fmt.Errorf("error generating gapics (may need to check logs for more errors): %v", err) + changes, err := gatherChanges(googleapisDir, genprotoDir) + if err != nil { + return nil, fmt.Errorf("error gathering commit info") } if err := recordGoogleapisHash(googleapisDir, genprotoDir); err != nil { - return fmt.Errorf("error recording most recent googleapis hash: %v", err) + return nil, err } - return nil + return changes, nil +} + +func gatherChanges(googleapisDir, genprotoDir string) ([]*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), true) + if err != nil { + return nil, err + } + changes, err := ParseChangeInfo(googleapisDir, commits) + if err != nil { + return nil, err + } + + return changes, nil } // recordGoogleapisHash parses the latest commit in googleapis and records it to // regen.txt in go-genproto. func recordGoogleapisHash(googleapisDir, genprotoDir string) error { - out := bytes.NewBuffer(nil) - c := command("git", "rev-list", "HEAD^..") - c.Stdout = out - c.Stderr = os.Stderr - c.Dir = googleapisDir - if err := c.Run(); err != nil { + commits, err := CommitsSinceHash(googleapisDir, "HEAD", true) + if err != nil { return err } - commits := strings.Split(strings.TrimSpace(out.String()), "\n") if len(commits) != 1 { return fmt.Errorf("only expected one commit, got %d", len(commits)) } @@ -72,26 +91,22 @@ func recordGoogleapisHash(googleapisDir, genprotoDir string) error { // build attempts to build all packages recursively from the given directory. func build(dir string) error { + log.Println("building generated code") c := command("go", "build", "./...") - c.Stdout = os.Stdout - c.Stderr = os.Stderr 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 := command("goimports", "-w", ".") - c.Stdout = os.Stdout - c.Stderr = os.Stderr c.Dir = dir if err := c.Run(); err != nil { return err } c = command("gofmt", "-s", "-d", "-w", "-l", ".") - c.Stdout = os.Stdout - c.Stderr = os.Stderr c.Dir = dir return c.Run() } @@ -100,8 +115,13 @@ type cmdWrapper struct { *exec.Cmd } +// command wraps a exec.Command to add some logging about commands being run. +// The commands stdout/stderr default to os.Stdout/os.Stderr respectfully. func command(name string, arg ...string) *cmdWrapper { - return &cmdWrapper{exec.Command(name, arg...)} + c := &cmdWrapper{exec.Command(name, arg...)} + c.Stdout = os.Stdout + c.Stderr = os.Stderr + return c } func (cw *cmdWrapper) Run() error { diff --git a/internal/gapicgen/generator/genproto.go b/internal/gapicgen/generator/genproto.go index 0d0e597a2d4..727d76561b6 100644 --- a/internal/gapicgen/generator/genproto.go +++ b/internal/gapicgen/generator/genproto.go @@ -20,7 +20,6 @@ import ( "fmt" "io/ioutil" "log" - "os" "path/filepath" "regexp" "strconv" @@ -42,6 +41,22 @@ var denylist = map[string]bool{ "google.golang.org/genproto/googleapis/devtools/containeranalysis/v1": true, } +// GenprotoGenerator is used to generate code for googleapis/go-genproto. +type GenprotoGenerator struct { + genprotoDir string + googleapisDir string + protoSrcDir string +} + +// NewGenprotoGenerator creates a new GenprotoGenerator. +func NewGenprotoGenerator(genprotoDir, googleapisDir, protoDir string) *GenprotoGenerator { + return &GenprotoGenerator{ + genprotoDir: genprotoDir, + googleapisDir: googleapisDir, + protoSrcDir: filepath.Join(protoDir, "/src"), + } +} + var skipPrefixes = []string{ "google.golang.org/genproto/googleapis/ads", } @@ -55,6 +70,7 @@ func hasPrefix(s string, prefixes []string) bool { return false } +// Regen regenerates the genproto repository. // regenGenproto regenerates the genproto repository. // // regenGenproto recursively walks through each directory named by given @@ -68,101 +84,56 @@ func hasPrefix(s string, prefixes []string) bool { // // Protoc is executed on remaining files, one invocation per set of files // declaring the same Go package. -func regenGenproto(ctx context.Context, genprotoDir, googleapisDir, protoDir string) error { +func (g *GenprotoGenerator) Regen(ctx context.Context) error { log.Println("regenerating genproto") - // The protoc include directory is actually the "src" directory of the repo. - protoDir += "/src" - // Create space to put generated .pb.go's. c := command("mkdir", "generated") - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = genprotoDir + c.Dir = g.genprotoDir if err := c.Run(); err != nil { return err } - // Record and map all .proto files to their Go packages. - seenFiles := make(map[string]bool) - pkgFiles := make(map[string][]string) - for _, root := range []string{googleapisDir} { - walkFn := func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - if !info.Mode().IsRegular() || !strings.HasSuffix(path, ".proto") { - return nil - } - - switch rel, err := filepath.Rel(root, path); { - case err != nil: - return err - case seenFiles[rel]: - return nil - default: - seenFiles[rel] = true - } - - pkg, err := goPkg(path) - if err != nil { - return err - } - pkgFiles[pkg] = append(pkgFiles[pkg], path) - return nil - } - if err := filepath.Walk(root, walkFn); err != nil { - return err - } + // Get the last processed googleapis hash. + lastHash, err := ioutil.ReadFile(filepath.Join(g.genprotoDir, "regen.txt")) + if err != nil { + return err } + pkgFiles, err := g.getUpdatedPackages(string(lastHash)) + if err != nil { + return err + } if len(pkgFiles) == 0 { return errors.New("couldn't find any pkgfiles") } - // Run protoc on all protos of all packages. + log.Println("generating from protos") grp, _ := errgroup.WithContext(ctx) - for pkg, fnames := range pkgFiles { + for pkg, fileNames := range pkgFiles { if !strings.HasPrefix(pkg, "google.golang.org/genproto") || denylist[pkg] || hasPrefix(pkg, skipPrefixes) { continue } pk := pkg - fn := fnames + fn := fileNames grp.Go(func() error { log.Println("running protoc on", pk) - return protoc(genprotoDir, googleapisDir, protoDir, fn) + return g.protoc(fn) }) } if err := grp.Wait(); err != nil { return err } - // Move all generated content to their correct locations in the repository, - // because protoc puts it in a folder called generated/. - - // The period at the end is analagous to * (copy everything in this dir). - c = command("cp", "-R", "generated/google.golang.org/genproto/.", ".") - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = genprotoDir - if err := c.Run(); err != nil { + if err := g.moveAndCleanupGeneratedSrc(); err != nil { return err } - c = command("rm", "-rf", "generated") - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = genprotoDir - if err := c.Run(); err != nil { + if err := vet(g.genprotoDir); err != nil { return err } - // Clean up and check it all compiles. - if err := vet(genprotoDir); err != nil { - return err - } - - if err := build(genprotoDir); err != nil { + if err := build(g.genprotoDir); err != nil { return err } @@ -171,8 +142,8 @@ func regenGenproto(ctx context.Context, genprotoDir, googleapisDir, protoDir str // goPkg reports the import path declared in the given file's `go_package` // option. If the option is missing, goPkg returns empty string. -func goPkg(fname string) (string, error) { - content, err := ioutil.ReadFile(fname) +func goPkg(fileName string) (string, error) { + content, err := ioutil.ReadFile(fileName) if err != nil { return "", err } @@ -191,14 +162,53 @@ func goPkg(fname string) (string, error) { return pkgName, nil } -// protoc executes the "protoc" command on files named in fnames, and outputs +// protoc executes the "protoc" command on files named in fileNames, and outputs // to "/generated". -func protoc(genprotoDir, googleapisDir, protoDir string, fnames []string) error { - args := []string{"--experimental_allow_proto3_optional", fmt.Sprintf("--go_out=plugins=grpc:%s/generated", genprotoDir), "-I", googleapisDir, "-I", protoDir} - args = append(args, fnames...) +func (g *GenprotoGenerator) protoc(fileNames []string) error { + args := []string{"--experimental_allow_proto3_optional", fmt.Sprintf("--go_out=plugins=grpc:%s/generated", g.genprotoDir), "-I", g.googleapisDir, "-I", g.protoSrcDir} + args = append(args, fileNames...) c := command("protoc", args...) - c.Stdout = os.Stdout - c.Stderr = os.Stderr - c.Dir = genprotoDir + c.Dir = g.genprotoDir return c.Run() } + +// 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) + if err != nil { + + } + pkgFiles := make(map[string][]string) + for _, v := range files { + if !strings.HasSuffix(v, ".proto") { + continue + } + path := filepath.Join(g.googleapisDir, v) + pkg, err := goPkg(path) + if err != nil { + return nil, err + } + pkgFiles[pkg] = append(pkgFiles[pkg], path) + } + return pkgFiles, nil +} + +// moveAndCleanupGeneratedSrc moves all generated src to their correct locations +// in the repository, because protoc puts it in a folder called `generated/``. +func (g *GenprotoGenerator) moveAndCleanupGeneratedSrc() error { + log.Println("moving generated code") + // The period at the end is analogous to * (copy everything in this dir). + c := command("cp", "-R", filepath.Join(g.genprotoDir, "generated", "google.golang.org", "genproto", "googleapis"), g.genprotoDir) + if err := c.Run(); err != nil { + return err + } + + c = command("rm", "-rf", "generated") + c.Dir = g.genprotoDir + if err := c.Run(); err != nil { + return err + } + + return nil +} diff --git a/internal/gapicgen/generator/git.go b/internal/gapicgen/generator/git.go new file mode 100644 index 00000000000..0e7faa4ed88 --- /dev/null +++ b/internal/gapicgen/generator/git.go @@ -0,0 +1,157 @@ +// Copyright 2020 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 generator + +import ( + "bytes" + "fmt" + "strings" +) + +// ChangeInfo represents a change and its associated metadata. +type ChangeInfo struct { + Body string + GoogleapisHash string + HasGapicChanges bool +} + +// ParseChangeInfo gets the ChangeInfo for a given googleapis hash. +func ParseChangeInfo(googleapisDir string, hashes []string) ([]*ChangeInfo, error) { + var changes []*ChangeInfo + // gapicPkgs is a map of googleapis inputDirectoryPath to the gapic package + // name used for conventional commits. + gapicPkgs := make(map[string]string) + for _, v := range microgenGapicConfigs { + gapicPkgs[v.inputDirectoryPath] = parseConventionalCommitPkg(v.importPath) + } + + for _, hash := range hashes { + // Get commit title and body + rawBody := bytes.NewBuffer(nil) + c := command("git", "show", "--pretty=format:%B", "-s", hash) + c.Stdout = rawBody + c.Dir = googleapisDir + if err := c.Run(); err != nil { + return nil, err + } + + // Add link so corresponding googleapis commit. + body := fmt.Sprintf("%s\nSource-Link: https://github.com/googleapis/googleapis/commit/%s", strings.TrimSpace(rawBody.String()), hash) + + // Try to map files updated to a package in google-cloud-go. Assumes only + // one servies protos are updated per commit. Multile versions are okay. + files, err := filesChanged(googleapisDir, hash) + if err != nil { + return nil, err + } + var pkg string + for _, file := range files { + ss := strings.Split(file, "/") + if len(ss) == 0 { + continue + } + // remove filename from path + strings.Join(ss[:len(ss)-1], "/") + tempPkg := gapicPkgs[strings.Join(ss[:len(ss)-1], "/")] + if tempPkg != "" { + pkg = tempPkg + break + } + } + if pkg == "" { + changes = append(changes, &ChangeInfo{ + Body: body, + GoogleapisHash: hash, + }) + continue + } + + // Try to add in pkg affected into conventional commit scope. + bodyParts := strings.SplitN(body, "\n", 2) + if len(bodyParts) > 0 { + titleParts := strings.SplitN(bodyParts[0], ":", 2) + if len(titleParts) == 2 { + // If a scope is already provided, remove it. + if i := strings.Index(titleParts[0], "("); i > 0 { + titleParts[0] = titleParts[0][:i] + } + titleParts[0] = fmt.Sprintf("%s(%s)", titleParts[0], pkg) + bodyParts[0] = strings.Join(titleParts, ":") + } + body = strings.Join(bodyParts, "\n") + } + + changes = append(changes, &ChangeInfo{ + Body: body, + GoogleapisHash: hash, + HasGapicChanges: true, + }) + } + return changes, nil +} + +func parseConventionalCommitPkg(importPath string) string { + s := strings.TrimPrefix(importPath, "cloud.google.com/go/") + ss := strings.Split(s, "/") + // remove the version, i.e /apiv1 + return strings.Join(ss[:len(ss)-1], "/") +} + +// CommitsSinceHash gathers all of the commits since the provided hash for the +// given gitDir. The inclusive parameter tells if the provided hash should also +// be returned in the slice. +func CommitsSinceHash(gitDir, hash string, inclusive bool) ([]string, error) { + var commitRange string + if inclusive { + commitRange = fmt.Sprintf("%s^..", hash) + } else { + commitRange = fmt.Sprintf("%s..", hash) + } + + out := bytes.NewBuffer(nil) + c := command("git", "rev-list", commitRange) + c.Stdout = out + c.Dir = gitDir + if err := c.Run(); err != nil { + return nil, err + } + return strings.Split(strings.TrimSpace(out.String()), "\n"), nil +} + +// UpdateFilesSinceHash returns a listed of files updated since the provided +// hash for the given gitDir. +func UpdateFilesSinceHash(gitDir, hash string) ([]string, error) { + out := bytes.NewBuffer(nil) + c := command("git", "diff-tree", "--no-commit-id", "--name-only", "-r", fmt.Sprintf("%s..HEAD", hash)) + c.Stdout = out + c.Dir = gitDir + if err := c.Run(); err != nil { + return nil, err + } + return strings.Split(out.String(), "\n"), nil +} + +// filesChanged returns a list of files changed in a commit for the provdied +// hash in the given gitDir. +func filesChanged(gitDir, hash string) ([]string, error) { + out := bytes.NewBuffer(nil) + c := command("git", "show", "--pretty=format:", "--name-only", hash) + c.Stdout = out + c.Dir = gitDir + if err := c.Run(); err != nil { + return nil, err + } + return strings.Split(out.String(), "\n"), nil +} diff --git a/internal/gapicgen/generator/git_test.go b/internal/gapicgen/generator/git_test.go new file mode 100644 index 00000000000..b971259a4be --- /dev/null +++ b/internal/gapicgen/generator/git_test.go @@ -0,0 +1,36 @@ +// Copyright 2020 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 generator + +import "testing" + +func TestParseConventionalCommitPkg(t *testing.T) { + tests := []struct { + name string + importPath string + want string + }{ + {name: "one path element", importPath: "cloud.google.com/go/foo/apiv1", want: "foo"}, + {name: "two path elements", importPath: "cloud.google.com/go/foo/bar/apiv1", want: "foo/bar"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := parseConventionalCommitPkg(tc.importPath); got != tc.want { + t.Errorf("parseConventionalCommitPkg(%q) = %q, want %q", tc.importPath, got, tc.want) + } + }) + } +}