veyron/lib/modules: introduce sh.StartExternalCommand
This CL splits up the shell.Start function into shell.Start and
shell.StartExternalCommand. This allows users (such as the e2e tests) to
run commands that have not been pre-registered. The shell will not
attempt to run these commands in the envelope.
Change-Id: I920b6172e6750aa3e6de4c031d5dd5e3fef95936
diff --git a/lib/modules/core/core.go b/lib/modules/core/core.go
index c2919fa..c937287 100644
--- a/lib/modules/core/core.go
+++ b/lib/modules/core/core.go
@@ -47,9 +47,6 @@
// echoClient <name> <text>
// invoke <name>.Echo(<text>)
//
-// exec <command> [args...]
-// executes the given command with the given arguments
-//
// proxyd <names>...
// runs a proxy server
package core
@@ -67,5 +64,4 @@
ProxyServerCommand = "proxyd"
WSPRCommand = "wsprd"
TestIdentitydCommand = "test_identityd"
- ExecCommand = "exec"
)
diff --git a/lib/modules/core/core_test.go b/lib/modules/core/core_test.go
index e5071c2..15a7c6a 100644
--- a/lib/modules/core/core_test.go
+++ b/lib/modules/core/core_test.go
@@ -202,7 +202,7 @@
func TestExec(t *testing.T) {
sh, cleanup := newShell(t)
defer cleanup()
- h, err := sh.Start(core.ExecCommand, nil, []string{"/bin/sh", "-c", "echo hello world"}...)
+ h, err := sh.StartExternalCommand(nil, []string{"/bin/sh", "-c", "echo hello world"}...)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
@@ -218,7 +218,7 @@
func TestExecWithEnv(t *testing.T) {
sh, cleanup := newShell(t)
defer cleanup()
- h, err := sh.Start(core.ExecCommand, []string{"BLAH=hello world"}, "/bin/sh", "-c", "printenv BLAH")
+ h, err := sh.StartExternalCommand([]string{"BLAH=hello world"}, "/bin/sh", "-c", "printenv BLAH")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
diff --git a/lib/modules/core/exec.go b/lib/modules/core/exec.go
deleted file mode 100644
index be41d0d..0000000
--- a/lib/modules/core/exec.go
+++ /dev/null
@@ -1,20 +0,0 @@
-package core
-
-import (
- "io"
- "syscall"
-
- "v.io/core/veyron/lib/modules"
-)
-
-func init() {
- modules.RegisterChild(ExecCommand, "", execCommand)
-}
-
-func execCommand(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
- envSlice := []string{}
- for key, value := range env {
- envSlice = append(envSlice, key+"="+value)
- }
- return syscall.Exec(args[1], args[1:], envSlice)
-}
diff --git a/lib/modules/exec.go b/lib/modules/exec.go
index b7e77c0..547ac0d 100644
--- a/lib/modules/exec.go
+++ b/lib/modules/exec.go
@@ -70,6 +70,10 @@
return &execHandle{name: name, entryPoint: shellEntryPoint + "=" + name, procErrCh: make(chan error, 1)}
}
+func newExecHandleForExternalCommand(name string) command {
+ return &execHandle{name: name, procErrCh: make(chan error, 1)}
+}
+
func (eh *execHandle) Stdout() io.Reader {
eh.mu.Lock()
defer eh.mu.Unlock()
@@ -114,8 +118,16 @@
eh.mu.Lock()
defer eh.mu.Unlock()
eh.sh = sh
- newargs, newenv := eh.envelope(sh, env, args[1:]...)
- cmd := exec.Command(os.Args[0], newargs[1:]...)
+ cmdPath := args[0]
+ newargs, newenv := args, env
+
+ // If an entry point is specified, use the envelope execution environment.
+ if eh.entryPoint != "" {
+ cmdPath = os.Args[0]
+ newargs, newenv = eh.envelope(sh, env, args[1:]...)
+ }
+
+ cmd := exec.Command(cmdPath, newargs[1:]...)
cmd.Env = newenv
stderr, err := newLogfile("stderr", eh.name)
if err != nil {
@@ -160,7 +172,6 @@
return nil, err
}
vlog.VI(1).Infof("Started: %q, pid %d", eh.name, cmd.Process.Pid)
- err = handle.WaitForReady(sh.startTimeout)
go func() {
eh.procErrCh <- eh.handle.Wait(0)
// It's now safe to close eh.stdout, since Wait only returns
@@ -170,7 +181,7 @@
eh.stdout.Close()
}()
- return eh, err
+ return eh, nil
}
func (eh *execHandle) Pid() int {
@@ -215,3 +226,7 @@
return procErr
}
+
+func (eh *execHandle) WaitForReady(timeout time.Duration) error {
+ return eh.handle.WaitForReady(timeout)
+}
diff --git a/lib/modules/func.go b/lib/modules/func.go
index e7372e6..5c5bc4c 100644
--- a/lib/modules/func.go
+++ b/lib/modules/func.go
@@ -5,6 +5,7 @@
"io"
"os"
"sync"
+ "time"
"v.io/core/veyron2/vlog"
)
@@ -143,3 +144,7 @@
fh.mu.Unlock()
return funcErr
}
+
+func (fh *functionHandle) WaitForReady(time.Duration) error {
+ return nil
+}
diff --git a/lib/modules/registry.go b/lib/modules/registry.go
index 7b01a1e..8331f9c 100644
--- a/lib/modules/registry.go
+++ b/lib/modules/registry.go
@@ -39,7 +39,7 @@
return r.cmds[name]
}
-// RegisterChild adds a new command to the registery that will be run
+// RegisterChild adds a new command to the registry that will be run
// as a subprocess. It must be called before Dispatch or DispatchInTest is
// called, typically from an init function.
func RegisterChild(name, help string, main Main) {
diff --git a/lib/modules/shell.go b/lib/modules/shell.go
index c80aae7..c6cd898 100644
--- a/lib/modules/shell.go
+++ b/lib/modules/shell.go
@@ -37,6 +37,7 @@
package modules
import (
+ "errors"
"fmt"
"io"
"io/ioutil"
@@ -213,6 +214,14 @@
return registry.help(command)
}
+func (sh *Shell) StartExternalCommand(env []string, args ...string) (Handle, error) {
+ if len(args) == 0 {
+ return nil, errors.New("no arguments specified to StartExternalCommand")
+ }
+ c := newExecHandleForExternalCommand(args[0])
+ return sh.startCommand(c, env, args...)
+}
+
// Start starts the specified command, it returns a Handle which can be
// used for interacting with that command.
//
@@ -236,6 +245,26 @@
// Commands must have already been registered using RegisterFunction
// or RegisterChild.
func (sh *Shell) Start(name string, env []string, args ...string) (Handle, error) {
+ cmd := registry.getCommand(name)
+ if cmd == nil {
+ return nil, fmt.Errorf("%s: not registered", name)
+ }
+ expanded := append([]string{name}, sh.expand(args...)...)
+ c := cmd.factory()
+ h, err := sh.startCommand(c, env, expanded...)
+ if err != nil {
+ // If the error is a timeout, then h can be used to recover
+ // any output from the process.
+ return h, err
+ }
+
+ if err := h.WaitForReady(sh.waitTimeout); err != nil {
+ return h, err
+ }
+ return h, nil
+}
+
+func (sh *Shell) startCommand(c command, env []string, args ...string) (Handle, error) {
cenv, err := sh.setupCommandEnv(env)
if err != nil {
return nil, err
@@ -244,16 +273,10 @@
if err != nil {
return nil, err
}
- cmd := registry.getCommand(name)
- if cmd == nil {
- return nil, fmt.Errorf("%s: not registered", name)
- }
- expanded := append([]string{name}, sh.expand(args...)...)
- h, err := cmd.factory().start(sh, p, cenv, expanded...)
+
+ h, err := c.start(sh, p, cenv, args...)
if err != nil {
- // If the error is a timeout, then h can be used to recover
- // any output from the process.
- return h, err
+ return nil, err
}
sh.mu.Lock()
sh.handles[h] = struct{}{}
@@ -459,6 +482,11 @@
// Pid returns the pid of the process running the command
Pid() int
+
+ // WaitForReady waits until the child process signals to us that it is
+ // ready. If this does not occur within the given timeout duration, a
+ // timeout error is returned.
+ WaitForReady(timeout time.Duration) error
}
// command is used to abstract the implementations of inprocess and subprocess
diff --git a/lib/testutil/integration/util.go b/lib/testutil/integration/util.go
index 46a2a6b..208fad1 100644
--- a/lib/testutil/integration/util.go
+++ b/lib/testutil/integration/util.go
@@ -262,9 +262,9 @@
locationString = fmt.Sprintf("(requested at %s:%d) ", filepath.Base(file), line)
}
b.env.t.Logf("%sstarting %s %s", locationString, b.Path(), strings.Join(args, " "))
- handle, err := b.env.shell.Start("exec", b.envVars, append([]string{b.Path()}, args...)...)
+ handle, err := b.env.shell.StartExternalCommand(b.envVars, append([]string{b.Path()}, args...)...)
if err != nil {
- b.env.t.Fatalf("Start(%v, %v) failed: %v", b.Path(), strings.Join(args, ", "), err)
+ b.env.t.Fatalf("StartExternalCommand(%v, %v) failed: %v", b.Path(), strings.Join(args, ", "), err)
}
b.env.t.Logf("started PID %d\n", handle.Pid())
return &integrationTestBinaryInvocation{
