Skip to content

Commit 617f3ed

Browse files
h9jianggopherbot
authored andcommitted
internal/task: update dependency in master branch after minor release
1. Extract the awaitSubmission method so that it can be shared by multiple workflows. 2. Add an executeAndMonitorChange helper, to run a script and return any change to the file list provided. 3. Add a task to the gopls release flow to upgrade dependency for x/tools' gopls submodule in master branch. (only for minor version release) 4. Update the gopls release flow to accept CL reviewers as a parameter. A local relui screenshot is at https://go.dev/issue/57643#issuecomment-2294232653 For golang/go#57643 Change-Id: I9cb6495956409ce9a3712584278ba0e031e48cfc Reviewed-on: https://go-review.googlesource.com/c/build/+/606336 Auto-Submit: Hongxiang Jiang <[email protected]> Reviewed-by: Robert Findley <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent ec1c451 commit 617f3ed

File tree

3 files changed

+228
-47
lines changed

3 files changed

+228
-47
lines changed

cmd/relui/main.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,12 @@ func main() {
320320
}
321321
dh.RegisterDefinition("Prepare a pre-release gopls candidate", prereleaseGoplsTasks.NewDefinition())
322322

323+
releaseGoplsTasks := task.ReleaseGoplsTasks{
324+
Gerrit: gerritClient,
325+
CloudBuild: cloudBuildClient,
326+
}
327+
dh.RegisterDefinition("Release gopls", releaseGoplsTasks.NewDefinition())
328+
323329
privateSyncTask := &task.PrivateMasterSyncTask{
324330
Git: gitClient,
325331
PrivateGerritURL: "https://go-internal.googlesource.com/golang/go-private",

internal/task/releasegopls.go

Lines changed: 139 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ func (r *PrereleaseGoplsTasks) NewDefinition() *wf.Definition {
4343
branchCreated := wf.Action1(wd, "creating new branch if minor release", r.createBranchIfMinor, semv, wf.After(issue))
4444

4545
configChangeID := wf.Task3(wd, "updating branch's codereview.cfg", r.updateCodeReviewConfig, semv, reviewers, issue, wf.After(branchCreated))
46-
configCommit := wf.Task1(wd, "await config CL submission", r.AwaitSubmission, configChangeID)
46+
configCommit := wf.Task1(wd, "await config CL submission", clAwaiter{r.Gerrit}.awaitSubmission, configChangeID)
4747

4848
dependencyChangeID := wf.Task4(wd, "updating gopls' x/tools dependency", r.updateXToolsDependency, semv, prerelease, reviewers, issue, wf.After(configCommit))
49-
dependencyCommit := wf.Task1(wd, "await gopls' x/tools dependency CL submission", r.AwaitSubmission, dependencyChangeID)
49+
dependencyCommit := wf.Task1(wd, "await gopls' x/tools dependency CL submission", clAwaiter{r.Gerrit}.awaitSubmission, dependencyChangeID)
5050

5151
verified := wf.Action1(wd, "verify installing latest gopls using release branch dependency commit", r.verifyGoplsInstallation, dependencyCommit)
5252
prereleaseVersion := wf.Task3(wd, "tag pre-release", r.tagPrerelease, semv, dependencyCommit, prerelease, wf.After(verified))
@@ -154,10 +154,10 @@ func (r *PrereleaseGoplsTasks) createBranchIfMinor(ctx *wf.TaskContext, semv sem
154154
//
155155
// It returns an empty string if no such CL is found, otherwise it returns the
156156
// CL's change ID.
157-
func (r *PrereleaseGoplsTasks) openCL(ctx *wf.TaskContext, branch, title string) (string, error) {
157+
func openCL(ctx *wf.TaskContext, gerrit GerritClient, branch, title string) (string, error) {
158158
// Query for an existing pending config CL, to avoid duplication.
159159
query := fmt.Sprintf(`message:%q status:open owner:[email protected] repo:tools branch:%q -age:7d`, title, branch)
160-
changes, err := r.Gerrit.QueryChanges(ctx, query)
160+
changes, err := gerrit.QueryChanges(ctx, query)
161161
if err != nil {
162162
return "", err
163163
}
@@ -178,7 +178,7 @@ func (r *PrereleaseGoplsTasks) updateCodeReviewConfig(ctx *wf.TaskContext, semv
178178
branch := goplsReleaseBranchName(semv)
179179
clTitle := fmt.Sprintf("all: update %s for %s", configFile, branch)
180180

181-
openCL, err := r.openCL(ctx, branch, clTitle)
181+
openCL, err := openCL(ctx, r.Gerrit, branch, clTitle)
182182
if err != nil {
183183
return "", fmt.Errorf("failed to find the open CL of title %q in branch %q: %w", clTitle, branch, err)
184184
}
@@ -275,7 +275,7 @@ func (r *PrereleaseGoplsTasks) updateXToolsDependency(ctx *wf.TaskContext, semv
275275

276276
branch := goplsReleaseBranchName(semv)
277277
clTitle := fmt.Sprintf("gopls: update go.mod for v%v.%v.%v-%s", semv.Major, semv.Minor, semv.Patch, pre)
278-
openCL, err := r.openCL(ctx, branch, clTitle)
278+
openCL, err := openCL(ctx, r.Gerrit, branch, clTitle)
279279
if err != nil {
280280
return "", fmt.Errorf("failed to find the open CL of title %q in branch %q: %w", clTitle, branch, err)
281281
}
@@ -284,38 +284,23 @@ func (r *PrereleaseGoplsTasks) updateXToolsDependency(ctx *wf.TaskContext, semv
284284
return openCL, nil
285285
}
286286

287-
outputFiles := []string{"gopls/go.mod.before", "gopls/go.mod", "gopls/go.sum.before", "gopls/go.sum"}
288-
const scriptFmt = `git checkout %s
289-
git rev-parse --abbrev-ref HEAD
290-
cp gopls/go.mod gopls/go.mod.before
291-
cp gopls/go.sum gopls/go.sum.before
292-
cd gopls
293-
go mod edit -dropreplace=golang.org/x/tools
294-
go get golang.org/x/tools@%s
295-
go mod tidy -compat=1.19
296-
`
297287
head, err := r.Gerrit.ReadBranchHead(ctx, "tools", branch)
298288
if err != nil {
299289
return "", err
300290
}
301-
build, err := r.CloudBuild.RunScript(ctx, fmt.Sprintf(scriptFmt, branch, head), "tools", outputFiles)
302-
if err != nil {
303-
return "", err
304-
}
291+
// TODO(hxjiang): Remove -compat flag once gopls no longer supports building
292+
// with older Go versions.
293+
script := fmt.Sprintf(`cd gopls
294+
go mod edit -dropreplace=golang.org/x/tools
295+
go get golang.org/x/tools@%s
296+
go mod tidy -compat=1.19
297+
`, head)
305298

306-
outputs, err := buildToOutputs(ctx, r.CloudBuild, build)
299+
changedFiles, err := executeAndMonitorChange(ctx, r.CloudBuild, branch, script, []string{"gopls/go.mod", "gopls/go.sum"})
307300
if err != nil {
308301
return "", err
309302
}
310303

311-
changedFiles := map[string]string{}
312-
for i := 0; i < len(outputFiles); i += 2 {
313-
before, after := outputs[outputFiles[i]], outputs[outputFiles[i+1]]
314-
if before != after {
315-
changedFiles[outputFiles[i+1]] = after
316-
}
317-
}
318-
319304
// Skip CL creation as nothing changed.
320305
if len(changedFiles) == 0 {
321306
return "", nil
@@ -324,7 +309,7 @@ go mod tidy -compat=1.19
324309
changeInput := gerrit.ChangeInput{
325310
Project: "tools",
326311
Branch: branch,
327-
Subject: fmt.Sprintf("%s\n\nThis is an automated CL which updates the go.mod go.sum.\n\nFor golang/go#%v", clTitle, issue),
312+
Subject: fmt.Sprintf("%s\n\nThis is an automated CL which updates the go.mod and go.sum.\n\nFor golang/go#%v", clTitle, issue),
328313
}
329314

330315
ctx.Printf("creating auto-submit change under branch %q in x/tools repo.", branch)
@@ -391,21 +376,6 @@ func (r *PrereleaseGoplsTasks) tagPrerelease(ctx *wf.TaskContext, semv semversio
391376
return version, nil
392377
}
393378

394-
// AwaitSubmission waits for the CL with the given change ID to be submitted.
395-
//
396-
// The return value is the submitted commit hash, or "" if changeID is "".
397-
func (r *PrereleaseGoplsTasks) AwaitSubmission(ctx *wf.TaskContext, changeID string) (string, error) {
398-
if changeID == "" {
399-
ctx.Printf("not awaiting: no CL was created")
400-
return "", nil
401-
}
402-
403-
ctx.Printf("awaiting review/submit of %v", ChangeLink(changeID))
404-
return AwaitCondition(ctx, 10*time.Second, func() (string, bool, error) {
405-
return r.Gerrit.Submitted(ctx, changeID, "")
406-
})
407-
}
408-
409379
func (r *PrereleaseGoplsTasks) isValidVersion(ctx *wf.TaskContext, ver string) (semversion, error) {
410380
if !semver.IsValid(ver) {
411381
return semversion{}, fmt.Errorf("the input %q version does not follow semantic version schema", ver)
@@ -529,17 +499,22 @@ func (r *PrereleaseGoplsTasks) possibleGoplsVersions(ctx *wf.TaskContext) ([]str
529499
// ReleaseGoplsTasks implements a new workflow definition include all the tasks
530500
// to release gopls.
531501
type ReleaseGoplsTasks struct {
532-
Gerrit GerritClient
502+
Gerrit GerritClient
503+
CloudBuild CloudBuildClient
533504
}
534505

535506
// NewDefinition create a new workflow definition for gopls release.
536507
func (r *ReleaseGoplsTasks) NewDefinition() *wf.Definition {
537508
wd := wf.New(wf.ACL{Groups: []string{groups.ToolsTeam}})
538509

539510
version := wf.Param(wd, wf.ParamDef[string]{Name: "pre-release version"})
511+
reviewers := wf.Param(wd, reviewersParam)
540512

541513
semv := wf.Task1(wd, "validating input version", r.isValidPrereleaseVersion, version)
542-
_ = wf.Action1(wd, "tag the release", r.tagRelease, semv)
514+
tagged := wf.Action1(wd, "tag the release", r.tagRelease, semv)
515+
516+
changeID := wf.Task2(wd, "updating x/tools dependency in master branch in gopls sub dir", r.updateDependencyIfMinor, reviewers, semv, wf.After(tagged))
517+
_ = wf.Task1(wd, "await x/tools gopls dependency CL submission in gopls sub dir", clAwaiter{r.Gerrit}.awaitSubmission, changeID)
543518

544519
return wd
545520
}
@@ -637,3 +612,120 @@ func (r *ReleaseGoplsTasks) tagRelease(ctx *wf.TaskContext, semv semversion) err
637612
ctx.Printf("tagged commit %s with tag %s", info.Revision, releaseTag)
638613
return nil
639614
}
615+
616+
// updateDependencyIfMinor update the dependency of x/tools repo in master
617+
// branch.
618+
//
619+
// Returns the change ID.
620+
func (r *ReleaseGoplsTasks) updateDependencyIfMinor(ctx *wf.TaskContext, reviewers []string, semv semversion) (string, error) {
621+
if semv.Patch != 0 {
622+
return "", nil
623+
}
624+
625+
clTitle := fmt.Sprintf("gopls/go.mod: update dependencies following the v%v.%v.%v release", semv.Major, semv.Minor, semv.Patch)
626+
openCL, err := openCL(ctx, r.Gerrit, "master", clTitle)
627+
if err != nil {
628+
return "", fmt.Errorf("failed to find the open CL of title %q in master branch: %w", clTitle, err)
629+
}
630+
if openCL != "" {
631+
ctx.Printf("not creating CL: found existing CL %s", openCL)
632+
return openCL, nil
633+
}
634+
635+
// TODO(hxjiang): Remove -compat flag once gopls no longer supports building
636+
// with older Go versions.
637+
const script = `cd gopls
638+
pwd
639+
go get -u all
640+
go mod tidy -compat=1.19
641+
`
642+
changed, err := executeAndMonitorChange(ctx, r.CloudBuild, "master", script, []string{"gopls/go.mod", "gopls/go.sum"})
643+
if err != nil {
644+
return "", err
645+
}
646+
647+
// Skip CL creation as nothing changed.
648+
if len(changed) == 0 {
649+
return "", nil
650+
}
651+
652+
changeInput := gerrit.ChangeInput{
653+
Project: "tools",
654+
Branch: "master",
655+
Subject: fmt.Sprintf("%s\n\nThis is an automated CL which updates the go.mod and go.sum.", clTitle),
656+
}
657+
658+
ctx.Printf("creating auto-submit change under master branch in x/tools repo.")
659+
return r.Gerrit.CreateAutoSubmitChange(ctx, changeInput, reviewers, changed)
660+
}
661+
662+
// executeAndMonitorChange runs the specified script on the designated branch,
663+
// tracking changes to the provided files.
664+
//
665+
// Returns a map where keys are the filenames of modified files and values are
666+
// their corresponding content after script execution.
667+
func executeAndMonitorChange(ctx *wf.TaskContext, cloudBuild CloudBuildClient, branch, script string, watchFiles []string) (map[string]string, error) {
668+
// Checkout to the provided branch.
669+
fullScript := fmt.Sprintf(`git checkout %s
670+
git rev-parse --abbrev-ref HEAD
671+
git rev-parse --ref HEAD
672+
`, branch)
673+
// Make a copy of all file that need to watch.
674+
// If the file does not exist, create a empty file and a empty before file.
675+
for _, file := range watchFiles {
676+
if strings.Contains(file, "'") {
677+
return nil, fmt.Errorf("file name %q contains '", file)
678+
}
679+
fullScript += fmt.Sprintf(`if [ -f '%[1]s' ]; then
680+
cp '%[1]s' '%[1]s.before'
681+
else
682+
touch '%[1]s' '%[1]s.before'
683+
fi
684+
`, file)
685+
}
686+
// Execute the script provided.
687+
fullScript += script
688+
689+
// Output files before the script execution and after the script execution.
690+
outputFiles := []string{}
691+
for _, file := range watchFiles {
692+
outputFiles = append(outputFiles, file+".before")
693+
outputFiles = append(outputFiles, file)
694+
}
695+
build, err := cloudBuild.RunScript(ctx, fullScript, "tools", outputFiles)
696+
if err != nil {
697+
return nil, err
698+
}
699+
700+
outputs, err := buildToOutputs(ctx, cloudBuild, build)
701+
if err != nil {
702+
return nil, err
703+
}
704+
705+
changed := map[string]string{}
706+
for i := 0; i < len(outputFiles); i += 2 {
707+
if before, after := outputs[outputFiles[i]], outputs[outputFiles[i+1]]; before != after {
708+
changed[outputFiles[i+1]] = after
709+
}
710+
}
711+
712+
return changed, nil
713+
}
714+
715+
// A clAwaiter closes over a GerritClient to provide a reusable workflow task
716+
// for awaiting the submission of a Gerrit change.
717+
type clAwaiter struct {
718+
GerritClient
719+
}
720+
721+
func (c clAwaiter) awaitSubmission(ctx *wf.TaskContext, changeID string) (string, error) {
722+
if changeID == "" {
723+
ctx.Printf("not awaiting: no CL was created")
724+
return "", nil
725+
}
726+
727+
ctx.Printf("awaiting review/submit of %v", ChangeLink(changeID))
728+
return AwaitCondition(ctx, 10*time.Second, func() (string, bool, error) {
729+
return c.Submitted(ctx, changeID, "")
730+
})
731+
}

internal/task/releasegopls_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package task
77
import (
88
"context"
99
"fmt"
10+
"reflect"
1011
"testing"
1112

1213
"github.com/google/go-cmp/cmp"
@@ -871,3 +872,85 @@ func TestTagRelease(t *testing.T) {
871872
})
872873
}
873874
}
875+
876+
func TestExecuteAndMonitorChange(t *testing.T) {
877+
mustHaveShell(t)
878+
879+
testcases := []struct {
880+
name string
881+
branch string
882+
script string
883+
watch []string
884+
want map[string]string
885+
}{
886+
{
887+
name: "write all three files with different content",
888+
branch: "master",
889+
script: `echo -n "foo" > file_a
890+
echo -n "foo" > file_b
891+
echo -n "foo" > file_c
892+
`,
893+
watch: []string{"file_a", "file_b", "file_c"},
894+
want: map[string]string{"file_a": "foo", "file_b": "foo", "file_c": "foo"},
895+
},
896+
{
897+
name: "ignore file_c changes",
898+
branch: "master",
899+
script: `echo -n "foo" > file_a
900+
echo -n "foo" > file_b
901+
echo -n "foo" > file_c
902+
`,
903+
watch: []string{"file_a", "file_b"},
904+
want: map[string]string{"file_a": "foo", "file_b": "foo"},
905+
},
906+
{
907+
name: "write two files with different content",
908+
branch: "master",
909+
script: `echo -n "foo" > file_a
910+
echo -n "foo" > file_b
911+
`,
912+
watch: []string{"file_a", "file_b", "file_c"},
913+
want: map[string]string{"file_a": "foo", "file_b": "foo"},
914+
},
915+
{
916+
name: "write one file with different content in foo branch",
917+
branch: "foo",
918+
script: `echo -n "foo" > file_a`,
919+
watch: []string{"file_a", "file_b", "file_c"},
920+
want: map[string]string{"file_a": "foo"},
921+
},
922+
{
923+
name: "create a file in foo branch",
924+
branch: "foo",
925+
script: `echo -n "foo" > file_d`,
926+
watch: []string{"file_a", "file_b", "file_c", "file_d"},
927+
want: map[string]string{"file_d": "foo"},
928+
},
929+
}
930+
931+
for _, tc := range testcases {
932+
t.Run(tc.name, func(t *testing.T) {
933+
tools := NewFakeRepo(t, "tools")
934+
initial := tools.Commit(map[string]string{
935+
"gopls/go.mod": "module golang.org/x/tools\n",
936+
"gopls/go.sum": "\n",
937+
"file_a": "file_a",
938+
"file_b": "file_b",
939+
"file_c": "file_c",
940+
})
941+
if tc.branch != "master" {
942+
tools.Branch(tc.branch, initial)
943+
}
944+
945+
cloudBuild := NewFakeCloudBuild(t, NewFakeGerrit(t, tools), "", nil, fakeGo)
946+
got, err := executeAndMonitorChange(&workflow.TaskContext{Context: context.Background(), Logger: &testLogger{t, ""}}, cloudBuild, tc.branch, tc.script, tc.watch)
947+
if err != nil {
948+
t.Fatal(err)
949+
}
950+
951+
if !reflect.DeepEqual(got, tc.want) {
952+
t.Errorf("executeAndMonitorChange() = %v want = %v", got, tc.want)
953+
}
954+
})
955+
}
956+
}

0 commit comments

Comments
 (0)