all: add env support, freebsd-386, freebsd-amd64-race, re-enable plan9
Update golang/go#9491
Update golang/go#8639
Change-Id: I1f43c751777f10a6d5870ca9bbb8c2a3430189bf
Reviewed-on: https://go-review.googlesource.com/3170
Reviewed-by: Andrew Gerrand <adg@golang.org>
diff --git a/buildlet/buildletclient.go b/buildlet/buildletclient.go
index 8098a73..0e4f98b 100644
--- a/buildlet/buildletclient.go
+++ b/buildlet/buildletclient.go
@@ -128,6 +128,10 @@
// Args are the arguments to pass to the cmd given to Client.Exec.
Args []string
+ // ExtraEnv are KEY=VALUE pairs to append to the buildlet
+ // process's environment.
+ ExtraEnv []string
+
// SystemLevel controls whether the command is run outside of
// the buildlet's environment.
SystemLevel bool
@@ -154,6 +158,7 @@
"cmd": {cmd},
"mode": {mode},
"cmdArg": opts.Args,
+ "env": opts.ExtraEnv,
}
req, err := http.NewRequest("POST", c.URL()+"/exec", strings.NewReader(form.Encode()))
if err != nil {
diff --git a/cmd/buildlet/Makefile b/cmd/buildlet/Makefile
index 93f57d9..46cb95b 100644
--- a/cmd/buildlet/Makefile
+++ b/cmd/buildlet/Makefile
@@ -7,7 +7,12 @@
buildlet.freebsd-amd64: buildlet.go
GOOS=freebsd GOARCH=amd64 go build -o $@
- cat $@ | (cd ../upload && go run upload.go --public go-builder-data/$@)
+
+# We use the same binary for freebsd-386 as freebsd-amd64:
+.PHONY: freebsd
+freebsd: buildlet.freebsd-amd64
+ cat buildlet.freebsd-amd64 | (cd ../upload && go run upload.go --public go-builder-data/buildlet.freebsd-amd64)
+ cat buildlet.freebsd-amd64 | (cd ../upload && go run upload.go --public go-builder-data/buildlet.freebsd-386)
buildlet.linux-amd64: buildlet.go
GOOS=linux GOARCH=amd64 go build -o $@
diff --git a/cmd/buildlet/buildlet.go b/cmd/buildlet/buildlet.go
index 75ce410..92a1563 100644
--- a/cmd/buildlet/buildlet.go
+++ b/cmd/buildlet/buildlet.go
@@ -39,7 +39,7 @@
var (
haltEntireOS = flag.Bool("halt", true, "halt OS in /halt handler. If false, the buildlet process just ends.")
- scratchDir = flag.String("scratchdir", "", "Temporary directory to use. The contents of this directory may be deleted at any time. If empty, TempDir is used to create one.")
+ workDir = flag.String("workdir", "", "Temporary directory to use. The contents of this directory may be deleted at any time. If empty, TempDir is used to create one.")
listenAddr = flag.String("listen", defaultListenAddr(), "address to listen on. Warning: this service is inherently insecure and offers no protection of its own. Do not expose this port to the world.")
)
@@ -72,33 +72,21 @@
if onGCE {
fixMTU()
}
- if runtime.GOOS == "plan9" {
- // Plan 9 is too slow on GCE, so stop running run.rc after the basics.
- // See https://golang.org/cl/2522 and https://golang.org/issue/9491
- // TODO(bradfitz): once the buildlet has environment variable support,
- // the coordinator can send this in, and this variable can be part of
- // the build configuration struct instead of hard-coded here.
- // But no need for environment variables quite yet.
- os.Setenv("GOTESTONLY", "std")
- }
- if *scratchDir == "" {
+ if *workDir == "" {
dir, err := ioutil.TempDir("", "buildlet-scatch")
if err != nil {
- log.Fatalf("error creating scratchdir with ioutil.TempDir: %v", err)
+ log.Fatalf("error creating workdir with ioutil.TempDir: %v", err)
}
- *scratchDir = dir
+ *workDir = dir
}
- // TODO(bradfitz): if this becomes more of a general tool,
- // perhaps we want to remove this hard-coded here. Also,
- // if/once the exec handler ever gets generic environment
- // variable support, it would make sense to remove this too
- // and push it to the client. This hard-codes policy. But
- // that's okay for now.
- os.Setenv("GOROOT_BOOTSTRAP", filepath.Join(*scratchDir, "go1.4"))
- os.Setenv("WORKDIR", *scratchDir) // mostly for demos
+ // This is hard-coded because the client-supplied environment has
+ // no way to expand relative paths from the workDir.
+ // TODO(bradfitz): if we ever need more than this, make some mechanism.
+ os.Setenv("GOROOT_BOOTSTRAP", filepath.Join(*workDir, "go1.4"))
+ os.Setenv("WORKDIR", *workDir) // mostly for demos
- if _, err := os.Lstat(*scratchDir); err != nil {
- log.Fatalf("invalid --scratchdir %q: %v", *scratchDir, err)
+ if _, err := os.Lstat(*workDir); err != nil {
+ log.Fatalf("invalid --workdir %q: %v", *workDir, err)
}
http.HandleFunc("/", handleRoot)
http.HandleFunc("/debug/goroutines", handleGoroutines)
@@ -208,7 +196,7 @@
if err != nil {
return err
}
- if _, err := io.WriteString(f, "mtu 1400\n"); err != nil { // not 1460
+ if _, err := io.WriteString(f, "mtu 1460\n"); err != nil {
f.Close()
return err
}
@@ -230,46 +218,18 @@
}
}
-// mtuWriter is a hack for environments where we can't (or can't yet)
-// fix the machine's MTU.
-// Instead of telling the operating system the MTU, we just cut up our
-// writes into small pieces to make sure we don't get too near the
-// MTU, and we hope the kernel doesn't coalesce different flushed
-// writes back together into the same TCP IP packets.
-// TODO(bradfitz): this has never proven to work. See:
-// https://github.com/golang/go/issues/9491#issuecomment-70881865
-// But we do depend on its http.Flusher.Flush-per-Write behavior, so we
-// can't delete this entirely.
-type mtuWriter struct {
+// flushWriter is an io.Writer that Flushes after each Write if the
+// underlying Writer implements http.Flusher.
+type flushWriter struct {
rw http.ResponseWriter
}
-func (mw mtuWriter) Write(p []byte) (n int, err error) {
- const mtu = 1000 // way less than 1460; since HTTP response headers might be in there too
- for len(p) > 0 {
- chunk := p
- if len(chunk) > mtu {
- chunk = p[:mtu]
- }
- n0, err := mw.rw.Write(chunk)
- n += n0
- if n0 != len(chunk) && err == nil {
- err = io.ErrShortWrite
- }
- if err != nil {
- return n, err
- }
- p = p[n0:]
- mw.rw.(http.Flusher).Flush()
- if len(p) > 0 && runtime.GOOS == "plan9" {
- // Try to prevent the kernel from Nagel-ing the IP packets
- // together into one that's too large.
- // This didn't fix anything, though.
- // See https://github.com/golang/go/issues/9491#issuecomment-70881865
- //time.Sleep(250 * time.Millisecond)
- }
+func (fw flushWriter) Write(p []byte) (n int, err error) {
+ n, err = fw.rw.Write(p)
+ if f, ok := fw.rw.(http.Flusher); ok {
+ f.Flush()
}
- return n, nil
+ return
}
func handleRoot(w http.ResponseWriter, r *http.Request) {
@@ -281,10 +241,9 @@
}
// unauthenticated /debug/goroutines handler
-func handleGoroutines(rw http.ResponseWriter, r *http.Request) {
- w := mtuWriter{rw}
+func handleGoroutines(w http.ResponseWriter, r *http.Request) {
log.Printf("Dumping goroutines.")
- rw.Header().Set("Content-Type", "text/plain; charset=utf-8")
+ w.Header().Set("Content-Type", "text/plain; charset=utf-8")
buf := make([]byte, 2<<20)
buf = buf[:runtime.Stack(buf, true)]
w.Write(buf)
@@ -319,19 +278,19 @@
return true
}
-func handleGetTGZ(rw http.ResponseWriter, r *http.Request) {
+func handleGetTGZ(w http.ResponseWriter, r *http.Request) {
if r.Method != "GET" {
- http.Error(rw, "requires GET method", http.StatusBadRequest)
+ http.Error(w, "requires GET method", http.StatusBadRequest)
return
}
dir := r.FormValue("dir")
if !validRelativeDir(dir) {
- http.Error(rw, "bogus dir", http.StatusBadRequest)
+ http.Error(w, "bogus dir", http.StatusBadRequest)
return
}
- zw := gzip.NewWriter(mtuWriter{rw})
+ zw := gzip.NewWriter(w)
tw := tar.NewWriter(zw)
- base := filepath.Join(*scratchDir, filepath.FromSlash(dir))
+ base := filepath.Join(*workDir, filepath.FromSlash(dir))
err := filepath.Walk(base, func(path string, fi os.FileInfo, err error) error {
if err != nil {
return err
@@ -374,7 +333,7 @@
log.Printf("Walk error: %v", err)
// Decent way to signal failure to the caller, since it'll break
// the chunked response, rather than have a valid EOF.
- conn, _, _ := rw.(http.Hijacker).Hijack()
+ conn, _, _ := w.(http.Hijacker).Hijack()
conn.Close()
}
tw.Close()
@@ -409,7 +368,7 @@
}
urlParam, _ := url.ParseQuery(r.URL.RawQuery)
- baseDir := *scratchDir
+ baseDir := *workDir
if dir := urlParam.Get("dir"); dir != "" {
if !validRelativeDir(dir) {
http.Error(w, "bogus dir", http.StatusBadRequest)
@@ -527,7 +486,7 @@
http.Error(w, "requires 'cmd' parameter", http.StatusBadRequest)
return
}
- absCmd = filepath.Join(*scratchDir, filepath.FromSlash(cmdPath))
+ absCmd = filepath.Join(*workDir, filepath.FromSlash(cmdPath))
}
if f, ok := w.(http.Flusher); ok {
@@ -536,13 +495,14 @@
cmd := exec.Command(absCmd, r.PostForm["cmdArg"]...)
if sysMode {
- cmd.Dir = *scratchDir
+ cmd.Dir = *workDir
} else {
cmd.Dir = filepath.Dir(absCmd)
}
- cmdOutput := mtuWriter{w}
+ cmdOutput := flushWriter{w}
cmd.Stdout = cmdOutput
cmd.Stderr = cmdOutput
+ cmd.Env = append(os.Environ(), r.PostForm["env"]...)
err := cmd.Start()
if err == nil {
go func() {
diff --git a/cmd/coordinator/coordinator.go b/cmd/coordinator/coordinator.go
index 2398142..9462b68 100644
--- a/cmd/coordinator/coordinator.go
+++ b/cmd/coordinator/coordinator.go
@@ -42,8 +42,6 @@
)
func init() {
- // Disabled until its networking/MTU problems are fixed:
- // https://github.com/golang/go/issues/9491#issuecomment-70881865
delete(dashboard.Builders, "plan9-386-gcepartial")
}
@@ -797,6 +795,7 @@
remoteErr, err := bc.Exec(path.Join("go", conf.AllScript()), buildlet.ExecOpts{
Output: st,
OnStartExec: func() { st.logEventTime("running_exec") },
+ ExtraEnv: conf.Env(),
})
if err != nil {
return err
diff --git a/cmd/gomote/run.go b/cmd/gomote/run.go
index aeaa0a9..f959c8a 100644
--- a/cmd/gomote/run.go
+++ b/cmd/gomote/run.go
@@ -22,6 +22,8 @@
}
var sys bool
fs.BoolVar(&sys, "system", false, "run inside the system, and not inside the workdir; this is implicit if cmd starts with '/'")
+ var env stringSlice
+ fs.Var(&env, "e", "Environment variable KEY=value. The -e flag may be repeated multiple times to add multiple things to the environment.")
fs.Parse(args)
if fs.NArg() < 2 {
@@ -37,9 +39,26 @@
SystemLevel: sys || strings.HasPrefix(cmd, "/"),
Output: os.Stdout,
Args: fs.Args()[2:],
+ ExtraEnv: []string(env),
})
if execErr != nil {
return fmt.Errorf("Error trying to execute %s: %v", cmd, execErr)
}
return remoteErr
}
+
+// stringSlice implements flag.Value, specifically for storing environment
+// variable key=value pairs.
+type stringSlice []string
+
+func (*stringSlice) String() string { return "" } // default value
+
+func (ss *stringSlice) Set(v string) error {
+ if v != "" {
+ if !strings.Contains(v, "=") {
+ return fmt.Errorf("-e argument %q doesn't contains an '=' sign.", v)
+ }
+ *ss = append(*ss, v)
+ }
+ return nil
+}
diff --git a/dashboard/builders.go b/dashboard/builders.go
index 685aafc..f079a29 100644
--- a/dashboard/builders.go
+++ b/dashboard/builders.go
@@ -30,13 +30,17 @@
Go14URL string // URL to built Go 1.4 tar.gz
// Docker-specific settings: (used if VMImage == "")
- Image string // Docker image to use to build
- cmd string // optional -cmd flag (relative to go/src/)
- env []string // extra environment ("key=value") pairs
- dashURL string // url of the build dashboard
- tool string // the tool this configuration is for
+ Image string // Docker image to use to build
+ cmd string // optional -cmd flag (relative to go/src/)
+ dashURL string // url of the build dashboard
+ tool string // the tool this configuration is for
+
+ // Use by both VMs and Docker:
+ env []string // extra environment ("key=value") pairs
}
+func (c *BuildConfig) Env() []string { return append([]string(nil), c.env...) }
+
func (c *BuildConfig) GOOS() string { return c.Name[:strings.Index(c.Name, "-")] }
func (c *BuildConfig) GOARCH() string {
@@ -52,14 +56,18 @@
// do the build and run its standard set of tests.
// Example values are "src/all.bash", "src/all.bat", "src/all.rc".
func (c *BuildConfig) AllScript() string {
+ if strings.HasSuffix(c.Name, "-race") {
+ if strings.HasPrefix(c.Name, "windows-") {
+ return "src/race.bat"
+ }
+ return "src/race.bash"
+ }
if strings.HasPrefix(c.Name, "windows-") {
return "src/all.bat"
}
if strings.HasPrefix(c.Name, "plan9-") {
return "src/all.rc"
}
- // TODO(bradfitz): race.bash, etc, once the race builder runs
- // via the buildlet.
return "src/all.bash"
}
@@ -149,19 +157,41 @@
// VMs:
addBuilder(BuildConfig{
+ Name: "freebsd-amd64-gce101",
+ VMImage: "freebsd-amd64-gce101",
+ machineType: "n1-highcpu-2",
+ Go14URL: "https://storage.googleapis.com/go-builder-data/go1.4-freebsd-amd64.tar.gz",
+ env: []string{"CC=clang"},
+ })
+ addBuilder(BuildConfig{
+ Name: "freebsd-amd64-race",
+ VMImage: "freebsd-amd64-gce101",
+ machineType: "n1-highcpu-4",
+ Go14URL: "https://storage.googleapis.com/go-builder-data/go1.4-freebsd-amd64.tar.gz",
+ env: []string{"CC=clang"},
+ })
+ addBuilder(BuildConfig{
+ Name: "freebsd-386-gce101",
+ VMImage: "freebsd-amd64-gce101",
+ machineType: "n1-highcpu-2",
+ Go14URL: "https://storage.googleapis.com/go-builder-data/go1.4-freebsd-amd64.tar.gz",
+ env: []string{"GOARCH=386", "CC=clang"},
+ })
+ addBuilder(BuildConfig{
Name: "openbsd-amd64-gce56",
VMImage: "openbsd-amd64-56",
machineType: "n1-highcpu-2",
Go14URL: "https://storage.googleapis.com/go-builder-data/go1.4-openbsd-amd64.tar.gz",
})
addBuilder(BuildConfig{
+ Name: "plan9-386-gcepartial",
+ VMImage: "plan9-386",
+ Go14URL: "https://storage.googleapis.com/go-builder-data/go1.4-plan9-386.tar.gz",
// It's named "partial" because the buildlet sets
// GOTESTONLY=std to stop after the "go test std"
// tests because it's so slow otherwise.
- // TODO(braditz): move that env variable to the
- // coordinator and into this config.
- Name: "plan9-386-gcepartial",
- VMImage: "plan9-386",
+ env: []string{"GOTESTONLY=std"},
+
// We *were* using n1-standard-1 because Plan 9 can only
// reliably use a single CPU. Using 2 or 4 and we see
// test failures. See:
