lib: add pkg lookpath, and make gosh use it

Details:
- Moves lookpath functionality out of v.io/x/lib/envvar, to
a new package named v.io/x/lib/lookpath
- Updates gosh to use this package, using the dirs from the
Shell's PATH variable

MultiPart: 2/2

Change-Id: I1a357758183cccf3a950176a3bc5bdf48d7fb887
diff --git a/cmdline/.api b/cmdline/.api
index b48e367..0c11b43 100644
--- a/cmdline/.api
+++ b/cmdline/.api
@@ -6,7 +6,7 @@
 pkg cmdline, func Parse(*Command, *Env, []string) (Runner, []string, error)
 pkg cmdline, func ParseAndRun(*Command, *Env, []string) error
 pkg cmdline, method (*Env) LookPath(string) string
-pkg cmdline, method (*Env) LookPathAll(string, map[string]bool) []string
+pkg cmdline, method (*Env) LookPathPrefix(string, map[string]bool) []string
 pkg cmdline, method (*Env) TimerPop()
 pkg cmdline, method (*Env) TimerPush(string)
 pkg cmdline, method (*Env) UsageErrorf(string, ...interface{}) error
diff --git a/cmdline/env.go b/cmdline/env.go
index d9fbbd3..8bc3509 100644
--- a/cmdline/env.go
+++ b/cmdline/env.go
@@ -12,6 +12,7 @@
 	"strings"
 
 	"v.io/x/lib/envvar"
+	"v.io/x/lib/lookpath"
 	"v.io/x/lib/textutil"
 	"v.io/x/lib/timing"
 )
@@ -75,19 +76,19 @@
 }
 
 // LookPath returns the absolute path of the executable with the given name,
-// based on the directories in PATH.  Calls envvar.LookPath.
+// based on the directories in PATH.  Calls lookpath.Look.
 func (e *Env) LookPath(name string) string {
 	e.TimerPush("lookpath " + name)
 	defer e.TimerPop()
-	return envvar.LookPath(e.pathDirs(), name)
+	return lookpath.Look(e.pathDirs(), name)
 }
 
-// LookPathAll returns the absolute paths of all executables with the given name
-// prefix, based on the directories in PATH.  Calls envvar.LookPath.
-func (e *Env) LookPathAll(prefix string, names map[string]bool) []string {
-	e.TimerPush("lookpathall " + prefix)
+// LookPathPrefix returns the absolute paths of all executables with the given
+// name prefix, based on the directories in PATH.  Calls lookpath.LookPrefix.
+func (e *Env) LookPathPrefix(prefix string, names map[string]bool) []string {
+	e.TimerPush("lookpathprefix " + prefix)
 	defer e.TimerPop()
-	return envvar.LookPathAll(e.pathDirs(), prefix, names)
+	return lookpath.LookPrefix(e.pathDirs(), prefix, names)
 }
 
 func usageErrorf(env *Env, usage func(*Env, io.Writer), format string, args ...interface{}) error {
diff --git a/cmdline/help.go b/cmdline/help.go
index 6971b8b..a7ee4fa 100644
--- a/cmdline/help.go
+++ b/cmdline/help.go
@@ -227,7 +227,7 @@
 	}
 	if cmd.LookPath {
 		cmdPrefix := cmd.Name + "-"
-		for _, subCmd := range env.LookPathAll(cmdPrefix, cmd.subNames(cmdPrefix)) {
+		for _, subCmd := range env.LookPathPrefix(cmdPrefix, cmd.subNames(cmdPrefix)) {
 			runner := binaryRunner{subCmd, cmdPath}
 			var buffer bytes.Buffer
 			envCopy := env.clone()
@@ -310,7 +310,7 @@
 	var extChildren []string
 	cmdPrefix := cmd.Name + "-"
 	if cmd.LookPath {
-		extChildren = env.LookPathAll(cmdPrefix, cmd.subNames(cmdPrefix))
+		extChildren = env.LookPathPrefix(cmdPrefix, cmd.subNames(cmdPrefix))
 	}
 	hasSubcommands := len(cmd.Children) > 0 || len(extChildren) > 0
 	if hasSubcommands {
diff --git a/envvar/.api b/envvar/.api
index ec7414e..569d0b4 100644
--- a/envvar/.api
+++ b/envvar/.api
@@ -3,8 +3,6 @@
 pkg envvar, func CopySlice([]string) []string
 pkg envvar, func JoinKeyValue(string, string) string
 pkg envvar, func JoinTokens([]string, string) string
-pkg envvar, func LookPath([]string, string) string
-pkg envvar, func LookPathAll([]string, string, map[string]bool) []string
 pkg envvar, func MapToSlice(map[string]string) []string
 pkg envvar, func MergeMaps(...map[string]string) map[string]string
 pkg envvar, func MergeSlices(...[]string) []string
diff --git a/gosh/buffered_pipe_test.go b/gosh/buffered_pipe_test.go
index b3d45b9..0ba942f 100644
--- a/gosh/buffered_pipe_test.go
+++ b/gosh/buffered_pipe_test.go
@@ -15,17 +15,17 @@
 func TestReadWriteAfterClose(t *testing.T) {
 	p := newBufferedPipe()
 	if n, err := p.Write([]byte("foo")); n != 3 || err != nil {
-		t.Errorf("write got (%v,%v) want (3,nil)", n, err)
+		t.Errorf("write got (%v, %v), want (3, <nil>)", n, err)
 	}
 	if n, err := p.Write([]byte("barbaz")); n != 6 || err != nil {
-		t.Errorf("write got (%v,%v) want (6,nil)", n, err)
+		t.Errorf("write got (%v, %v), want (6, <nil>)", n, err)
 	}
 	if err := p.Close(); err != nil {
 		t.Errorf("close failed: %v", err)
 	}
 	// Read after close returns all data terminated by EOF.
 	if b, err := ioutil.ReadAll(p); string(b) != "foobarbaz" || err != nil {
-		t.Errorf("read got (%s,%v) want (foobarbaz,nil)", b, err)
+		t.Errorf("read got (%s, %v), want (foobarbaz, <nil>)", b, err)
 	}
 	// Write after close fails.
 	n, err := p.Write([]byte("already closed"))
diff --git a/gosh/cmd.go b/gosh/cmd.go
index b08b2fd..9357094 100644
--- a/gosh/cmd.go
+++ b/gosh/cmd.go
@@ -8,6 +8,7 @@
 	"bytes"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 	"os"
 	"os/exec"
@@ -15,6 +16,8 @@
 	"sync"
 	"syscall"
 	"time"
+
+	"v.io/x/lib/lookpath"
 )
 
 var (
@@ -256,11 +259,12 @@
 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
+		dirs := splitTokens(sh.Vars["PATH"], ":")
+		lp := lookpath.Look(dirs, name)
+		if lp == "" {
+			return nil, fmt.Errorf("gosh: failed to locate executable: %s", name)
 		}
+		name = lp
 	}
 	return newCmdInternal(sh, vars, name, args)
 }
diff --git a/gosh/env_util.go b/gosh/env_util.go
index a9966d7..b12ccf3 100644
--- a/gosh/env_util.go
+++ b/gosh/env_util.go
@@ -72,3 +72,14 @@
 func copyMap(m map[string]string) map[string]string {
 	return mergeMaps(m)
 }
+
+// splitTokens is like strings.Split(value, sep), but drops empty tokens.
+func splitTokens(s, sep string) []string {
+	var tokens []string
+	for _, token := range strings.Split(s, sep) {
+		if token != "" {
+			tokens = append(tokens, token)
+		}
+	}
+	return tokens
+}
diff --git a/gosh/internal/hello_world/main.go b/gosh/internal/hello_world/main.go
new file mode 100644
index 0000000..afc2f0c
--- /dev/null
+++ b/gosh/internal/hello_world/main.go
@@ -0,0 +1,16 @@
+// 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 main
+
+import (
+	"fmt"
+
+	"v.io/x/lib/gosh"
+)
+
+func main() {
+	gosh.InitChildMain()
+	fmt.Println("Hello, world!")
+}
diff --git a/gosh/shell_test.go b/gosh/shell_test.go
index 996e003..9078c0e 100644
--- a/gosh/shell_test.go
+++ b/gosh/shell_test.go
@@ -243,32 +243,44 @@
 	eq(t, c.Stdout(), "Hello, world!\n")
 }
 
-// Tests that BuildGoPkg works when the -o flag is passed.
+// Tests that BuildGoPkg works even when the -o flag is passed.
+// TODO(sadovsky): Add tests for (1) missing name after -o, (2) multiple
+// instances of -o, (3) using --o instead of -o, and perhaps other cases as
+// well.
 func TestBuildGoPkg(t *testing.T) {
 	sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
 	defer sh.Cleanup()
 
-	binPath := sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_server")
-	c := sh.Cmd(binPath)
-	c.Start()
-	addr := c.AwaitVars("addr")["addr"]
-	neq(t, addr, "")
-
-	cwd, err := os.Getwd()
-	ok(t, err)
-	absName := filepath.Join(cwd, "x")
-	binPath = sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_client", "-o", absName)
-	defer os.Remove(absName)
-	c = sh.Cmd(absName, "-addr="+addr)
+	tempDir := sh.MakeTempDir()
+	absName := filepath.Join(tempDir, "hw")
+	sh.BuildGoPkg("v.io/x/lib/gosh/internal/hello_world", "-o", absName)
+	c := sh.Cmd(absName)
 	eq(t, c.Stdout(), "Hello, world!\n")
 
-	relName := filepath.Join(sh.Opts.BinDir, "y")
-	binPath = sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_client", "-o", "y")
-	defer os.Remove(relName)
-	c = sh.Cmd(relName, "-addr="+addr)
+	absName = filepath.Join(sh.Opts.BinDir, "hw")
+	sh.BuildGoPkg("v.io/x/lib/gosh/internal/hello_world", "-o", "hw")
+	c = sh.Cmd(absName)
 	eq(t, c.Stdout(), "Hello, world!\n")
 }
 
+// Tests that Shell.Cmd uses Shell.Vars["PATH"] to locate executables with
+// relative names.
+func TestLookPath(t *testing.T) {
+	sh := gosh.NewShell(gosh.Opts{Fatalf: makeFatalf(t), Logf: t.Logf})
+	defer sh.Cleanup()
+
+	tempDir := sh.MakeTempDir()
+	sh.Vars["PATH"] += ":" + tempDir
+	absName, relName := filepath.Join(tempDir, "hw"), "hw"
+	sh.BuildGoPkg("v.io/x/lib/gosh/internal/hello_world", "-o", absName)
+	c := sh.Cmd(relName)
+	eq(t, c.Stdout(), "Hello, world!\n")
+
+	// Test the case where we cannot find the executable.
+	sh.Vars["PATH"] = ""
+	setsErr(t, sh, func() { sh.Cmd("yes") })
+}
+
 var (
 	sendVarsFunc = gosh.RegisterFunc("sendVarsFunc", func(vars map[string]string) {
 		gosh.SendVars(vars)
diff --git a/lookpath/.api b/lookpath/.api
new file mode 100644
index 0000000..c414a19
--- /dev/null
+++ b/lookpath/.api
@@ -0,0 +1,2 @@
+pkg lookpath, func Look([]string, string) string
+pkg lookpath, func LookPrefix([]string, string, map[string]bool) []string
diff --git a/envvar/lookpath.go b/lookpath/lookpath.go
similarity index 78%
rename from envvar/lookpath.go
rename to lookpath/lookpath.go
index 2c9983e..d58ca21 100644
--- a/envvar/lookpath.go
+++ b/lookpath/lookpath.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package envvar
+package lookpath
 
 import (
 	"io/ioutil"
@@ -11,10 +11,10 @@
 	"strings"
 )
 
-// LookPath returns the absolute path of the executable with the given name,
-// based on the given dirs.  If multiple exectuables match the name, the first
-// match in dirs is returned.  Invalid dirs are silently ignored.
-func LookPath(dirs []string, name string) string {
+// Look returns the absolute path of the executable with the given name, based
+// on the given dirs.  If multiple executables match the name, the first match
+// in dirs is returned.  Invalid dirs are silently ignored.
+func Look(dirs []string, name string) string {
 	if strings.Contains(name, string(filepath.Separator)) {
 		return ""
 	}
@@ -35,15 +35,15 @@
 	return ""
 }
 
-// LookPathAll returns the absolute paths of all executables with the given name
-// prefix, based on the given dirs.  If multiple exectuables match the prefix
+// LookPrefix returns the absolute paths of all executables with the given name
+// prefix, based on the given dirs.  If multiple executables match the prefix
 // with the same name, the first match in dirs is returned.  Invalid dirs are
 // silently ignored.  Returns a list of paths sorted by name.
 //
 // The names are filled in as the method runs, to ensure the first matching
 // property.  As a consequence, you may pass in a pre-populated names map to
 // prevent matching those names.  It is fine to pass in a nil names map.
-func LookPathAll(dirs []string, prefix string, names map[string]bool) []string {
+func LookPrefix(dirs []string, prefix string, names map[string]bool) []string {
 	if strings.Contains(prefix, string(filepath.Separator)) {
 		return nil
 	}
diff --git a/envvar/lookpath_test.go b/lookpath/lookpath_test.go
similarity index 88%
rename from envvar/lookpath_test.go
rename to lookpath/lookpath_test.go
index 57d5933..9077213 100644
--- a/envvar/lookpath_test.go
+++ b/lookpath/lookpath_test.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package envvar
+package lookpath_test
 
 import (
 	"io/ioutil"
@@ -10,6 +10,8 @@
 	"path/filepath"
 	"reflect"
 	"testing"
+
+	"v.io/x/lib/lookpath"
 )
 
 func mkdir(t *testing.T, d ...string) string {
@@ -44,7 +46,7 @@
 	}
 }
 
-func TestLookPath(t *testing.T) {
+func TestLook(t *testing.T) {
 	tmpDir, cleanup := initTmpDir(t)
 	defer cleanup()
 	dirA, dirB := mkdir(t, tmpDir, "a"), mkdir(t, tmpDir, "b")
@@ -67,17 +69,17 @@
 		{[]string{dirA, dirB}, "foo", aFoo},
 		{[]string{dirA, dirB}, "bar", aBar},
 		{[]string{dirA, dirB}, "baz", bBaz},
-		// Make sure we find bExe, since aExe isn't executable
+		// Make sure we find bExe, since aExe isn't executable.
 		{[]string{dirA, dirB}, "exe", bExe},
 	}
 	for _, test := range tests {
-		if got, want := LookPath(test.Dirs, test.Name), test.Want; got != want {
+		if got, want := lookpath.Look(test.Dirs, test.Name), test.Want; got != want {
 			t.Errorf("dirs=%v name=%v got %v, want %v", test.Dirs, test.Name, got, want)
 		}
 	}
 }
 
-func TestLookPathAll(t *testing.T) {
+func TestLookPrefix(t *testing.T) {
 	tmpDir, cleanup := initTmpDir(t)
 	defer cleanup()
 	dirA, dirB := mkdir(t, tmpDir, "a"), mkdir(t, tmpDir, "b")
@@ -110,12 +112,12 @@
 		{[]string{dirA, dirB}, "b", nil, []string{bBaa, aBar, bBaz, aBzz}},
 		// Don't find baz, since it's already provided.
 		{[]string{dirA, dirB}, "b", map[string]bool{"baz": true}, []string{bBaa, aBar, aBzz}},
-		// Make sure we find bExe, since aExe isn't executable
+		// Make sure we find bExe, since aExe isn't executable.
 		{[]string{dirA, dirB}, "exe", nil, []string{bExe}},
 		{[]string{dirA, dirB}, "e", nil, []string{bExe}},
 	}
 	for _, test := range tests {
-		if got, want := LookPathAll(test.Dirs, test.Prefix, test.Names), test.Want; !reflect.DeepEqual(got, want) {
+		if got, want := lookpath.LookPrefix(test.Dirs, test.Prefix, test.Names), test.Want; !reflect.DeepEqual(got, want) {
 			t.Errorf("dirs=%v prefix=%v names=%v got %v, want %v", test.Dirs, test.Prefix, test.Names, got, want)
 		}
 	}