Merge "v.io/jiri/runutil: improvements inspired by caprita:-)"
diff --git a/runutil/.api b/runutil/.api
index d303875..ba929ec 100644
--- a/runutil/.api
+++ b/runutil/.api
@@ -1,3 +1,4 @@
+pkg runutil, func IsFNLHost() bool
pkg runutil, func LookPath(string, map[string]string) (string, error)
pkg runutil, func NewRun(map[string]string, io.Reader, io.Writer, io.Writer, bool, bool, bool) *Run
pkg runutil, func NewSequence(map[string]string, io.Reader, io.Writer, io.Writer, bool, bool, bool) *Sequence
@@ -6,6 +7,8 @@
pkg runutil, method (*Run) Chmod(string, os.FileMode) error
pkg runutil, method (*Run) Command(string, ...string) error
pkg runutil, method (*Run) CommandWithOpts(Opts, string, ...string) error
+pkg runutil, method (*Run) Copy(*os.File, io.Reader) (int64, error)
+pkg runutil, method (*Run) Create(string) (*os.File, error)
pkg runutil, method (*Run) DirectoryExists(string) bool
pkg runutil, method (*Run) FileExists(string) bool
pkg runutil, method (*Run) Function(func() error, string, ...interface{}) error
@@ -13,6 +16,7 @@
pkg runutil, method (*Run) IsDir(string) (bool, error)
pkg runutil, method (*Run) MkdirAll(string, os.FileMode) error
pkg runutil, method (*Run) Open(string) (*os.File, error)
+pkg runutil, method (*Run) OpenFile(string, int, os.FileMode) (*os.File, error)
pkg runutil, method (*Run) Output([]string)
pkg runutil, method (*Run) OutputWithOpts(Opts, []string)
pkg runutil, method (*Run) ReadDir(string) ([]os.FileInfo, error)
@@ -23,6 +27,7 @@
pkg runutil, method (*Run) Stat(string) (os.FileInfo, error)
pkg runutil, method (*Run) Symlink(string, string) error
pkg runutil, method (*Run) TempDir(string, string) (string, error)
+pkg runutil, method (*Run) TempFile(string, string) (*os.File, error)
pkg runutil, method (*Run) TimedCommand(time.Duration, string, ...string) error
pkg runutil, method (*Run) TimedCommandWithOpts(time.Duration, Opts, string, ...string) error
pkg runutil, method (*Run) WriteFile(string, []byte, os.FileMode) error
@@ -36,6 +41,7 @@
pkg runutil, method (*Sequence) FileExists(string) (bool, error)
pkg runutil, method (*Sequence) GetOpts() Opts
pkg runutil, method (*Sequence) IsDir(string) (bool, error)
+pkg runutil, method (*Sequence) Last(string, ...string) error
pkg runutil, method (*Sequence) MkdirAll(string, os.FileMode) *Sequence
pkg runutil, method (*Sequence) Open(string) (*os.File, error)
pkg runutil, method (*Sequence) Opts(Opts) *Sequence
diff --git a/runutil/sequence.go b/runutil/sequence.go
index 1d1b564..34f0533 100644
--- a/runutil/sequence.go
+++ b/runutil/sequence.go
@@ -5,8 +5,13 @@
package runutil
import (
+ "bytes"
+ "fmt"
"io"
+ "io/ioutil"
"os"
+ "path/filepath"
+ "runtime"
"time"
)
@@ -19,6 +24,11 @@
// methods and the result of that first error is returned by the
// Done method or any of the other 'terminating methods' (see below).
//
+// When not in verbose mode (set either via NewSequence or an Opt) output
+// from commands will be suppressed unless they return an error. When
+// verbose mode is set, all output is written to the supplied stdout
+// and stderr.
+//
// Modifier methods are provided that influence the behaviour of the
// next invocation of the Run method to set timeouts (Timeout) and to
// capture output (Capture), an additional modifier method (Opts) is
@@ -27,15 +37,17 @@
// For example, the following will result in a timeout error.
//
// err := s.Timed(time.Second).Run("sleep","10").Done()
+// err := s.Timed(time.Second).Last("sleep","10")
//
// A sequence of commands must be terminated with a call to a 'terminating'
-// method. The simplest is the Done method used in the examples above, but there
-// are other methods which typically return results in addition to error, such
-// as ReadFile(filename string) ([]byte, error). Here the usage would be:
+// method. The simplest are the Done or Last methods used in the examples above,
+// but there are other methods which typically return results in addition to
+// error, such as ReadFile(filename string) ([]byte, error). Here the usage
+// would be:
//
// o.Stdout, _ = os.Create("foo")
// data, err := s.Opts(o).Run("echo","b").ReadFile("foo")
-// // data == "foo"
+// // data == "b"
//
// Note that terminating functions, even those that take an action, may
// return an error generated by a previous method.
@@ -46,8 +58,10 @@
type Sequence struct {
r *Run
err error
+ caller string
stdout, stderr io.Writer
opts *Opts
+ dirs []string
timeout time.Duration
}
@@ -97,6 +111,9 @@
}
func (s *Sequence) Error() error {
+ if s.err != nil && len(s.caller) > 0 {
+ return fmt.Errorf("%s: %v", s.caller, s.err)
+ }
return s.err
}
@@ -104,6 +121,19 @@
s.opts = &opts
}
+func fmtError(depth int, err error, detail string) string {
+ _, file, line, _ := runtime.Caller(depth + 1)
+ return fmt.Sprintf("%s:%d: %s", filepath.Base(file), line, detail)
+}
+
+func (s *Sequence) setError(err error, detail string) {
+ if err == nil || s.err != nil {
+ return
+ }
+ s.err = err
+ s.caller = fmtError(2, err, detail)
+}
+
func (s *Sequence) reset() {
s.stdout, s.stderr, s.opts = nil, nil, nil
s.timeout = 0
@@ -128,7 +158,29 @@
func (s *Sequence) initAndDefer() func() {
if s.stdout == nil && s.stderr == nil {
- return func() {}
+ f, err := ioutil.TempFile("", "seq")
+ if err != nil {
+ return func() {}
+ }
+ opts := s.GetOpts()
+ stdout := opts.Stdout
+ opts.Stdout = f
+ opts.Stderr = f
+ s.setOpts(opts)
+ return func() {
+ filename := f.Name()
+ f.Close()
+ if opts.Verbose || s.err != nil {
+ // TODO(cnicolaou): probably best to stream this out rather
+ // than buffer the whole file into memory.
+ data, err := ioutil.ReadFile(filename)
+ if err == nil {
+ fmt.Fprint(stdout, string(data))
+ }
+ }
+ os.Remove(filename)
+ s.opts = nil
+ }
}
opts := s.GetOpts()
rStdin, wStdin := io.Pipe()
@@ -152,16 +204,87 @@
}
}
+func fmtArgs(args ...interface{}) string {
+ if len(args) == 0 {
+ return ""
+ }
+ out := &bytes.Buffer{}
+ for _, a := range args {
+ if _, ok := a.(string); ok {
+ out.WriteString(fmt.Sprintf(" ,%q", a))
+ } else {
+ out.WriteString(fmt.Sprintf(" ,%s", a))
+ }
+ }
+ return out.String()
+}
+
+func fmtStringArgs(args ...string) string {
+ if len(args) == 0 {
+ return ""
+ }
+ out := &bytes.Buffer{}
+ for _, a := range args {
+ out.WriteString(", \"")
+ out.WriteString(a)
+ out.WriteString("\"")
+ }
+ return out.String()
+}
+
+// Pushd pushes the current directory onto a stack and changes directory
+// to the specified one. Calling any terminating function will pop back
+// to the first element in the stack on completion of that function.
+func (s *Sequence) Pushd(dir string) *Sequence {
+ cwd, err := os.Getwd()
+ if err != nil {
+ s.setError(err, "Pushd("+dir+"): os.Getwd")
+ return s
+ }
+ s.dirs = append(s.dirs, cwd)
+ s.setError(s.r.Chdir(dir), "Pushd("+dir+")")
+ return s
+}
+
+// Popd popds the last directory from the directory stack and chdir's to it.
+// Calling any termination function will pop back to the first element in
+// the stack on completion of that function.
+func (s *Sequence) Popd() *Sequence {
+ if s.err != nil {
+ return s
+ }
+ if len(s.dirs) == 0 {
+ s.setError(fmt.Errorf("directory stack is empty"), "Popd()")
+ return s
+ }
+ last := s.dirs[len(s.dirs)-1]
+ s.dirs = s.dirs[:len(s.dirs)-1]
+ s.setError(s.r.Chdir(last), "Popd() -> "+last)
+ return s
+}
+
// Run runs the given command as a subprocess.
func (s *Sequence) Run(path string, args ...string) *Sequence {
if s.err != nil {
return s
}
defer s.initAndDefer()()
- s.err = s.r.command(s.timeout, s.GetOpts(), path, args...)
+ s.setError(s.r.command(s.timeout, s.GetOpts(), path, args...), fmt.Sprintf("Run(%q%s)", path, fmtStringArgs(args...)))
return s
}
+// Last runs the given command as a subprocess and returns an error
+// immediately terminating the sequence, it is equivalent to
+// calling s.Run(path, args...).Done().
+func (s *Sequence) Last(path string, args ...string) error {
+ if s.err != nil {
+ return s.Done()
+ }
+ defer s.initAndDefer()()
+ s.setError(s.r.command(s.timeout, s.GetOpts(), path, args...), fmt.Sprintf("Last(%q%s)", path, fmtStringArgs(args...)))
+ return s.Done()
+}
+
// Call runs the given function. Note that Capture and Timeout have no
// effect on invocations of Call, but Opts can control logging.
func (s *Sequence) Call(fn func() error, format string, args ...interface{}) *Sequence {
@@ -169,7 +292,7 @@
return s
}
defer s.initAndDefer()()
- s.err = s.r.FunctionWithOpts(s.GetOpts(), fn, format, args...)
+ s.setError(s.r.FunctionWithOpts(s.GetOpts(), fn, format, args...), fmt.Sprintf("Call(%s,%s%s)", fn, format, fmtArgs(args)))
return s
}
@@ -179,7 +302,7 @@
if s.err != nil {
return s
}
- s.err = s.r.Chdir(dir)
+ s.setError(s.r.Chdir(dir), "Chdir("+dir+")")
return s
}
@@ -190,7 +313,7 @@
return s
}
if err := s.r.Chmod(dir, mode); err != nil {
- s.err = err
+ s.setError(err, fmt.Sprintf("Chmod(%s, %s)", dir, mode))
}
return s
}
@@ -201,7 +324,7 @@
if s.err != nil {
return s
}
- s.err = s.r.MkdirAll(dir, mode)
+ s.setError(s.r.MkdirAll(dir, mode), fmt.Sprintf("MkdirAll(%s, %s)", dir, mode))
return s
}
@@ -211,7 +334,7 @@
if s.err != nil {
return s
}
- s.err = s.r.RemoveAll(dir)
+ s.setError(s.r.RemoveAll(dir), fmt.Sprintf("RemoveAll(%s)", dir))
return s
}
@@ -221,7 +344,7 @@
if s.err != nil {
return s
}
- s.err = s.r.Remove(file)
+ s.setError(s.r.Remove(file), fmt.Sprintf("Remove(%s)", file))
return s
}
@@ -231,7 +354,7 @@
if s.err != nil {
return s
}
- s.err = s.r.Rename(src, dst)
+ s.setError(s.r.Rename(src, dst), fmt.Sprintf("Rename(%s, %s)", src, dst))
return s
}
@@ -241,7 +364,7 @@
if s.err != nil {
return s
}
- s.err = s.r.Symlink(src, dst)
+ s.setError(s.r.Symlink(src, dst), fmt.Sprintf("Symlink(%s, %s)", src, dst))
return s
}
@@ -255,12 +378,32 @@
return s
}
-// Done returns the error stored in the Sequence. Done is a terminating function.
+// Done returns the error stored in the Sequence and pops back to the first
+// entry in the directory stack if Pushd has been called. Done is a terminating
+// function. There is no need to ensure that Done is called before returning
+// from a function that uses a sequence unless it is necessary to pop the
+// stack.
func (s *Sequence) Done() error {
- err := s.err
+ rerr := s.Error()
s.err = nil
+ s.caller = ""
s.reset()
- return err
+ if len(s.dirs) > 0 {
+ cwd := s.dirs[0]
+ s.dirs = nil
+ if err := s.r.Chdir(cwd); err != nil {
+ detail := "Done: Chdir(" + cwd + ")"
+ if rerr == nil {
+ s.setError(err, detail)
+ } else {
+ // In the unlikely event that Chdir fails in addition to an
+ // earlier error, we append an appropriate error message.
+ s.err = fmt.Errorf("%v\n%v", rerr, fmtError(1, err, detail))
+ }
+ return s.Error()
+ }
+ }
+ return rerr
}
// Open is a wrapper around os.Open that handles options such as
@@ -270,7 +413,7 @@
return nil, s.Done()
}
f, err := s.r.Open(name)
- s.err = err
+ s.setError(err, fmt.Sprintf("Open(%s)", name))
return f, s.Done()
}
@@ -281,7 +424,7 @@
return nil, s.Done()
}
fi, err := s.r.ReadDir(dirname)
- s.err = err
+ s.setError(err, fmt.Sprintf("ReadDir(%s)", dirname))
return fi, s.Done()
}
@@ -292,10 +435,21 @@
return nil, s.Done()
}
data, err := s.r.ReadFile(filename)
- s.err = err
+ s.setError(err, fmt.Sprintf("ReadFile(%s)", filename))
return data, s.Done()
}
+// WriteFile is a wrapper around ioutil.WriteFile that handles options
+// such as "verbose" or "dry run".
+func (s *Sequence) WriteFile(filename string, data []byte, perm os.FileMode) *Sequence {
+ if s.err != nil {
+ return s
+ }
+ err := s.r.WriteFile(filename, data, perm)
+ s.setError(err, fmt.Sprintf("WriteFile(%s, %10s, %s)", filename, data, perm))
+ return s
+}
+
// Stat is a wrapper around os.Stat that handles options such as
// "verbose" or "dry run". Stat is a terminating function.
func (s *Sequence) Stat(name string) (os.FileInfo, error) {
@@ -303,7 +457,7 @@
return nil, s.Done()
}
fi, err := s.r.Stat(name)
- s.err = err
+ s.setError(err, fmt.Sprintf("Stat(%s)", name))
return fi, s.Done()
}
@@ -314,18 +468,18 @@
return "", s.Done()
}
name, err := s.r.TempDir(dir, prefix)
- s.err = err
+ s.setError(err, fmt.Sprintf("TempDir(%s,%s)", dir, prefix))
return name, s.Done()
}
// IsDir is a wrapper around os.Stat with appropriate logging.
// IsDir is a terminating function.
-func (s *Sequence) IsDir(name string) (bool, error) {
+func (s *Sequence) IsDir(dirname string) (bool, error) {
if s.err != nil {
return false, s.Done()
}
- t, err := s.r.IsDir(name)
- s.err = err
+ t, err := s.r.IsDir(dirname)
+ s.setError(err, fmt.Sprintf("IsDir(%s)", dirname))
return t, s.Done()
}
diff --git a/runutil/sequence_test.go b/runutil/sequence_test.go
index 566f500..92f8b9d 100644
--- a/runutil/sequence_test.go
+++ b/runutil/sequence_test.go
@@ -9,28 +9,35 @@
"fmt"
"io/ioutil"
"os"
+ "path/filepath"
+ "regexp"
+ "strings"
"testing"
"time"
"v.io/jiri/runutil"
)
+func rmLineNumbers(pattern, s string) string {
+ re := regexp.MustCompile("(.*.go):" + pattern + ":(.*)")
+ return re.ReplaceAllString(s, "$1:-:$2")
+}
+
func ExampleSequence() {
seq := runutil.NewSequence(nil, os.Stdin, ioutil.Discard, ioutil.Discard, false, false, true)
err := seq.
Capture(os.Stdout, nil).Run("echo", "a").
- Capture(os.Stdout, nil).Run("echo", "b").
- Done()
+ Capture(os.Stdout, nil).Last("echo", "b")
err = seq.
Run("echo", "c").
Run("xxxxxxx").
- Capture(os.Stdout, nil).Run("echo", "d").
- Done()
- fmt.Println(err)
+ Capture(os.Stdout, nil).Last("echo", "d")
+ // Get rid of the line#s in the error output.
+ fmt.Println(rmLineNumbers("3.", err.Error()))
// Output:
// a
// b
- // exec: "xxxxxxx": executable file not found in $PATH
+ // sequence_test.go:-: Run("xxxxxxx"): exec: "xxxxxxx": executable file not found in $PATH
}
func TestSequence(t *testing.T) {
@@ -66,14 +73,13 @@
t.Errorf("got %v, want %v", got, want)
}
out.Reset()
- err = seq.Run("./bound-to-fail").Done()
+ err = seq.Run("./bound-to-fail", "fail").Done()
if err == nil {
t.Fatalf("should have experience an error")
}
- if got, want := err.Error(), "fork/exec ./bound-to-fail: no such file or directory"; got != want {
+ if got, want := rmLineNumbers("7.", err.Error()), "sequence_test.go:-: Run(\"./bound-to-fail\", \"fail\"): fork/exec ./bound-to-fail: no such file or directory"; got != want {
t.Errorf("got %v, want %v", got, want)
}
-
err = seq.
Capture(&out, nil).Run("echo", "works, despite previous error").Done()
if err != nil {
@@ -83,9 +89,8 @@
t.Errorf("got %v, want %v", got, want)
}
out.Reset()
-
err = seq.Timeout(time.Second).Run("sleep", "10").Done()
- if got, want := err.Error(), "command timed out"; got != want {
+ if got, want := rmLineNumbers("9.", err.Error()), "sequence_test.go:-: Run(\"sleep\", \"10\"): command timed out"; got != want {
t.Errorf("got %v, want %v", got, want)
}
@@ -102,8 +107,7 @@
}
err := seq.
Capture(&out, nil).Opts(opts).Run("sh", "-c", "echo $MYTEST").
- Capture(&out, nil).Run("sh", "-c", "echo $MYTEST").
- Done()
+ Capture(&out, nil).Last("sh", "-c", "echo $MYTEST")
if err != nil {
t.Fatal(err)
}
@@ -124,6 +128,50 @@
}
}
+func TestSequenceOutputOnError(t *testing.T) {
+ var out bytes.Buffer
+ // Only the output from the command that generates an error is written
+ // to out when not in verbose mode.
+ seq := runutil.NewSequence(nil, os.Stdin, &out, os.Stderr, false, false, false)
+ err := seq.Run("sh", "-c", "echo not me").
+ Run("sh", "-c", "echo ooh; echo ah; echo me; exit 1").
+ Last("sh", "-c", "echo not me either")
+ if err == nil {
+ t.Errorf("expected an error")
+ }
+ if got, want := out.String(), "oh\nah"; !strings.Contains(got, want) {
+ t.Errorf("got %v doesn't contain %v", got, want)
+ }
+ if got, notWant := out.String(), "not me either"; strings.Contains(got, notWant) {
+ t.Errorf("got %v contains %v", got, notWant)
+ }
+
+ out.Reset()
+ err = seq.Run("sh", "-c", "echo hard to not include me").
+ Run("sh", "-c", "echo ooh; echo ah; echo me").
+ Last("sh", "-c", "echo not me either")
+ if err != nil {
+ t.Error(err)
+ }
+ if got, want := len(out.String()), 0; got != want {
+ t.Logf(out.String())
+ t.Errorf("got %v, want %v", got, want)
+ }
+
+ out.Reset()
+ // All output is written to out when in verbose mode.
+ seq = runutil.NewSequence(nil, os.Stdin, &out, os.Stderr, false, false, true)
+ err = seq.Run("sh", "-c", "echo AA").
+ Run("sh", "-c", "echo BB; exit 1").
+ Last("sh", "-c", "echo CC")
+ if got, want := strings.Count(out.String(), "\n"), 6; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := strings.Count(out.String(), "AA"), 2; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+}
+
type timestamped struct {
times []time.Time
data [][]byte
@@ -178,6 +226,56 @@
}
}
+func getwd(t *testing.T) string {
+ here, err := os.Getwd()
+ if err != nil {
+ t.Fatal(err)
+ }
+ return here
+}
+
+func TestSequencePushPop(t *testing.T) {
+ here := getwd(t)
+ s := runutil.NewSequence(nil, os.Stdin, os.Stdout, os.Stderr, false, false, true)
+ components := []string{here, "test", "a", "b", "c"}
+ tree := filepath.Join(components...)
+ s.MkdirAll(tree, os.FileMode(0755))
+ if err := s.Error(); err != nil {
+ t.Fatal(err)
+ }
+ defer os.RemoveAll(filepath.Join(here, "test"))
+
+ td := ""
+ for _, d := range components {
+ s.Pushd(d)
+ td = filepath.Join(td, d)
+ if got, want := getwd(t), td; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ }
+ s.Done()
+ if got, want := getwd(t), here; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+
+ s.Pushd("test").Pushd("a").Pushd("b")
+ if got, want := getwd(t), filepath.Join(here, "test", "a", "b"); got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ err := s.Pushd("x").Done()
+ if err == nil {
+ t.Fatal(fmt.Errorf("expected an error"))
+ }
+ // Make sure the stack is unwound on error.
+ if got, want := getwd(t), here; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ if err := os.Chdir(here); err != nil {
+ panic(fmt.Sprintf("failed to chdir back to %s", here))
+ }
+
+ }
+}
+
// TODO(cnicolaou):
// - tests for functions
// - tests for terminating functions, make sure they clean up correctly.
diff --git a/tool/context.go b/tool/context.go
index fc2da35..0efecbb 100644
--- a/tool/context.go
+++ b/tool/context.go
@@ -89,12 +89,10 @@
func NewContext(opts ContextOpts) *Context {
initOpts(newContextOpts(), &opts)
run := runutil.NewRun(opts.Env, opts.Stdin, opts.Stdout, opts.Stderr, *opts.Color, *opts.DryRun, *opts.Verbose)
- seq := runutil.NewSequence(opts.Env, opts.Stdin, opts.Stdout, opts.Stderr, *opts.Color, *opts.DryRun, *opts.Verbose)
start := runutil.NewStart(opts.Env, opts.Stdin, opts.Stdout, opts.Stderr, *opts.Color, *opts.DryRun, *opts.Verbose)
return &Context{
opts: opts,
run: run,
- seq: seq,
start: start,
}
}
@@ -197,9 +195,10 @@
return ctx.run
}
-// Seq returns the sequence instance of the context.
-func (ctx Context) Seq() *runutil.Sequence {
- return ctx.seq
+// NewSeq returns a new instance of Sequence initialized using the options
+// stored in the context.
+func (ctx Context) NewSeq() *runutil.Sequence {
+ return runutil.NewSequence(ctx.opts.Env, ctx.opts.Stdin, ctx.opts.Stdout, ctx.opts.Stderr, *ctx.opts.Color, *ctx.opts.DryRun, *ctx.opts.Verbose)
}
// Start returns the start instance of the context.