Skip to content

Commit 941ab02

Browse files
authored
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.
1 parent 2760fe5 commit 941ab02

File tree

10 files changed

+452
-169
lines changed

10 files changed

+452
-169
lines changed

internal/gapicgen/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@ gapicgen contains three binaries:
1313
gapic regen CL that needs to have reviewers added and go.mod update, and then
1414
does so. Intended to be run periodically as a bot, but humans can use it too.
1515

16-
See the README.md in each folder for more specific instructions.
16+
See the README.md in each folder for more specific instructions.

internal/gapicgen/cmd/genbot/README.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ It is intended to be used as a bot, though it can be run locally too.
77

88
### Github
99

10-
For Github, you need to generate/supply a Personal Access Token. More information on how that's done is here:
11-
https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line
10+
For Github, you need to generate/supply a Personal Access Token. More
11+
information on how that's done can be found here:
12+
[creating a personal access token](https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line).
1213

1314
## Running locally
1415

@@ -48,9 +49,9 @@ docker run -t --rm --privileged \
4849

4950
## FAQ
5051

51-
#### How do I bump to a later version of the microgenerator?
52+
### How to bump to a later version of the microgenerator
5253

53-
```
54+
```shell
5455
cd /path/to/internal/gapicgen
5556
go get -u github.com/googleapis/gapic-generator-go/cmd/protoc-gen-go_gapic
5657
```

internal/gapicgen/cmd/genbot/generate.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,15 @@ func generate(ctx context.Context, githubClient *GithubClient) error {
6060
return gitClone("https://github.com/googleapis/google-cloud-go", gocloudDir)
6161
})
6262
grp.Go(func() error {
63-
return gitClone("https://github.com/google/protobuf", protoDir)
63+
return gitClone("https://github.com/protocolbuffers/protobuf", protoDir)
6464
})
6565
if err := grp.Wait(); err != nil {
6666
log.Println(err)
6767
}
6868

6969
// Regen.
70-
if err := generator.Generate(ctx, googleapisDir, genprotoDir, gocloudDir, protoDir, ""); err != nil {
70+
changes, err := generator.Generate(ctx, googleapisDir, genprotoDir, gocloudDir, protoDir, "")
71+
if err != nil {
7172
return err
7273
}
7374

@@ -85,17 +86,17 @@ func generate(ctx context.Context, githubClient *GithubClient) error {
8586
switch {
8687
case genprotoHasChanges && gocloudHasChanges:
8788
// Both have changes.
88-
genprotoPRNum, err := githubClient.CreateGenprotoPR(ctx, genprotoDir, true)
89+
genprotoPRNum, err := githubClient.CreateGenprotoPR(ctx, genprotoDir, true, changes)
8990
if err != nil {
9091
return fmt.Errorf("error creating PR for genproto (may need to check logs for more errors): %v", err)
9192
}
9293

93-
gocloudPRNum, err := githubClient.CreateGocloudPR(ctx, gocloudDir, genprotoPRNum)
94+
gocloudPRNum, err := githubClient.CreateGocloudPR(ctx, gocloudDir, genprotoPRNum, changes)
9495
if err != nil {
9596
return fmt.Errorf("error creating CL for veneers (may need to check logs for more errors): %v", err)
9697
}
9798

98-
if err := githubClient.AmendWithPRURL(ctx, genprotoPRNum, genprotoDir, gocloudPRNum); err != nil {
99+
if err := githubClient.AmendGenprotoPR(ctx, genprotoPRNum, genprotoDir, gocloudPRNum, changes); err != nil {
99100
return fmt.Errorf("error amending genproto PR: %v", err)
100101
}
101102

@@ -105,7 +106,7 @@ func generate(ctx context.Context, githubClient *GithubClient) error {
105106
log.Println(gocloudPRURL)
106107
case genprotoHasChanges:
107108
// Only genproto has changes.
108-
genprotoPRNum, err := githubClient.CreateGenprotoPR(ctx, genprotoDir, false)
109+
genprotoPRNum, err := githubClient.CreateGenprotoPR(ctx, genprotoDir, false, changes)
109110
if err != nil {
110111
return fmt.Errorf("error creating PR for genproto (may need to check logs for more errors): %v", err)
111112
}
@@ -115,7 +116,7 @@ func generate(ctx context.Context, githubClient *GithubClient) error {
115116
log.Println("gocloud had no changes")
116117
case gocloudHasChanges:
117118
// Only gocloud has changes.
118-
gocloudPRNum, err := githubClient.CreateGocloudPR(ctx, gocloudDir, -1)
119+
gocloudPRNum, err := githubClient.CreateGocloudPR(ctx, gocloudDir, -1, changes)
119120
if err != nil {
120121
return fmt.Errorf("error creating CL for veneers (may need to check logs for more errors): %v", err)
121122
}
@@ -145,7 +146,7 @@ func gitClone(repo, dir string) error {
145146
func hasChanges(dir string) (bool, error) {
146147
// Write command output to both os.Stderr and local, so that we can check
147148
// whether there are modified files.
148-
inmem := bytes.NewBuffer([]byte{}) // TODO(deklerk): Try `var inmem bytes.Buffer`.
149+
inmem := &bytes.Buffer{}
149150
w := io.MultiWriter(os.Stderr, inmem)
150151

151152
c := exec.Command("bash", "-c", "git status --short")

internal/gapicgen/cmd/genbot/github.go

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"strings"
2727
"time"
2828

29+
"cloud.google.com/go/internal/gapicgen/generator"
2930
"github.com/google/go-github/v32/github"
3031
"github.com/shurcooL/githubv4"
3132
"golang.org/x/oauth2"
@@ -178,13 +179,15 @@ func (gc *GithubClient) GetRegenPR(ctx context.Context, repo string, status stri
178179
// CreateGenprotoPR creates a PR for a given genproto change.
179180
//
180181
// hasCorrespondingPR indicates that there is a corresponding google-cloud-go PR.
181-
func (gc *GithubClient) CreateGenprotoPR(ctx context.Context, genprotoDir string, hasCorrespondingPR bool) (prNumber int, _ error) {
182+
func (gc *GithubClient) CreateGenprotoPR(ctx context.Context, genprotoDir string, hasCorrespondingPR bool, changes []*generator.ChangeInfo) (prNumber int, _ error) {
182183
log.Println("creating genproto PR")
183-
184-
body := genprotoCommitBody
184+
var sb strings.Builder
185+
sb.WriteString(genprotoCommitBody)
185186
if !hasCorrespondingPR {
186-
body += "\n\nThere is no corresponding google-cloud-go PR.\n"
187+
sb.WriteString("\n\nThere is no corresponding google-cloud-go PR.\n")
188+
sb.WriteString(formatChanges(changes, false))
187189
}
190+
body := sb.String()
188191

189192
c := exec.Command("/bin/bash", "-c", `
190193
set -ex
@@ -222,6 +225,7 @@ git push origin $BRANCH_NAME
222225
Body: &body,
223226
Head: &head,
224227
Base: &base,
228+
Draft: github.Bool(true),
225229
})
226230
if err != nil {
227231
return 0, err
@@ -246,18 +250,21 @@ git push origin $BRANCH_NAME
246250
return pr.GetNumber(), nil
247251
}
248252

249-
// CreateGocloudPR creats a PR for a given google-cloud-go change.
250-
func (gc *GithubClient) CreateGocloudPR(ctx context.Context, gocloudDir string, genprotoPRNum int) (prNumber int, _ error) {
253+
// CreateGocloudPR creates a PR for a given google-cloud-go change.
254+
func (gc *GithubClient) CreateGocloudPR(ctx context.Context, gocloudDir string, genprotoPRNum int, changes []*generator.ChangeInfo) (prNumber int, _ error) {
251255
log.Println("creating google-cloud-go PR")
252256

253-
var body string
257+
var sb strings.Builder
254258
var draft bool
259+
sb.WriteString(gocloudCommitBody)
255260
if genprotoPRNum > 0 {
256-
body = gocloudCommitBody + fmt.Sprintf("\n\nCorresponding genproto PR: https://github.com/googleapis/go-genproto/pull/%d\n", genprotoPRNum)
261+
sb.WriteString(fmt.Sprintf("\n\nCorresponding genproto PR: https://github.com/googleapis/go-genproto/pull/%d\n", genprotoPRNum))
257262
draft = true
258263
} else {
259-
body = gocloudCommitBody + "\n\nThere is no corresponding genproto PR.\n"
264+
sb.WriteString("\n\nThere is no corresponding genproto PR.\n")
260265
}
266+
sb.WriteString(formatChanges(changes, true))
267+
body := sb.String()
261268

262269
c := exec.Command("/bin/bash", "-c", `
263270
set -ex
@@ -304,11 +311,14 @@ git push origin $BRANCH_NAME
304311
return pr.GetNumber(), nil
305312
}
306313

307-
// AmendWithPRURL amends the given genproto PR with a link to the given
314+
// AmendGenprotoPR amends the given genproto PR with a link to the given
308315
// google-cloud-go PR.
309-
func (gc *GithubClient) AmendWithPRURL(ctx context.Context, genprotoPRNum int, genprotoDir string, gocloudPRNum int) error {
310-
newBody := genprotoCommitBody + fmt.Sprintf("\n\nCorresponding google-cloud-go PR: googleapis/google-cloud-go#%d\n", gocloudPRNum)
311-
316+
func (gc *GithubClient) AmendGenprotoPR(ctx context.Context, genprotoPRNum int, genprotoDir string, gocloudPRNum int, changes []*generator.ChangeInfo) error {
317+
var body strings.Builder
318+
body.WriteString(genprotoCommitBody)
319+
body.WriteString(fmt.Sprintf("\n\nCorresponding google-cloud-go PR: googleapis/google-cloud-go#%d\n", gocloudPRNum))
320+
body.WriteString(formatChanges(changes, false))
321+
sBody := body.String()
312322
c := exec.Command("/bin/bash", "-c", `
313323
set -ex
314324
@@ -325,15 +335,15 @@ git push -f origin $BRANCH_NAME
325335
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.
326336
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.
327337
fmt.Sprintf("COMMIT_TITLE=%s", genprotoCommitTitle),
328-
fmt.Sprintf("COMMIT_BODY=%s", newBody),
338+
fmt.Sprintf("COMMIT_BODY=%s", sBody),
329339
fmt.Sprintf("BRANCH_NAME=%s", genprotoBranchName),
330340
}
331341
c.Dir = genprotoDir
332342
if err := c.Run(); err != nil {
333343
return err
334344
}
335345
_, _, err := gc.cV3.PullRequests.Edit(ctx, "googleapis", "go-genproto", genprotoPRNum, &github.PullRequest{
336-
Body: &newBody,
346+
Body: &sBody,
337347
})
338348
return err
339349
}
@@ -356,3 +366,31 @@ func (gc *GithubClient) MarkPRReadyForReview(ctx context.Context, repo string, n
356366
}
357367
return nil
358368
}
369+
370+
func formatChanges(changes []*generator.ChangeInfo, onlyGapicChanges bool) string {
371+
if len(changes) == 0 {
372+
return ""
373+
}
374+
var sb strings.Builder
375+
sb.WriteString("\nChanges:\n")
376+
for _, c := range changes {
377+
if onlyGapicChanges && !c.HasGapicChanges {
378+
continue
379+
}
380+
sb.WriteString("- ")
381+
ss := strings.Split(c.Body, "\n")
382+
for i, s := range ss {
383+
if i == 0 {
384+
sb.WriteString(fmt.Sprintf("%s\n", s))
385+
continue
386+
}
387+
if s == "" {
388+
sb.WriteString("\n")
389+
continue
390+
}
391+
sb.WriteString(fmt.Sprintf(" %s\n", s))
392+
}
393+
sb.WriteString("\n")
394+
}
395+
return sb.String()
396+
}

internal/gapicgen/cmd/genlocal/main.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package main
2020
import (
2121
"context"
2222
"flag"
23+
"fmt"
2324
"io/ioutil"
2425
"log"
2526
"os"
@@ -54,32 +55,40 @@ func main() {
5455
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.")
5556
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.")
5657
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.`)
58+
verbose := flag.Bool("verbose", false, "Enables verbose logging.")
5759
flag.Parse()
5860

5961
ctx := context.Background()
6062

6163
// Clone repositories if needed.
62-
6364
grp, _ := errgroup.WithContext(ctx)
6465
gitClone(grp, "https://github.com/googleapis/googleapis.git", *googleapisDir, tmpDir)
6566
gitClone(grp, "https://github.com/googleapis/go-genproto", *genprotoDir, tmpDir)
6667
gitClone(grp, "https://github.com/googleapis/google-cloud-go", *gocloudDir, tmpDir)
67-
gitClone(grp, "https://github.com/google/protobuf", *protoDir, tmpDir)
68+
gitClone(grp, "https://github.com/protocolbuffers/protobuf", *protoDir, tmpDir)
6869
if err := grp.Wait(); err != nil {
6970
log.Println(err)
7071
}
7172

7273
// Regen.
73-
74-
if err := generator.Generate(ctx, *googleapisDir, *genprotoDir, *gocloudDir, *protoDir, *gapicToGenerate); err != nil {
74+
changes, err := generator.Generate(ctx, *googleapisDir, *genprotoDir, *gocloudDir, *protoDir, *gapicToGenerate)
75+
if err != nil {
7576
log.Printf("Generator ran (and failed) in %s\n", tmpDir)
7677
log.Fatal(err)
7778
}
7879

7980
// Log results.
80-
8181
log.Println(genprotoDir)
8282
log.Println(gocloudDir)
83+
84+
if *verbose {
85+
log.Println("Changes:")
86+
fmt.Println()
87+
for _, v := range changes {
88+
fmt.Println("********************************************")
89+
fmt.Println(v.Body)
90+
}
91+
}
8392
}
8493

8594
// gitClone clones a repository in the given directory if dir is not in tmpDir.

0 commit comments

Comments
 (0)