Skip to content

Commit 774ccf3

Browse files
committed
internal/task, gerrit: create a new branch for minor release
1. Add new method CreateBranch to interface GerritClient. a. Implement CreateBranch real gerrit client follow gerrit documentation. b. Implement CreateBranch using git branch for fake client. 2. Change fake gerrit client implementation of ReadBranchHead to return empty string if encounter error to align return value with the real client. 3. Change return type of isValidVersion to semantic version to avoid future code duplication. 4. Add a new action CreateBranchIfMinor to gopls release after converting the input version string to semantic version struct. A local relui screenshot is at https://go.dev/issue/57643#issuecomment-2258617146. For golang/go#57643 Change-Id: I36eb88625ed6036eb322c39b345aee3f1ffa3f5c Reviewed-on: https://go-review.googlesource.com/c/build/+/601655 Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 49c121e commit 774ccf3

File tree

5 files changed

+191
-23
lines changed

5 files changed

+191
-23
lines changed

gerrit/gerrit.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,14 @@ type BranchInfo struct {
844844
CanDelete bool `json:"can_delete"`
845845
}
846846

847+
// The BranchInput entity contains information for the creation of a new branch.
848+
// See https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#branch-input
849+
type BranchInput struct {
850+
Ref string `json:"ref,omitempty"`
851+
Revision string `json:"revision,omitempty"`
852+
// ValidationOptions is optional.
853+
}
854+
847855
// GetProjectBranches returns the branches for the project name. The branches are stored in a map
848856
// keyed by reference.
849857
func (c *Client) GetProjectBranches(ctx context.Context, name string) (map[string]BranchInfo, error) {
@@ -868,6 +876,15 @@ func (c *Client) GetBranch(ctx context.Context, project, branch string) (BranchI
868876
return res, err
869877
}
870878

879+
// CreateBranch create a new branch in the project.
880+
//
881+
// See https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#create-branch
882+
func (c *Client) CreateBranch(ctx context.Context, project, branch string, input BranchInput) (BranchInfo, error) {
883+
var res BranchInfo
884+
err := c.do(ctx, &res, "PUT", fmt.Sprintf("/projects/%s/branches/%s", url.PathEscape(project), url.PathEscape(branch)), reqBodyJSON{&input}, wantResStatus(http.StatusCreated))
885+
return res, err
886+
}
887+
871888
// GetFileContent gets a file's contents at a particular commit.
872889
//
873890
// See https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#get-content-from-commit.

internal/task/fakes.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,24 @@ func (g *FakeGerrit) ReadBranchHead(ctx context.Context, project, branch string)
220220
}
221221
// TODO: If the branch doesn't exist, return an error matching gerrit.ErrResourceNotExist.
222222
out, err := repo.dir.RunCommand(ctx, "rev-parse", "refs/heads/"+branch)
223-
return strings.TrimSpace(string(out)), err
223+
if err != nil {
224+
// Returns empty string if the error is nil to align the same behavior with
225+
// the real Gerrit client.
226+
return "", err
227+
}
228+
return strings.TrimSpace(string(out)), nil
229+
}
230+
231+
func (g *FakeGerrit) CreateBranch(ctx context.Context, project, branch string, input gerrit.BranchInput) (string, error) {
232+
repo, err := g.repo(project)
233+
if err != nil {
234+
return "", err
235+
}
236+
if _, err = repo.dir.RunCommand(ctx, "branch", branch, input.Revision); err != nil {
237+
return "", err
238+
}
239+
240+
return g.ReadBranchHead(ctx, project, branch)
224241
}
225242

226243
func (g *FakeGerrit) ReadFile(ctx context.Context, project string, commit string, file string) ([]byte, error) {

internal/task/gerrit.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ type GerritClient interface {
3838
// ReadBranchHead returns the head of a branch in project.
3939
// If the branch doesn't exist, it returns an error matching gerrit.ErrResourceNotExist.
4040
ReadBranchHead(ctx context.Context, project, branch string) (string, error)
41+
// CreateBranch create the given branch and returns the created branch's revision.
42+
CreateBranch(ctx context.Context, project, branch string, input gerrit.BranchInput) (string, error)
4143
// ListProjects lists all the projects on the server.
4244
ListProjects(ctx context.Context) ([]string, error)
4345
// ReadFile reads a file from project at the specified commit.
@@ -194,6 +196,14 @@ func (c *RealGerritClient) ReadBranchHead(ctx context.Context, project, branch s
194196
return branchInfo.Revision, nil
195197
}
196198

199+
func (c *RealGerritClient) CreateBranch(ctx context.Context, project, branch string, input gerrit.BranchInput) (string, error) {
200+
branchInfo, err := c.Client.CreateBranch(ctx, project, branch, input)
201+
if err != nil {
202+
return "", err
203+
}
204+
return branchInfo.Revision, nil
205+
}
206+
197207
func (c *RealGerritClient) ListProjects(ctx context.Context) ([]string, error) {
198208
projects, err := c.Client.ListProjects(ctx)
199209
if err != nil {

internal/task/releasegopls.go

Lines changed: 57 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"slices"
1010
"strings"
1111

12+
"golang.org/x/build/gerrit"
1213
wf "golang.org/x/build/internal/workflow"
1314
"golang.org/x/mod/semver"
1415
)
@@ -26,36 +27,71 @@ func (r *ReleaseGoplsTasks) NewDefinition() *wf.Definition {
2627
// TODO(hxjiang): provide potential release versions in the relui where the
2728
// coordinator can choose which version to release instead of manual input.
2829
version := wf.Param(wd, wf.ParamDef[string]{Name: "version"})
29-
isValid := wf.Task1(wd, "validating input version", r.isValidVersion, version)
30-
wf.Output(wd, "valid", isValid)
30+
semversion := wf.Task1(wd, "validating input version", r.isValidVersion, version)
31+
_ = wf.Action1(wd, "creating new branch if minor release", r.createBranchIfMinor, semversion)
3132
return wd
3233
}
3334

34-
func (r *ReleaseGoplsTasks) isValidVersion(ctx *wf.TaskContext, ver string) (bool, error) {
35+
// createBranchIfMinor create the release branch if the input version is a minor
36+
// release.
37+
// All patch releases under the same minor version share the same release branch.
38+
func (r *ReleaseGoplsTasks) createBranchIfMinor(ctx *wf.TaskContext, semv semversion) error {
39+
branch := fmt.Sprintf("gopls-release-branch.%v.%v", semv.Major, semv.Minor)
40+
41+
// Require gopls release branch existence if this is a non-minor release.
42+
if semv.Patch != 0 {
43+
_, err := r.Gerrit.ReadBranchHead(ctx, "tools", branch)
44+
return err
45+
}
46+
47+
// Return early if the branch already exist.
48+
// This scenario should only occur if the initial minor release flow failed
49+
// or was interrupted and subsequently re-triggered.
50+
if _, err := r.Gerrit.ReadBranchHead(ctx, "tools", branch); err == nil {
51+
return nil
52+
}
53+
54+
// Create the release branch using the revision from the head of master branch.
55+
head, err := r.Gerrit.ReadBranchHead(ctx, "tools", "master")
56+
if err != nil {
57+
return err
58+
}
59+
60+
ctx.Printf("Creating branch %s at revision %s.\n", branch, head)
61+
_, err = r.Gerrit.CreateBranch(ctx, "tools", branch, gerrit.BranchInput{Revision: head})
62+
return err
63+
}
64+
65+
func (r *ReleaseGoplsTasks) isValidVersion(ctx *wf.TaskContext, ver string) (semversion, error) {
3566
if !semver.IsValid(ver) {
36-
return false, nil
67+
return semversion{}, fmt.Errorf("the input %q version does not follow semantic version schema", ver)
3768
}
3869

3970
versions, err := r.possibleGoplsVersions(ctx)
4071
if err != nil {
41-
return false, fmt.Errorf("failed to get latest Gopls version tags from x/tool: %w", err)
72+
return semversion{}, fmt.Errorf("failed to get latest Gopls version tags from x/tool: %w", err)
73+
}
74+
75+
if !slices.Contains(versions, ver) {
76+
return semversion{}, fmt.Errorf("the input %q is not next version of any existing versions", ver)
4277
}
4378

44-
return slices.Contains(versions, ver), nil
79+
semver, _ := parseSemver(ver)
80+
return semver, nil
4581
}
4682

4783
// semversion is a parsed semantic version.
4884
type semversion struct {
49-
major, minor, patch int
50-
pre string
85+
Major, Minor, Patch int
86+
Pre string
5187
}
5288

5389
// parseSemver attempts to parse semver components out of the provided semver
5490
// v. If v is not valid semver in canonical form, parseSemver returns false.
5591
func parseSemver(v string) (_ semversion, ok bool) {
5692
var parsed semversion
57-
v, parsed.pre, _ = strings.Cut(v, "-")
58-
if _, err := fmt.Sscanf(v, "v%d.%d.%d", &parsed.major, &parsed.minor, &parsed.patch); err == nil {
93+
v, parsed.Pre, _ = strings.Cut(v, "-")
94+
if _, err := fmt.Sscanf(v, "v%d.%d.%d", &parsed.Major, &parsed.Minor, &parsed.Patch); err == nil {
5995
ok = true
6096
}
6197
return parsed, ok
@@ -89,32 +125,32 @@ func (r *ReleaseGoplsTasks) possibleGoplsVersions(ctx *wf.TaskContext) ([]string
89125
semv, ok := parseSemver(v)
90126
semVersions = append(semVersions, semv)
91127

92-
if majorMinorPatch[semv.major] == nil {
93-
majorMinorPatch[semv.major] = map[int]map[int]bool{}
128+
if majorMinorPatch[semv.Major] == nil {
129+
majorMinorPatch[semv.Major] = map[int]map[int]bool{}
94130
}
95-
if majorMinorPatch[semv.major][semv.minor] == nil {
96-
majorMinorPatch[semv.major][semv.minor] = map[int]bool{}
131+
if majorMinorPatch[semv.Major][semv.Minor] == nil {
132+
majorMinorPatch[semv.Major][semv.Minor] = map[int]bool{}
97133
}
98-
majorMinorPatch[semv.major][semv.minor][semv.patch] = true
134+
majorMinorPatch[semv.Major][semv.Minor][semv.Patch] = true
99135
}
100136

101137
var possible []string
102138
seen := map[string]bool{}
103139
for _, v := range semVersions {
104-
nextMajor := fmt.Sprintf("v%d.%d.%d", v.major+1, 0, 0)
105-
if _, ok := majorMinorPatch[v.major+1]; !ok && !seen[nextMajor] {
140+
nextMajor := fmt.Sprintf("v%d.%d.%d", v.Major+1, 0, 0)
141+
if _, ok := majorMinorPatch[v.Major+1]; !ok && !seen[nextMajor] {
106142
seen[nextMajor] = true
107143
possible = append(possible, nextMajor)
108144
}
109145

110-
nextMinor := fmt.Sprintf("v%d.%d.%d", v.major, v.minor+1, 0)
111-
if _, ok := majorMinorPatch[v.major][v.minor+1]; !ok && !seen[nextMinor] {
146+
nextMinor := fmt.Sprintf("v%d.%d.%d", v.Major, v.Minor+1, 0)
147+
if _, ok := majorMinorPatch[v.Major][v.Minor+1]; !ok && !seen[nextMinor] {
112148
seen[nextMinor] = true
113149
possible = append(possible, nextMinor)
114150
}
115151

116-
nextPatch := fmt.Sprintf("v%d.%d.%d", v.major, v.minor, v.patch+1)
117-
if _, ok := majorMinorPatch[v.major][v.minor][v.patch+1]; !ok && !seen[nextPatch] {
152+
nextPatch := fmt.Sprintf("v%d.%d.%d", v.Major, v.Minor, v.Patch+1)
153+
if _, ok := majorMinorPatch[v.Major][v.Minor][v.Patch+1]; !ok && !seen[nextPatch] {
118154
seen[nextPatch] = true
119155
possible = append(possible, nextPatch)
120156
}

internal/task/releasegopls_test.go

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"testing"
1010

1111
"github.com/google/go-cmp/cmp"
12+
"golang.org/x/build/gerrit"
1213
"golang.org/x/build/internal/workflow"
1314
)
1415

@@ -73,7 +74,7 @@ func TestPossibleGoplsVersions(t *testing.T) {
7374
Gerrit: gerrit,
7475
}
7576

76-
got, err := tasks.possibleGoplsVersions(&workflow.TaskContext{Context: context.Background()})
77+
got, err := tasks.possibleGoplsVersions(&workflow.TaskContext{Context: context.Background(), Logger: &testLogger{t, ""}})
7778
if err != nil {
7879
t.Fatalf("possibleGoplsVersions() should not return error, but return %v", err)
7980
}
@@ -83,3 +84,90 @@ func TestPossibleGoplsVersions(t *testing.T) {
8384
})
8485
}
8586
}
87+
88+
func TestCreateBranchIfMinor(t *testing.T) {
89+
ctx := context.Background()
90+
tests := []struct {
91+
name string
92+
version string
93+
existingBranch string
94+
wantErr bool
95+
wantBranch string
96+
}{
97+
{
98+
name: "should create a release branch for a minor release",
99+
version: "v1.2.0",
100+
wantErr: false,
101+
wantBranch: "gopls-release-branch.1.2",
102+
},
103+
{
104+
name: "should return nil if the release branch already exist for a minor release",
105+
version: "v1.2.0",
106+
existingBranch: "gopls-release-branch.1.2",
107+
wantErr: false,
108+
},
109+
{
110+
name: "should not create a release branch for a patch release",
111+
version: "v1.2.4",
112+
existingBranch: "gopls-release-branch.1.2",
113+
wantErr: false,
114+
wantBranch: "",
115+
},
116+
{
117+
name: "should throw error for patch release if release branch is missing",
118+
version: "v1.3.1",
119+
wantErr: true,
120+
wantBranch: "",
121+
},
122+
}
123+
124+
for _, tc := range tests {
125+
t.Run(tc.name, func(t *testing.T) {
126+
tools := NewFakeRepo(t, "tools")
127+
_ = tools.Commit(map[string]string{
128+
"go.mod": "module golang.org/x/tools\n",
129+
"go.sum": "\n",
130+
})
131+
_ = tools.Commit(map[string]string{
132+
"README.md": "THIS IS READ ME.",
133+
})
134+
135+
gerritClient := NewFakeGerrit(t, tools)
136+
137+
masterHead, err := gerritClient.ReadBranchHead(ctx, "tools", "master")
138+
if err != nil {
139+
t.Fatalf("ReadBranchHead should be able to get revision of master branch's head: %v", err)
140+
}
141+
142+
if tc.existingBranch != "" {
143+
if _, err := gerritClient.CreateBranch(ctx, "tools", tc.existingBranch, gerrit.BranchInput{Revision: masterHead}); err != nil {
144+
t.Fatalf("failed to create the branch %q: %v", tc.existingBranch, err)
145+
}
146+
}
147+
148+
tasks := &ReleaseGoplsTasks{
149+
Gerrit: gerritClient,
150+
}
151+
152+
semv, _ := parseSemver(tc.version)
153+
err = tasks.createBranchIfMinor(&workflow.TaskContext{Context: ctx, Logger: &testLogger{t, ""}}, semv)
154+
155+
if tc.wantErr && err == nil {
156+
t.Errorf("createBranchIfMinor() should return error but return nil")
157+
} else if !tc.wantErr && err != nil {
158+
t.Errorf("createBranchIfMinor() should return nil but return err: %v", err)
159+
}
160+
161+
// Created branch should have same revision as master branch's HEAD.
162+
if tc.wantBranch != "" {
163+
gotRevision, err := gerritClient.ReadBranchHead(ctx, "tools", tc.wantBranch)
164+
if err != nil {
165+
t.Errorf("ReadBranchHead should be able to get revision of %s branch's head: %v", tc.wantBranch, err)
166+
}
167+
if masterHead != gotRevision {
168+
t.Errorf("createBranchIfMinor() = %q, want %q", gotRevision, masterHead)
169+
}
170+
}
171+
})
172+
}
173+
}

0 commit comments

Comments
 (0)