lib: Use lookpath package consistently.
Changes the signature of the lookpath functions to:
Look(env map[string]string, ...) (string, error)
LookPrefix(env map[string]string, ...) ([]string, error)
The env map is passed in, rather than a slice of dirs, to
facilitate supporting other platforms in the future. E.g. on
windows we need to consult both the PATH and PATHDIRS environment
variables.
We now return an error, to more closely match LookPath in
os/exec. The implementation of these methods has also changed,
to more closely match the os/exec implementation.
Also removed the copy of LookPath in runutil, and changed code to
use the lookpath package instead.
Fixes v.io/i/1157
MultiPart: 3/3
Change-Id: I3890f4debd8d519dc8d86eb4eed6759d9f6fd18e
diff --git a/cmdline/.api b/cmdline/.api
index a5db6d2..3f5d6fb 100644
--- a/cmdline/.api
+++ b/cmdline/.api
@@ -5,8 +5,8 @@
pkg cmdline, func Main(*Command)
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) LookPathPrefix(string, map[string]bool) []string
+pkg cmdline, method (*Env) LookPath(string) (string, error)
+pkg cmdline, method (*Env) LookPathPrefix(string, map[string]bool) ([]string, error)
pkg cmdline, method (*Env) TimerPop()
pkg cmdline, method (*Env) TimerPush(string)
pkg cmdline, method (*Env) UsageErrorf(string, ...interface{}) error
diff --git a/cmdline/cmdline.go b/cmdline/cmdline.go
index 678731d..1ebac61 100644
--- a/cmdline/cmdline.go
+++ b/cmdline/cmdline.go
@@ -382,7 +382,7 @@
}
if cmd.LookPath {
// Look for a matching executable in PATH.
- if subCmd := env.LookPath(cmd.Name + "-" + subName); subCmd != "" {
+ if subCmd, _ := env.LookPath(cmd.Name + "-" + subName); subCmd != "" {
extArgs := append(flagsAsArgs(setFlags), subArgs...)
return binaryRunner{subCmd, cmdPath}, extArgs, nil
}
diff --git a/cmdline/env.go b/cmdline/env.go
index 8bc3509..06325c5 100644
--- a/cmdline/env.go
+++ b/cmdline/env.go
@@ -9,7 +9,6 @@
"io"
"os"
"strconv"
- "strings"
"v.io/x/lib/envvar"
"v.io/x/lib/lookpath"
@@ -77,18 +76,18 @@
// LookPath returns the absolute path of the executable with the given name,
// based on the directories in PATH. Calls lookpath.Look.
-func (e *Env) LookPath(name string) string {
+func (e *Env) LookPath(name string) (string, error) {
e.TimerPush("lookpath " + name)
defer e.TimerPop()
- return lookpath.Look(e.pathDirs(), name)
+ return lookpath.Look(e.Vars, name)
}
// 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 {
+func (e *Env) LookPathPrefix(prefix string, names map[string]bool) ([]string, error) {
e.TimerPush("lookpathprefix " + prefix)
defer e.TimerPop()
- return lookpath.LookPrefix(e.pathDirs(), prefix, names)
+ return lookpath.LookPrefix(e.Vars, prefix, names)
}
func usageErrorf(env *Env, usage func(*Env, io.Writer), format string, args ...interface{}) error {
@@ -126,10 +125,6 @@
return e.Vars["CMDLINE_PREFIX"]
}
-func (e *Env) pathDirs() []string {
- return strings.Split(e.Vars["PATH"], string(os.PathListSeparator))
-}
-
func (e *Env) firstCall() bool {
return e.Vars["CMDLINE_FIRST_CALL"] == ""
}
diff --git a/cmdline/help.go b/cmdline/help.go
index a7ee4fa..69a652a 100644
--- a/cmdline/help.go
+++ b/cmdline/help.go
@@ -127,7 +127,7 @@
}
if cmd.LookPath {
// Look for a matching executable in PATH.
- if subCmd := env.LookPath(cmd.Name + "-" + subName); subCmd != "" {
+ if subCmd, _ := env.LookPath(cmd.Name + "-" + subName); subCmd != "" {
runner := binaryRunner{subCmd, cmdPath}
envCopy := env.clone()
envCopy.Vars["CMDLINE_STYLE"] = config.style.String()
@@ -227,7 +227,8 @@
}
if cmd.LookPath {
cmdPrefix := cmd.Name + "-"
- for _, subCmd := range env.LookPathPrefix(cmdPrefix, cmd.subNames(cmdPrefix)) {
+ subCmds, _ := env.LookPathPrefix(cmdPrefix, cmd.subNames(cmdPrefix))
+ for _, subCmd := range subCmds {
runner := binaryRunner{subCmd, cmdPath}
var buffer bytes.Buffer
envCopy := env.clone()
@@ -310,7 +311,7 @@
var extChildren []string
cmdPrefix := cmd.Name + "-"
if cmd.LookPath {
- extChildren = env.LookPathPrefix(cmdPrefix, cmd.subNames(cmdPrefix))
+ extChildren, _ = env.LookPathPrefix(cmdPrefix, cmd.subNames(cmdPrefix))
}
hasSubcommands := len(cmd.Children) > 0 || len(extChildren) > 0
if hasSubcommands {
diff --git a/gosh/cmd.go b/gosh/cmd.go
index 9922b21..23347ed 100644
--- a/gosh/cmd.go
+++ b/gosh/cmd.go
@@ -259,9 +259,8 @@
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 {
- dirs := splitTokens(sh.Vars["PATH"], ":")
- lp := lookpath.Look(dirs, name)
- if lp == "" {
+ lp, err := lookpath.Look(sh.Vars, name)
+ if err != nil {
return nil, fmt.Errorf("gosh: failed to locate executable: %s", name)
}
name = lp
diff --git a/gosh/env_util.go b/gosh/env_util.go
index b12ccf3..a9966d7 100644
--- a/gosh/env_util.go
+++ b/gosh/env_util.go
@@ -72,14 +72,3 @@
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/lookpath/.api b/lookpath/.api
index c414a19..fe51477 100644
--- a/lookpath/.api
+++ b/lookpath/.api
@@ -1,2 +1,2 @@
-pkg lookpath, func Look([]string, string) string
-pkg lookpath, func LookPrefix([]string, string, map[string]bool) []string
+pkg lookpath, func Look(map[string]string, string) (string, error)
+pkg lookpath, func LookPrefix(map[string]string, string, map[string]bool) ([]string, error)
diff --git a/lookpath/lookpath.go b/lookpath/lookpath.go
index d58ca21..7ee21a6 100644
--- a/lookpath/lookpath.go
+++ b/lookpath/lookpath.go
@@ -2,77 +2,119 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
+// Package lookpath implements utilities to find executables.
package lookpath
+// TODO(toddw): implement for non-unix systems.
+
import (
"io/ioutil"
+ "os"
+ "os/exec"
"path/filepath"
"sort"
"strings"
)
-// 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 ""
+func splitPath(env map[string]string) []string {
+ var dirs []string
+ for _, dir := range strings.Split(env["PATH"], string(filepath.ListSeparator)) {
+ if dir != "" {
+ dirs = append(dirs, dir)
+ }
+ }
+ return dirs
+}
+
+func isExecutable(info os.FileInfo) bool {
+ mode := info.Mode()
+ return !mode.IsDir() && mode&0111 != 0
+}
+
+// Look returns the absolute path of the executable with the given name. If
+// name only contains a single path component, the dirs in env["PATH"] are
+// consulted, and the first match is returned. Otherwise, for multi-component
+// paths, the absolute path of the name is looked up directly.
+//
+// The behavior is the same as LookPath in the os/exec package, but allows the
+// env to be passed in explicitly.
+func Look(env map[string]string, name string) (string, error) {
+ var dirs []string
+ base := filepath.Base(name)
+ if base == name {
+ dirs = splitPath(env)
+ } else {
+ dirs = []string{filepath.Dir(name)}
}
for _, dir := range dirs {
- fileInfos, err := ioutil.ReadDir(dir)
+ file, err := filepath.Abs(filepath.Join(dir, base))
if err != nil {
continue
}
- for _, fileInfo := range fileInfos {
- if m := fileInfo.Mode(); !m.IsRegular() || (m&0111 == 0) {
- continue
- }
- if fileInfo.Name() == name {
- return filepath.Join(dir, name)
- }
+ info, err := os.Stat(file)
+ if err != nil {
+ continue
}
+ if !isExecutable(info) {
+ continue
+ }
+ return file, nil
}
- return ""
+ return "", &exec.Error{name, exec.ErrNotFound}
}
// 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.
+// prefix. If prefix only contains a single path component, the directories in
+// env["PATH"] are consulted. Otherwise, for multi-component prefixes, only the
+// directory containing the prefix is consulted. If multiple executables with
+// the same base name match the prefix in different directories, the first match
+// is returned. Returns a list of paths sorted by base 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 LookPrefix(dirs []string, prefix string, names map[string]bool) []string {
- if strings.Contains(prefix, string(filepath.Separator)) {
- return nil
- }
+func LookPrefix(env map[string]string, prefix string, names map[string]bool) ([]string, error) {
if names == nil {
names = make(map[string]bool)
}
+ var dirs []string
+ if filepath.Base(prefix) == prefix {
+ dirs = splitPath(env)
+ } else {
+ dirs = []string{filepath.Dir(prefix)}
+ }
var all []string
for _, dir := range dirs {
- fileInfos, err := ioutil.ReadDir(dir)
+ dir, err := filepath.Abs(dir)
if err != nil {
continue
}
- for _, fileInfo := range fileInfos {
- if m := fileInfo.Mode(); !m.IsRegular() || (m&0111 == 0) {
+ infos, err := ioutil.ReadDir(dir)
+ if err != nil {
+ continue
+ }
+ for _, info := range infos {
+ if !isExecutable(info) {
continue
}
- name, prefixLen := fileInfo.Name(), len(prefix)
- if len(name) < prefixLen || name[:prefixLen] != prefix {
+ name := info.Name()
+ file := filepath.Join(dir, name)
+ index := strings.LastIndex(file, prefix)
+ if index == -1 || strings.ContainsRune(file[index+len(prefix):], filepath.Separator) {
continue
}
if names[name] {
continue
}
names[name] = true
- all = append(all, filepath.Join(dir, name))
+ all = append(all, file)
}
}
- sort.Sort(byBase(all))
- return all
+ if len(all) > 0 {
+ sort.Sort(byBase(all))
+ return all, nil
+ }
+ return nil, &exec.Error{prefix + "*", exec.ErrNotFound}
}
type byBase []string
diff --git a/lookpath/lookpath_test.go b/lookpath/lookpath_test.go
index 9077213..614c2aa 100644
--- a/lookpath/lookpath_test.go
+++ b/lookpath/lookpath_test.go
@@ -5,10 +5,13 @@
package lookpath_test
import (
+ "fmt"
"io/ioutil"
"os"
+ "os/exec"
"path/filepath"
"reflect"
+ "strings"
"testing"
"v.io/x/lib/lookpath"
@@ -46,35 +49,61 @@
}
}
+func pathEnv(dir ...string) map[string]string {
+ return map[string]string{"PATH": strings.Join(dir, string(filepath.ListSeparator))}
+}
+
+func isNotFoundError(err error, name string) bool {
+ e, ok := err.(*exec.Error)
+ return ok && e.Name == name && e.Err == exec.ErrNotFound
+}
+
func TestLook(t *testing.T) {
tmpDir, cleanup := initTmpDir(t)
defer cleanup()
dirA, dirB := mkdir(t, tmpDir, "a"), mkdir(t, tmpDir, "b")
aFoo, aBar := mkfile(t, dirA, "foo", 0755), mkfile(t, dirA, "bar", 0755)
bBar, bBaz := mkfile(t, dirB, "bar", 0755), mkfile(t, dirB, "baz", 0755)
- _, bExe := mkfile(t, dirA, "exe", 0644), mkfile(t, dirB, "exe", 0755)
+ aExe, bExe := mkfile(t, dirA, "exe", 0644), mkfile(t, dirB, "exe", 0755)
tests := []struct {
- Dirs []string
+ Env map[string]string
Name string
Want string
}{
{nil, "", ""},
{nil, "foo", ""},
- {[]string{dirA}, "foo", aFoo},
- {[]string{dirA}, "bar", aBar},
- {[]string{dirA}, "baz", ""},
- {[]string{dirB}, "foo", ""},
- {[]string{dirB}, "bar", bBar},
- {[]string{dirB}, "baz", bBaz},
- {[]string{dirA, dirB}, "foo", aFoo},
- {[]string{dirA, dirB}, "bar", aBar},
- {[]string{dirA, dirB}, "baz", bBaz},
+ {pathEnv(dirA), "foo", aFoo},
+ {pathEnv(dirA), "bar", aBar},
+ {pathEnv(dirA), "baz", ""},
+ {pathEnv(dirB), "foo", ""},
+ {pathEnv(dirB), "bar", bBar},
+ {pathEnv(dirB), "baz", bBaz},
+ {pathEnv(dirA, dirB), "foo", aFoo},
+ {pathEnv(dirA, dirB), "bar", aBar},
+ {pathEnv(dirA, dirB), "baz", bBaz},
// Make sure we find bExe, since aExe isn't executable.
- {[]string{dirA, dirB}, "exe", bExe},
+ {pathEnv(dirA, dirB), "exe", bExe},
+ // Absolute name lookups.
+ {nil, dirA, ""},
+ {nil, dirB, ""},
+ {nil, aFoo, aFoo},
+ {nil, aBar, aBar},
+ {nil, bBar, bBar},
+ {nil, bBaz, bBaz},
+ {nil, aExe, ""},
+ {nil, bExe, bExe},
}
for _, test := range tests {
- 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)
+ hdr := fmt.Sprintf("env=%v name=%v", test.Env, test.Name)
+ look, err := lookpath.Look(test.Env, test.Name)
+ if got, want := look, test.Want; got != want {
+ t.Errorf("%s got %v, want %v", hdr, got, want)
+ }
+ if (look == "") == (err == nil) {
+ t.Errorf("%s got mismatched look=%v err=%v", hdr, look, err)
+ }
+ if err != nil && !isNotFoundError(err, test.Name) {
+ t.Errorf("%s got wrong error %v", hdr, err)
}
}
}
@@ -86,39 +115,65 @@
aFoo, aBar := mkfile(t, dirA, "foo", 0755), mkfile(t, dirA, "bar", 0755)
bBar, bBaz := mkfile(t, dirB, "bar", 0755), mkfile(t, dirB, "baz", 0755)
aBzz, bBaa := mkfile(t, dirA, "bzz", 0755), mkfile(t, dirB, "baa", 0755)
- _, bExe := mkfile(t, dirA, "exe", 0644), mkfile(t, dirB, "exe", 0755)
+ aExe, bExe := mkfile(t, dirA, "exe", 0644), mkfile(t, dirB, "exe", 0755)
tests := []struct {
- Dirs []string
+ Env map[string]string
Prefix string
Names map[string]bool
Want []string
}{
{nil, "", nil, nil},
{nil, "foo", nil, nil},
- {[]string{dirA}, "foo", nil, []string{aFoo}},
- {[]string{dirA}, "bar", nil, []string{aBar}},
- {[]string{dirA}, "baz", nil, nil},
- {[]string{dirA}, "f", nil, []string{aFoo}},
- {[]string{dirA}, "b", nil, []string{aBar, aBzz}},
- {[]string{dirB}, "foo", nil, nil},
- {[]string{dirB}, "bar", nil, []string{bBar}},
- {[]string{dirB}, "baz", nil, []string{bBaz}},
- {[]string{dirB}, "f", nil, nil},
- {[]string{dirB}, "b", nil, []string{bBaa, bBar, bBaz}},
- {[]string{dirA, dirB}, "foo", nil, []string{aFoo}},
- {[]string{dirA, dirB}, "bar", nil, []string{aBar}},
- {[]string{dirA, dirB}, "baz", nil, []string{bBaz}},
- {[]string{dirA, dirB}, "f", nil, []string{aFoo}},
- {[]string{dirA, dirB}, "b", nil, []string{bBaa, aBar, bBaz, aBzz}},
+ {pathEnv(dirA), "foo", nil, []string{aFoo}},
+ {pathEnv(dirA), "bar", nil, []string{aBar}},
+ {pathEnv(dirA), "baz", nil, nil},
+ {pathEnv(dirA), "f", nil, []string{aFoo}},
+ {pathEnv(dirA), "b", nil, []string{aBar, aBzz}},
+ {pathEnv(dirB), "foo", nil, nil},
+ {pathEnv(dirB), "bar", nil, []string{bBar}},
+ {pathEnv(dirB), "baz", nil, []string{bBaz}},
+ {pathEnv(dirB), "f", nil, nil},
+ {pathEnv(dirB), "b", nil, []string{bBaa, bBar, bBaz}},
+ {pathEnv(dirA, dirB), "foo", nil, []string{aFoo}},
+ {pathEnv(dirA, dirB), "bar", nil, []string{aBar}},
+ {pathEnv(dirA, dirB), "baz", nil, []string{bBaz}},
+ {pathEnv(dirA, dirB), "f", nil, []string{aFoo}},
+ {pathEnv(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}},
+ {pathEnv(dirA, dirB), "b", map[string]bool{"baz": true}, []string{bBaa, aBar, aBzz}},
// Make sure we find bExe, since aExe isn't executable.
- {[]string{dirA, dirB}, "exe", nil, []string{bExe}},
- {[]string{dirA, dirB}, "e", nil, []string{bExe}},
+ {pathEnv(dirA, dirB), "exe", nil, []string{bExe}},
+ {pathEnv(dirA, dirB), "e", nil, []string{bExe}},
+ // Absolute prefix lookups.
+ {nil, dirA, nil, nil},
+ {nil, dirB, nil, nil},
+ {nil, aFoo, nil, []string{aFoo}},
+ {nil, aBar, nil, []string{aBar}},
+ {nil, bBar, nil, []string{bBar}},
+ {nil, bBaz, nil, []string{bBaz}},
+ {nil, aBzz, nil, []string{aBzz}},
+ {nil, bBaa, nil, []string{bBaa}},
+ {nil, filepath.Join(dirA, "f"), nil, []string{aFoo}},
+ {nil, filepath.Join(dirA, "b"), nil, []string{aBar, aBzz}},
+ {nil, filepath.Join(dirB, "f"), nil, nil},
+ {nil, filepath.Join(dirB, "b"), nil, []string{bBaa, bBar, bBaz}},
+ {nil, filepath.Join(dirB, "b"), map[string]bool{"baz": true}, []string{bBaa, bBar}},
+ {nil, aExe, nil, nil},
+ {nil, filepath.Join(dirA, "e"), nil, nil},
+ {nil, bExe, nil, []string{bExe}},
+ {nil, filepath.Join(dirB, "e"), nil, []string{bExe}},
}
for _, test := range tests {
- 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)
+ hdr := fmt.Sprintf("env=%v prefix=%v names=%v", test.Env, test.Prefix, test.Names)
+ look, err := lookpath.LookPrefix(test.Env, test.Prefix, test.Names)
+ if got, want := look, test.Want; !reflect.DeepEqual(got, want) {
+ t.Errorf("%s got %v, want %v", hdr, got, want)
+ }
+ if (look == nil) == (err == nil) {
+ t.Errorf("%s got mismatched look=%v err=%v", hdr, look, err)
+ }
+ if err != nil && !isNotFoundError(err, test.Prefix+"*") {
+ t.Errorf("%s got wrong error %v", hdr, err)
}
}
}
