From 1bb28a5ec978d81b28158332276c6536adbed661 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Mon, 22 Aug 2022 14:38:46 -0400 Subject: [PATCH] internal/task: finish initial implementation of x/ repo tagging This one got away from me a little, sorry. Add cycle detection and breaking: we need to break some cycles involving x/tools. We could consider fixing them by creating nested modules but I don't want to deal with that now. Implement UpdateGoMod: fetch the latest Go release, download it to a buildlet with network access, and run go get to update to the previously tagged versions. But only do this if there are dependencies, both to save time and because go get throws an error if you try to get nothing. Implement Tag: if there hasn't been a change since the last tag, reuse that one. Otherwise, tag it with the selected new verison. For golang/go#48523. Change-Id: I29c05eb639fd748af1e0e356596adc8c3f15ccf0 Reviewed-on: https://go-review.googlesource.com/c/build/+/425091 Run-TryBot: Heschi Kreinick Reviewed-by: Dmitri Shuralyov Auto-Submit: Heschi Kreinick Reviewed-by: Dmitri Shuralyov TryBot-Result: Gopher Robot --- cmd/relui/main.go | 6 + internal/task/buildrelease.go | 6 +- internal/task/gerrit.go | 6 + internal/task/tagx.go | 338 +++++++++++++++++++++++++++++----- internal/task/tagx_test.go | 83 +++++++++ 5 files changed, 395 insertions(+), 44 deletions(-) diff --git a/cmd/relui/main.go b/cmd/relui/main.go index 7335af32ea..c690d3186e 100644 --- a/cmd/relui/main.go +++ b/cmd/relui/main.go @@ -194,6 +194,12 @@ func main() { log.Fatalf("RegisterReleaseWorkflows: %v", err) } + tagTasks := &task.TagXReposTasks{ + Gerrit: gerritClient, + CreateBuildlet: coordinator.CreateBuildlet, + } + dh.RegisterDefinition("Tag x/ repos", tagTasks.NewDefinition()) + w := relui.NewWorker(dh, dbPool, relui.NewPGListener(dbPool)) go w.Run(ctx) if err := w.ResumeAll(ctx); err != nil { diff --git a/internal/task/buildrelease.go b/internal/task/buildrelease.go index f79a2164e1..0ac494ddff 100644 --- a/internal/task/buildrelease.go +++ b/internal/task/buildrelease.go @@ -310,14 +310,14 @@ func (b *BuildletStep) BuildMSI(ctx *workflow.TaskContext, binaryArchive io.Read }) return err } - return b.fetchFile(ctx, msi, "msi") + return fetchFile(ctx, b.Buildlet, msi, "msi") } // fetchFile fetches the specified directory from the given buildlet, and // writes the first file it finds in that directory to dest. -func (b *BuildletStep) fetchFile(ctx *workflow.TaskContext, dest io.Writer, dir string) error { +func fetchFile(ctx *workflow.TaskContext, client buildlet.RemoteClient, dest io.Writer, dir string) error { ctx.Printf("Downloading file from %q.", dir) - tgz, err := b.Buildlet.GetTar(context.Background(), dir) + tgz, err := client.GetTar(context.Background(), dir) if err != nil { return err } diff --git a/internal/task/gerrit.go b/internal/task/gerrit.go index 691b5cc416..47fde8076f 100644 --- a/internal/task/gerrit.go +++ b/internal/task/gerrit.go @@ -22,6 +22,8 @@ type GerritClient interface { // trybots. If the CL is submitted, returns the submitted commit hash. // If parentCommit is non-empty, the submitted CL's parent must match it. Submitted(ctx context.Context, changeID, parentCommit string) (string, bool, error) + + GetTag(ctx context.Context, project, tag string) (gerrit.TagInfo, 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. @@ -146,6 +148,10 @@ func (c *RealGerritClient) ListTags(ctx context.Context, project string) ([]stri return tagNames, nil } +func (c *RealGerritClient) GetTag(ctx context.Context, project, tag string) (gerrit.TagInfo, error) { + return c.Client.GetTag(ctx, project, tag) +} + func (c *RealGerritClient) ReadBranchHead(ctx context.Context, project, branch string) (string, error) { branchInfo, err := c.Client.GetBranch(ctx, project, branch) if err != nil { diff --git a/internal/task/tagx.go b/internal/task/tagx.go index 4965a063cf..59f745f1c5 100644 --- a/internal/task/tagx.go +++ b/internal/task/tagx.go @@ -1,18 +1,28 @@ package task import ( + "bytes" + "context" + "encoding/json" "errors" "fmt" + "net/http" + "reflect" + "regexp" + "strconv" "strings" + "golang.org/x/build/buildlet" "golang.org/x/build/gerrit" wf "golang.org/x/build/internal/workflow" "golang.org/x/mod/modfile" "golang.org/x/mod/semver" + "golang.org/x/net/context/ctxhttp" ) type TagXReposTasks struct { - Gerrit GerritClient + Gerrit GerritClient + CreateBuildlet func(context.Context, string) (buildlet.RemoteClient, error) } func (x *TagXReposTasks) NewDefinition() *wf.Definition { @@ -22,10 +32,12 @@ func (x *TagXReposTasks) NewDefinition() *wf.Definition { return wd } +// TagRepo contains information about a repo that can be tagged. type TagRepo struct { - Name string - Next string - Deps []string + Name string // Gerrit project name, e.g. "tools". + ModPath string // Module path, e.g. "golang.org/x/tools". + Deps []string // Dependency module paths. + Version string // After a tagging decision has been made, the version dependencies should upgrade to. } func (x *TagXReposTasks) SelectRepos(ctx *wf.TaskContext) ([]TagRepo, error) { @@ -45,9 +57,22 @@ func (x *TagXReposTasks) SelectRepos(ctx *wf.TaskContext) ([]TagRepo, error) { repos = append(repos, *repo) } } + + if cycles := checkCycles(repos); len(cycles) != 0 { + return nil, fmt.Errorf("cycles detected (there may be more): %v", cycles) + } + return repos, nil } +var breakCycles = map[string]bool{ + // golang.org/x/text/message/pipeline is "UNDER DEVELOPMENT"; ignore its deps. + "golang.org/x/text->golang.org/x/tools": true, + "golang.org/x/text->golang.org/x/mod": true, + // test-only dependency, the reverse is much more real + "golang.org/x/mod->golang.org/x/tools": true, +} + func (x *TagXReposTasks) readRepo(ctx *wf.TaskContext, project string) (*TagRepo, error) { head, err := x.Gerrit.ReadBranchHead(ctx, project, "master") if errors.Is(err, gerrit.ErrResourceNotExist) { @@ -70,68 +95,131 @@ func (x *TagXReposTasks) readRepo(ctx *wf.TaskContext, project string) (*TagRepo if err != nil { return nil, err } - if !strings.HasPrefix(mf.Module.Mod.Path, "golang.org/x") { + + // TODO(heschi): ignoring nested modules for now. We should find and handle + // x/exp/event, maybe by reading release tags? But don't tag gopls... + isX := func(path string) bool { + return strings.HasPrefix(path, "golang.org/x/") && + !strings.Contains(strings.TrimPrefix(path, "golang.org/x/"), "/") + } + if !isX(mf.Module.Mod.Path) { ctx.Printf("ignoring %v: not golang.org/x", project) return nil, nil } - tags, err := x.Gerrit.ListTags(ctx, project) - if err != nil { - return nil, err + result := &TagRepo{ + Name: project, + ModPath: mf.Module.Mod.Path, } - highestRelease := "" - for _, tag := range tags { - if semver.IsValid(tag) && semver.Prerelease(tag) == "" && - (highestRelease == "" || semver.Compare(highestRelease, tag) < 0) { - highestRelease = tag + for _, req := range mf.Require { + if !isX(req.Mod.Path) { + continue } - } - nextTag := "v0.1.0" - if highestRelease != "" { - var err error - nextTag, err = nextVersion(highestRelease) - if err != nil { - return nil, fmt.Errorf("couldn't pick next version for %v: %v", project, err) + if breakCycles[result.ModPath+"->"+req.Mod.Path] { + continue } + result.Deps = append(result.Deps, req.Mod.Path) + } + return result, nil +} - result := &TagRepo{ - Name: project, - Next: nextTag, +// checkCycles returns all the shortest dependency cycles in repos. +func checkCycles(repos []TagRepo) [][]string { + reposByModule := map[string]TagRepo{} + for _, repo := range repos { + reposByModule[repo.ModPath] = repo } - for _, req := range mf.Require { - if strings.HasPrefix(req.Mod.Path, "golang.org/x") { - result.Deps = append(result.Deps, req.Mod.Path) + + var cycles [][]string + + for _, repo := range reposByModule { + cycles = append(cycles, checkCycles1(reposByModule, repo, nil)...) + } + + var shortestCycles [][]string + for _, cycle := range cycles { + switch { + case len(shortestCycles) == 0 || len(shortestCycles[0]) > len(cycle): + shortestCycles = [][]string{cycle} + case len(shortestCycles[0]) == len(cycle): + found := false + for _, existing := range shortestCycles { + if reflect.DeepEqual(existing, cycle) { + found = true + break + } + } + if !found { + shortestCycles = append(shortestCycles, cycle) + } } } - return result, nil + return shortestCycles } +func checkCycles1(reposByModule map[string]TagRepo, repo TagRepo, stack []string) [][]string { + var cycles [][]string + stack = append(stack, repo.ModPath) + for i, s := range stack[:len(stack)-1] { + if s == repo.ModPath { + cycles = append(cycles, append([]string(nil), stack[i:]...)) + } + } + if len(cycles) != 0 { + return cycles + } + + for _, dep := range repo.Deps { + cycles = append(cycles, checkCycles1(reposByModule, reposByModule[dep], stack)...) + } + return cycles +} + +// BuildPlan adds the tasks needed to update repos to wd. func (x *TagXReposTasks) BuildPlan(wd *wf.Definition, repos []TagRepo) error { - updated := map[string]wf.Dependency{} + // repo.ModPath to the wf.Value produced by updating it. + updated := map[string]wf.Value[TagRepo]{} // Find all repositories whose dependencies are satisfied and update // them, proceeding until all are updated or no progress can be made. for len(updated) != len(repos) { progress := false for _, repo := range repos { + if _, ok := updated[repo.ModPath]; ok { + continue + } dep, ok := x.planRepo(wd, repo, updated) if !ok { continue } - updated[repo.Name] = dep + updated[repo.ModPath] = dep progress = true } if !progress { - return fmt.Errorf("cycle detected, sorry") + var missing []string + for _, r := range repos { + if updated[r.ModPath] == nil { + missing = append(missing, r.Name) + } + } + return fmt.Errorf("failed to progress the plan: todo: %v", missing) } } + var allDeps []wf.Dependency + for _, dep := range updated { + allDeps = append(allDeps, dep) + } + done := wf.Task0(wd, "done", func(_ context.Context) (string, error) { return "done!", nil }, wf.After(allDeps...)) + wf.Output(wd, "done", done) return nil } -func (x *TagXReposTasks) planRepo(wd *wf.Definition, repo TagRepo, updated map[string]wf.Dependency) (wf.Dependency, bool) { - var deps []wf.Dependency +// planRepo returns a Value containing the tagged repository's information, or +// nil, false if its dependencies haven't been planned yet. +func (x *TagXReposTasks) planRepo(wd *wf.Definition, repo TagRepo, updated map[string]wf.Value[TagRepo]) (_ wf.Value[TagRepo], ready bool) { + var deps []wf.Value[TagRepo] for _, repoDeps := range repo.Deps { if dep, ok := updated[repoDeps]; ok { deps = append(deps, dep) @@ -140,21 +228,189 @@ func (x *TagXReposTasks) planRepo(wd *wf.Definition, repo TagRepo, updated map[s } } wd = wd.Sub(repo.Name) - commit := wf.Task1(wd, "update go.mod", x.UpdateGoMod, wf.Const(repo), wf.After(deps...)) - green := wf.Action2(wd, "wait for green post-submit", x.AwaitGreen, wf.Const(repo), commit) - tagged := wf.Task2(wd, "tag", x.TagRepo, wf.Const(repo), commit, wf.After(green)) - wf.Output(wd, "tagged version", tagged) + repoName, branch := wf.Const(repo.Name), wf.Const("master") + + head := wf.Task2(wd, "read branch head", x.Gerrit.ReadBranchHead, repoName, branch) + tagCommit := head + if len(deps) != 0 { + gomod := wf.Task3(wd, "generate updated go.mod", x.UpdateGoMod, wf.Const(repo), wf.Slice(deps...), head) + cl := wf.Task2(wd, "mail updated go.mod", x.MailGoMod, repoName, gomod) + versionTasks := &VersionTasks{Gerrit: x.Gerrit} + tagCommit = wf.Task2(wd, "wait for submit", versionTasks.AwaitCL, cl, wf.Const("")) + } + green := wf.Action2(wd, "wait for green post-submit", x.AwaitGreen, repoName, tagCommit) + tagged := wf.Task2(wd, "tag if appropriate", x.MaybeTag, wf.Const(repo), tagCommit, wf.After(green)) return tagged, true } -func (x *TagXReposTasks) UpdateGoMod(ctx *wf.TaskContext, repo TagRepo) (string, error) { - return "", nil +type UpdatedModSum struct { + Mod, Sum string +} + +func (x *TagXReposTasks) UpdateGoMod(ctx *wf.TaskContext, repo TagRepo, deps []TagRepo, commit string) (UpdatedModSum, error) { + binaries, err := latestGoBinaries(ctx) + if err != nil { + return UpdatedModSum{}, err + } + bc, err := x.CreateBuildlet(ctx, "linux-amd64-longtest") // longtest to allow network access. A little yucky. + if err != nil { + return UpdatedModSum{}, err + } + defer bc.Close() + + if err := bc.PutTarFromURL(ctx, binaries, ""); err != nil { + return UpdatedModSum{}, err + } + archive := fmt.Sprintf("https://go.googlesource.com/%v/+archive/%v.tar.gz", repo.Name, commit) + if err := bc.PutTarFromURL(ctx, archive, "repo"); err != nil { + return UpdatedModSum{}, err + } + + writer := &LogWriter{Logger: ctx} + go writer.Run(ctx) + + args := []string{"get"} + for _, dep := range deps { + args = append(args, dep.ModPath+"@"+dep.Version) + } + remoteErr, execErr := bc.Exec(ctx, "go/bin/go", buildlet.ExecOpts{ + Dir: "repo", + Args: args, + Output: writer, + }) + if execErr != nil { + return UpdatedModSum{}, execErr + } + if remoteErr != nil { + return UpdatedModSum{}, fmt.Errorf("Command failed: %v", remoteErr) + } + + remoteErr, execErr = bc.Exec(ctx, "go/bin/go", buildlet.ExecOpts{ + Dir: "repo", + Args: []string{"mod", "tidy"}, + Output: writer, + }) + if execErr != nil { + return UpdatedModSum{}, execErr + } + if remoteErr != nil { + return UpdatedModSum{}, fmt.Errorf("Command failed: %v", remoteErr) + } + + remoteErr, execErr = bc.Exec(ctx, "bash", buildlet.ExecOpts{ + Dir: ".", + Args: []string{"-c", "mkdir fetchgomod && cp repo/go.mod fetchgomod && touch repo/go.sum && mkdir fetchgosum && cp repo/go.sum fetchgosum"}, + Output: writer, + SystemLevel: true, + }) + if execErr != nil { + return UpdatedModSum{}, execErr + } + if remoteErr != nil { + return UpdatedModSum{}, fmt.Errorf("Command failed: %v", remoteErr) + } + + mod := &bytes.Buffer{} + if err := fetchFile(ctx, bc, mod, "fetchgomod"); err != nil { + return UpdatedModSum{}, err + } + sum := &bytes.Buffer{} + if err := fetchFile(ctx, bc, sum, "fetchgosum"); err != nil { + return UpdatedModSum{}, err + } + + return UpdatedModSum{mod.String(), sum.String()}, nil +} + +func latestGoBinaries(ctx *wf.TaskContext) (string, error) { + resp, err := ctxhttp.Get(ctx, http.DefaultClient, "https://go.dev/dl/?mode=json") + if err != nil { + return "", err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("unexpected status %v", resp.Status) + } + + releases := []*WebsiteRelease{} + if err := json.NewDecoder(resp.Body).Decode(&releases); err != nil { + return "", err + } + for _, r := range releases { + for _, f := range r.Files { + if f.Arch == "amd64" && f.OS == "linux" && f.Kind == "archive" { + return "https://go.dev/dl/" + f.Filename, nil + } + } + } + return "", fmt.Errorf("no linux-amd64??") +} + +func (x *TagXReposTasks) MailGoMod(ctx *wf.TaskContext, repo string, gomod UpdatedModSum) (string, error) { + if dryRun { + return "", fmt.Errorf("nope") + } + return x.Gerrit.CreateAutoSubmitChange(ctx, gerrit.ChangeInput{ + Project: repo, + Branch: "master", + Subject: "update x/ dependencies", + }, nil, map[string]string{ + "go.mod": gomod.Mod, + "go.sum": gomod.Sum, + }) } -func (x *TagXReposTasks) AwaitGreen(ctx *wf.TaskContext, repo TagRepo, commit string) error { +func (x *TagXReposTasks) AwaitGreen(ctx *wf.TaskContext, repo, commit string) error { return nil } -func (x *TagXReposTasks) TagRepo(ctx *wf.TaskContext, repo TagRepo, commit string) (string, error) { - return repo.Next, nil +const dryRun = true + +// MaybeTag tags repo at commit with the next version, unless commit is already +// the latest tagged version. repo is returned with Version populated. +func (x *TagXReposTasks) MaybeTag(ctx *wf.TaskContext, repo TagRepo, commit string) (TagRepo, error) { + tags, err := x.Gerrit.ListTags(ctx, repo.Name) + if err != nil { + return TagRepo{}, err + } + highestRelease := "" + for _, tag := range tags { + if semver.IsValid(tag) && semver.Prerelease(tag) == "" && + (highestRelease == "" || semver.Compare(highestRelease, tag) < 0) { + highestRelease = tag + } + } + if highestRelease == "" || dryRun { + repo.Version = "latest" + return repo, nil + } + + tagInfo, err := x.Gerrit.GetTag(ctx, repo.Name, highestRelease) + if err != nil { + return TagRepo{}, fmt.Errorf("reading project %v tag %v: %v", repo.Name, highestRelease, err) + } + if tagInfo.Revision == commit { + repo.Version = highestRelease + return repo, nil + } + + repo.Version, err = nextMinor(highestRelease) + if err != nil { + return TagRepo{}, fmt.Errorf("couldn't pick next version for %v: %v", repo.Name, err) + } + return repo, x.Gerrit.Tag(ctx, repo.Name, repo.Version, commit) +} + +var majorMinorRestRe = regexp.MustCompile(`^v(\d+)\.(\d+)\..*$`) + +func nextMinor(version string) (string, error) { + parts := majorMinorRestRe.FindStringSubmatch(version) + if parts == nil { + return "", fmt.Errorf("malformatted version %q", version) + } + minor, err := strconv.Atoi(parts[2]) + if err != nil { + return "", fmt.Errorf("malformatted version %q (%v)", version, err) + } + return fmt.Sprintf("v%s.%d.0", parts[1], minor+1), nil } diff --git a/internal/task/tagx_test.go b/internal/task/tagx_test.go index 28185fb1fd..4a3e5aa452 100644 --- a/internal/task/tagx_test.go +++ b/internal/task/tagx_test.go @@ -3,8 +3,10 @@ package task import ( "context" "flag" + "strings" "testing" + "github.com/google/go-cmp/cmp" "golang.org/x/build/gerrit" "golang.org/x/build/internal/workflow" ) @@ -33,3 +35,84 @@ func TestSelectReposLive(t *testing.T) { t.Logf("%#v", r) } } + +func TestCycles(t *testing.T) { + tests := []struct { + repos []TagRepo + want []string + }{ + { + repos: []TagRepo{ + {Name: "text", Deps: []string{"tools"}}, + {Name: "tools", Deps: []string{"text"}}, + {Name: "sys"}, + {Name: "net", Deps: []string{"sys"}}, + }, + want: []string{ + "tools,text,tools", + "text,tools,text", + }, + }, + { + repos: []TagRepo{ + {Name: "text", Deps: []string{"tools"}}, + {Name: "tools", Deps: []string{"fake"}}, + {Name: "fake", Deps: []string{"text"}}, + }, + want: []string{ + "tools,fake,text,tools", + "text,tools,fake,text", + "fake,text,tools,fake", + }, + }, + { + repos: []TagRepo{ + {Name: "text", Deps: []string{"tools"}}, + {Name: "tools", Deps: []string{"fake", "text"}}, + {Name: "fake", Deps: []string{"tools"}}, + }, + want: []string{ + "tools,text,tools", + "text,tools,text", + "tools,fake,tools", + "fake,tools,fake", + }, + }, + { + repos: []TagRepo{ + {Name: "text", Deps: []string{"tools"}}, + {Name: "tools", Deps: []string{"fake", "text"}}, + {Name: "fake1", Deps: []string{"fake2"}}, + {Name: "fake2", Deps: []string{"tools"}}, + }, + want: []string{ + "tools,text,tools", + "text,tools,text", + }, + }, + } + + for _, tt := range tests { + var repos []TagRepo + for _, r := range tt.repos { + repos = append(repos, TagRepo{ + Name: r.Name, + ModPath: r.Name, + Deps: r.Deps, + }) + } + cycles := checkCycles(repos) + got := map[string]bool{} + for _, cycle := range cycles { + got[strings.Join(cycle, ",")] = true + } + want := map[string]bool{} + for _, cycle := range tt.want { + want[cycle] = true + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("%v result unexpected: %v", tt.repos, diff) + } + } +}