internal/gapicgen: check if there are changes to be committed
Currently, genbot always assumes that BOTH genproto and gocloud have changes to
be committed. That is frequently not true. This CL makes genbot check first, and
adjust PR/CL messages accordingly.
Change-Id: I054bbd711467eb0f43de90701c0bfd84f1e41e66
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/49851
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tyler Bui-Palsulich <tbp@google.com>
diff --git a/internal/gapicgen/cmd/genbot/gapics.go b/internal/gapicgen/cmd/genbot/gapics.go
index 0190874..f05ea95 100644
--- a/internal/gapicgen/cmd/genbot/gapics.go
+++ b/internal/gapicgen/cmd/genbot/gapics.go
@@ -47,13 +47,17 @@
// clGocloud creates a CL for the given gocloud change (including a link to
// the given genproto PR).
+//
+// genprotoPRNum may be -1 to indicate there is no corresponding genproto PR.
func clGocloud(ctx context.Context, gocloudDir string, genprotoPRNum int) (url string, _ error) {
log.Println("creating gocloud CL")
- newBody := fmt.Sprintf(`%s
-
-Corresponding genproto PR: https://github.com/googleapis/go-genproto/pull/%d
-`, gerritCommitBody, genprotoPRNum)
+ var newBody string
+ if genprotoPRNum > 0 {
+ newBody = gerritCommitBody + fmt.Sprintf("\n\nCorresponding genproto PR: https://github.com/googleapis/go-genproto/pull/%d\n", genprotoPRNum)
+ } else {
+ newBody = gerritCommitBody + "\n\nThere is no corresponding genproto PR.\n"
+ }
// Write command output to both os.Stderr and local, so that we can check
// for gerrit URL.
diff --git a/internal/gapicgen/cmd/genbot/genproto.go b/internal/gapicgen/cmd/genbot/genproto.go
index 23571ae..1e3eae4 100644
--- a/internal/gapicgen/cmd/genbot/genproto.go
+++ b/internal/gapicgen/cmd/genbot/genproto.go
@@ -54,9 +54,16 @@
var genprotoReviewers = []string{"jadekler", "hongalex", "broady", "noahdietz", "tritone", "codyoss", "tbpg"}
// prGenproto creates a PR for a given genproto change.
-func prGenproto(ctx context.Context, githubClient *github.Client, genprotoDir string) (prNumber int, _ error) {
+//
+// hasCorrespondingCL indicates that there is a corresponding gocloud CL.
+func prGenproto(ctx context.Context, githubClient *github.Client, genprotoDir string, hasCorrespondingCL bool) (prNumber int, _ error) {
log.Println("creating genproto PR")
+ body := genprotoCommitBody
+ if !hasCorrespondingCL {
+ body += "\n\nThere is no corresponding gocloud CL.\n"
+ }
+
c := exec.Command("/bin/bash", "-c", `
set -ex
@@ -77,7 +84,7 @@
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", genprotoCommitBody),
+ fmt.Sprintf("COMMIT_BODY=%s", body),
fmt.Sprintf("BRANCH_NAME=%s", genprotoBranchName),
}
c.Dir = genprotoDir
@@ -88,10 +95,9 @@
head := fmt.Sprintf("googleapis:" + genprotoBranchName)
base := "master"
t := genprotoCommitTitle // Because we have to take the address.
- b := genprotoCommitBody // Because we have to take the address.
pr, _, err := githubClient.PullRequests.Create(ctx, "googleapis", "go-genproto", &github.NewPullRequest{
Title: &t,
- Body: &b,
+ Body: &body,
Head: &head,
Base: &base,
})
@@ -121,10 +127,7 @@
// amendPRWithCLURL amends the given genproto PR with a link to the given
// gocloud CL.
func amendPRWithCLURL(ctx context.Context, githubClient *github.Client, genprotoPRNum int, genprotoDir, gocloudCL string) error {
- newBody := fmt.Sprintf(`%s
-
-Corresponding gocloud CL: %s
-`, genprotoCommitBody, gocloudCL)
+ newBody := genprotoCommitBody + fmt.Sprintf("\n\nCorresponding gocloud CL: %s\n", gocloudCL)
c := exec.Command("/bin/bash", "-c", `
set -ex
diff --git a/internal/gapicgen/cmd/genbot/main.go b/internal/gapicgen/cmd/genbot/main.go
index 1333344..1b74f55 100644
--- a/internal/gapicgen/cmd/genbot/main.go
+++ b/internal/gapicgen/cmd/genbot/main.go
@@ -17,12 +17,15 @@
package main
import (
+ "bytes"
"context"
"flag"
"fmt"
+ "io"
"io/ioutil"
"log"
"os"
+ "os/exec"
"path/filepath"
"cloud.google.com/go/internal/gapicgen"
@@ -188,25 +191,63 @@
// Create PRs/CLs.
- genprotoPRNum, err := prGenproto(ctx, githubClient, genprotoDir)
+ genprotoHasChanges, err := hasChanges(genprotoDir)
if err != nil {
- log.Fatalf("error creating PR for genproto (may need to check logs for more errors): %v", err)
+ log.Fatal(err)
}
- gocloudCL, err := clGocloud(ctx, gocloudDir, genprotoPRNum)
+ gocloudHasChanges, err := hasChanges(gocloudDir)
if err != nil {
- log.Fatalf("error creating CL for veneers (may need to check logs for more errors): %v", err)
+ log.Fatal(err)
}
- if err := amendPRWithCLURL(ctx, githubClient, genprotoPRNum, genprotoDir, gocloudCL); err != nil {
- log.Fatalf("error amending genproto PR: %v", err)
+ switch {
+ case genprotoHasChanges && gocloudHasChanges:
+ // Both have changes.
+
+ genprotoPRNum, err := prGenproto(ctx, githubClient, genprotoDir, true)
+ if err != nil {
+ log.Fatalf("error creating PR for genproto (may need to check logs for more errors): %v", err)
+ }
+
+ gocloudCL, err := clGocloud(ctx, gocloudDir, genprotoPRNum)
+ if err != nil {
+ log.Fatalf("error creating CL for veneers (may need to check logs for more errors): %v", err)
+ }
+
+ if err := amendPRWithCLURL(ctx, githubClient, genprotoPRNum, genprotoDir, gocloudCL); err != nil {
+ log.Fatalf("error amending genproto PR: %v", err)
+ }
+
+ genprotoPRURL := fmt.Sprintf("https://github.com/googleapis/go-genproto/pull/%d", genprotoPRNum)
+ log.Println(genprotoPRURL)
+ log.Println(gocloudCL)
+ case genprotoHasChanges:
+ // Only genproto has changes.
+
+ genprotoPRNum, err := prGenproto(ctx, githubClient, genprotoDir, false)
+ if err != nil {
+ log.Fatalf("error creating PR for genproto (may need to check logs for more errors): %v", err)
+ }
+
+ genprotoPRURL := fmt.Sprintf("https://github.com/googleapis/go-genproto/pull/%d", genprotoPRNum)
+ log.Println(genprotoPRURL)
+ log.Println("gocloud had no changes")
+ case gocloudHasChanges:
+ // Only gocloud has changes.
+
+ gocloudCL, err := clGocloud(ctx, gocloudDir, -1)
+ if err != nil {
+ log.Fatalf("error creating CL for veneers (may need to check logs for more errors): %v", err)
+ }
+
+ log.Println("genproto had no changes")
+ log.Println(gocloudCL)
+ default:
+ // Neither have changes.
+
+ log.Println("Neither genproto nor gocloud had changes")
}
-
- // Log results.
-
- genprotoPRURL := fmt.Sprintf("https://github.com/googleapis/go-genproto/pull/%d", genprotoPRNum)
- log.Println(genprotoPRURL)
- log.Println(gocloudCL)
}
// gitClone clones a repository in the given directory.
@@ -219,3 +260,18 @@
})
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.NewBuffer([]byte{}) // TODO(deklerk): Try `var inmem bytes.Buffer`.
+ w := io.MultiWriter(os.Stderr, inmem)
+
+ c := exec.Command("bash", "-c", "git status --short")
+ c.Stdout = w
+ c.Stderr = os.Stderr
+ c.Stdin = os.Stdin // Prevents "the input device is not a TTY" error.
+
+ return inmem.Len() > 0, nil
+}
