cmd/coordinator, internal/buildgo: create buildgo package

This package contains the BuilderRev type moved from cmd/coordinator.

The rest of the CL is simply updating coordinator with the new
exported names of the type and its fields.

This refactoring is in preparation for moving the benchmark building and running
code into a separate package.

(Most of the diff could have been avoided with a type alias, but I
assume we'd rather not do that.)

Updates golang/go#19871

Change-Id: Ib6ce49431c8529d6b4e72725d3cd652b9d0160db
Reviewed-on: https://go-review.googlesource.com/44175
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/cmd/coordinator/benchmarks.go b/cmd/coordinator/benchmarks.go
index 5b800ce..eae63ba 100644
--- a/cmd/coordinator/benchmarks.go
+++ b/cmd/coordinator/benchmarks.go
@@ -6,6 +6,7 @@
 
 import (
 	"bytes"
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -16,6 +17,7 @@
 	"golang.org/x/build/buildlet"
 	"golang.org/x/build/cmd/coordinator/spanlog"
 	"golang.org/x/build/dashboard"
+	"golang.org/x/build/internal/buildgo"
 )
 
 // benchRuns is the number of times to run each benchmark binary
@@ -224,8 +226,8 @@
 }
 
 // parentRev returns the parent of this build's commit (but only if this build comes from a trySet).
-func (st *buildStatus) parentRev() (pbr builderRev, err error) {
-	pbr = st.builderRev // copy
+func (st *buildStatus) parentRev() (pbr buildgo.BuilderRev, err error) {
+	pbr = st.BuilderRev // copy
 	rev := st.trySet.ci.Revisions[st.trySet.ci.CurrentRevision]
 	if rev.Commit == nil {
 		err = fmt.Errorf("commit information missing for revision %q", st.trySet.ci.CurrentRevision)
@@ -236,25 +238,25 @@
 		err = errors.New("commit has no parent")
 		return
 	}
-	pbr.rev = rev.Commit.Parents[0].CommitID
+	pbr.Rev = rev.Commit.Parents[0].CommitID
 	return
 }
 
-func (st *buildStatus) buildRev(sl spanlog.Logger, conf dashboard.BuildConfig, bc *buildlet.Client, w io.Writer, goroot string, br builderRev) error {
-	if br.snapshotExists() {
-		return bc.PutTarFromURL(br.snapshotURL(), "go-parent")
+func (st *buildStatus) buildRev(sl spanlog.Logger, conf dashboard.BuildConfig, bc *buildlet.Client, w io.Writer, goroot string, br buildgo.BuilderRev) error {
+	if br.SnapshotExists(context.TODO(), buildEnv) {
+		return bc.PutTarFromURL(br.SnapshotURL(buildEnv), goroot)
 	}
-	if err := bc.PutTar(versionTgz(br.rev), "go-parent"); err != nil {
+	if err := bc.PutTar(versionTgz(br.Rev), goroot); err != nil {
 		return err
 	}
-	srcTar, err := getSourceTgz(sl, "go", br.rev)
+	srcTar, err := getSourceTgz(sl, "go", br.Rev)
 	if err != nil {
 		return err
 	}
-	if err := bc.PutTar(srcTar, "go-parent"); err != nil {
+	if err := bc.PutTar(srcTar, goroot); err != nil {
 		return err
 	}
-	remoteErr, err := st.runMake(bc, "go-parent", w)
+	remoteErr, err := st.runMake(bc, goroot, w)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/coordinator/coordinator.go b/cmd/coordinator/coordinator.go
index 7a91651..a8c22c7 100644
--- a/cmd/coordinator/coordinator.go
+++ b/cmd/coordinator/coordinator.go
@@ -54,6 +54,7 @@
 	"golang.org/x/build/cmd/coordinator/spanlog"
 	"golang.org/x/build/dashboard"
 	"golang.org/x/build/gerrit"
+	"golang.org/x/build/internal/buildgo"
 	"golang.org/x/build/internal/lru"
 	"golang.org/x/build/internal/singleflight"
 	"golang.org/x/build/livelog"
@@ -111,7 +112,7 @@
 
 var (
 	statusMu   sync.Mutex // guards the following four structures; see LOCK ORDER comment above
-	status     = map[builderRev]*buildStatus{}
+	status     = map[buildgo.BuilderRev]*buildStatus{}
 	statusDone []*buildStatus         // finished recently, capped to maxStatusDone
 	tries      = map[tryKey]*trySet{} // trybot builds
 	tryList    []tryKey
@@ -299,7 +300,7 @@
 		}
 	}()
 
-	workc := make(chan builderRev)
+	workc := make(chan buildgo.BuilderRev)
 
 	if *mode == "dev" {
 		// TODO(crawshaw): do more in dev mode
@@ -329,7 +330,7 @@
 		case work := <-workc:
 			if !mayBuildRev(work) {
 				if inStaging {
-					if _, ok := dashboard.Builders[work.name]; ok && logCantBuildStaging.Allow() {
+					if _, ok := dashboard.Builders[work.Name]; ok && logCantBuildStaging.Allow() {
 						log.Printf("may not build %v; skipping", work)
 					}
 				}
@@ -406,14 +407,14 @@
 	statusMu.Lock()
 	defer statusMu.Unlock()
 	for rev := range status {
-		if rev.name == typ {
+		if rev.Name == typ {
 			n++
 		}
 	}
 	return
 }
 
-func isBuilding(work builderRev) bool {
+func isBuilding(work buildgo.BuilderRev) bool {
 	statusMu.Lock()
 	defer statusMu.Unlock()
 	_, building := status[work]
@@ -428,21 +429,21 @@
 // mayBuildRev reports whether the build type & revision should be started.
 // It returns true if it's not already building, and if a reverse buildlet is
 // required, if an appropriate machine is registered.
-func mayBuildRev(rev builderRev) bool {
+func mayBuildRev(rev buildgo.BuilderRev) bool {
 	if isBuilding(rev) {
 		return false
 	}
 	if buildEnv.MaxBuilds > 0 && numCurrentBuilds() >= buildEnv.MaxBuilds {
 		return false
 	}
-	buildConf, ok := dashboard.Builders[rev.name]
+	buildConf, ok := dashboard.Builders[rev.Name]
 	if !ok {
 		if logUnknownBuilder.Allow() {
-			log.Printf("unknown builder %q", rev.name)
+			log.Printf("unknown builder %q", rev.Name)
 		}
 		return false
 	}
-	if buildConf.MaxAtOnce > 0 && numCurrentBuildsOfType(rev.name) >= buildConf.MaxAtOnce {
+	if buildConf.MaxAtOnce > 0 && numCurrentBuildsOfType(rev.Name) >= buildConf.MaxAtOnce {
 		return false
 	}
 	if buildConf.IsReverse() && !reversePool.CanBuild(buildConf.HostType) {
@@ -454,7 +455,7 @@
 	return true
 }
 
-func setStatus(work builderRev, st *buildStatus) {
+func setStatus(work buildgo.BuilderRev, st *buildStatus) {
 	statusMu.Lock()
 	defer statusMu.Unlock()
 	// TODO: panic if status[work] already exists. audit all callers.
@@ -465,7 +466,7 @@
 	status[work] = st
 }
 
-func markDone(work builderRev) {
+func markDone(work buildgo.BuilderRev) {
 	statusMu.Lock()
 	defer statusMu.Unlock()
 	st, ok := status[work]
@@ -483,7 +484,7 @@
 // statusPtrStr disambiguates which status to return if there are
 // multiple in the history (e.g. recent failures where the build
 // didn't finish for reasons outside of all.bash failing)
-func getStatus(work builderRev, statusPtrStr string) *buildStatus {
+func getStatus(work buildgo.BuilderRev, statusPtrStr string) *buildStatus {
 	statusMu.Lock()
 	defer statusMu.Unlock()
 	match := func(st *buildStatus) bool {
@@ -493,15 +494,15 @@
 		return st
 	}
 	for _, st := range statusDone {
-		if st.builderRev == work && match(st) {
+		if st.BuilderRev == work && match(st) {
 			return st
 		}
 	}
 	for k, ts := range tries {
-		if k.Commit == work.rev {
+		if k.Commit == work.Rev {
 			ts.mu.Lock()
 			for _, st := range ts.builds {
-				if st.builderRev == work && match(st) {
+				if st.BuilderRev == work && match(st) {
 					ts.mu.Unlock()
 					return st
 				}
@@ -549,7 +550,7 @@
 		}
 		bs.mu.Unlock()
 		fmt.Fprintf(w, "<tr valign=top><td align=left>%s</td><td align=center>%s</td><td><pre>%s</pre></td></tr>\n",
-			bs.name,
+			bs.Name,
 			status,
 			bs.HTMLStatusLine())
 	}
@@ -571,11 +572,11 @@
 }
 
 func handleLogs(w http.ResponseWriter, r *http.Request) {
-	br := builderRev{
-		name:    r.FormValue("name"),
-		rev:     r.FormValue("rev"),
-		subName: r.FormValue("subName"), // may be empty
-		subRev:  r.FormValue("subRev"),  // may be empty
+	br := buildgo.BuilderRev{
+		Name:    r.FormValue("name"),
+		Rev:     r.FormValue("rev"),
+		SubName: r.FormValue("subName"), // may be empty
+		SubRev:  r.FormValue("subRev"),  // may be empty
 	}
 	st := getStatus(br, r.FormValue("st"))
 	if st == nil {
@@ -630,8 +631,8 @@
 func writeStatusHeader(w http.ResponseWriter, st *buildStatus) {
 	st.mu.Lock()
 	defer st.mu.Unlock()
-	fmt.Fprintf(w, "  builder: %s\n", st.name)
-	fmt.Fprintf(w, "      rev: %s\n", st.rev)
+	fmt.Fprintf(w, "  builder: %s\n", st.Name)
+	fmt.Fprintf(w, "      rev: %s\n", st.Rev)
 	workaroundFlush(w)
 	fmt.Fprintf(w, " buildlet: %s\n", st.bc)
 	fmt.Fprintf(w, "  started: %v\n", st.startTime)
@@ -662,10 +663,10 @@
 // findWorkLoop polls https://build.golang.org/?mode=json looking for new work
 // for the main dashboard. It does not support gccgo.
 // TODO(bradfitz): it also currently does not support subrepos.
-func findWorkLoop(work chan<- builderRev) {
+func findWorkLoop(work chan<- buildgo.BuilderRev) {
 	// Useful for debugging a single run:
 	if inStaging && false {
-		//work <- builderRev{name: "linux-arm", rev: "c9778ec302b2e0e0d6027e1e0fca892e428d9657", subName: "tools", subRev: "ac303766f5f240c1796eeea3dc9bf34f1261aa35"}
+		//work <- buildgo.BuilderRev{name: "linux-arm", rev: "c9778ec302b2e0e0d6027e1e0fca892e428d9657", subName: "tools", subRev: "ac303766f5f240c1796eeea3dc9bf34f1261aa35"}
 		const debugArm = false
 		if debugArm {
 			for !reversePool.CanBuild("host-linux-arm") {
@@ -673,14 +674,14 @@
 				time.Sleep(time.Second)
 			}
 			log.Printf("ARM machine(s) registered.")
-			work <- builderRev{name: "linux-arm", rev: "3129c67db76bc8ee13a1edc38a6c25f9eddcbc6c"}
+			work <- buildgo.BuilderRev{Name: "linux-arm", Rev: "3129c67db76bc8ee13a1edc38a6c25f9eddcbc6c"}
 		} else {
-			work <- builderRev{name: "windows-amd64-2008", rev: "3129c67db76bc8ee13a1edc38a6c25f9eddcbc6c"}
-			work <- builderRev{name: "windows-386-gce", rev: "3129c67db76bc8ee13a1edc38a6c25f9eddcbc6c"}
+			work <- buildgo.BuilderRev{Name: "windows-amd64-2008", Rev: "3129c67db76bc8ee13a1edc38a6c25f9eddcbc6c"}
+			work <- buildgo.BuilderRev{Name: "windows-386-gce", Rev: "3129c67db76bc8ee13a1edc38a6c25f9eddcbc6c"}
 		}
 
 		// Still run findWork but ignore what it does.
-		ignore := make(chan builderRev)
+		ignore := make(chan buildgo.BuilderRev)
 		go func() {
 			for range ignore {
 			}
@@ -696,7 +697,7 @@
 	}
 }
 
-func findWork(work chan<- builderRev) error {
+func findWork(work chan<- buildgo.BuilderRev) error {
 	var bs types.BuildStatus
 	if err := dash("GET", "", url.Values{"mode": {"json"}}, nil, &bs); err != nil {
 		return err
@@ -766,24 +767,24 @@
 				continue
 			}
 
-			var rev builderRev
+			var rev buildgo.BuilderRev
 			if br.Repo == "go" {
-				rev = builderRev{
-					name: bs.Builders[i],
-					rev:  br.Revision,
+				rev = buildgo.BuilderRev{
+					Name: bs.Builders[i],
+					Rev:  br.Revision,
 				}
 			} else {
-				rev = builderRev{
-					name:    bs.Builders[i],
-					rev:     br.GoRevision,
-					subName: br.Repo,
-					subRev:  br.Revision,
+				rev = buildgo.BuilderRev{
+					Name:    bs.Builders[i],
+					Rev:     br.GoRevision,
+					SubName: br.Repo,
+					SubRev:  br.Revision,
 				}
-				if awaitSnapshot && !rev.snapshotExists() {
+				if awaitSnapshot && !rev.SnapshotExists(context.TODO(), buildEnv) {
 					continue
 				}
 			}
-			if rev.skipBuild() {
+			if skipBuild(rev) {
 				continue
 			}
 			if !isBuilding(rev) {
@@ -802,8 +803,8 @@
 			continue
 		}
 		for _, rev := range goRevisions {
-			br := builderRev{name: b, rev: rev}
-			if !br.skipBuild() && !isBuilding(br) {
+			br := buildgo.BuilderRev{Name: b, Rev: rev}
+			if !skipBuild(br) && !isBuilding(br) {
 				work <- br
 			}
 		}
@@ -987,18 +988,18 @@
 	return ts, nil
 }
 
-func tryKeyToBuilderRev(builder string, key tryKey) builderRev {
+func tryKeyToBuilderRev(builder string, key tryKey) buildgo.BuilderRev {
 	if key.Repo == "go" {
-		return builderRev{
-			name: builder,
-			rev:  key.Commit,
+		return buildgo.BuilderRev{
+			Name: builder,
+			Rev:  key.Commit,
 		}
 	}
-	return builderRev{
-		name:    builder,
-		rev:     getRepoHead("go"),
-		subName: key.Repo,
-		subRev:  key.Commit,
+	return buildgo.BuilderRev{
+		Name:    builder,
+		Rev:     getRepoHead("go"),
+		SubName: key.Repo,
+		SubRev:  key.Commit,
 	}
 }
 
@@ -1108,7 +1109,7 @@
 
 	ts.mu.Lock()
 	if hasBenchResults {
-		ts.benchResults = append(ts.benchResults, bs.name)
+		ts.benchResults = append(ts.benchResults, bs.Name)
 	}
 	ts.remain--
 	remain := ts.remain
@@ -1122,7 +1123,7 @@
 	if !succeeded {
 		s1 := sha1.New()
 		io.WriteString(s1, buildLog)
-		objName := fmt.Sprintf("%s/%s_%x.log", bs.rev[:8], bs.name, s1.Sum(nil)[:4])
+		objName := fmt.Sprintf("%s/%s_%x.log", bs.Rev[:8], bs.Name, s1.Sum(nil)[:4])
 		wr, failLogURL := newFailureLogBlob(objName)
 		if _, err := io.WriteString(wr, buildLog); err != nil {
 			log.Printf("Failed to write to GCS: %v", err)
@@ -1137,7 +1138,7 @@
 		bs.failURL = failLogURL
 		bs.mu.Unlock()
 		ts.mu.Lock()
-		fmt.Fprintf(&ts.errMsg, "Failed on %s: %s\n", bs.name, failLogURL)
+		fmt.Fprintf(&ts.errMsg, "Failed on %s: %s\n", bs.Name, failLogURL)
 		ts.mu.Unlock()
 
 		if numFail == 1 && remain > 0 {
@@ -1147,7 +1148,7 @@
 						"This change failed on %s:\n"+
 						"See %s\n\n"+
 						"Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.",
-					bs.name, failLogURL),
+					bs.Name, failLogURL),
 			}); err != nil {
 				log.Printf("Failed to call Gerrit: %v", err)
 				return
@@ -1179,28 +1180,18 @@
 	}
 }
 
-// builderRev is a build configuration type and a revision.
-type builderRev struct {
-	name string // e.g. "linux-amd64-race"
-	rev  string // lowercase hex core repo git hash
-
-	// optional sub-repository details (both must be present)
-	subName string // e.g. "net"
-	subRev  string // lowercase hex sub-repo git hash
-}
-
-func (br builderRev) skipBuild() bool {
-	if strings.HasPrefix(br.name, "netbsd-386") {
+func skipBuild(br buildgo.BuilderRev) bool {
+	if strings.HasPrefix(br.Name, "netbsd-386") {
 		// Hangs during make.bash. TODO: remove once Issue 19339 is fixed.
 		return true
 	}
-	if strings.HasPrefix(br.name, "netbsd-amd64") {
+	if strings.HasPrefix(br.Name, "netbsd-amd64") {
 		// Broken and unloved. Wasting resources.
 		// Still available via gomote, but not building for now.
 		// TODO: remove once Issue 19652 is fixed.
 		return true
 	}
-	switch br.subName {
+	switch br.SubName {
 	case "build", // has external deps
 		"exp",    // always broken, depends on mobile which is broken
 		"mobile", // always broken (gl, etc). doesn't compile.
@@ -1208,13 +1199,13 @@
 		"oauth2": // has external deps
 		return true
 	case "perf":
-		if br.name == "linux-amd64-nocgo" {
+		if br.Name == "linux-amd64-nocgo" {
 			// The "perf" repo requires sqlite, which
 			// requires cgo. Skip the no-cgo builder.
 			return true
 		}
 	case "net":
-		if br.name == "darwin-amd64-10_8" || br.name == "darwin-386-10_8" {
+		if br.Name == "darwin-amd64-10_8" || br.Name == "darwin-386-10_8" {
 			// One of the tests seems to panic the kernel
 			// and kill our buildlet which goes in a loop.
 			return true
@@ -1223,24 +1214,6 @@
 	return false
 }
 
-func (br builderRev) isSubrepo() bool {
-	return br.subName != ""
-}
-
-func (br builderRev) subRevOrGoRev() string {
-	if br.subRev != "" {
-		return br.subRev
-	}
-	return br.rev
-}
-
-func (br builderRev) repoOrGo() string {
-	if br.subName == "" {
-		return "go"
-	}
-	return br.subName
-}
-
 type eventTimeLogger interface {
 	LogEventTime(event string, optText ...string)
 }
@@ -1331,18 +1304,18 @@
 	}
 }
 
-func newBuild(rev builderRev) (*buildStatus, error) {
+func newBuild(rev buildgo.BuilderRev) (*buildStatus, error) {
 	// Note: can't acquire statusMu in newBuild, as this is called
 	// from findTryWork -> newTrySet, which holds statusMu.
 
-	conf, ok := dashboard.Builders[rev.name]
+	conf, ok := dashboard.Builders[rev.Name]
 	if !ok {
-		return nil, fmt.Errorf("unknown builder type %q", rev.name)
+		return nil, fmt.Errorf("unknown builder type %q", rev.Name)
 	}
 	ctx, cancel := context.WithCancel(context.Background())
 	return &buildStatus{
 		buildID:    "B" + randHex(9),
-		builderRev: rev,
+		BuilderRev: rev,
 		conf:       conf,
 		startTime:  time.Now(),
 		ctx:        ctx,
@@ -1354,7 +1327,7 @@
 // The buildStatus's context is closed when the build is complete,
 // successfully or not.
 func (st *buildStatus) start() {
-	setStatus(st.builderRev, st)
+	setStatus(st.BuilderRev, st)
 	go func() {
 		err := st.build()
 		if err == errSkipBuildDueToDeps {
@@ -1362,12 +1335,12 @@
 		} else {
 			if err != nil {
 				fmt.Fprintf(st, "\n\nError: %v\n", err)
-				log.Println(st.builderRev, "failed:", err)
+				log.Println(st.BuilderRev, "failed:", err)
 			}
 			st.setDone(err == nil)
 			putBuildRecord(st.buildRecord())
 		}
-		markDone(st.builderRev)
+		markDone(st.BuilderRev)
 	}()
 }
 
@@ -1424,7 +1397,7 @@
 // ready, such that they're ready when make.bash is done. But we don't
 // want to start too early, lest we waste idle resources during make.bash.
 func (st *buildStatus) getHelpersReadySoon() {
-	if st.isSubrepo() || st.conf.NumTestHelpers(st.isTry()) == 0 || st.conf.IsReverse() {
+	if st.IsSubrepo() || st.conf.NumTestHelpers(st.isTry()) == 0 || st.conf.IsReverse() {
 		return
 	}
 	time.AfterFunc(st.expectedMakeBashDuration()-st.expectedBuildletStartDuration(),
@@ -1458,7 +1431,7 @@
 	if st.useSnapshotMemo != nil {
 		return *st.useSnapshotMemo
 	}
-	b := st.conf.SplitMakeRun() && st.builderRev.snapshotExists()
+	b := st.conf.SplitMakeRun() && st.BuilderRev.SnapshotExists(context.TODO(), buildEnv)
 	st.useSnapshotMemo = &b
 	return b
 }
@@ -1474,7 +1447,7 @@
 	if kubeErr != nil {
 		return nil
 	}
-	config := crossCompileConfigs[st.name]
+	config := crossCompileConfigs[st.Name]
 	if config == nil {
 		return nil
 	}
@@ -1494,7 +1467,7 @@
 	for {
 		tries++
 		res, err := maintnerClient.HasAncestor(ctx, &apipb.HasAncestorRequest{
-			Commit:   st.rev,
+			Commit:   st.Rev,
 			Ancestor: dep,
 		})
 		if err != nil {
@@ -1526,12 +1499,12 @@
 		for _, dep := range deps {
 			has, err := st.checkDep(ctx, dep)
 			if err != nil {
-				fmt.Fprintf(st, "Error checking whether commit %s includes ancestor %s: %v\n", st.rev, dep, err)
+				fmt.Fprintf(st, "Error checking whether commit %s includes ancestor %s: %v\n", st.Rev, dep, err)
 				return err
 			}
 			if !has {
 				st.LogEventTime(eventSkipBuildMissingDep)
-				fmt.Fprintf(st, "skipping build; commit %s lacks ancestor %s\n", st.rev, dep)
+				fmt.Fprintf(st, "skipping build; commit %s lacks ancestor %s\n", st.Rev, dep)
 				return errSkipBuildDueToDeps
 			}
 		}
@@ -1542,7 +1515,7 @@
 
 	sp := st.CreateSpan("checking_for_snapshot")
 	if inStaging {
-		err := storageClient.Bucket(buildEnv.SnapBucket).Object(st.snapshotObjectName()).Delete(context.Background())
+		err := storageClient.Bucket(buildEnv.SnapBucket).Object(st.SnapshotObjectName()).Delete(context.Background())
 		st.LogEventTime("deleted_snapshot", fmt.Sprint(err))
 	}
 	snapshotExists := st.useSnapshot()
@@ -1572,7 +1545,7 @@
 
 	if st.useSnapshot() {
 		sp := st.CreateSpan("write_snapshot_tar")
-		if err := bc.PutTarFromURL(st.snapshotURL(), "go"); err != nil {
+		if err := bc.PutTarFromURL(st.SnapshotURL(buildEnv), "go"); err != nil {
 			return sp.Done(fmt.Errorf("failed to put snapshot to buildlet: %v", err))
 		}
 		sp.Done(nil)
@@ -1587,9 +1560,9 @@
 	}
 
 	execStartTime := time.Now()
-	fmt.Fprintf(st, "%s at %v", st.name, st.rev)
-	if st.isSubrepo() {
-		fmt.Fprintf(st, " building %v at %v", st.subName, st.subRev)
+	fmt.Fprintf(st, "%s at %v", st.Name, st.Rev)
+	if st.IsSubrepo() {
+		fmt.Fprintf(st, " building %v at %v", st.SubName, st.SubRev)
 	}
 	fmt.Fprint(st, "\n\n")
 
@@ -1639,7 +1612,7 @@
 				buildLog += "\n" + remoteErr.Error()
 			}
 		}
-		if err := recordResult(st.builderRev, remoteErr == nil, buildLog, time.Since(execStartTime)); err != nil {
+		if err := recordResult(st.BuilderRev, remoteErr == nil, buildLog, time.Since(execStartTime)); err != nil {
 			if remoteErr != nil {
 				return fmt.Errorf("Remote error was %q but failed to report it to the dashboard: %v", remoteErr, err)
 			}
@@ -1660,10 +1633,10 @@
 		ProcessID: processID,
 		StartTime: st.startTime,
 		IsTry:     st.isTry(),
-		GoRev:     st.rev,
-		Rev:       st.subRevOrGoRev(),
-		Repo:      st.repoOrGo(),
-		Builder:   st.name,
+		GoRev:     st.Rev,
+		Rev:       st.SubRevOrGoRev(),
+		Repo:      st.RepoOrGo(),
+		Builder:   st.Name,
 		OS:        st.conf.GOOS(),
 		Arch:      st.conf.GOARCH(),
 	}
@@ -1688,10 +1661,10 @@
 	rec := &types.SpanRecord{
 		BuildID: st.buildID,
 		IsTry:   st.isTry(),
-		GoRev:   st.rev,
-		Rev:     st.subRevOrGoRev(),
-		Repo:    st.repoOrGo(),
-		Builder: st.name,
+		GoRev:   st.Rev,
+		Rev:     st.SubRevOrGoRev(),
+		Repo:    st.RepoOrGo(),
+		Builder: st.Name,
 		OS:      st.conf.GOOS(),
 		Arch:    st.conf.GOARCH(),
 
@@ -1712,7 +1685,7 @@
 	if !*shouldRunBench {
 		return false
 	}
-	return st.isTry() && !st.isSubrepo() && st.conf.RunBench
+	return st.isTry() && !st.IsSubrepo() && st.conf.RunBench
 }
 
 // runAllSharded runs make.bash and then shards the test execution.
@@ -1741,7 +1714,7 @@
 		return nil, err
 	}
 
-	if st.isSubrepo() {
+	if st.IsSubrepo() {
 		remoteErr, err = st.runSubrepoTests()
 	} else {
 		remoteErr, err = st.runTests(st.getHelpers())
@@ -1886,7 +1859,7 @@
 		sp.Done(nil)
 	}
 
-	if st.name == "linux-amd64-racecompile" {
+	if st.Name == "linux-amd64-racecompile" {
 		return st.runConcurrentGoBuildStdCmd(bc)
 	}
 
@@ -1962,24 +1935,6 @@
 	return nil
 }
 
-// snapshotExists reports whether the snapshot exists in storage.
-// It returns potentially false negatives on network errors.
-// Callers must not depend on this as more than an optimization.
-func (br *builderRev) snapshotExists() bool {
-	req, err := http.NewRequest("HEAD", br.snapshotURL(), nil)
-	if err != nil {
-		panic(err)
-	}
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	defer cancel()
-	res, err := http.DefaultClient.Do(req.WithContext(ctx))
-	if err != nil {
-		log.Printf("snapshotExists check: %v", err)
-		return false
-	}
-	return res.StatusCode == http.StatusOK
-}
-
 func (st *buildStatus) writeGoSource() error {
 	return st.writeGoSourceTo(st.bc)
 }
@@ -1987,11 +1942,11 @@
 func (st *buildStatus) writeGoSourceTo(bc *buildlet.Client) error {
 	// Write the VERSION file.
 	sp := st.CreateSpan("write_version_tar")
-	if err := bc.PutTar(versionTgz(st.rev), "go"); err != nil {
+	if err := bc.PutTar(versionTgz(st.Rev), "go"); err != nil {
 		return sp.Done(fmt.Errorf("writing VERSION tgz: %v", err))
 	}
 
-	srcTar, err := getSourceTgz(st, "go", st.rev)
+	srcTar, err := getSourceTgz(st, "go", st.Rev)
 	if err != nil {
 		return err
 	}
@@ -2020,18 +1975,6 @@
 	))
 }
 
-// snapshotObjectName is the cloud storage object name of the
-// built Go tree for this builder and Go rev (not the sub-repo).
-// The entries inside this tarball do not begin with "go/".
-func (br *builderRev) snapshotObjectName() string {
-	return fmt.Sprintf("%v/%v/%v.tar.gz", "go", br.name, br.rev)
-}
-
-// snapshotURL is the absolute URL of the snapshot object (see above).
-func (br *builderRev) snapshotURL() string {
-	return buildEnv.SnapshotURL(br.name, br.rev)
-}
-
 func (st *buildStatus) writeSnapshot(bc *buildlet.Client) (err error) {
 	sp := st.CreateSpan("write_snapshot_to_gcs")
 	defer func() { sp.Done(err) }()
@@ -2050,7 +1993,7 @@
 	}
 	defer tgz.Close()
 
-	wr := storageClient.Bucket(buildEnv.SnapBucket).Object(st.snapshotObjectName()).NewWriter(ctx)
+	wr := storageClient.Bucket(buildEnv.SnapBucket).Object(st.SnapshotObjectName()).NewWriter(ctx)
 	wr.ContentType = "application/octet-stream"
 	wr.ACL = append(wr.ACL, storage.ACLRule{Entity: storage.AllUsers, Role: storage.RoleReader})
 	if _, err := io.Copy(wr, tgz); err != nil {
@@ -2107,7 +2050,7 @@
 // only do this for slow builders running redundant tests. (That is,
 // tests which have identical behavior across different ports)
 func (st *buildStatus) shouldSkipTest(testName string) bool {
-	if inStaging && st.name == "linux-arm" && false {
+	if inStaging && st.Name == "linux-arm" && false {
 		if strings.HasPrefix(testName, "go_test:") && testName < "go_test:runtime" {
 			return true
 		}
@@ -2117,7 +2060,7 @@
 		// Old vetall test name, before the sharding in CL 37572.
 		return true
 	case "api":
-		return st.isTry() && st.name != "linux-amd64"
+		return st.isTry() && st.Name != "linux-amd64"
 	}
 	return false
 }
@@ -2130,7 +2073,7 @@
 		set.items = append(set.items, &testItem{
 			set:      set,
 			name:     name,
-			duration: testDuration(st.builderRev.name, name),
+			duration: testDuration(st.BuilderRev.Name, name),
 			take:     make(chan token, 1),
 			done:     make(chan token),
 		})
@@ -2141,7 +2084,7 @@
 			set:      set,
 			name:     name,
 			bench:    bench,
-			duration: testDuration(st.builderRev.name, name),
+			duration: testDuration(st.BuilderRev.Name, name),
 			take:     make(chan token, 1),
 			done:     make(chan token),
 		})
@@ -2534,7 +2477,7 @@
 }
 
 func (st *buildStatus) runSubrepoTests() (remoteErr, err error) {
-	st.LogEventTime("fetching_subrepo", st.subName)
+	st.LogEventTime("fetching_subrepo", st.SubName)
 
 	workDir, err := st.bc.WorkDir()
 	if err != nil {
@@ -2545,7 +2488,7 @@
 	gopath := st.conf.FilePathJoin(workDir, "gopath")
 
 	fetched := map[string]bool{}
-	toFetch := []string{st.subName}
+	toFetch := []string{st.SubName}
 
 	// fetch checks out the provided sub-repo to the buildlet's workspace.
 	fetch := func(repo, rev string) error {
@@ -2602,7 +2545,7 @@
 		}
 		// For the repo under test, choose that specific revision.
 		if i == 0 {
-			rev = st.subRev
+			rev = st.SubRev
 		}
 		if err := fetch(repo, rev); err != nil {
 			return nil, err
@@ -2616,7 +2559,7 @@
 		}
 	}
 
-	sp := st.CreateSpan("running_subrepo_tests", st.subName)
+	sp := st.CreateSpan("running_subrepo_tests", st.SubName)
 	defer func() { sp.Done(err) }()
 	return st.bc.Exec(path.Join("go", "bin", "go"), buildlet.ExecOpts{
 		Output: st,
@@ -2626,7 +2569,7 @@
 			"GOPATH="+gopath,
 			"GO15VENDOREXPERIMENT=1"),
 		Path: []string{"$WORKDIR/go/bin", "$PATH"},
-		Args: []string{"test", "-short", subrepoPrefix + st.subName + "/..."},
+		Args: []string{"test", "-short", subrepoPrefix + st.SubName + "/..."},
 	})
 }
 
@@ -2700,7 +2643,7 @@
 					defer st.LogEventTime("DEV_HELPER_SLEEP", bc.Name())
 				}
 				st.LogEventTime("got_empty_test_helper", bc.String())
-				if err := bc.PutTarFromURL(st.snapshotURL(), "go"); err != nil {
+				if err := bc.PutTarFromURL(st.SnapshotURL(buildEnv), "go"); err != nil {
 					log.Printf("failed to extract snapshot for helper %s: %v", bc.Name(), err)
 					return
 				}
@@ -2846,14 +2789,14 @@
 	}
 	fmt.Fprintf(&benchFiles[0].out, "cl: %d\nps: %d\ntry: %s\nbuildlet: %s\nbranch: %s\nrepo: https://go.googlesource.com/%s\n",
 		st.trySet.ci.ChangeNumber, ps, st.trySet.tryID,
-		st.name, st.trySet.ci.Branch, st.trySet.ci.Project,
+		st.Name, st.trySet.ci.Branch, st.trySet.ci.Project,
 	)
 	if inStaging {
 		benchFiles[0].out.WriteString("staging: true\n")
 	}
 	benchFiles[1].out.Write(benchFiles[0].out.Bytes())
 	fmt.Fprintf(&benchFiles[0].out, "commit: %s\n", rev.Commit.Parents[0].CommitID)
-	fmt.Fprintf(&benchFiles[1].out, "commit: %s\n", st.builderRev.rev)
+	fmt.Fprintf(&benchFiles[1].out, "commit: %s\n", st.BuilderRev.Rev)
 	return benchFiles
 }
 
@@ -3037,7 +2980,7 @@
 
 	// First do the go_test:* ones. partitionGoTests
 	// only returns those, which are the ones we merge together.
-	stdSets := partitionGoTests(s.st.builderRev.name, names)
+	stdSets := partitionGoTests(s.st.BuilderRev.Name, names)
 	for _, set := range stdSets {
 		tis := make([]*testItem, len(set))
 		for i, name := range set {
@@ -3135,7 +3078,7 @@
 // buildStatus is the status of a build.
 type buildStatus struct {
 	// Immutable:
-	builderRev
+	buildgo.BuilderRev
 	buildID   string // "B" + 9 random hex
 	conf      dashboard.BuildConfig
 	startTime time.Time // actually time of newBuild (~same thing); TODO(bradfitz): rename this createTime
@@ -3179,7 +3122,7 @@
 func (st *buildStatus) isRunningLocked() bool { return st.done.IsZero() }
 
 func (st *buildStatus) logf(format string, args ...interface{}) {
-	log.Printf("[build %s %s]: %s", st.name, st.rev, fmt.Sprintf(format, args...))
+	log.Printf("[build %s %s]: %s", st.Name, st.Rev, fmt.Sprintf(format, args...))
 }
 
 // span is an event covering a region of time.
@@ -3292,10 +3235,10 @@
 
 	var buf bytes.Buffer
 	fmt.Fprintf(&buf, "<a href='https://github.com/golang/go/wiki/DashboardBuilders'>%s</a> rev <a href='%s%s'>%s</a>",
-		st.name, urlPrefix, st.rev, st.rev[:8])
-	if st.isSubrepo() {
+		st.Name, urlPrefix, st.Rev, st.Rev[:8])
+	if st.IsSubrepo() {
 		fmt.Fprintf(&buf, " (sub-repo %s rev <a href='%s%s'>%s</a>)",
-			st.subName, urlPrefix, st.subRev, st.subRev[:8])
+			st.SubName, urlPrefix, st.SubRev, st.SubRev[:8])
 	}
 	if ts := st.trySet; ts != nil {
 		fmt.Fprintf(&buf, " (<a href='/try?commit=%v'>trybot set</a> for <a href='https://go-review.googlesource.com/#/q/%s'>%s</a>)",
@@ -3339,9 +3282,9 @@
 	if *mode == "dev" {
 		urlPrefix = "https://localhost:8119"
 	}
-	u := fmt.Sprintf("%v/temporarylogs?name=%s&rev=%s&st=%p", urlPrefix, st.name, st.rev, st)
-	if st.isSubrepo() {
-		u += fmt.Sprintf("&subName=%v&subRev=%v", st.subName, st.subRev)
+	u := fmt.Sprintf("%v/temporarylogs?name=%s&rev=%s&st=%p", urlPrefix, st.Name, st.Rev, st)
+	if st.IsSubrepo() {
+		u += fmt.Sprintf("&subName=%v&subRev=%v", st.SubName, st.SubRev)
 	}
 	return u
 }
diff --git a/cmd/coordinator/dash.go b/cmd/coordinator/dash.go
index 3c7afd4..752cb15 100644
--- a/cmd/coordinator/dash.go
+++ b/cmd/coordinator/dash.go
@@ -22,6 +22,8 @@
 	"sync"
 	"time"
 
+	"golang.org/x/build/internal/buildgo"
+
 	"cloud.google.com/go/compute/metadata"
 )
 
@@ -90,22 +92,22 @@
 // recordResult sends build results to the dashboard.
 // This is not used for trybot failures; only failures after commit.
 // The URLs end up looking like https://build.golang.org/log/$HEXDIGEST
-func recordResult(br builderRev, ok bool, buildLog string, runTime time.Duration) error {
+func recordResult(br buildgo.BuilderRev, ok bool, buildLog string, runTime time.Duration) error {
 	req := map[string]interface{}{
-		"Builder":     br.name,
+		"Builder":     br.Name,
 		"PackagePath": "",
-		"Hash":        br.rev,
+		"Hash":        br.Rev,
 		"GoHash":      "",
 		"OK":          ok,
 		"Log":         buildLog,
 		"RunTime":     runTime,
 	}
-	if br.isSubrepo() {
-		req["PackagePath"] = subrepoPrefix + br.subName
-		req["Hash"] = br.subRev
-		req["GoHash"] = br.rev
+	if br.IsSubrepo() {
+		req["PackagePath"] = subrepoPrefix + br.SubName
+		req["Hash"] = br.SubRev
+		req["GoHash"] = br.Rev
 	}
-	args := url.Values{"key": {builderKey(br.name)}, "builder": {br.name}}
+	args := url.Values{"key": {builderKey(br.Name)}, "builder": {br.Name}}
 	if *mode == "dev" {
 		log.Printf("In dev mode, not recording result: %v", req)
 		return nil
@@ -138,14 +140,14 @@
 	logsURL := st.logsURLLocked()
 	st.mu.Unlock()
 	args := url.Values{
-		"builder": []string{st.name},
-		"key":     []string{builderKey(st.name)},
-		"hash":    []string{st.rev},
+		"builder": []string{st.Name},
+		"key":     []string{builderKey(st.Name)},
+		"hash":    []string{st.Rev},
 		"url":     []string{logsURL},
 	}
-	if st.isSubrepo() {
-		args.Set("hash", st.subRev)
-		args.Set("gohash", st.rev)
+	if st.IsSubrepo() {
+		args.Set("hash", st.SubRev)
+		args.Set("gohash", st.Rev)
 	}
 	u := buildEnv.DashBase() + "building?" + args.Encode()
 	for {
diff --git a/cmd/coordinator/debug.go b/cmd/coordinator/debug.go
index 1ace760..212372e 100644
--- a/cmd/coordinator/debug.go
+++ b/cmd/coordinator/debug.go
@@ -15,13 +15,14 @@
 	"strings"
 	"text/template"
 
+	"golang.org/x/build/internal/buildgo"
 	"golang.org/x/build/types"
 )
 
 // handleDoSomeWork adds the last committed CL as work to do.
 //
 // Only available in dev mode.
-func handleDoSomeWork(work chan<- builderRev) func(w http.ResponseWriter, r *http.Request) {
+func handleDoSomeWork(work chan<- buildgo.BuilderRev) func(w http.ResponseWriter, r *http.Request) {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if r.Method == "GET" {
 			w.Header().Set("Content-Type", "text/html; charset=utf-8")
@@ -63,7 +64,7 @@
 		}
 		fmt.Fprintf(w, "found work: %v\n", revs)
 		for _, rev := range revs {
-			work <- builderRev{name: mode, rev: rev}
+			work <- buildgo.BuilderRev{Name: mode, Rev: rev}
 		}
 	}
 }
diff --git a/cmd/coordinator/status.go b/cmd/coordinator/status.go
index b4164e0..f3d47d0 100644
--- a/cmd/coordinator/status.go
+++ b/cmd/coordinator/status.go
@@ -56,7 +56,7 @@
 				key.ChangeTriple(), key.Commit, key.Commit[:8])
 			fmt.Fprintf(&buf, "   Remain: %d, fails: %v\n", state.remain, state.failed)
 			for _, bs := range ts.builds {
-				fmt.Fprintf(&buf, "  %s: running=%v\n", bs.name, bs.isRunning())
+				fmt.Fprintf(&buf, "  %s: running=%v\n", bs.Name, bs.isRunning())
 			}
 		}
 	}
diff --git a/internal/buildgo/buildgo.go b/internal/buildgo/buildgo.go
new file mode 100644
index 0000000..41d591f
--- /dev/null
+++ b/internal/buildgo/buildgo.go
@@ -0,0 +1,75 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package buildgo provides tools for pushing and building the Go
+// distribution on buildlets.
+package buildgo
+
+import (
+	"context"
+	"fmt"
+	"log"
+	"net/http"
+	"time"
+
+	"golang.org/x/build/buildenv"
+)
+
+// BuilderRev is a build configuration type and a revision.
+type BuilderRev struct {
+	Name string // e.g. "linux-amd64-race"
+	Rev  string // lowercase hex core repo git hash
+
+	// optional sub-repository details (both must be present)
+	SubName string // e.g. "net"
+	SubRev  string // lowercase hex sub-repo git hash
+}
+
+func (br BuilderRev) IsSubrepo() bool {
+	return br.SubName != ""
+}
+
+func (br BuilderRev) SubRevOrGoRev() string {
+	if br.SubRev != "" {
+		return br.SubRev
+	}
+	return br.Rev
+}
+
+func (br BuilderRev) RepoOrGo() string {
+	if br.SubName == "" {
+		return "go"
+	}
+	return br.SubName
+}
+
+// SnapshotObjectName is the cloud storage object name of the
+// built Go tree for this builder and Go rev (not the sub-repo).
+// The entries inside this tarball do not begin with "go/".
+func (br *BuilderRev) SnapshotObjectName() string {
+	return fmt.Sprintf("%v/%v/%v.tar.gz", "go", br.Name, br.Rev)
+}
+
+// SnapshotURL is the absolute URL of the snapshot object (see above).
+func (br *BuilderRev) SnapshotURL(buildEnv *buildenv.Environment) string {
+	return buildEnv.SnapshotURL(br.Name, br.Rev)
+}
+
+// snapshotExists reports whether the snapshot exists in storage.
+// It returns potentially false negatives on network errors.
+// Callers must not depend on this as more than an optimization.
+func (br *BuilderRev) SnapshotExists(ctx context.Context, buildEnv *buildenv.Environment) bool {
+	req, err := http.NewRequest("HEAD", br.SnapshotURL(buildEnv), nil)
+	if err != nil {
+		panic(err)
+	}
+	ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
+	defer cancel()
+	res, err := http.DefaultClient.Do(req.WithContext(ctx))
+	if err != nil {
+		log.Printf("SnapshotExists check: %v", err)
+		return false
+	}
+	return res.StatusCode == http.StatusOK
+}