jiri: 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
Change-Id: Ib413dc6ca021b7304f9d67fcf2e76f32e6f315bc
MultiPart: 1/3
diff --git a/profiles/profilescmdline/manager_cmdline.go b/profiles/profilescmdline/manager_cmdline.go
index fd91958..c3cf226 100644
--- a/profiles/profilescmdline/manager_cmdline.go
+++ b/profiles/profilescmdline/manager_cmdline.go
@@ -36,6 +36,7 @@
"v.io/jiri/profiles"
"v.io/jiri/profiles/profilesmanager"
"v.io/x/lib/cmdline"
+ "v.io/x/lib/lookpath"
)
// newCmdInstall represents the "profile install" command.
@@ -284,15 +285,14 @@
func findProfileSubcommands(jirix *jiri.X) []string {
if !runSubcommands {
- return []string{}
+ return nil
}
fi, err := os.Stat(filepath.Join(jirix.Root, jiri.ProfilesDBDir))
if err == nil && fi.IsDir() {
- env := cmdline.EnvFromOS()
- env.Vars["PATH"] = jirix.Env()["PATH"]
- return env.LookPathPrefix("jiri-profile", map[string]bool{})
+ cmds, _ := lookpath.LookPrefix(jirix.Env(), "jiri-profile-", nil)
+ return cmds
}
- return []string{}
+ return nil
}
func allAvailableManagers(jirix *jiri.X) ([]string, error) {
diff --git a/runutil/.api b/runutil/.api
index 4f9083e..c288c41 100644
--- a/runutil/.api
+++ b/runutil/.api
@@ -3,7 +3,6 @@
pkg runutil, func IsNotExist(error) bool
pkg runutil, func IsPermission(error) bool
pkg runutil, func IsTimeout(error) bool
-pkg runutil, func LookPath(string, map[string]string) (string, error)
pkg runutil, func NewSequence(map[string]string, io.Reader, io.Writer, io.Writer, bool, bool, bool) Sequence
pkg runutil, func TranslateExitCode(error) error
pkg runutil, method (*Handle) Kill() error
diff --git a/runutil/executor.go b/runutil/executor.go
index c8cebce..e969e0c 100644
--- a/runutil/executor.go
+++ b/runutil/executor.go
@@ -16,6 +16,7 @@
"time"
"v.io/x/lib/envvar"
+ "v.io/x/lib/lookpath"
)
const (
@@ -82,16 +83,17 @@
e.printf(e.opts.stdout, format, args...)
}
err := fn()
- if err != nil {
- if opts.verbose {
- e.printf(e.opts.stdout, "FAILED: %v", err)
- }
- return err
- }
if opts.verbose {
- e.printf(e.opts.stdout, "OK")
+ e.printf(e.opts.stdout, okOrFailed(err))
}
- return nil
+ return err
+}
+
+func okOrFailed(err error) string {
+ if err != nil {
+ return fmt.Sprintf("FAILED: %v", err)
+ }
+ return "OK"
}
// output logs the given list of lines using the given
@@ -128,8 +130,7 @@
// Check if <path> identifies a binary in the PATH environment
// variable of the opts.Env.
- binary, err := LookPath(path, opts.env)
- if err == nil {
+ if binary, err := lookpath.Look(opts.env, path); err == nil {
// If so, make sure to execute this binary. This step
// enables us to "shadow" binaries included in the
// PATH environment variable of the host OS (which
@@ -159,31 +160,21 @@
e.printf(e.opts.stdout, strings.Replace(strings.Join(args, " "), "%", "%%", -1))
}
- if wait {
- if timeout == 0 {
- if err = command.Run(); err != nil {
- if opts.verbose {
- e.printf(e.opts.stdout, "FAILED")
- }
- } else {
- if opts.verbose {
- e.printf(e.opts.stdout, "OK")
- }
- }
- } else {
- err = e.timedCommand(timeout, opts, command)
- }
- } else {
+ var err error
+ switch {
+ case !wait:
err = command.Start()
- if err != nil {
- if opts.verbose {
- e.printf(e.opts.stdout, "FAILED")
- }
- } else {
- if opts.verbose {
- e.printf(e.opts.stdout, "OK")
- }
+ if opts.verbose {
+ e.printf(e.opts.stdout, okOrFailed(err))
}
+ case timeout == 0:
+ err = command.Run()
+ if opts.verbose {
+ e.printf(e.opts.stdout, okOrFailed(err))
+ }
+ default:
+ err = e.timedCommand(timeout, opts, command)
+ // Verbose output handled in timedCommand.
}
return command, err
}
@@ -204,7 +195,7 @@
}()
if err := command.Start(); err != nil {
if opts.verbose {
- e.printf(e.opts.stdout, "FAILED")
+ e.printf(e.opts.stdout, "FAILED: %v", err)
}
return err
}
@@ -223,14 +214,8 @@
}
return commandTimedOutErr
case err := <-done:
- if err != nil {
- if opts.verbose {
- e.printf(e.opts.stdout, "FAILED")
- }
- } else {
- if opts.verbose {
- e.printf(e.opts.stdout, "OK")
- }
+ if opts.verbose {
+ e.printf(e.opts.stdout, okOrFailed(err))
}
return err
}
diff --git a/runutil/executor_test.go b/runutil/executor_test.go
index 562137c..17679f7 100644
--- a/runutil/executor_test.go
+++ b/runutil/executor_test.go
@@ -55,7 +55,7 @@
if err := e.run(forever, e.opts, "go", "run", "./testdata/fail_hello.go"); err == nil {
t.Fatalf(`Command("go run ./testdata/fail_hello.go") did not fail when it should`)
}
- if got, want := removeTimestamps(t, &out), ">> go run ./testdata/fail_hello.go\nhello\n>> FAILED\n"; got != want {
+ if got, want := removeTimestamps(t, &out), ">> go run ./testdata/fail_hello.go\nhello\n>> FAILED: exit status 1\n"; got != want {
t.Fatalf("unexpected output:\ngot\n%v\nwant\n%v", got, want)
}
}
@@ -84,7 +84,7 @@
if err := e.run(forever, opts, "go", "run", "./testdata/fail_hello.go"); err == nil {
t.Fatalf(`CommandWithOpts("go run ./testdata/fail_hello.go") did not fail when it should`)
}
- if got, want := removeTimestamps(t, &runOut), ">> go run ./testdata/fail_hello.go\n>> FAILED\n"; got != want {
+ if got, want := removeTimestamps(t, &runOut), ">> go run ./testdata/fail_hello.go\n>> FAILED: exit status 1\n"; got != want {
t.Fatalf("unexpected output:\ngot\n%v\nwant\n%v", got, want)
}
if got, want := strings.TrimSpace(cmdOut.String()), "hello"; got != want {
diff --git a/runutil/lookpath.go b/runutil/lookpath.go
deleted file mode 100644
index 2b2c48a..0000000
--- a/runutil/lookpath.go
+++ /dev/null
@@ -1,61 +0,0 @@
-// 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 runutil
-
-import (
- "fmt"
- "os"
- "path/filepath"
- "strings"
-)
-
-// isExistingExecutable checks whether the given path points to an
-// existing executable.
-func isExistingExecutable(path string) (bool, error) {
- if fileInfo, err := os.Stat(path); err == nil {
- if mode := fileInfo.Mode(); !mode.IsDir() && (mode.Perm()&os.FileMode(0111)) != 0 {
- return true, nil
- }
- } else if !os.IsNotExist(err) {
- return false, fmt.Errorf("Stat(%v) failed: %v", path, err)
- }
- return false, nil
-}
-
-// LookPath inputs a command name and searches the PATH environment
-// variable of the snapshot for an executable file that would be run
-// had this command actually been invoked. The function returns an
-// absolute path to the executable file. In other words, LookPath
-// implements functionality similar to the UNIX "which" command
-// relative to the snapshot environment.
-func LookPath(file string, env map[string]string) (string, error) {
- if filepath.IsAbs(file) {
- ok, err := isExistingExecutable(file)
- if err != nil {
- return "", err
- } else if ok {
- return file, nil
- }
- return "", fmt.Errorf("failed to find %v", file)
- }
- envPath := env["PATH"]
- for _, dir := range strings.Split(envPath, string(os.PathListSeparator)) {
- path := filepath.Join(dir, file)
- ok, err := isExistingExecutable(path)
- if err != nil {
- return "", err
- } else if ok {
- if !filepath.IsAbs(path) {
- var err error
- path, err = filepath.Abs(path)
- if err != nil {
- return "", fmt.Errorf("Abs(%v) failed: %v", path, err)
- }
- }
- return path, nil
- }
- }
- return "", fmt.Errorf("failed to find %v in %q", file, envPath)
-}
diff --git a/runutil/lookpath_test.go b/runutil/lookpath_test.go
deleted file mode 100644
index 5f92d38..0000000
--- a/runutil/lookpath_test.go
+++ /dev/null
@@ -1,252 +0,0 @@
-// 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 runutil
-
-import (
- "io/ioutil"
- "os"
- "path/filepath"
- "testing"
-)
-
-func pathsMatch(t *testing.T, path1, path2 string) bool {
- eval1, err := filepath.EvalSymlinks(path1)
- if err != nil {
- t.Fatalf("EvalSymlinks(%v) failed: %v", path1, err)
- }
- eval2, err := filepath.EvalSymlinks(path2)
- if err != nil {
- t.Fatalf("EvalSymlinks(%v) failed: %v", path2, err)
- }
- return eval1 == eval2
-}
-
-// TestLookPathCommandOK checks that LookPath() succeeds when given an
-// existing command.
-func TestLookPathCommandOK(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", "")
- if err != nil {
- t.Fatalf("TempDir() failed: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatalf("Getwd() failed: %v", err)
- }
- if err := os.Chdir(tmpDir); err != nil {
- t.Fatalf("Chdir(%v) failed: %v", tmpDir, err)
- }
- defer os.Chdir(cwd)
- cmd := "jiri-unlikely-binary-name"
- absPath := filepath.Join(tmpDir, cmd)
-
- tmpFile, err := os.OpenFile(absPath, os.O_CREATE, os.FileMode(0755))
- if err != nil {
- t.Fatalf("OpenFile(%v) failed: %v", absPath, err)
- }
- defer tmpFile.Close()
- command := filepath.Base(absPath)
- env := map[string]string{"PATH": tmpDir}
- got, err := LookPath(command, env)
- if err != nil {
- t.Fatalf("LookPath(%v) failed: %v", command, err)
- }
- if want := absPath; !pathsMatch(t, got, want) {
- t.Fatalf("unexpected output: got %v, want %v", got, want)
- }
-}
-
-// TestLookPathCommandFail checks that LookPath() fails when given a
-// non-existing command.
-func TestLookPathCommandFail(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", "")
- if err != nil {
- t.Fatalf("TempDir() failed: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatalf("Getwd() failed: %v", err)
- }
- if err := os.Chdir(tmpDir); err != nil {
- t.Fatalf("Chdir(%v) failed: %v", tmpDir, err)
- }
- defer os.Chdir(cwd)
- absPath := filepath.Join(tmpDir, "jiri-unlikely-binary-name")
- env := map[string]string{"PATH": tmpDir}
- if _, err := LookPath(filepath.Base(absPath), env); err == nil {
- t.Fatalf("LookPath(%v) did not fail when it should", filepath.Base(absPath))
- }
-}
-
-// TestLookPathAbsoluteOk checks that LookPath() succeeds when given
-// an existing absolute path.
-func TestLookPathAbsoluteOK(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", "")
- if err != nil {
- t.Fatalf("TempDir() failed: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatalf("Getwd() failed: %v", err)
- }
- if err := os.Chdir(tmpDir); err != nil {
- t.Fatalf("Chdir(%v) failed: %v", tmpDir, err)
- }
- defer os.Chdir(cwd)
- absPath := filepath.Join(tmpDir, "jiri-unlikely-binary-name")
- tmpFile, err := os.OpenFile(absPath, os.O_CREATE, os.FileMode(0755))
- if err != nil {
- t.Fatalf("OpenFile(%v) failed: %v", absPath, err)
- }
- defer tmpFile.Close()
- env := map[string]string{"PATH": tmpDir}
- got, err := LookPath(absPath, env)
- if err != nil {
- t.Fatalf("LookPath(%v) failed: %v", absPath, err)
- }
- if want := absPath; !pathsMatch(t, got, want) {
- t.Fatalf("unexpected output: got %v, want %v", got, want)
- }
-}
-
-// TestLookPathAbsoluteFail checks that LookPath() fails when given a
-// non-existing absolute path.
-func TestLookPathAbsoluteFail(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", "")
- if err != nil {
- t.Fatalf("TempDir() failed: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatalf("Getwd() failed: %v", err)
- }
- if err := os.Chdir(tmpDir); err != nil {
- t.Fatalf("Chdir(%v) failed: %v", tmpDir, err)
- }
- defer os.Chdir(cwd)
- absPath := filepath.Join(tmpDir, "jiri-unlikely-binary-name")
- env := map[string]string{"PATH": tmpDir}
- if _, err := LookPath(absPath, env); err == nil {
- t.Fatalf("LookPath(%v) did not fail when it should", absPath)
- }
-}
-
-// TestLookPathAbsoluteExecFail checks that LookPath() fails when
-// given an existing absolute path to a non-executable file.
-func TestLookPathAbsoluteExecFail(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", "")
- if err != nil {
- t.Fatalf("TempDir() failed: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatalf("Getwd() failed: %v", err)
- }
- if err := os.Chdir(tmpDir); err != nil {
- t.Fatalf("Chdir(%v) failed: %v", tmpDir, err)
- }
- defer os.Chdir(cwd)
- absPath := filepath.Join(tmpDir, "jiri-unlikely-binary-name")
- tmpFile, err := os.OpenFile(absPath, os.O_CREATE, os.FileMode(0644))
- if err != nil {
- t.Fatalf("OpenFile(%v) failed: %v", absPath, err)
- }
- defer tmpFile.Close()
- env := map[string]string{"PATH": tmpDir}
- if _, err := LookPath(absPath, env); err == nil {
- t.Fatalf("LookPath(%v) did not fail when it should", absPath)
- }
-}
-
-// TestLookPathRelativeOK checks that LookPath() succeeds when given
-// an existing relative path.
-func TestLookPathRelativeOK(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", "")
- if err != nil {
- t.Fatalf("TempDir() failed: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatalf("Getwd() failed: %v", err)
- }
- if err := os.Chdir(tmpDir); err != nil {
- t.Fatalf("Chdir(%v) failed: %v", tmpDir, err)
- }
- defer os.Chdir(cwd)
- cmd := "jiri-unlikely-binary-name"
- absPath := filepath.Join(tmpDir, cmd)
- tmpFile, err := os.OpenFile(absPath, os.O_CREATE, os.FileMode(0755))
- if err != nil {
- t.Fatalf("OpenFile(%v) failed: %v", absPath, err)
- }
- defer tmpFile.Close()
- relPath := "." + string(os.PathSeparator) + filepath.Base(absPath)
- env := map[string]string{"PATH": tmpDir}
- got, err := LookPath(relPath, env)
- if err != nil {
- t.Fatalf("LookPath(%v) failed: %v", relPath, err)
- }
- if want := absPath; !pathsMatch(t, got, want) {
- t.Fatalf("unexpected output: got %v, want %v", got, want)
- }
-}
-
-// TestLookPathRelativeFail checks that LookPath() fails when given a
-// non-existing relative path.
-func TestLookPathRelativeFail(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", "")
- if err != nil {
- t.Fatalf("TempDir() failed: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatalf("Getwd() failed: %v", err)
- }
- if err := os.Chdir(tmpDir); err != nil {
- t.Fatalf("Chdir(%v) failed: %v", tmpDir, err)
- }
- defer os.Chdir(cwd)
- absPath := filepath.Join(tmpDir, "jiri-unlikely-binary-name")
- relPath := "." + string(os.PathSeparator) + filepath.Base(absPath)
- env := map[string]string{"PATH": tmpDir}
- if _, err := LookPath(relPath, env); err == nil {
- t.Fatalf("LookPath(%v) did not fail when it should", relPath)
- }
-}
-
-// TestLookPathRelativeExecFail checks that LookPath() fails when
-// given an existing relative path to a non-executable file.
-func TestLookPathRelativeExecFail(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", "")
- if err != nil {
- t.Fatalf("TempDir() failed: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatalf("Getwd() failed: %v", err)
- }
- if err := os.Chdir(tmpDir); err != nil {
- t.Fatalf("Chdir(%v) failed: %v", tmpDir, err)
- }
- defer os.Chdir(cwd)
- absPath := filepath.Join(tmpDir, "jiri-unlikely-binary-name")
- tmpFile, err := os.OpenFile(absPath, os.O_CREATE, os.FileMode(0644))
- if err != nil {
- t.Fatalf("OpenFile(%v) failed: %v", absPath, err)
- }
- defer tmpFile.Close()
- relPath := "." + string(os.PathSeparator) + filepath.Base(absPath)
- env := map[string]string{"PATH": tmpDir}
- if _, err := LookPath(relPath, env); err == nil {
- t.Fatalf("LookPath(%v) did not fail when it should", relPath)
- }
-}
diff --git a/runutil/sequence_test.go b/runutil/sequence_test.go
index 3bb7421..0dab62b 100644
--- a/runutil/sequence_test.go
+++ b/runutil/sequence_test.go
@@ -19,7 +19,6 @@
"time"
"v.io/jiri/runutil"
- "v.io/x/lib/envvar"
)
func rmLineNumbers(s string) string {
@@ -601,11 +600,7 @@
func TestStart(t *testing.T) {
s := runutil.NewSequence(nil, os.Stdin, os.Stdout, os.Stderr, false, false, true)
- path, err := runutil.LookPath("sleep", envvar.SliceToMap(os.Environ()))
- if err != nil {
- t.Fatal(err)
- }
- h, err := s.Start(path, "100")
+ h, err := s.Start("sh", "-c", "sleep 100")
if err != nil {
t.Fatal(err)
}
