From de2befcaa2a886499db9da6d4d04d28398c8d44b Mon Sep 17 00:00:00 2001 From: Cody Oss <6331106+codyoss@users.noreply.github.com> Date: Tue, 26 Apr 2022 15:40:42 -0600 Subject: [PATCH] fix(internal/gapicgen): properly update modules that have no gapic changes (#5945) Previously this was happening thanks to some logic the revolved around looking for UNKNOWN date in a generated gapic client. This logic was removed around a month ago and hense PRs that have genproto changes but no gapic changes have needed some manual updates. With this change the update process now parses out what kind of changes happened based on context aware commit messages. This means even if there we not chnages to the gapic client genproto will still get updated to the latest version if there were changes to message types. --- internal/gapicgen/generator/gapics.go | 28 --------------- internal/gapicgen/git/github.go | 49 +++++++++++++++++++++++++++ internal/gapicgen/git/github_test.go | 39 +++++++++++++++++++++ 3 files changed, 88 insertions(+), 28 deletions(-) create mode 100644 internal/gapicgen/git/github_test.go diff --git a/internal/gapicgen/generator/gapics.go b/internal/gapicgen/generator/gapics.go index 9f304696b49..070ff69cd91 100644 --- a/internal/gapicgen/generator/gapics.go +++ b/internal/gapicgen/generator/gapics.go @@ -29,7 +29,6 @@ import ( "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" ) @@ -617,33 +616,6 @@ func ParseAPIShortnames(googleapisDir string, confs []*MicrogenConfig, manualEnt 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 -} - func docURL(cloudDir, importPath string) (string, error) { suffix := strings.TrimPrefix(importPath, "cloud.google.com/go/") mod, err := gocmd.CurrentMod(filepath.Join(cloudDir, suffix)) diff --git a/internal/gapicgen/git/github.go b/internal/gapicgen/git/github.go index 4d813b16752..07f48f83549 100644 --- a/internal/gapicgen/git/github.go +++ b/internal/gapicgen/git/github.go @@ -24,6 +24,7 @@ import ( "os/user" "path" "path/filepath" + "regexp" "strings" "time" @@ -68,6 +69,8 @@ genbot to assign reviewers to the google-cloud-go PR. ` ) +var conventionalCommitScopeRe = regexp.MustCompile(`.*\((.*)\): .*`) + // PullRequest represents a GitHub pull request. type PullRequest struct { Author string @@ -402,6 +405,15 @@ func updateDeps(tmpDir string) error { updatedModDirs[modDir] = true } + // Find modules based on conventional commit messages. + commitModDirs, err := processCommitMessage(tmpDir) + if err != nil { + return err + } + for _, v := range commitModDirs { + updatedModDirs[v] = true + } + // Update required modules. for modDir := range updatedModDirs { log.Printf("Updating module dir %q", modDir) @@ -442,3 +454,40 @@ fi c.Dir = tmpDir return c.Run() } + +// processCommitMessage process the context aware commits mentioned in the PR +// body to determine what modules need to have dependency bumps. +func processCommitMessage(dir string) ([]string, error) { + c := execv.Command("git", "log", "-1", "--pretty=%B") + c.Dir = dir + out, err := c.Output() + if err != nil { + return nil, err + } + outStr := string(out) + ss := strings.Split(outStr, "Changes:\n\n") + if len(ss) != 2 { + return nil, fmt.Errorf("unable to process commit msg") + } + commits := strings.Split(ss[1], "\n\n") + var modDirs []string + for _, v := range commits { + pkg := parsePackage(v) + if pkg == "" { + continue + } + modDirs = append(modDirs, filepath.Join(dir, pkg)) + } + return modDirs, nil +} + +// parsePackage parses a package name from the conventional commit scope of a +// commit message. +func parsePackage(msg string) string { + matches := conventionalCommitScopeRe.FindStringSubmatch(msg) + if len(matches) < 2 { + return "" + } + return matches[1] + +} diff --git a/internal/gapicgen/git/github_test.go b/internal/gapicgen/git/github_test.go new file mode 100644 index 00000000000..7bd473c399c --- /dev/null +++ b/internal/gapicgen/git/github_test.go @@ -0,0 +1,39 @@ +// Copyright 2022 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 git + +import "testing" + +func TestParsePackage(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {"found basic package", "feat(foo): something", "foo"}, + {"found nested package", "fix(foo/bar): something", "foo/bar"}, + {"no scope", "chore: something", ""}, + {"bad commit msg", "something happend", ""}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := parsePackage(tc.input) + if got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +}