diff --git a/buildlet/buildletclient.go b/buildlet/buildletclient.go index 8098a73d..0e4f98b1 100644 --- a/buildlet/buildletclient.go +++ b/buildlet/buildletclient.go @@ -128,6 +128,10 @@ type ExecOpts struct { // 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 @@ func (c *Client) Exec(cmd string, opts ExecOpts) (remoteErr, execErr error) { "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 93f57d9c..46cb95b2 100644 --- a/cmd/buildlet/Makefile +++ b/cmd/buildlet/Makefile @@ -7,7 +7,12 @@ buildlet.darwin-amd64: buildlet.go 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 75ce410d..92a15634 100644 --- a/cmd/buildlet/buildlet.go +++ b/cmd/buildlet/buildlet.go @@ -39,7 +39,7 @@ import ( 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 @@ func main() { 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 @@ func fixMTU_plan9() error { 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 @@ func fixMTU() { } } -// 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 @@ func handleRoot(w http.ResponseWriter, r *http.Request) { } // 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 @@ func validRelativeDir(dir string) bool { 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 @@ func handleGetTGZ(rw http.ResponseWriter, r *http.Request) { 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 @@ func handleWriteTGZ(w http.ResponseWriter, r *http.Request) { } 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 @@ func handleExec(w http.ResponseWriter, r *http.Request) { 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 @@ func handleExec(w http.ResponseWriter, r *http.Request) { 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 23981429..9462b687 100644 --- a/cmd/coordinator/coordinator.go +++ b/cmd/coordinator/coordinator.go @@ -42,8 +42,6 @@ import ( ) 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 @@ func buildInVM(conf dashboard.BuildConfig, st *buildStatus) (retErr error) { 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 aeaa0a9d..f959c8a1 100644 --- a/cmd/gomote/run.go +++ b/cmd/gomote/run.go @@ -22,6 +22,8 @@ func run(args []string) error { } 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 @@ func run(args []string) error { 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 685aafcd..f079a29e 100644 --- a/dashboard/builders.go +++ b/dashboard/builders.go @@ -30,13 +30,17 @@ type BuildConfig struct { 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 @@ func (c *BuildConfig) GOARCH() string { // 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" } @@ -148,6 +156,27 @@ func init() { addBuilder(BuildConfig{Name: "linux-amd64-clang", Image: "gobuilders/linux-x86-clang"}) // 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", @@ -155,13 +184,14 @@ func init() { 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: