lib: gosh/v23test: address gosh TODOs, update v23 code
MultiPart: 3/5
Change-Id: I349048b7014210fb9c45f814f247d601fac7ab7b
diff --git a/gosh/.api b/gosh/.api
index 3206ab5..472dccb 100644
--- a/gosh/.api
+++ b/gosh/.api
@@ -8,16 +8,20 @@
pkg gosh, func SendReady()
pkg gosh, func SendVars(map[string]string)
pkg gosh, func WatchParent()
+pkg gosh, method (*Cmd) AddStderrWriter(io.Writer)
+pkg gosh, method (*Cmd) AddStdoutWriter(io.Writer)
pkg gosh, method (*Cmd) AwaitReady()
pkg gosh, method (*Cmd) AwaitVars(...string) map[string]string
-pkg gosh, method (*Cmd) CombinedOutput() string
-pkg gosh, method (*Cmd) Output() (string, string)
+pkg gosh, method (*Cmd) Clone() *Cmd
pkg gosh, method (*Cmd) Process() *os.Process
pkg gosh, method (*Cmd) Run()
pkg gosh, method (*Cmd) Shutdown(os.Signal)
pkg gosh, method (*Cmd) Start()
pkg gosh, method (*Cmd) StderrPipe() io.Reader
+pkg gosh, method (*Cmd) StdinPipe() io.WriteCloser
+pkg gosh, method (*Cmd) Stdout() string
pkg gosh, method (*Cmd) StdoutPipe() io.Reader
+pkg gosh, method (*Cmd) StdoutStderr() (string, string)
pkg gosh, method (*Cmd) Wait()
pkg gosh, method (*Fn) Call(...interface{}) error
pkg gosh, method (*Shell) AddToCleanup(func())
@@ -39,16 +43,17 @@
pkg gosh, type Cmd struct, Err error
pkg gosh, type Cmd struct, ExitErrorIsOk bool
pkg gosh, type Cmd struct, OutputDir string
-pkg gosh, type Cmd struct, Stdin io.Reader
-pkg gosh, type Cmd struct, SuppressOutput bool
+pkg gosh, type Cmd struct, Path string
+pkg gosh, type Cmd struct, PropagateOutput bool
+pkg gosh, type Cmd struct, Stdin string
pkg gosh, type Cmd struct, Vars map[string]string
pkg gosh, type Fn struct
pkg gosh, type Opts struct
pkg gosh, type Opts struct, BinDir string
pkg gosh, type Opts struct, ChildOutputDir string
-pkg gosh, type Opts struct, Errorf func(string, ...interface{})
+pkg gosh, type Opts struct, Fatalf func(string, ...interface{})
pkg gosh, type Opts struct, Logf func(string, ...interface{})
-pkg gosh, type Opts struct, SuppressChildOutput bool
+pkg gosh, type Opts struct, PropagateChildOutput bool
pkg gosh, type Shell struct
pkg gosh, type Shell struct, Args []string
pkg gosh, type Shell struct, Err error
diff --git a/gosh/buffered_pipe.go b/gosh/buffered_pipe.go
index 0cae34f..b7cd44b 100644
--- a/gosh/buffered_pipe.go
+++ b/gosh/buffered_pipe.go
@@ -6,15 +6,14 @@
import (
"bytes"
- "errors"
"io"
"sync"
)
type bufferedPipe struct {
- cond *sync.Cond
- buf bytes.Buffer
- err error
+ cond *sync.Cond
+ buf bytes.Buffer
+ closed bool
}
// NewBufferedPipe returns a new thread-safe pipe backed by an unbounded
@@ -29,11 +28,12 @@
p.cond.L.Lock()
defer p.cond.L.Unlock()
for {
+ // Read any remaining data before checking whether the pipe is closed.
if p.buf.Len() > 0 {
return p.buf.Read(d)
}
- if p.err != nil {
- return 0, p.err
+ if p.closed {
+ return 0, io.EOF
}
p.cond.Wait()
}
@@ -43,8 +43,8 @@
func (p *bufferedPipe) Write(d []byte) (n int, err error) {
p.cond.L.Lock()
defer p.cond.L.Unlock()
- if p.err != nil {
- return 0, errors.New("write on closed pipe")
+ if p.closed {
+ return 0, io.ErrClosedPipe
}
defer p.cond.Signal()
return p.buf.Write(d)
@@ -54,9 +54,9 @@
func (p *bufferedPipe) Close() error {
p.cond.L.Lock()
defer p.cond.L.Unlock()
- if p.err == nil {
+ if !p.closed {
defer p.cond.Signal()
- p.err = io.EOF
+ p.closed = true
}
return nil
}
diff --git a/gosh/buffered_pipe_test.go b/gosh/buffered_pipe_test.go
new file mode 100644
index 0000000..593f77b
--- /dev/null
+++ b/gosh/buffered_pipe_test.go
@@ -0,0 +1,18 @@
+// Copyright 2015 The Vanadium 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 gosh_test
+
+import (
+ "testing"
+
+ "v.io/x/lib/gosh"
+)
+
+func TestReadAfterClose(t *testing.T) {
+ p := gosh.NewBufferedPipe()
+ p.Write([]byte("foo"))
+ p.Close()
+ eq(t, toString(t, p), "foo")
+}
diff --git a/gosh/cmd.go b/gosh/cmd.go
index 19ccdfb..9c69d01 100644
--- a/gosh/cmd.go
+++ b/gosh/cmd.go
@@ -13,6 +13,7 @@
"os"
"os/exec"
"path/filepath"
+ "strings"
"sync"
"time"
)
@@ -28,50 +29,58 @@
type Cmd struct {
// Err is the most recent error from this Cmd (may be nil).
Err error
+ // Path is the path of the command to run.
+ Path string
// Vars is the map of env vars for this Cmd.
Vars map[string]string
// Args is the list of args for this Cmd.
Args []string
- // Stdin specifies this Cmd's stdin. See comments in exec.Cmd for detailed
- // semantics.
- Stdin io.Reader
- // SuppressOutput is inherited from Shell.Opts.SuppressChildOutput.
- SuppressOutput bool
+ // PropagateOutput is inherited from Shell.Opts.PropagateChildOutput.
+ PropagateOutput bool
// OutputDir is inherited from Shell.Opts.ChildOutputDir.
OutputDir string
// ExitErrorIsOk specifies whether an *exec.ExitError should be reported via
// Shell.HandleError.
ExitErrorIsOk bool
+ // Stdin is a string to write to the child's stdin.
+ Stdin string
// Internal state.
- sh *Shell
- c *exec.Cmd
- name string
- calledWait bool
- stdoutWriters []io.Writer
- stderrWriters []io.Writer
- closeAfterWait []io.Closer
- condReady *sync.Cond
- recvReady bool // protected by condReady.L
- condVars *sync.Cond
- recvVars map[string]string // protected by condVars.L
+ sh *Shell
+ c *exec.Cmd
+ stdinWriteCloser io.WriteCloser // from exec.Cmd.StdinPipe
+ calledStart bool
+ calledWait bool
+ waitChan chan error
+ started bool // protected by sh.cleanupMu
+ exitedMu sync.Mutex
+ exited bool // protected by exitedMu
+ stdoutWriters []io.Writer
+ stderrWriters []io.Writer
+ closers []io.Closer
+ condReady *sync.Cond
+ recvReady bool // protected by condReady.L
+ condVars *sync.Cond
+ recvVars map[string]string // protected by condVars.L
}
-// TODO(sadovsky):
-// - Change "Errorf" to "Fatalf" everywhere.
-// - Drop "CombinedOutput", change "Output" to "StdoutStderr", and add "Stdout".
-// - Add Cmd.Clone method that returns a new Cmd with the same configuration.
-// - Add Cmd.StdinPipe method that creates a new bufferedPipe, returns a
-// WriteCloser, and stores a ReadCloser. Cmd.StdinPipe cannot be called more
-// than once.
-// - In Cmd.Start, start a goroutine that monitors the started process and
-// closes the stdinPipe ReadCloser (if it's not already closed) when the
-// process exits.
-// - Make it so awaitReady and awaitVars return an error if/when our goroutine
-// detects that the process has exited.
-// - Add unit test for piping from one Cmd's stdout to another's stdin. It
-// should be possible to wait on just the last Cmd.
-// - Add unit test to demonstrate that Cmd.Wait returns immediately for a
-// process that blocks on non-nil stdin if Cmd.StdinPipe was never called.
+// Clone returns a new Cmd with a copy of this Cmd's configuration.
+func (c *Cmd) Clone() *Cmd {
+ c.sh.Ok()
+ res, err := c.clone()
+ c.handleError(err)
+ return res
+}
+
+// StdinPipe returns a thread-safe WriteCloser backed by a buffered pipe for the
+// command's stdin. The returned WriteCloser will be closed when the process
+// exits. Must be called before Start. It is safe to call StdinPipe multiple
+// times; calls after the first return the pipe created by the first call.
+func (c *Cmd) StdinPipe() io.WriteCloser {
+ c.sh.Ok()
+ res, err := c.stdinPipe()
+ c.handleError(err)
+ return res
+}
// StdoutPipe returns a Reader backed by a buffered pipe for the command's
// stdout. Must be called before Start. May be called more than once; each
@@ -93,6 +102,22 @@
return res
}
+// AddStdoutWriter configures this Cmd to tee the child's stdout to the given
+// Writer. If this Writer is a Closer and is not os.Stdout or os.Stderr, it will
+// be closed when the process exits.
+func (c *Cmd) AddStdoutWriter(w io.Writer) {
+ c.sh.Ok()
+ c.handleError(c.addStdoutWriter(w))
+}
+
+// AddStderrWriter configures this Cmd to tee the child's stderr to the given
+// Writer. If this Writer is a Closer and is not os.Stdout or os.Stderr, it will
+// be closed when the process exits.
+func (c *Cmd) AddStderrWriter(w io.Writer) {
+ c.sh.Ok()
+ c.handleError(c.addStderrWriter(w))
+}
+
// Start starts the command.
func (c *Cmd) Start() {
c.sh.Ok()
@@ -136,24 +161,23 @@
c.handleError(c.run())
}
-// Output calls Start followed by Wait, then returns the command's stdout and
-// stderr.
-func (c *Cmd) Output() (string, string) {
+// Stdout calls Start followed by Wait, then returns the command's stdout.
+func (c *Cmd) Stdout() string {
c.sh.Ok()
- stdout, stderr, err := c.output()
- c.handleError(err)
- return stdout, stderr
-}
-
-// CombinedOutput calls Start followed by Wait, then returns the command's
-// combined stdout and stderr.
-func (c *Cmd) CombinedOutput() string {
- c.sh.Ok()
- res, err := c.combinedOutput()
+ res, err := c.stdout()
c.handleError(err)
return res
}
+// StdoutStderr calls Start followed by Wait, then returns the command's stdout
+// and stderr.
+func (c *Cmd) StdoutStderr() (string, string) {
+ c.sh.Ok()
+ stdout, stderr, err := c.stdoutStderr()
+ c.handleError(err)
+ return stdout, stderr
+}
+
// Process returns the underlying process handle for the command.
func (c *Cmd) Process() *os.Process {
c.sh.Ok()
@@ -165,20 +189,14 @@
////////////////////////////////////////
// Internals
-func newCmd(sh *Shell, vars map[string]string, name string, args ...string) (*Cmd, error) {
- // Mimics https://golang.org/src/os/exec/exec.go Command.
- if filepath.Base(name) == name {
- if lp, err := exec.LookPath(name); err != nil {
- return nil, err
- } else {
- name = lp
- }
- }
+func newCmdInternal(sh *Shell, vars map[string]string, path string, args []string) (*Cmd, error) {
c := &Cmd{
+ Path: path,
Vars: vars,
Args: args,
sh: sh,
- name: name,
+ c: &exec.Cmd{},
+ waitChan: make(chan error, 1),
condReady: sync.NewCond(&sync.Mutex{}),
condVars: sync.NewCond(&sync.Mutex{}),
recvVars: map[string]string{},
@@ -193,28 +211,60 @@
return c, nil
}
-func (c *Cmd) handleError(err error) {
- c.Err = err
- if c.ExitErrorIsOk {
- if _, ok := err.(*exec.ExitError); ok {
- return
+func newCmd(sh *Shell, vars map[string]string, name string, args ...string) (*Cmd, error) {
+ // Mimics https://golang.org/src/os/exec/exec.go Command.
+ if filepath.Base(name) == name {
+ if lp, err := exec.LookPath(name); err != nil {
+ return nil, err
+ } else {
+ name = lp
}
}
- c.sh.HandleError(err)
+ return newCmdInternal(sh, vars, name, args)
}
-func (c *Cmd) calledStart() bool {
- return c.c != nil
+func (c *Cmd) errorIsOk(err error) bool {
+ if c.ExitErrorIsOk {
+ if _, ok := err.(*exec.ExitError); ok {
+ return true
+ }
+ }
+ return err == nil
}
-func closeAll(closers []io.Closer) {
- for _, c := range closers {
- c.Close()
+func (c *Cmd) handleError(err error) {
+ c.Err = err
+ if !c.errorIsOk(err) {
+ c.sh.HandleError(err)
}
}
-func addWriter(writers *[]io.Writer, w io.Writer) {
+func (c *Cmd) addWriter(writers *[]io.Writer, w io.Writer) {
*writers = append(*writers, w)
+ // Check for os.Stdout and os.Stderr so that we don't close these when a
+ // single command exits. This technique isn't foolproof (since, for example,
+ // os.Stdout may be wrapped in another WriteCloser), but in practice it should
+ // be adequate.
+ if w != os.Stdout && w != os.Stderr {
+ if wc, ok := w.(io.Closer); ok {
+ c.closers = append(c.closers, wc)
+ }
+ }
+}
+
+func (c *Cmd) closeClosers() {
+ for _, c := range c.closers {
+ c.Close() // best-effort; ignore returned error
+ }
+}
+
+func (c *Cmd) isRunning() bool {
+ if !c.started {
+ return false
+ }
+ c.exitedMu.Lock()
+ defer c.exitedMu.Unlock()
+ return !c.exited
}
// recvWriter listens for gosh messages from a child process.
@@ -269,55 +319,100 @@
var writers *[]io.Writer
if f == os.Stdout {
writers = &c.stdoutWriters
+ c.addWriter(writers, &recvWriter{c: c})
} else {
writers = &c.stderrWriters
}
- if !c.SuppressOutput {
- addWriter(writers, f)
+ if c.PropagateOutput {
+ c.addWriter(writers, f)
}
if c.OutputDir != "" {
suffix := "stderr"
if f == os.Stdout {
suffix = "stdout"
}
- name := filepath.Join(c.OutputDir, filepath.Base(c.name)+"."+t+"."+suffix)
+ name := filepath.Join(c.OutputDir, filepath.Base(c.Path)+"."+t+"."+suffix)
file, err := os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600)
if err != nil {
return nil, err
}
- addWriter(writers, file)
- c.closeAfterWait = append(c.closeAfterWait, file)
- }
- if f == os.Stdout {
- addWriter(writers, &recvWriter{c: c})
+ c.addWriter(writers, file)
}
return io.MultiWriter(*writers...), nil
}
+func (c *Cmd) clone() (*Cmd, error) {
+ vars := make(map[string]string, len(c.Vars))
+ for k, v := range c.Vars {
+ vars[k] = v
+ }
+ args := make([]string, len(c.Args))
+ copy(args, c.Args)
+ res, err := newCmdInternal(c.sh, vars, c.Path, args)
+ if err != nil {
+ return nil, err
+ }
+ res.PropagateOutput = c.PropagateOutput
+ res.OutputDir = c.OutputDir
+ res.ExitErrorIsOk = c.ExitErrorIsOk
+ res.Stdin = c.Stdin
+ return res, nil
+}
+
+func (c *Cmd) stdinPipe() (io.WriteCloser, error) {
+ if c.calledStart {
+ return nil, errAlreadyCalledStart
+ }
+ if c.stdinWriteCloser != nil {
+ return c.stdinWriteCloser, nil
+ }
+ var err error
+ c.stdinWriteCloser, err = c.c.StdinPipe()
+ return c.stdinWriteCloser, err
+}
+
func (c *Cmd) stdoutPipe() (io.Reader, error) {
- if c.calledStart() {
+ if c.calledStart {
return nil, errAlreadyCalledStart
}
p := NewBufferedPipe()
- addWriter(&c.stdoutWriters, p)
- c.closeAfterWait = append(c.closeAfterWait, p)
+ c.addWriter(&c.stdoutWriters, p)
return p, nil
}
func (c *Cmd) stderrPipe() (io.Reader, error) {
- if c.calledStart() {
+ if c.calledStart {
return nil, errAlreadyCalledStart
}
p := NewBufferedPipe()
- addWriter(&c.stderrWriters, p)
- c.closeAfterWait = append(c.closeAfterWait, p)
+ c.addWriter(&c.stderrWriters, p)
return p, nil
}
-func (c *Cmd) start() error {
- if c.calledStart() {
+func (c *Cmd) addStdoutWriter(w io.Writer) error {
+ if c.calledStart {
return errAlreadyCalledStart
}
+ c.addWriter(&c.stdoutWriters, w)
+ return nil
+}
+
+func (c *Cmd) addStderrWriter(w io.Writer) error {
+ if c.calledStart {
+ return errAlreadyCalledStart
+ }
+ c.addWriter(&c.stderrWriters, w)
+ return nil
+}
+
+// TODO(sadovsky): Maybe wrap every child process with a "supervisor" process
+// that calls WatchParent().
+
+func (c *Cmd) start() error {
+ if c.calledStart {
+ return errAlreadyCalledStart
+ }
+ c.calledStart = true
// Protect against Cmd.start() writing to c.c.Process concurrently with
// signal-triggered Shell.cleanup() reading from it.
c.sh.cleanupMu.Lock()
@@ -325,9 +420,16 @@
if c.sh.calledCleanup {
return errAlreadyCalledCleanup
}
- c.c = exec.Command(c.name, c.Args...)
+ // Configure the command.
+ c.c.Path = c.Path
c.c.Env = mapToSlice(c.Vars)
- c.c.Stdin = c.Stdin
+ c.c.Args = append([]string{c.Path}, c.Args...)
+ if c.Stdin != "" {
+ if c.stdinWriteCloser != nil {
+ return errors.New("gosh: cannot both set Stdin and call StdinPipe")
+ }
+ c.c.Stdin = strings.NewReader(c.Stdin)
+ }
t := time.Now().Format("20060102.150405.000000")
var err error
if c.c.Stdout, err = c.initMultiWriter(os.Stdout, t); err != nil {
@@ -336,19 +438,38 @@
if c.c.Stderr, err = c.initMultiWriter(os.Stderr, t); err != nil {
return err
}
- // TODO(sadovsky): Maybe wrap every child process with a "supervisor" process
- // that calls WatchParent().
+ // Start the command.
err = c.c.Start()
if err != nil {
- closeAll(c.closeAfterWait)
+ c.exitedMu.Lock()
+ c.exited = true
+ c.exitedMu.Unlock()
+ c.closeClosers()
+ c.waitChan <- errors.New("gosh: start failed")
+ return err
}
- return err
+ c.started = true
+ // Spawn a "waiter" goroutine that calls exec.Cmd.Wait and thus waits for the
+ // process to exit. Calling exec.Cmd.Wait here rather than in gosh.Cmd.Wait
+ // ensures that the child process is reaped once it exits. Note, gosh.Cmd.wait
+ // blocks on waitChan.
+ go func() {
+ err := c.c.Wait()
+ c.exitedMu.Lock()
+ c.exited = true
+ c.exitedMu.Unlock()
+ c.closeClosers()
+ c.waitChan <- err
+ }()
+ return nil
}
-// TODO(sadovsky): Maybe add timeouts for Cmd.{awaitReady,awaitVars,wait}.
+// TODO(sadovsky): Make it so Cmd.{awaitReady,awaitVars} return an error if/when
+// we detect that the process has exited. Also, maybe add optional timeouts for
+// Cmd.{awaitReady,awaitVars,wait}.
func (c *Cmd) awaitReady() error {
- if !c.calledStart() {
+ if !c.started {
return errDidNotCallStart
} else if c.calledWait {
return errAlreadyCalledWait
@@ -363,7 +484,7 @@
}
func (c *Cmd) awaitVars(keys ...string) (map[string]string, error) {
- if !c.calledStart() {
+ if !c.started {
return nil, errDidNotCallStart
} else if c.calledWait {
return nil, errAlreadyCalledWait
@@ -392,21 +513,27 @@
}
func (c *Cmd) wait() error {
- if !c.calledStart() {
+ if !c.started {
return errDidNotCallStart
} else if c.calledWait {
return errAlreadyCalledWait
}
c.calledWait = true
- err := c.c.Wait()
- closeAll(c.closeAfterWait)
- return err
+ return <-c.waitChan
}
func (c *Cmd) shutdown(sig os.Signal) error {
- if !c.calledStart() {
+ if !c.started {
return errDidNotCallStart
}
+ // TODO(sadovsky): There's a race condition here and in
+ // Shell.terminateRunningCmds. If our Process.Wait returns immediately before
+ // we call Process.Signal, Process.Signal will return an error, "os: process
+ // already finished". Should we add Cmd.Signal and Cmd.Kill methods that
+ // special-case for this error message?
+ if !c.isRunning() {
+ return nil
+ }
if err := c.c.Process.Signal(sig); err != nil {
return err
}
@@ -425,41 +552,29 @@
return c.wait()
}
-func (c *Cmd) output() (string, string, error) {
+func (c *Cmd) stdout() (string, error) {
+ if c.calledStart {
+ return "", errAlreadyCalledStart
+ }
+ var stdout bytes.Buffer
+ c.addWriter(&c.stdoutWriters, &stdout)
+ err := c.run()
+ return stdout.String(), err
+}
+
+func (c *Cmd) stdoutStderr() (string, string, error) {
+ if c.calledStart {
+ return "", "", errAlreadyCalledStart
+ }
var stdout, stderr bytes.Buffer
- addWriter(&c.stdoutWriters, &stdout)
- addWriter(&c.stderrWriters, &stderr)
+ c.addWriter(&c.stdoutWriters, &stdout)
+ c.addWriter(&c.stderrWriters, &stderr)
err := c.run()
return stdout.String(), stderr.String(), err
}
-type threadSafeBuffer struct {
- mu sync.Mutex
- buf bytes.Buffer
-}
-
-func (b *threadSafeBuffer) Write(p []byte) (int, error) {
- b.mu.Lock()
- defer b.mu.Unlock()
- return b.buf.Write(p)
-}
-
-func (b *threadSafeBuffer) String() string {
- b.mu.Lock()
- defer b.mu.Unlock()
- return b.buf.String()
-}
-
-func (c *Cmd) combinedOutput() (string, error) {
- buf := &threadSafeBuffer{}
- addWriter(&c.stdoutWriters, buf)
- addWriter(&c.stderrWriters, buf)
- err := c.run()
- return buf.String(), err
-}
-
func (c *Cmd) process() (*os.Process, error) {
- if !c.calledStart() {
+ if !c.started {
return nil, errDidNotCallStart
}
return c.c.Process, nil
diff --git a/gosh/internal/gosh_example/main.go b/gosh/internal/gosh_example/main.go
index 51d4754..67fa15c 100644
--- a/gosh/internal/gosh_example/main.go
+++ b/gosh/internal/gosh_example/main.go
@@ -12,7 +12,7 @@
)
func ExampleCmds() {
- sh := gosh.NewShell(gosh.Opts{SuppressChildOutput: true})
+ sh := gosh.NewShell(gosh.Opts{})
defer sh.Cleanup()
// Start server.
@@ -26,37 +26,36 @@
// Run client.
binPath = sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_client")
c = sh.Cmd(binPath, "-addr="+addr)
- stdout, _ := c.Output()
- fmt.Print(stdout)
+ fmt.Print(c.Stdout())
}
var (
- get = gosh.Register("get", lib.Get)
- serve = gosh.Register("serve", lib.Serve)
+ getFn = gosh.Register("get", lib.Get)
+ serveFn = gosh.Register("serve", lib.Serve)
)
func ExampleFns() {
- sh := gosh.NewShell(gosh.Opts{SuppressChildOutput: true})
+ sh := gosh.NewShell(gosh.Opts{})
defer sh.Cleanup()
// Start server.
- c := sh.Fn(serve)
+ c := sh.Fn(serveFn)
c.Start()
c.AwaitReady()
addr := c.AwaitVars("Addr")["Addr"]
fmt.Println(addr)
// Run client.
- c = sh.Fn(get, addr)
- stdout, _ := c.Output()
- fmt.Print(stdout)
+ c = sh.Fn(getFn, addr)
+ fmt.Print(c.Stdout())
}
func ExampleShellMain() {
sh := gosh.NewShell(gosh.Opts{})
defer sh.Cleanup()
- stdout, _ := sh.Main(lib.HelloWorldMain).Output()
- fmt.Print(stdout)
+
+ c := sh.Main(lib.HelloWorldMain)
+ fmt.Print(c.Stdout())
}
func main() {
diff --git a/gosh/registry.go b/gosh/registry.go
index e2511eb..139a4f1 100644
--- a/gosh/registry.go
+++ b/gosh/registry.go
@@ -10,6 +10,7 @@
"bytes"
"encoding/base64"
"encoding/gob"
+ "errors"
"fmt"
"reflect"
)
@@ -29,20 +30,20 @@
// unique across the dependency graph; 'fni' must be a function that accepts
// gob-encodable arguments and returns an error or nothing.
func Register(name string, fni interface{}) *Fn {
- // TODO(sadovsky): Switch to using len(fns) as name, and maybe drop the name
- // argument, if it turns out that initialization order is deterministic.
+ // TODO(sadovsky): Include len(fns) in registered name if it turns out that
+ // initialization order is deterministic.
if _, ok := fns[name]; ok {
- panic(fmt.Errorf("%s: already registered", name))
+ panic(fmt.Errorf("gosh: %q is already registered", name))
}
v := reflect.ValueOf(fni)
t := v.Type()
if t.Kind() != reflect.Func {
- panic(fmt.Errorf("%s: not a function: %v", name, t.Kind()))
+ panic(fmt.Errorf("gosh: %q is not a function: %v", name, t.Kind()))
}
if t.NumOut() > 1 || t.NumOut() == 1 && t.Out(0) != errorType {
- panic(fmt.Errorf("%s: function must return an error or nothing: %v", name, t))
+ panic(fmt.Errorf("gosh: %q must return an error or nothing: %v", name, t))
}
- // Register the function's args with gob. Needed because Shell.Fn() takes
+ // Register the function's args with gob. Needed because Shell.Fn takes
// interface{} arguments.
for i := 0; i < t.NumIn(); i++ {
// Note: Clients are responsible for registering any concrete types stored
@@ -59,11 +60,11 @@
// Call calls the named function, which must have been registered.
func Call(name string, args ...interface{}) error {
- if fn, ok := fns[name]; !ok {
- return fmt.Errorf("unknown function: %s", name)
- } else {
- return fn.Call(args...)
+ fn, err := getFn(name)
+ if err != nil {
+ return err
}
+ return fn.Call(args...)
}
// Call calls the function fn with the input arguments args.
@@ -77,11 +78,7 @@
} else {
// Client passed nil; construct the zero value for this argument based on
// the function signature.
- at := t.In(i)
- if t.IsVariadic() && i == t.NumIn()-1 {
- at = at.Elem()
- }
- av = reflect.Zero(at)
+ av = reflect.Zero(argType(t, i))
}
in = append(in, av)
}
@@ -92,6 +89,52 @@
return nil
}
+// getFn returns the named function.
+func getFn(name string) (*Fn, error) {
+ fn, ok := fns[name]
+ if !ok {
+ return nil, fmt.Errorf("gosh: unknown function %q", name)
+ }
+ return fn, nil
+}
+
+// argType returns the type of the nth argument to a function of type t.
+func argType(t reflect.Type, n int) reflect.Type {
+ if !t.IsVariadic() || n < t.NumIn()-1 {
+ return t.In(n)
+ }
+ return t.In(t.NumIn() - 1).Elem()
+}
+
+// checkCall checks that the named function exists and can be called with the
+// given arguments. Modeled after the implementation of reflect.Value.call.
+func checkCall(name string, args ...interface{}) error {
+ fn, err := getFn(name)
+ if err != nil {
+ return err
+ }
+ t := fn.value.Type()
+ n := t.NumIn()
+ if t.IsVariadic() {
+ n--
+ }
+ if len(args) < n {
+ return errors.New("gosh: too few input arguments")
+ }
+ if !t.IsVariadic() && len(args) > n {
+ return errors.New("gosh: too many input arguments")
+ }
+ for i, arg := range args {
+ if arg == nil {
+ continue
+ }
+ if at, et := reflect.ValueOf(arg).Type(), argType(t, i); !at.AssignableTo(et) {
+ return fmt.Errorf("gosh: cannot use %s as type %s", at, et)
+ }
+ }
+ return nil
+}
+
////////////////////////////////////////
// invocation
@@ -102,10 +145,13 @@
// encInvocation encodes an invocation.
func encInvocation(name string, args ...interface{}) (string, error) {
+ if err := checkCall(name, args...); err != nil {
+ return "", err
+ }
inv := invocation{Name: name, Args: args}
buf := &bytes.Buffer{}
if err := gob.NewEncoder(buf).Encode(inv); err != nil {
- return "", fmt.Errorf("failed to encode invocation: %v", err)
+ return "", fmt.Errorf("gosh: failed to encode invocation: %v", err)
}
// Base64-encode the gob-encoded bytes so that the result can be used as an
// env var value.
@@ -120,7 +166,7 @@
err = gob.NewDecoder(bytes.NewReader(b)).Decode(&inv)
}
if err != nil {
- return "", nil, fmt.Errorf("failed to decode invocation: %v", err)
+ return "", nil, fmt.Errorf("gosh: failed to decode invocation: %v", err)
}
return inv.Name, inv.Args, nil
}
diff --git a/gosh/shell.go b/gosh/shell.go
index dbe8fbb..04705ee 100644
--- a/gosh/shell.go
+++ b/gosh/shell.go
@@ -38,7 +38,6 @@
errAlreadyCalledCleanup = errors.New("gosh: already called Shell.Cleanup")
errDidNotCallMaybeRunFnAndExit = errors.New("gosh: did not call Shell.MaybeRunFnAndExit")
errDidNotCallNewShell = errors.New("gosh: did not call gosh.NewShell")
- errShellErrIsNotNil = errors.New("gosh: Shell.Err is not nil")
)
// Shell represents a shell. Not thread-safe.
@@ -65,15 +64,15 @@
// Opts configures Shell.
type Opts struct {
- // Errorf is called whenever an error is encountered.
+ // Fatalf is called whenever an error is encountered.
// If not specified, defaults to panic(fmt.Sprintf(format, v...)).
- Errorf func(format string, v ...interface{})
+ Fatalf func(format string, v ...interface{})
// Logf is called to log things.
// If not specified, defaults to log.Printf(format, v...).
Logf func(format string, v ...interface{})
// Child stdout and stderr are propagated up to the parent's stdout and stderr
- // iff SuppressChildOutput is false.
- SuppressChildOutput bool
+ // iff PropagateChildOutput is true.
+ PropagateChildOutput bool
// If specified, each child's stdout and stderr streams are also piped to
// files in this directory.
// If not specified, defaults to GOSH_CHILD_OUTPUT_DIR.
@@ -90,12 +89,12 @@
return sh
}
-// HandleError sets sh.Err. If err is not nil, it also calls sh.Opts.Errorf.
+// HandleError sets sh.Err. If err is not nil, it also calls sh.Opts.Fatalf.
func (sh *Shell) HandleError(err error) {
sh.Ok()
sh.Err = err
- if err != nil && sh.Opts.Errorf != nil {
- sh.Opts.Errorf("%v", err)
+ if err != nil && sh.Opts.Fatalf != nil {
+ sh.Opts.Fatalf("%v", err)
}
}
@@ -210,7 +209,7 @@
}
// Panic on incorrect usage of Shell.
if sh.Err != nil {
- panic(errShellErrIsNotNil)
+ panic(fmt.Errorf("gosh: Shell.Err is not nil: %v", sh.Err))
}
sh.cleanupMu.Lock()
defer sh.cleanupMu.Unlock()
@@ -232,11 +231,11 @@
}()
}
-// Note: On error, newShell returns a *Shell with Opts.Errorf initialized to
+// Note: On error, newShell returns a *Shell with Opts.Fatalf initialized to
// simplify things for the caller.
func newShell(opts Opts) (*Shell, error) {
- if opts.Errorf == nil {
- opts.Errorf = func(format string, v ...interface{}) {
+ if opts.Fatalf == nil {
+ opts.Fatalf = func(format string, v ...interface{}) {
panic(fmt.Sprintf(format, v...))
}
}
@@ -263,15 +262,6 @@
}
}
}
- // Set this process's PGID to its PID so that its child processes can be
- // identified reliably.
- // http://man7.org/linux/man-pages/man2/setpgid.2.html
- // TODO(sadovsky): Is there any way to reliably kill all spawned subprocesses
- // without modifying external state?
- if err := syscall.Setpgid(0, 0); err != nil {
- sh.cleanup()
- return sh, err
- }
// Call sh.cleanup() if needed when a termination signal is received.
onTerminationSignal(func(sig os.Signal) {
sh.logf("Received signal: %v\n", sig)
@@ -302,7 +292,7 @@
if err != nil {
return nil, err
}
- c.SuppressOutput = sh.Opts.SuppressChildOutput
+ c.PropagateOutput = sh.Opts.PropagateChildOutput
c.OutputDir = sh.Opts.ChildOutputDir
return c, nil
}
@@ -330,7 +320,7 @@
// Check that fn has the required signature.
t := fn.value.Type()
if t.NumIn() != 0 || t.NumOut() != 0 {
- return nil, errors.New("main function must have no input or output parameters")
+ return nil, errors.New("gosh: main function must have no input or output parameters")
}
b, err := encInvocation(fn.name)
if err != nil {
@@ -345,14 +335,12 @@
// need not hold cleanupMu when accessing sh.cmds below.
var res error
for _, c := range sh.cmds {
- if !c.calledStart() || c.calledWait {
+ if !c.started || c.calledWait {
continue
}
- if err := c.wait(); err != nil {
- sh.logf("Cmd.Wait() failed: %v\n", err)
- if res == nil {
- res = err
- }
+ if err := c.wait(); !c.errorIsOk(err) {
+ sh.logf("%s (PID %d) failed: %v\n", c.Path, c.c.Process.Pid, err)
+ res = err
}
}
return res
@@ -392,7 +380,7 @@
if err != nil {
return "", err
}
- c.SuppressOutput = true
+ c.PropagateOutput = false
if err := c.run(); err != nil {
return "", err
}
@@ -444,7 +432,7 @@
func (sh *Shell) popd() error {
if len(sh.dirStack) == 0 {
- return errors.New("dir stack is empty")
+ return errors.New("gosh: dir stack is empty")
}
dir := sh.dirStack[len(sh.dirStack)-1]
if err := os.Chdir(dir); err != nil {
@@ -468,24 +456,21 @@
func (sh *Shell) forEachRunningCmd(fn func(*Cmd)) bool {
anyRunning := false
for _, c := range sh.cmds {
- if !c.calledStart() || c.c.Process == nil {
- continue // not started
- }
- if pgid, err := syscall.Getpgid(c.c.Process.Pid); err != nil || pgid != os.Getpid() {
- continue // not our child
- }
- anyRunning = true
- if fn != nil {
- fn(c)
+ if c.isRunning() {
+ anyRunning = true
+ if fn != nil {
+ fn(c)
+ }
}
}
return anyRunning
}
-// Note: It is safe to run Shell.terminateRunningCmds() concurrently with
-// Cmd.wait(). In particular, Shell.terminateRunningCmds() only reads
-// c.c.Process.{Pid,Path} and calls c.c.Process.{Signal,Kill}, all of which are
-// thread-safe with Cmd.wait().
+// Note: It is safe to run Shell.terminateRunningCmds concurrently with the
+// waiter goroutine and with Cmd.wait. In particular, Shell.terminateRunningCmds
+// only reads c.c.Process.Pid and calls c.c.Process.{Signal,Kill}, all of which
+// are thread-safe with the waiter goroutine and with Cmd.wait.
+// TODO(sadovsky): Not quite true. See TODO in Cmd.shutdown.
func (sh *Shell) terminateRunningCmds() {
// Try SIGINT first; if that doesn't work, use SIGKILL.
anyRunning := sh.forEachRunningCmd(func(c *Cmd) {
@@ -493,11 +478,11 @@
sh.logf("%d.Signal(os.Interrupt) failed: %v\n", c.c.Process.Pid, err)
}
})
- // If any child is still running, wait for 50ms.
+ // If any child is still running, wait for 100ms.
if anyRunning {
- time.Sleep(50 * time.Millisecond)
+ time.Sleep(100 * time.Millisecond)
anyRunning = sh.forEachRunningCmd(func(c *Cmd) {
- sh.logf("%s (PID %d) did not die\n", c.c.Path, c.c.Process.Pid)
+ sh.logf("%s (PID %d) did not die\n", c.Path, c.c.Process.Pid)
})
}
// If any child is still running, wait for another second, then send SIGKILL
@@ -515,14 +500,8 @@
func (sh *Shell) cleanup() {
sh.calledCleanup = true
- // Terminate all children that are still running. Note, newShell() calls
- // syscall.Setpgid().
- pgid, pid := syscall.Getpgrp(), os.Getpid()
- if pgid != pid {
- sh.logf("PGID (%d) != PID (%d); skipping subprocess termination\n", pgid, pid)
- } else {
- sh.terminateRunningCmds()
- }
+ // Terminate all children that are still running.
+ sh.terminateRunningCmds()
// Close and delete all temporary files.
for _, tempFile := range sh.tempFiles {
name := tempFile.Name()
diff --git a/gosh/shell_test.go b/gosh/shell_test.go
index 89a9555..a6df636 100644
--- a/gosh/shell_test.go
+++ b/gosh/shell_test.go
@@ -5,14 +5,14 @@
package gosh_test
// TODO(sadovsky): Add more tests:
-// - variadic function registration and invocation
// - effects of Shell.Cleanup
-// - Cmd.{Wait,Run}
-// - Shell.{Args,Wait,Rename,MakeTempFile,MakeTempDir}
+// - Cmd.Clone
+// - Shell.{Vars,Args,Rename,MakeTempFile,MakeTempDir}
// - Opts (including defaulting behavior)
// - {,Maybe}WatchParent
import (
+ "bufio"
"errors"
"fmt"
"io"
@@ -64,16 +64,71 @@
}
}
-func makeErrorf(t *testing.T) func(string, ...interface{}) {
+func toString(t *testing.T, r io.Reader) string {
+ b, err := ioutil.ReadAll(r)
+ ok(t, err)
+ return string(b)
+}
+
+func makeFatalf(t *testing.T) func(string, ...interface{}) {
return func(format string, v ...interface{}) {
debug.PrintStack()
t.Fatalf(format, v...)
}
}
-func TestPushdPopd(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+////////////////////////////////////////
+// Simple functions
+
+// Simplified versions of various Unix commands.
+var (
+ catFn = gosh.Register("cat", func() {
+ io.Copy(os.Stdout, os.Stdin)
+ })
+ echoFn = gosh.Register("echo", func() {
+ fmt.Println(os.Args[1])
+ })
+ readFn = gosh.Register("read", func() {
+ bufio.NewReader(os.Stdin).ReadString('\n')
+ })
+)
+
+// Functions with parameters.
+var (
+ exitFn = gosh.Register("exit", func(code int) {
+ os.Exit(code)
+ })
+ sleepFn = gosh.Register("sleep", func(d time.Duration, code int) {
+ time.Sleep(d)
+ os.Exit(code)
+ })
+ printFn = gosh.Register("print", func(v ...interface{}) {
+ fmt.Print(v...)
+ })
+ printfFn = gosh.Register("printf", func(format string, v ...interface{}) {
+ fmt.Printf(format, v...)
+ })
+)
+
+////////////////////////////////////////
+// Tests
+
+func TestCustomFatalf(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
+
+ var calledFatalf bool
+ sh.Opts.Fatalf = func(string, ...interface{}) { calledFatalf = true }
+ sh.HandleError(fakeError)
+ // Note, our deferred sh.Cleanup() should succeed despite this error.
+ nok(t, sh.Err)
+ eq(t, calledFatalf, true)
+}
+
+func TestPushdPopd(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
+ defer sh.Cleanup()
+
startDir, err := os.Getwd()
ok(t, err)
parentDir := filepath.Dir(startDir)
@@ -95,35 +150,39 @@
ok(t, err)
eq(t, cwd, startDir)
// The next sh.Popd() will fail.
- var calledErrorf bool
- sh.Opts.Errorf = func(string, ...interface{}) { calledErrorf = true }
+ sh.Opts.Fatalf = func(string, ...interface{}) {}
sh.Popd()
- // Note, our deferred sh.Cleanup() should succeed despite this error.
nok(t, sh.Err)
- eq(t, calledErrorf, true)
+}
+
+func evalSymlinks(t *testing.T, dir string) string {
+ var err error
+ dir, err = filepath.EvalSymlinks(dir)
+ ok(t, err)
+ return dir
+}
+
+func getwdEvalSymlinks(t *testing.T) string {
+ dir, err := os.Getwd()
+ ok(t, err)
+ return evalSymlinks(t, dir)
}
func TestPushdNoPopdCleanup(t *testing.T) {
- startDir, err := os.Getwd()
- ok(t, err)
-
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ startDir := getwdEvalSymlinks(t)
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
tmpDir := sh.MakeTempDir()
sh.Pushd(tmpDir)
- cwd, err := os.Getwd()
- ok(t, err)
- eq(t, cwd, tmpDir)
+ eq(t, getwdEvalSymlinks(t), evalSymlinks(t, tmpDir))
// There is no matching popd; the cwd is tmpDir, which is deleted by Cleanup.
// Cleanup needs to put us back in startDir, otherwise all subsequent Pushd
// calls will fail.
sh.Cleanup()
- cwd, err = os.Getwd()
- ok(t, err)
- eq(t, cwd, startDir)
+ eq(t, getwdEvalSymlinks(t), startDir)
}
func TestCmds(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
// Start server.
@@ -137,61 +196,178 @@
// Run client.
binPath = sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_client")
c = sh.Cmd(binPath, "-addr="+addr)
- stdout, _ := c.Output()
- eq(t, stdout, "Hello, world!\n")
+ eq(t, c.Stdout(), "Hello, world!\n")
}
var (
- get = gosh.Register("get", lib.Get)
- serve = gosh.Register("serve", lib.Serve)
+ getFn = gosh.Register("get", lib.Get)
+ serveFn = gosh.Register("serve", lib.Serve)
)
func TestFns(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
// Start server.
- c := sh.Fn(serve)
+ c := sh.Fn(serveFn)
c.Start()
c.AwaitReady()
addr := c.AwaitVars("Addr")["Addr"]
neq(t, addr, "")
// Run client.
- c = sh.Fn(get, addr)
- stdout, _ := c.Output()
- eq(t, stdout, "Hello, world!\n")
+ c = sh.Fn(getFn, addr)
+ eq(t, c.Stdout(), "Hello, world!\n")
}
func TestShellMain(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
- stdout, _ := sh.Main(lib.HelloWorldMain).Output()
- eq(t, stdout, "Hello, world!\n")
+
+ c := sh.Main(lib.HelloWorldMain)
+ eq(t, c.Stdout(), "Hello, world!\n")
}
-var write = gosh.Register("write", func(stdout, stderr bool) error {
- tenMs := 10 * time.Millisecond
+// Functions designed for TestRegistry.
+var (
+ printIntsFn = gosh.Register("printInts", func(v ...int) {
+ var vi []interface{}
+ for _, x := range v {
+ vi = append(vi, x)
+ }
+ fmt.Print(vi...)
+ })
+ printfIntsFn = gosh.Register("printfInts", func(format string, v ...int) {
+ var vi []interface{}
+ for _, x := range v {
+ vi = append(vi, x)
+ }
+ fmt.Printf(format, vi...)
+ })
+)
+
+// Tests function signature-checking and execution.
+func TestRegistry(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
+ defer sh.Cleanup()
+
+ // Variadic functions. Non-variadic functions are sufficiently covered in
+ // other tests.
+ eq(t, sh.Fn(printFn).Stdout(), "")
+ eq(t, sh.Fn(printFn, 0).Stdout(), "0")
+ eq(t, sh.Fn(printFn, 0, "foo").Stdout(), "0foo")
+ eq(t, sh.Fn(printfFn, "").Stdout(), "")
+ eq(t, sh.Fn(printfFn, "%v", 0).Stdout(), "0")
+ eq(t, sh.Fn(printfFn, "%v%v", 0, "foo").Stdout(), "0foo")
+ eq(t, sh.Fn(printIntsFn, 1, 2).Stdout(), "1 2")
+ eq(t, sh.Fn(printfIntsFn, "%v %v", 1, 2).Stdout(), "1 2")
+
+ // Error cases.
+ sh.Opts.Fatalf = func(string, ...interface{}) {}
+ reset := func() {
+ nok(t, sh.Err)
+ sh.Err = nil
+ }
+
+ // Too few arguments.
+ sh.Fn(exitFn)
+ reset()
+ sh.Fn(sleepFn, time.Second)
+ reset()
+ sh.Fn(printfFn)
+ reset()
+
+ // Too many arguments.
+ sh.Fn(exitFn, 0, 0)
+ reset()
+ sh.Fn(sleepFn, time.Second, 0, 0)
+ reset()
+
+ // Wrong argument types.
+ sh.Fn(exitFn, "foo")
+ reset()
+ sh.Fn(sleepFn, 0, 0)
+ reset()
+ sh.Fn(printfFn, 0)
+ reset()
+ sh.Fn(printfFn, 0, 0)
+ reset()
+
+ // Wrong variadic argument types.
+ sh.Fn(printIntsFn, 0.5)
+ reset()
+ sh.Fn(printIntsFn, 0, 0.5)
+ reset()
+ sh.Fn(printfIntsFn, "%v", 0.5)
+ reset()
+ sh.Fn(printfIntsFn, "%v", 0, 0.5)
+ reset()
+
+ // Unsupported argument types.
+ var p *int
+ sh.Fn(printFn, p)
+ reset()
+ sh.Fn(printfFn, "%v", p)
+ reset()
+}
+
+func TestStdin(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
+ defer sh.Cleanup()
+
+ c := sh.Main(catFn)
+ c.Stdin = "foo\n"
+ // We set c.Stdin and did not call c.StdinPipe(), so stdin should close and
+ // cat should exit immediately.
+ eq(t, c.Stdout(), "foo\n")
+
+ c = sh.Main(catFn)
+ c.StdinPipe().Write([]byte("foo\n"))
+ // The "cat" command only exits when stdin is closed, so we must explicitly
+ // close the stdin pipe. Note, it's safe to call c.StdinPipe multiple times.
+ c.StdinPipe().Close()
+ eq(t, c.Stdout(), "foo\n")
+
+ c = sh.Main(readFn)
+ c.StdinPipe().Write([]byte("foo\n"))
+ // The "read" command exits when it sees a newline, so Cmd.Wait (and thus
+ // Cmd.Run) should return immediately; it should not be necessary to close the
+ // stdin pipe.
+ c.Run()
+
+ c = sh.Main(catFn)
+ // No stdin, so cat should exit immediately.
+ eq(t, c.Stdout(), "")
+
+ // It's an error (detected at command start time) to both set c.Stdin and call
+ // c.StdinPipe. Note, this indirectly tests that Shell.Cleanup works even if
+ // some Cmd.Start failed.
+ c = sh.Main(catFn)
+ c.Stdin = "foo"
+ c.StdinPipe().Write([]byte("bar"))
+ c.StdinPipe().Close()
+ sh.Opts.Fatalf = func(string, ...interface{}) {}
+ c.Start()
+ nok(t, sh.Err)
+}
+
+var writeFn = gosh.Register("write", func(stdout, stderr bool) error {
if stdout {
- time.Sleep(tenMs)
if _, err := os.Stdout.Write([]byte("A")); err != nil {
return err
}
}
if stderr {
- time.Sleep(tenMs)
if _, err := os.Stderr.Write([]byte("B")); err != nil {
return err
}
}
if stdout {
- time.Sleep(tenMs)
if _, err := os.Stdout.Write([]byte("A")); err != nil {
return err
}
}
if stderr {
- time.Sleep(tenMs)
if _, err := os.Stderr.Write([]byte("B")); err != nil {
return err
}
@@ -199,92 +375,169 @@
return nil
})
-func toString(r io.Reader) string {
- if b, err := ioutil.ReadAll(r); err != nil {
- panic(err)
- } else {
- return string(b)
- }
-}
-
func TestStdoutStderr(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
// Write to stdout only.
- c := sh.Fn(write, true, false)
+ c := sh.Fn(writeFn, true, false)
stdoutPipe, stderrPipe := c.StdoutPipe(), c.StderrPipe()
- eq(t, c.CombinedOutput(), "AA")
- eq(t, toString(stdoutPipe), "AA")
- eq(t, toString(stderrPipe), "")
- stdout, stderr := sh.Fn(write, true, false).Output()
+ stdout, stderr := c.StdoutStderr()
eq(t, stdout, "AA")
eq(t, stderr, "")
+ eq(t, toString(t, stdoutPipe), "AA")
+ eq(t, toString(t, stderrPipe), "")
// Write to stderr only.
- c = sh.Fn(write, false, true)
+ c = sh.Fn(writeFn, false, true)
stdoutPipe, stderrPipe = c.StdoutPipe(), c.StderrPipe()
- eq(t, c.CombinedOutput(), "BB")
- eq(t, toString(stdoutPipe), "")
- eq(t, toString(stderrPipe), "BB")
- stdout, stderr = sh.Fn(write, false, true).Output()
+ stdout, stderr = c.StdoutStderr()
eq(t, stdout, "")
eq(t, stderr, "BB")
+ eq(t, toString(t, stdoutPipe), "")
+ eq(t, toString(t, stderrPipe), "BB")
// Write to both stdout and stderr.
- c = sh.Fn(write, true, true)
+ c = sh.Fn(writeFn, true, true)
stdoutPipe, stderrPipe = c.StdoutPipe(), c.StderrPipe()
- eq(t, c.CombinedOutput(), "ABAB")
- eq(t, toString(stdoutPipe), "AA")
- eq(t, toString(stderrPipe), "BB")
- stdout, stderr = sh.Fn(write, true, true).Output()
+ stdout, stderr = c.StdoutStderr()
eq(t, stdout, "AA")
eq(t, stderr, "BB")
+ eq(t, toString(t, stdoutPipe), "AA")
+ eq(t, toString(t, stderrPipe), "BB")
}
-var sleep = gosh.Register("sleep", func(d time.Duration) {
- time.Sleep(d)
+var writeMoreFn = gosh.Register("writeMore", func() {
+ sh := gosh.NewShell(gosh.Opts{})
+ defer sh.Cleanup()
+
+ c := sh.Fn(writeFn, true, true)
+ c.AddStdoutWriter(os.Stdout)
+ c.AddStderrWriter(os.Stderr)
+ c.Run()
+
+ fmt.Fprint(os.Stdout, " stdout done")
+ fmt.Fprint(os.Stderr, " stderr done")
})
+// Tests that it's safe to add os.Stdout and os.Stderr as writers.
+func TestAddWriters(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
+ defer sh.Cleanup()
+
+ stdout, stderr := sh.Fn(writeMoreFn).StdoutStderr()
+ eq(t, stdout, "AA stdout done")
+ eq(t, stderr, "BB stderr done")
+}
+
+// Tests piping from one Cmd's stdout/stderr to another's stdin. It should be
+// possible to wait on just the last Cmd.
+func TestPiping(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
+ defer sh.Cleanup()
+
+ echo := sh.Main(echoFn, "foo")
+ cat := sh.Main(catFn)
+ echo.AddStdoutWriter(cat.StdinPipe())
+ echo.Start()
+ eq(t, cat.Stdout(), "foo\n")
+
+ // This time, pipe both stdout and stderr to cat's stdin.
+ c := sh.Fn(writeFn, true, true)
+ cat = sh.Main(catFn)
+ c.AddStdoutWriter(cat.StdinPipe())
+ c.AddStderrWriter(cat.StdinPipe())
+ c.Start()
+ // Note, we can't assume any particular ordering of stdout and stderr, so we
+ // simply check the length of the combined output.
+ eq(t, len(cat.Stdout()), 4)
+}
+
func TestShutdown(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
for _, d := range []time.Duration{0, time.Second} {
for _, s := range []os.Signal{os.Interrupt, os.Kill} {
fmt.Println(d, s)
- c := sh.Fn(sleep, d)
+ c := sh.Fn(sleepFn, d, 0)
c.Start()
+ // Wait for a bit to allow the zero-sleep commands to exit, to test that
+ // Shutdown succeeds for an exited process and to avoid the race condition
+ // in Cmd.Shutdown.
time.Sleep(10 * time.Millisecond)
c.Shutdown(s)
}
}
}
-var exit = gosh.Register("exit", func(code int) {
- os.Exit(code)
-})
+func TestShellWait(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
+ defer sh.Cleanup()
+
+ d0 := time.Duration(0)
+ d200 := 200 * time.Millisecond
+
+ c0 := sh.Fn(sleepFn, d0, 0) // not started
+ c1 := sh.Fn(sleepFn, d0, 0) // failed to start
+ c2 := sh.Fn(sleepFn, d200, 0) // running and will succeed
+ c3 := sh.Fn(sleepFn, d200, 1) // running and will fail
+ c4 := sh.Fn(sleepFn, d0, 0) // succeeded
+ c5 := sh.Fn(sleepFn, d0, 0) // succeeded, called wait
+ c6 := sh.Fn(sleepFn, d0, 1) // failed
+ c7 := sh.Fn(sleepFn, d0, 1) // failed, called wait
+
+ c3.ExitErrorIsOk = true
+ c6.ExitErrorIsOk = true
+ c7.ExitErrorIsOk = true
+
+ // Configure the "failed to start" command.
+ c1.StdinPipe()
+ c1.Stdin = "foo"
+ sh.Opts.Fatalf = func(string, ...interface{}) {}
+ c1.Start()
+ nok(t, sh.Err)
+ sh.Err = nil
+ sh.Opts.Fatalf = makeFatalf(t)
+
+ // Start commands, call wait.
+ for _, c := range []*gosh.Cmd{c2, c3, c4, c5, c6, c7} {
+ c.Start()
+ }
+ c5.Wait()
+ c7.Wait()
+ sh.Wait()
+
+ // It should be possible to run existing unstarted commands, and to create and
+ // run new commands, after calling Shell.Wait.
+ c0.Run()
+ sh.Fn(sleepFn, d0, 0).Run()
+ sh.Fn(sleepFn, d0, 0).Start()
+
+ // Call Shell.Wait again.
+ sh.Wait()
+}
func TestExitErrorIsOk(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
defer sh.Cleanup()
// Exit code 0 is not an error.
- c := sh.Fn(exit, 0)
+ c := sh.Fn(exitFn, 0)
c.Run()
ok(t, c.Err)
ok(t, sh.Err)
// Exit code 1 is an error.
- c = sh.Fn(exit, 1)
+ c = sh.Fn(exitFn, 1)
c.ExitErrorIsOk = true
c.Run()
nok(t, c.Err)
ok(t, sh.Err)
// If ExitErrorIsOk is false, exit code 1 triggers sh.HandleError.
- sh.Opts.Errorf = func(string, ...interface{}) {}
- c = sh.Fn(exit, 1)
+ c = sh.Fn(exitFn, 1)
+ sh.Opts.Fatalf = func(string, ...interface{}) {}
c.Run()
nok(t, c.Err)
nok(t, sh.Err)
@@ -298,14 +551,14 @@
sh.Ok()
}()
func() { // errShellErrIsNotNil
- sh := gosh.NewShell(gosh.Opts{Errorf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: t.Logf})
defer sh.Cleanup()
sh.Err = fakeError
defer func() { neq(t, recover(), nil) }()
sh.Ok()
}()
func() { // errAlreadyCalledCleanup
- sh := gosh.NewShell(gosh.Opts{Errorf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: t.Logf})
sh.Cleanup()
defer func() { neq(t, recover(), nil) }()
sh.Ok()
@@ -320,14 +573,14 @@
sh.HandleError(fakeError)
}()
func() { // errShellErrIsNotNil
- sh := gosh.NewShell(gosh.Opts{Errorf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: t.Logf})
defer sh.Cleanup()
sh.Err = fakeError
defer func() { neq(t, recover(), nil) }()
sh.HandleError(fakeError)
}()
func() { // errAlreadyCalledCleanup
- sh := gosh.NewShell(gosh.Opts{Errorf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: t.Logf})
sh.Cleanup()
defer func() { neq(t, recover(), nil) }()
sh.HandleError(fakeError)
@@ -345,14 +598,14 @@
// Tests that sh.Cleanup succeeds even if sh.Err is not nil.
func TestCleanupAfterError(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
sh.Err = fakeError
sh.Cleanup()
}
// Tests that sh.Cleanup can be called multiple times.
func TestMultipleCleanup(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
sh.Cleanup()
sh.Cleanup()
}
diff --git a/vlog/flags_test.go b/vlog/flags_test.go
index 575504e..f241165 100644
--- a/vlog/flags_test.go
+++ b/vlog/flags_test.go
@@ -42,7 +42,7 @@
})
func TestFlags(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: t.Fatalf, Logf: t.Logf})
+ sh := gosh.NewShell(gosh.Opts{Fatalf: t.Fatalf, Logf: t.Logf})
defer sh.Cleanup()
sh.Fn(child).Run()
}
