internal/relui: add an integration test for release building

Adds an integration test. The test uses a fake HTTP server to serve the
fake source archive and bootstrap Go. Fake buildlets run locally in temp
directories, but otherwise act mostly like real ones, running real
commands and reading/writing real tar files. Each step produces a dummy output
that can be used for subsequent steps or verified at the end of the
test.

This is more of a smoke test than an exhaustive check of every detail.
Still, it's a lot better than running the workflow against live
buildlets for incremental development.

I had some vague ideas of recording the execution log for each buildlet
into a golden file, but after trying it out I think that it's not worth
it. There really are only 2-3 commands per target. For now I left the
code in case I change my mind.

In the process of writing the test I realized that all the filenames
were wrong, so fix those up.

For golang/go#51797.

Change-Id: Ie3cc8e335047faa8b50bd05ecc0deddaceb55f71
Reviewed-on: https://go-review.googlesource.com/c/build/+/403057
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
diff --git a/buildlet/buildletclient.go b/buildlet/buildletclient.go
index e5e3c4c..248d4f9 100644
--- a/buildlet/buildletclient.go
+++ b/buildlet/buildletclient.go
@@ -736,19 +736,19 @@
 
 // DirEntry is the information about a file on a buildlet.
 type DirEntry struct {
-	// line is of the form "drw-rw-rw\t<name>" and then if a regular file,
+	// Line is of the form "drw-rw-rw\t<name>" and then if a regular file,
 	// also "\t<size>\t<modtime>". in either case, without trailing newline.
 	// TODO: break into parsed fields?
-	line string
+	Line string
 }
 
 func (de DirEntry) String() string {
-	return de.line
+	return de.Line
 }
 
 // Name returns the relative path to the file, such as "src/net/http/" or "src/net/http/jar.go".
 func (de DirEntry) Name() string {
-	f := strings.Split(de.line, "\t")
+	f := strings.Split(de.Line, "\t")
 	if len(f) < 2 {
 		return ""
 	}
@@ -757,26 +757,26 @@
 
 // Perm returns the permission bits in string form, such as "-rw-r--r--" or "drwxr-xr-x".
 func (de DirEntry) Perm() string {
-	i := strings.IndexByte(de.line, '\t')
+	i := strings.IndexByte(de.Line, '\t')
 	if i == -1 {
 		return ""
 	}
-	return de.line[:i]
+	return de.Line[:i]
 }
 
 // IsDir reports whether de describes a directory. That is,
 // it tests for the os.ModeDir bit being set in de.Perm().
 func (de DirEntry) IsDir() bool {
-	if len(de.line) == 0 {
+	if len(de.Line) == 0 {
 		return false
 	}
-	return de.line[0] == 'd'
+	return de.Line[0] == 'd'
 }
 
 // Digest returns the SHA-1 digest of the file, such as "da39a3ee5e6b4b0d3255bfef95601890afd80709".
 // It returns the empty string if the digest isn't included.
 func (de DirEntry) Digest() string {
-	f := strings.Split(de.line, "\t")
+	f := strings.Split(de.Line, "\t")
 	if len(f) < 5 {
 		return ""
 	}
@@ -825,7 +825,7 @@
 	sc := bufio.NewScanner(resp.Body)
 	for sc.Scan() {
 		line := strings.TrimSpace(sc.Text())
-		fn(DirEntry{line: line})
+		fn(DirEntry{Line: line})
 	}
 	return sc.Err()
 }
diff --git a/buildlet/fakebuildletclient.go b/buildlet/fakebuildletclient.go
index b025f0c..9850386 100644
--- a/buildlet/fakebuildletclient.go
+++ b/buildlet/fakebuildletclient.go
@@ -127,7 +127,7 @@
 drwxr-xr-x      tmp/`
 	lines := strings.Split(lsOutput, "\n")
 	for _, line := range lines {
-		fn(DirEntry{line: line})
+		fn(DirEntry{Line: line})
 	}
 	return nil
 }
diff --git a/cmd/release/release.go b/cmd/release/release.go
index e4ff690..7070ef3 100644
--- a/cmd/release/release.go
+++ b/cmd/release/release.go
@@ -121,9 +121,11 @@
 	}
 }
 
+const gerritURL = "https://go.googlesource.com"
+
 func doRelease(ctx *workflow.TaskContext, revision, version string, target *releasetargets.Target, stagingDir string, watch bool) error {
 	srcBuf := &bytes.Buffer{}
-	if err := task.WriteSourceArchive(ctx, revision, version, srcBuf); err != nil {
+	if err := task.WriteSourceArchive(ctx, gerritURL, revision, version, srcBuf); err != nil {
 		return fmt.Errorf("Building source archive: %v", err)
 	}
 
@@ -259,7 +261,7 @@
 	if err != nil {
 		return err
 	}
-	if err := task.WriteSourceArchive(ctx, revision, version, w); err != nil {
+	if err := task.WriteSourceArchive(ctx, gerritURL, revision, version, w); err != nil {
 		return err
 	}
 	return w.Close()
diff --git a/cmd/relui/main.go b/cmd/relui/main.go
index 30e175b..9380c96 100644
--- a/cmd/relui/main.go
+++ b/cmd/relui/main.go
@@ -104,6 +104,7 @@
 		log.Fatalf("Could not connect to GCS: %v", err)
 	}
 	releaseTasks := &relui.BuildReleaseTasks{
+		GerritURL:      "https://go.googlesource.com",
 		CreateBuildlet: coordinator.CreateBuildlet,
 		GCSClient:      gcsClient,
 		ScratchURL:     *scratchFilesBase,
diff --git a/internal/gcsfs/osfs.go b/internal/gcsfs/osfs.go
index 84ea66a..2bf8be4 100644
--- a/internal/gcsfs/osfs.go
+++ b/internal/gcsfs/osfs.go
@@ -53,7 +53,7 @@
 
 func (dir dirFS) Create(name string) (WriteFile, error) {
 	if !fs.ValidPath(name) || runtime.GOOS == "windows" && containsAny(name, `\:`) {
-		return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid}
+		return nil, &fs.PathError{Op: "create", Path: name, Err: fs.ErrInvalid}
 	}
 	fullName := path.Join(string(dir), name)
 	if err := os.MkdirAll(path.Dir(fullName), 0700); err != nil {
diff --git a/internal/relui/buildrelease_test.go b/internal/relui/buildrelease_test.go
new file mode 100644
index 0000000..87a4afd
--- /dev/null
+++ b/internal/relui/buildrelease_test.go
@@ -0,0 +1,473 @@
+package relui
+
+import (
+	"archive/tar"
+	"archive/zip"
+	"bytes"
+	"compress/gzip"
+	"context"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/google/uuid"
+	"golang.org/x/build/buildlet"
+	"golang.org/x/build/internal/untar"
+	"golang.org/x/build/internal/workflow"
+)
+
+func TestRelease(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("Requires bash shell scripting support.")
+	}
+	s := httptest.NewServer(http.HandlerFunc(serveTarballs))
+	defer s.Close()
+	fakeBuildlets := &fakeBuildlets{
+		t:       t,
+		dir:     t.TempDir(),
+		httpURL: s.URL,
+		logs:    map[string][]*[]string{},
+	}
+	stagingDir := t.TempDir()
+	tasks := BuildReleaseTasks{
+		GerritURL:      s.URL,
+		GCSClient:      nil,
+		ScratchURL:     "file://" + filepath.ToSlash(t.TempDir()),
+		StagingURL:     "file://" + filepath.ToSlash(stagingDir),
+		CreateBuildlet: fakeBuildlets.createBuildlet,
+	}
+	wd, err := tasks.newBuildReleaseWorkflow("go1.18")
+	if err != nil {
+		t.Fatal(err)
+	}
+	w, err := workflow.Start(wd, map[string]interface{}{
+		"Revision": "0",
+		"Version":  "go1.18releasetest1",
+		"Targets to skip testing (space-separated target names or 'all') (optional)": "",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	out, err := w.Run(context.TODO(), &verboseListener{t})
+	if err != nil {
+		t.Fatal(err)
+	}
+	artifacts := out["Staged artifacts"].([]artifact)
+	byName := map[string]artifact{}
+	for _, a := range artifacts {
+		byName[a.filename] = a
+	}
+
+	checkTGZ(t, stagingDir, byName["go1.18releasetest1.src.tar.gz"], map[string]string{
+		"go/VERSION":       "go1.18releasetest1",
+		"go/src/make.bash": makeScript,
+	})
+	checkMSI(t, stagingDir, byName["go1.18releasetest1.windows-amd64.msi"])
+	checkTGZ(t, stagingDir, byName["go1.18releasetest1.linux-amd64.tar.gz"], map[string]string{
+		"go/VERSION":                        "go1.18releasetest1",
+		"go/tool/something_orother/compile": "",
+		"go/pkg/something_orother/race.a":   "",
+	})
+	checkZip(t, stagingDir, byName["go1.18releasetest1.windows-arm64.zip"], map[string]string{
+		"go/VERSION":                        "go1.18releasetest1",
+		"go/tool/something_orother/compile": "",
+	})
+	checkZip(t, stagingDir, byName["go1.18releasetest1.windows-386.zip"], map[string]string{
+		"go/VERSION":                        "go1.18releasetest1",
+		"go/tool/something_orother/compile": "",
+	})
+
+	// TODO: consider logging this to golden files?
+	for name, logs := range fakeBuildlets.logs {
+		t.Logf("%v buildlets:", name)
+		for _, group := range logs {
+			for _, line := range *group {
+				t.Log(line)
+			}
+		}
+	}
+}
+
+// makeScript pretends to be make.bash. It creates a fake go command that
+// knows how to fake the commands the release process runs.
+const makeScript = `#!/bin/bash
+
+GO=../
+mkdir -p $GO/bin
+
+cat <<'EOF' >$GO/bin/go
+#!/bin/bash -eu
+case "$1 $2" in
+"run releaselet.go")
+    # We're building an MSI. The command should be run in the gomote work dir.
+	ls go/src/make.bash >/dev/null
+	mkdir msi
+	echo "I'm an MSI!" > msi/thisisanmsi.msi
+	;;
+"install -race")
+	# Installing the race mode stdlib. Doesn't matter where it's run.
+	mkdir -p $(dirname $0)/../pkg/something_orother/
+	touch $(dirname $0)/../pkg/something_orother/race.a
+	;;
+*)
+	echo "unexpected command $@"
+	exit 1
+	;;
+esac
+EOF
+chmod 0755 $GO/bin/go
+
+cp $GO/bin/go $GO/bin/go.exe
+# We don't know what GOOS_GOARCH we're "building" for, write some junk for
+# versimilitude.
+mkdir -p $GO/tool/something_orother/
+touch $GO/tool/something_orother/compile
+`
+
+// allScript pretends to be all.bash. It is hardcoded to pass.
+const allScript = `#!/bin/bash -eu
+
+echo "I'm a test! :D"
+exit 0
+`
+
+// serveTarballs serves the files the release process relies on.
+// PutTarFromURL is hardcoded to read from this server.
+func serveTarballs(w http.ResponseWriter, r *http.Request) {
+	gzw := gzip.NewWriter(w)
+	tw := tar.NewWriter(gzw)
+	writeFile := func(name, contents string) {
+		if err := tw.WriteHeader(&tar.Header{
+			Typeflag: tar.TypeReg,
+			Name:     name,
+			Size:     int64(len(contents)),
+			Mode:     0777,
+		}); err != nil {
+			panic(err)
+		}
+		if _, err := tw.Write([]byte(contents)); err != nil {
+			panic(err)
+		}
+	}
+
+	switch {
+	case strings.Contains(r.URL.Path, "+archive"):
+		writeFile("src/make.bash", makeScript)
+		writeFile("src/make.bat", makeScript)
+		writeFile("src/all.bash", allScript)
+		writeFile("src/all.bat", allScript)
+	case strings.Contains(r.URL.Path, "go-builder-data/go"):
+		writeFile("bin/go", "I'm a dummy bootstrap go command!")
+	default:
+		panic("unknown url requested: " + r.URL.String())
+	}
+
+	if err := tw.Close(); err != nil {
+		panic(err)
+	}
+	if err := gzw.Close(); err != nil {
+		panic(err)
+	}
+}
+
+func checkMSI(t *testing.T, stagingDir string, a artifact) {
+	t.Run(a.filename, func(t *testing.T) {
+		b, err := ioutil.ReadFile(filepath.Join(stagingDir, a.stagingPath))
+		if err != nil {
+			t.Fatalf("reading %v: %v", a.filename, err)
+		}
+		if got, want := string(b), "I'm an MSI!\n"; got != want {
+			t.Fatalf("%v contains %q, want %q", a.filename, got, want)
+		}
+	})
+}
+
+func checkTGZ(t *testing.T, stagingDir string, a artifact, contents map[string]string) {
+	t.Run(a.filename, func(t *testing.T) {
+		b, err := ioutil.ReadFile(filepath.Join(stagingDir, a.stagingPath))
+		if err != nil {
+			t.Fatalf("reading %v: %v", a.filename, err)
+		}
+		gzr, err := gzip.NewReader(bytes.NewReader(b))
+		if err != nil {
+			t.Fatalf("unzipping %v: %v", a.filename, err)
+		}
+		tr := tar.NewReader(gzr)
+		for {
+			h, err := tr.Next()
+			if err == io.EOF {
+				break
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			want, ok := contents[h.Name]
+			if !ok {
+				continue
+			}
+			b, err := ioutil.ReadAll(tr)
+			if err != nil {
+				t.Fatal(err)
+			}
+			delete(contents, h.Name)
+			if string(b) != want {
+				t.Errorf("contents of %v were %q, want %q", h.Name, string(b), want)
+			}
+		}
+		if len(contents) != 0 {
+			t.Fatalf("not all files were found: missing %v", contents)
+		}
+	})
+}
+
+func checkZip(t *testing.T, stagingDir string, a artifact, contents map[string]string) {
+	t.Run(a.filename, func(t *testing.T) {
+		b, err := ioutil.ReadFile(filepath.Join(stagingDir, a.stagingPath))
+		if err != nil {
+			t.Fatalf("reading %v: %v", a.filename, err)
+		}
+		zr, err := zip.NewReader(bytes.NewReader(b), int64(len(b)))
+		if err != nil {
+			t.Fatal(err)
+		}
+		for _, f := range zr.File {
+			want, ok := contents[f.Name]
+			if !ok {
+				continue
+			}
+			r, err := zr.Open(f.Name)
+			if err != nil {
+				t.Fatal(err)
+			}
+			b, err := ioutil.ReadAll(r)
+			if err != nil {
+				t.Fatal(err)
+			}
+			delete(contents, f.Name)
+			if string(b) != want {
+				t.Errorf("contents of %v were %q, want %q", f.Name, string(b), want)
+			}
+		}
+		if len(contents) != 0 {
+			t.Fatalf("not all files were found: missing %v", contents)
+		}
+	})
+}
+
+type fakeBuildlets struct {
+	t       *testing.T
+	dir     string
+	httpURL string
+
+	mu     sync.Mutex
+	nextID int
+	logs   map[string][]*[]string
+}
+
+func (b *fakeBuildlets) createBuildlet(kind string) (buildlet.Client, error) {
+	b.mu.Lock()
+	buildletDir := filepath.Join(b.dir, kind, fmt.Sprint(b.nextID))
+	logs := &[]string{}
+	b.nextID++
+	b.logs[kind] = append(b.logs[kind], logs)
+	b.mu.Unlock()
+	logf := func(format string, args ...interface{}) {
+		line := fmt.Sprintf(format, args...)
+		line = strings.ReplaceAll(line, buildletDir, "$WORK")
+		*logs = append(*logs, line)
+	}
+	logf("--- create buildlet ---")
+
+	return &fakeBuildlet{
+		t:       b.t,
+		kind:    kind,
+		dir:     buildletDir,
+		httpURL: b.httpURL,
+		logf:    logf,
+	}, nil
+}
+
+type fakeBuildlet struct {
+	buildlet.Client
+	t       *testing.T
+	kind    string
+	dir     string
+	httpURL string
+	logf    func(string, ...interface{})
+	closed  bool
+}
+
+func (b *fakeBuildlet) Close() error {
+	if !b.closed {
+		b.logf("--- destroy buildlet ---")
+		b.closed = true
+	}
+	return nil
+}
+
+func (b *fakeBuildlet) Exec(ctx context.Context, cmd string, opts buildlet.ExecOpts) (remoteErr error, execErr error) {
+	b.logf("exec %v %v\n\twd %q env %v", cmd, opts.Args, opts.Dir, opts.ExtraEnv)
+	absCmd := filepath.Join(b.dir, cmd)
+retry:
+	c := exec.CommandContext(ctx, absCmd, opts.Args...)
+	c.Env = append(os.Environ(), opts.ExtraEnv...)
+	buf := &bytes.Buffer{}
+	var w io.Writer = buf
+	if opts.Output != nil {
+		w = io.MultiWriter(w, opts.Output)
+	}
+	c.Stdout = w
+	c.Stderr = w
+	if opts.Dir == "" {
+		c.Dir = filepath.Dir(absCmd)
+	} else {
+		c.Dir = filepath.Join(b.dir, opts.Dir)
+	}
+	err := c.Run()
+	// Work around Unix foolishness. See go.dev/issue/22315.
+	if err != nil && strings.Contains(err.Error(), "text file busy") {
+		time.Sleep(100 * time.Millisecond)
+		goto retry
+	}
+	if err != nil {
+		return nil, fmt.Errorf("command %v %v failed: %v output: %q", cmd, opts.Args, err, buf.String())
+	}
+	return nil, nil
+}
+
+func (b *fakeBuildlet) GetTar(ctx context.Context, dir string) (io.ReadCloser, error) {
+	b.logf("get tar of %q", dir)
+	buf := &bytes.Buffer{}
+	zw := gzip.NewWriter(buf)
+	tw := tar.NewWriter(zw)
+	base := filepath.Join(b.dir, filepath.FromSlash(dir))
+	// Copied pretty much wholesale from buildlet.go.
+	err := filepath.Walk(base, func(path string, fi os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		rel := strings.TrimPrefix(filepath.ToSlash(strings.TrimPrefix(path, base)), "/")
+		th, err := tar.FileInfoHeader(fi, path)
+		if err != nil {
+			return err
+		}
+		th.Name = rel
+		if fi.IsDir() && !strings.HasSuffix(th.Name, "/") {
+			th.Name += "/"
+		}
+		if th.Name == "/" {
+			return nil
+		}
+		if err := tw.WriteHeader(th); err != nil {
+			return err
+		}
+		if fi.Mode().IsRegular() {
+			f, err := os.Open(path)
+			if err != nil {
+				return err
+			}
+			defer f.Close()
+			if _, err := io.Copy(tw, f); err != nil {
+				return err
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	if err := tw.Close(); err != nil {
+		return nil, err
+	}
+	if err := zw.Close(); err != nil {
+		return nil, err
+	}
+	return ioutil.NopCloser(buf), nil
+}
+
+func (b *fakeBuildlet) ListDir(ctx context.Context, dir string, opts buildlet.ListDirOpts, fn func(buildlet.DirEntry)) error {
+	// We call this when something goes wrong, so we need it to "succeed".
+	// It's not worth implementing; return some nonsense.
+	fn(buildlet.DirEntry{
+		Line: "ListDir is silently unimplemented, sorry",
+	})
+	return nil
+}
+
+func (b *fakeBuildlet) Put(ctx context.Context, r io.Reader, path string, mode os.FileMode) error {
+	b.logf("write file %q with mode %0o", path, mode)
+	f, err := os.OpenFile(filepath.Join(b.dir, path), os.O_CREATE|os.O_RDWR, mode)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	if _, err := io.Copy(f, r); err != nil {
+		return err
+	}
+	return f.Close()
+}
+
+func (b *fakeBuildlet) PutTar(ctx context.Context, r io.Reader, dir string) error {
+	b.logf("put tar to %q", dir)
+	return untar.Untar(r, filepath.Join(b.dir, dir))
+}
+
+func (b *fakeBuildlet) PutTarFromURL(ctx context.Context, tarURL string, dir string) error {
+	b.logf("put tar from %v to %q", tarURL, dir)
+	u, err := url.Parse(tarURL)
+	if err != nil {
+		return err
+	}
+	resp, err := http.Get(b.httpURL + u.Path)
+	if err != nil {
+		return err
+	}
+	if resp.StatusCode != http.StatusOK {
+		return fmt.Errorf("unexpected status for %q: %v", tarURL, resp.Status)
+	}
+	defer resp.Body.Close()
+	return untar.Untar(resp.Body, filepath.Join(b.dir, dir))
+}
+
+func (b *fakeBuildlet) WorkDir(ctx context.Context) (string, error) {
+	return b.dir, nil
+}
+
+type verboseListener struct{ t *testing.T }
+
+func (l *verboseListener) TaskStateChanged(_ uuid.UUID, _ string, st *workflow.TaskState) error {
+	switch {
+	case !st.Finished:
+		l.t.Logf("task %-10v: started", st.Name)
+	case st.Error != "":
+		l.t.Logf("task %-10v: error: %v", st.Name, st.Error)
+	default:
+		l.t.Logf("task %-10v: done: %v", st.Name, st.Result)
+	}
+	return nil
+}
+
+func (l *verboseListener) Logger(_ uuid.UUID, task string) workflow.Logger {
+	return &testLogger{t: l.t, task: task}
+}
+
+type testLogger struct {
+	t    *testing.T
+	task string
+}
+
+func (l *testLogger) Printf(format string, v ...interface{}) {
+	l.t.Logf("task %-10v: LOG: %s", l.task, fmt.Sprintf(format, v...))
+}
diff --git a/internal/relui/workflows.go b/internal/relui/workflows.go
index f6ae866..98d1955 100644
--- a/internal/relui/workflows.go
+++ b/internal/relui/workflows.go
@@ -207,7 +207,7 @@
 
 	source := wd.Task("Build source archive", tasks.buildSource, revision, version)
 	// Artifact file paths.
-	var artifacts []workflow.Value
+	artifacts := []workflow.Value{source}
 	// Empty values that represent the dependency on tests passing.
 	var testResults []workflow.Value
 	for _, target := range targets {
@@ -236,7 +236,8 @@
 	}
 	// Eventually we need to sign artifacts and perhaps summarize test results.
 	// For now, just mush them all together.
-	stagedArtifacts := wd.Task("Stage artifacts for signing", tasks.copyToStaging, wd.Slice(artifacts))
+	stagedArtifacts := wd.Task("Stage artifacts for signing", tasks.copyToStaging, version, wd.Slice(artifacts))
+	wd.Output("Staged artifacts", stagedArtifacts)
 	results := wd.Task("Combine results", combineResults, stagedArtifacts, wd.Slice(testResults))
 	wd.Output("Build results", results)
 	return wd, nil
@@ -244,31 +245,32 @@
 
 // BuildReleaseTasks serves as an adapter to the various build tasks in the task package.
 type BuildReleaseTasks struct {
+	GerritURL              string
 	GCSClient              *storage.Client
 	ScratchURL, StagingURL string
 	CreateBuildlet         func(string) (buildlet.Client, error)
 }
 
 func (b *BuildReleaseTasks) buildSource(ctx *workflow.TaskContext, revision, version string) (artifact, error) {
-	return b.runBuildStep(ctx, nil, "", artifact{}, "source.tar.gz", func(_ *task.BuildletStep, _ io.Reader, w io.Writer) error {
-		return task.WriteSourceArchive(ctx, revision, version, w)
+	return b.runBuildStep(ctx, nil, "", artifact{}, "src.tar.gz", func(_ *task.BuildletStep, _ io.Reader, w io.Writer) error {
+		return task.WriteSourceArchive(ctx, b.GerritURL, revision, version, w)
 	})
 }
 
 func (b *BuildReleaseTasks) buildBinary(ctx *workflow.TaskContext, target *releasetargets.Target, source artifact) (artifact, error) {
-	return b.runBuildStep(ctx, target, target.Builder, source, target.Name+"-binary.tar.gz", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
+	return b.runBuildStep(ctx, target, target.Builder, source, "tar.gz", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
 		return bs.BuildBinary(ctx, r, w)
 	})
 }
 
 func (b *BuildReleaseTasks) buildMSI(ctx *workflow.TaskContext, target *releasetargets.Target, binary artifact) (artifact, error) {
-	return b.runBuildStep(ctx, target, target.Builder, binary, target.Name+".msi", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
+	return b.runBuildStep(ctx, target, target.Builder, binary, "msi", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
 		return bs.BuildMSI(ctx, r, w)
 	})
 }
 
 func (b *BuildReleaseTasks) convertToZip(ctx *workflow.TaskContext, target *releasetargets.Target, binary artifact) (artifact, error) {
-	return b.runBuildStep(ctx, nil, "", binary, target.Name+".zip", func(_ *task.BuildletStep, r io.Reader, w io.Writer) error {
+	return b.runBuildStep(ctx, target, "", binary, "zip", func(_ *task.BuildletStep, r io.Reader, w io.Writer) error {
 		return task.ConvertTGZToZIP(r, w)
 	})
 }
@@ -294,22 +296,22 @@
 // runBuildStep is a convenience function that manages resources a build step might need.
 // If target and buildlet name are specified, a BuildletStep will be passed to f.
 // If inputName is specified, it will be opened and passed as a Reader to f.
-// If outputName is specified, a unique filename will be generated based off it, the file
-// will be opened and passed as a Writer to f, and its name will be returned as the result.
+// If outputSuffix is specified, a unique filename will be generated based off
+// it (and the target name, if any), the file will be opened and passed as a
+// Writer to f, and an artifact representing it will be returned as the result.
 func (b *BuildReleaseTasks) runBuildStep(
 	ctx *workflow.TaskContext,
 	target *releasetargets.Target,
 	buildletName string,
 	input artifact,
-	outputBase string,
+	outputSuffix string,
 	f func(*task.BuildletStep, io.Reader, io.Writer) error,
 ) (artifact, error) {
-	if (target == nil) != (buildletName == "") {
-		return artifact{}, fmt.Errorf("target and buildlet must be specified together")
-	}
-
 	var step *task.BuildletStep
-	if target != nil {
+	if buildletName != "" {
+		if target == nil {
+			return artifact{}, fmt.Errorf("target must be specified to use a buildlet")
+		}
 		ctx.Printf("Creating buildlet %v.", buildletName)
 		client, err := b.CreateBuildlet(buildletName)
 		if err != nil {
@@ -342,13 +344,17 @@
 		defer in.Close()
 	}
 	var out io.WriteCloser
-	var outputName string
+	var scratchPath string
 	hash := sha256.New()
 	size := &sizeWriter{}
 	var multiOut io.Writer
-	if outputBase != "" {
-		outputName = fmt.Sprintf("%v/%v-%v", ctx.WorkflowID.String(), outputBase, rand.Int63())
-		out, err = gcsfs.Create(scratchFS, outputName)
+	if outputSuffix != "" {
+		scratchName := outputSuffix
+		if target != nil {
+			scratchName = target.Name + "." + outputSuffix
+		}
+		scratchPath = fmt.Sprintf("%v/%v-%v", ctx.WorkflowID.String(), scratchName, rand.Int63())
+		out, err = gcsfs.Create(scratchFS, scratchPath)
 		if err != nil {
 			return artifact{}, err
 		}
@@ -377,20 +383,27 @@
 	}
 	return artifact{
 		target:      target,
-		scratchPath: outputName,
-		filename:    outputBase,
+		scratchPath: scratchPath,
+		suffix:      outputSuffix,
 		sha256:      fmt.Sprintf("%x", string(hash.Sum([]byte(nil)))),
 		size:        size.size,
 	}, nil
 }
 
 type artifact struct {
-	target      *releasetargets.Target
+	// The target platform of this artifact, or nil for source.
+	target *releasetargets.Target
+	// The scratch path of this artifact.
 	scratchPath string
+	// The path the artifact was staged to for the signing process.
 	stagingPath string
-	filename    string
-	sha256      string
-	size        int
+	// The filename suffix of the artifact, e.g. "tar.gz" or "src.tar.gz",
+	// combined with the version and target name to produce filename.
+	suffix string
+	// The final filename of this artifact as it will be downloaded.
+	filename string
+	sha256   string
+	size     int
 }
 
 type sizeWriter struct {
@@ -406,7 +419,7 @@
 	return fmt.Sprintf("%#v\n\n", artifacts) + strings.Join(tests, "\n"), nil
 }
 
-func (tasks *BuildReleaseTasks) copyToStaging(ctx *workflow.TaskContext, artifacts []artifact) ([]artifact, error) {
+func (tasks *BuildReleaseTasks) copyToStaging(ctx *workflow.TaskContext, version string, artifacts []artifact) ([]artifact, error) {
 	scratchFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.ScratchURL)
 	if err != nil {
 		return nil, err
@@ -418,7 +431,12 @@
 	var stagedArtifacts []artifact
 	for _, a := range artifacts {
 		staged := a
-		staged.stagingPath = path.Join(ctx.WorkflowID.String(), a.filename)
+		if a.target != nil {
+			staged.filename = version + "." + a.target.Name + "." + a.suffix
+		} else {
+			staged.filename = version + "." + a.suffix
+		}
+		staged.stagingPath = path.Join(ctx.WorkflowID.String(), staged.filename)
 		stagedArtifacts = append(stagedArtifacts, staged)
 
 		in, err := scratchFS.Open(a.scratchPath)
@@ -439,5 +457,5 @@
 			return nil, err
 		}
 	}
-	return artifacts, nil
+	return stagedArtifacts, nil
 }
diff --git a/internal/task/buildrelease.go b/internal/task/buildrelease.go
index abecafb..b5801f4 100644
--- a/internal/task/buildrelease.go
+++ b/internal/task/buildrelease.go
@@ -24,9 +24,9 @@
 )
 
 // WriteSourceArchive writes a source archive to out, based on revision with version written in as VERSION.
-func WriteSourceArchive(ctx *workflow.TaskContext, revision, version string, out io.Writer) error {
+func WriteSourceArchive(ctx *workflow.TaskContext, gerritURL, revision, version string, out io.Writer) error {
 	ctx.Printf("Create source archive.")
-	tarURL := "https://go.googlesource.com/go/+archive/" + revision + ".tar.gz"
+	tarURL := gerritURL + "/go/+archive/" + revision + ".tar.gz"
 	resp, err := http.Get(tarURL)
 	if err != nil {
 		return err