internal/relui: check for stray commits
We want to make sure we're tagging what we release. Check that the
revision that we used to build the source archive (and then do
everything else) is still the branch head once the build is done, and
then that it's the parent commit of the version CL if any.
For golang/go#51797.
Fixes golang/go#39317.
Change-Id: I718b6d03ad0de542aed9c94a585e6b052a5d986d
Reviewed-on: https://go-review.googlesource.com/c/build/+/411197
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Auto-Submit: Heschi Kreinick <heschi@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
diff --git a/gerrit/gerrit.go b/gerrit/gerrit.go
index ed707a8..e6e31d0 100644
--- a/gerrit/gerrit.go
+++ b/gerrit/gerrit.go
@@ -781,6 +781,7 @@
ErrProjectNotExist = errors.New("gerrit: requested project does not exist")
ErrChangeNotExist = errors.New("gerrit: requested change does not exist")
ErrTagNotExist = errors.New("gerrit: requested tag does not exist")
+ ErrBranchNotExist = errors.New("gerrit: requested branch does not exist")
)
// GetProjectInfo returns info about a project.
@@ -817,6 +818,19 @@
return m, nil
}
+// GetBranch gets a particular branch in project. If the branch doesn't exist, the
+// error will be ErrBranchNotExist.
+//
+// See https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#get-branch.
+func (c *Client) GetBranch(ctx context.Context, project, branch string) (BranchInfo, error) {
+ var res BranchInfo
+ err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/branches/%s", project, branch))
+ if he, ok := err.(*HTTPError); ok && he.Res.StatusCode == 404 {
+ return BranchInfo{}, ErrBranchNotExist
+ }
+ return res, err
+}
+
// WebLinkInfo is information about a web link.
// See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#web-link-info
type WebLinkInfo struct {
diff --git a/internal/relui/buildrelease_test.go b/internal/relui/buildrelease_test.go
index 0c2034b..b47b706 100644
--- a/internal/relui/buildrelease_test.go
+++ b/internal/relui/buildrelease_test.go
@@ -375,7 +375,7 @@
return "fake~12345", nil
}
-func (g *fakeGerrit) AwaitSubmit(ctx context.Context, changeID string) (string, error) {
+func (g *fakeGerrit) AwaitSubmit(ctx context.Context, changeID, baseCommit string) (string, error) {
return "fakehash", nil
}
@@ -388,6 +388,10 @@
return nil
}
+func (g *fakeGerrit) ReadBranchHead(ctx context.Context, project, branch string) (string, error) {
+ return fmt.Sprintf("fake HEAD commit for %v/%v", project, branch), nil
+}
+
type fakeGitHub struct {
}
diff --git a/internal/relui/workflows.go b/internal/relui/workflows.go
index 1597d5b..29b6824 100644
--- a/internal/relui/workflows.go
+++ b/internal/relui/workflows.go
@@ -234,30 +234,33 @@
branch = "master"
}
branchVal := wd.Constant(branch)
- // TODO(heschi): read the real branch HEAD here, and check that it's the base commit for the version CL if there is one.
- tagCommit := wd.Constant(branch)
+ branchHead := wd.Task("Read branch HEAD", version.ReadBranchHead, branchVal)
// Select version, check milestones.
nextVersion := wd.Task("Get next version", version.GetNextVersion, kindVal)
milestones := wd.Task("Pick milestones", milestone.FetchMilestones, nextVersion, kindVal)
checked := wd.Action("Check blocking issues", milestone.CheckBlockers, milestones, nextVersion, kindVal)
dlcl := wd.Task("Mail DL CL", version.MailDLCL, wd.Slice([]workflow.Value{nextVersion}), wd.Constant(false))
- dlclCommit := wd.Task("Wait for DL CL", version.AwaitCL, dlcl)
+ dlclCommit := wd.Task("Wait for DL CL", version.AwaitCL, dlcl, wd.Constant(""))
wd.Output("Download CL submitted", dlclCommit)
// Build, test, and sign release.
- signedAndTestedArtifacts, err := build.addBuildTasks(wd, "go1.19", nextVersion, branchVal, skipTests, checked)
+ signedAndTestedArtifacts, err := build.addBuildTasks(wd, "go1.19", nextVersion, branchHead, skipTests, checked)
if err != nil {
return err
}
// Tag version and upload to CDN/website.
uploaded := wd.Action("Upload artifacts to CDN", build.uploadArtifacts, signedAndTestedArtifacts)
+
+ tagCommit := branchHead
if branch != "master" {
- versionCL := wd.Task("Mail version CL", version.CreateAutoSubmitVersionCL, branchVal, nextVersion, uploaded)
- tagCommit = wd.Task("Wait for version CL submission", version.AwaitCL, versionCL)
+ branchHeadChecked := wd.Action("Check for modified branch head", version.CheckBranchHead, branchVal, branchHead, uploaded)
+ versionCL := wd.Task("Mail version CL", version.CreateAutoSubmitVersionCL, branchVal, nextVersion, branchHeadChecked)
+ tagCommit = wd.Task("Wait for version CL submission", version.AwaitCL, versionCL, branchHead)
}
tagged := wd.Action("Tag version", version.TagRelease, nextVersion, tagCommit, uploaded)
+
pushed := wd.Action("Push issues", milestone.PushIssues, milestones, nextVersion, kindVal, tagged)
published := wd.Task("Publish to website", build.publishArtifacts, nextVersion, signedAndTestedArtifacts, pushed)
wd.Output("Publish results", published)
diff --git a/internal/task/gerrit.go b/internal/task/gerrit.go
index 8b9355a..b15a794 100644
--- a/internal/task/gerrit.go
+++ b/internal/task/gerrit.go
@@ -16,11 +16,14 @@
CreateAutoSubmitChange(ctx context.Context, input gerrit.ChangeInput, contents map[string]string) (string, error)
// AwaitSubmit waits for the specified change to be auto-submitted or fail
// trybots. If the CL is submitted, returns the submitted commit hash.
- AwaitSubmit(ctx context.Context, changeID string) (string, error)
+ // If parentCommit is non-empty, the submitted CL's parent must match it.
+ AwaitSubmit(ctx context.Context, changeID, parentCommit string) (string, error)
// Tag creates a tag on project at the specified commit.
Tag(ctx context.Context, project, tag, commit string) error
// ListTags returns all the tags on project.
ListTags(ctx context.Context, project string) ([]string, error)
+ // ReadBranchHead returns the head of a branch in project.
+ ReadBranchHead(ctx context.Context, project, branch string) (string, error)
}
type RealGerritClient struct {
@@ -59,15 +62,19 @@
return changeID, nil
}
-func (c *RealGerritClient) AwaitSubmit(ctx context.Context, changeID string) (string, error) {
+func (c *RealGerritClient) AwaitSubmit(ctx context.Context, changeID, parentCommit string) (string, error) {
for {
detail, err := c.Client.GetChangeDetail(ctx, changeID, gerrit.QueryChangesOpt{
- Fields: []string{"CURRENT_REVISION", "DETAILED_LABELS"},
+ Fields: []string{"CURRENT_REVISION", "DETAILED_LABELS", "CURRENT_COMMIT"},
})
if err != nil {
return "", err
}
if detail.Status == "MERGED" {
+ parents := detail.Revisions[detail.CurrentRevision].Commit.Parents
+ if parentCommit != "" && (len(parents) != 1 || parents[0].CommitID != parentCommit) {
+ return "", fmt.Errorf("expected merged CL %v to have one parent commit %v, has %v", ChangeLink(changeID), parentCommit, parents)
+ }
return detail.CurrentRevision, nil
}
for _, approver := range detail.Labels["TryBot-Result"].All {
@@ -116,6 +123,14 @@
return tagNames, nil
}
+func (c *RealGerritClient) ReadBranchHead(ctx context.Context, project, branch string) (string, error) {
+ branchInfo, err := c.Client.GetBranch(ctx, project, branch)
+ if err != nil {
+ return "", err
+ }
+ return branchInfo.Revision, nil
+}
+
// ChangeLink returns a link to the review page for the CL with the specified
// change ID. The change ID must be in the project~cl# form.
func ChangeLink(changeID string) string {
diff --git a/internal/task/version.go b/internal/task/version.go
index 8e7c5fe..1bc01b8 100644
--- a/internal/task/version.go
+++ b/internal/task/version.go
@@ -85,9 +85,25 @@
}
// AwaitCL waits for the specified CL to be submitted.
-func (t *VersionTasks) AwaitCL(ctx *workflow.TaskContext, changeID string) (string, error) {
+func (t *VersionTasks) AwaitCL(ctx *workflow.TaskContext, changeID, baseCommit string) (string, error) {
ctx.Printf("Awaiting review/submit of %v", ChangeLink(changeID))
- return t.Gerrit.AwaitSubmit(ctx, changeID)
+ return t.Gerrit.AwaitSubmit(ctx, changeID, baseCommit)
+}
+
+// ReadBranchHead returns the current HEAD revision of branch.
+func (t *VersionTasks) ReadBranchHead(ctx *workflow.TaskContext, branch string) (string, error) {
+ return t.Gerrit.ReadBranchHead(ctx, t.GoProject, branch)
+}
+
+func (t *VersionTasks) CheckBranchHead(ctx *workflow.TaskContext, branch, expectedCommit string) error {
+ head, err := t.ReadBranchHead(ctx, branch)
+ if err != nil {
+ return err
+ }
+ if head != expectedCommit {
+ return fmt.Errorf("head of branch %q is %q, wanted %q", branch, head, expectedCommit)
+ }
+ return nil
}
// TagRelease tags commit as version.
diff --git a/internal/task/version_test.go b/internal/task/version_test.go
index 5602520..c0b88be 100644
--- a/internal/task/version_test.go
+++ b/internal/task/version_test.go
@@ -102,7 +102,7 @@
if err != nil {
t.Fatal(err)
}
- _, err = tasks.AwaitCL(ctx, changeID)
+ _, err = tasks.AwaitCL(ctx, changeID, "")
if strings.Contains(err.Error(), "trybots failed") {
t.Logf("Trybots failed, as they usually do: %v. Abandoning CL and ending test.", err)
if err := cl.AbandonChange(ctx, changeID, "test is done"); err != nil {
@@ -119,7 +119,7 @@
if err != nil {
t.Fatalf("cleaning up VERSION: %v", err)
}
- if _, err := tasks.AwaitCL(ctx, changeID); err != nil {
+ if _, err := tasks.AwaitCL(ctx, changeID, ""); err != nil {
t.Fatalf("cleaning up VERSION: %v", err)
}
}
