veyron/lib/modules: provide a means to auto-register subprocesses.

Change-Id: I8f01f3da16bf223e7af1613d637742608c84cc86
diff --git a/lib/modules/core/core.go b/lib/modules/core/core.go
index ccb14dd..9aa5767 100644
--- a/lib/modules/core/core.go
+++ b/lib/modules/core/core.go
@@ -61,20 +61,15 @@
 
 // NewShell returns a new Shell instance with the core commands installed.
 func NewShell() *modules.Shell {
-	shell := modules.NewShell()
+	shell := modules.NewShell(".*")
 	Install(shell)
 	return shell
 }
 
 // Install installs the core commands into the supplied Shell.
 func Install(shell *modules.Shell) {
-	shell.AddSubprocess(RootMTCommand, "")
-	shell.AddSubprocess(MTCommand, `<mount point>
-	reads NAMESPACE_ROOT from its environment and mounts a new mount table at <mount point>`)
 	shell.AddFunction(LSCommand, ls, `<glob>...
 	issues glob requests using the current processes namespace library`)
-	shell.AddSubprocess(LSExternalCommand, `<glob>...
-	runs a subprocess to issue glob requests using the subprocesses namespace library`)
 	shell.AddFunction(ResolveCommand, resolveObject, `<name>
 	resolves name to obtain an object server address`)
 	shell.AddFunction(ResolveMTCommand, resolveMT, `<name>
@@ -89,8 +84,4 @@
 	turns the namespace cache on or off`)
 	shell.AddFunction(MountCommand, mountServer, `<mountpoint> <server> <ttl> [M][R]
 	invokes namespace.Mount(<mountpoint>, <server>, <ttl>)`)
-	shell.AddSubprocess(EchoClientCommand, `<name> <message>...
-	invokes name.Echo(message)`)
-	shell.AddSubprocess(EchoServerCommand, `<name> <text>
-	runs an Echo server mounted at <name> and configured to return <text>: as a prefix in its response`)
 }
diff --git a/lib/modules/core/echo.go b/lib/modules/core/echo.go
index 4a2e5a5..8456539 100644
--- a/lib/modules/core/echo.go
+++ b/lib/modules/core/echo.go
@@ -15,14 +15,14 @@
 )
 
 func init() {
-	modules.RegisterChild(EchoServerCommand, echoServer)
-	modules.RegisterChild(EchoClientCommand, echoClient)
+	modules.RegisterChild(EchoServerCommand, `<name> <message>...
+	invokes name.Echo(message)`, echoServer)
+	modules.RegisterChild(EchoClientCommand, `<name> <text>
+	runs an Echo server mounted at <name> and configured to return <text>: as a prefix in its response`, echoClient)
 }
 
 type treeDispatcher struct{ id string }
 
-var _ ipc.Dispatcher = (*treeDispatcher)(nil)
-
 func (d treeDispatcher) Lookup(suffix, method string) (ipc.Invoker, security.Authorizer, error) {
 	return ipc.ReflectInvoker(&echoServerObject{d.id, suffix}), nil, nil
 }
diff --git a/lib/modules/core/mounttable.go b/lib/modules/core/mounttable.go
index b013862..f1ff8fa 100644
--- a/lib/modules/core/mounttable.go
+++ b/lib/modules/core/mounttable.go
@@ -17,9 +17,12 @@
 )
 
 func init() {
-	modules.RegisterChild(RootMTCommand, rootMountTable)
-	modules.RegisterChild(MTCommand, mountTable)
-	modules.RegisterChild(LSExternalCommand, ls)
+	modules.RegisterChild(RootMTCommand, "", rootMountTable)
+	modules.RegisterChild(MTCommand, `<mount point>
+	reads NAMESPACE_ROOT from its environment and mounts a new mount table at <mount point>`, mountTable)
+	modules.RegisterChild(LSExternalCommand, `<glob>...
+	issues glob requests using the current processes namespace library`,
+		ls)
 }
 
 func mountTable(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
diff --git a/lib/modules/exec.go b/lib/modules/exec.go
index db7f7be..5ca6f09 100644
--- a/lib/modules/exec.go
+++ b/lib/modules/exec.go
@@ -6,6 +6,7 @@
 	"io"
 	"os"
 	"os/exec"
+	"regexp"
 	"strings"
 	"sync"
 	"time"
@@ -202,10 +203,10 @@
 
 const ShellEntryPoint = "VEYRON_SHELL_HELPER_PROCESS_ENTRY_POINT"
 
-func RegisterChild(name string, main Main) {
+func RegisterChild(name, help string, main Main) {
 	child.Lock()
 	defer child.Unlock()
-	child.mains[name] = main
+	child.mains[name] = &childEntryPoint{main, help}
 }
 
 // DispatchInTest will execute the requested subproccess command from within
@@ -238,16 +239,36 @@
 }
 
 func (child *childRegistrar) dispatch() error {
+	ch, _ := vexec.GetChildHandle()
+	// Only signal that the child is ready or failed if we successfully get
+	// a child handle. We most likely failed to get a child handle
+	// because the subprocess was run directly from the command line.
+
 	command := os.Getenv(ShellEntryPoint)
 	if len(command) == 0 {
-		return fmt.Errorf("Failed to find entrypoint %q", ShellEntryPoint)
+		err := fmt.Errorf("Failed to find entrypoint %q", ShellEntryPoint)
+		if ch != nil {
+			ch.SetFailed(err)
+		}
+		return err
 	}
+
 	child.Lock()
 	m := child.mains[command]
 	child.Unlock()
+
 	if m == nil {
-		return fmt.Errorf("Shell command %q not registered", command)
+		err := fmt.Errorf("Shell command %q not registered", command)
+		if ch != nil {
+			ch.SetFailed(err)
+		}
+		return err
 	}
+
+	if ch != nil {
+		ch.SetReady()
+	}
+
 	go func(pid int) {
 		for {
 			_, err := os.FindProcess(pid)
@@ -258,14 +279,34 @@
 			time.Sleep(time.Second)
 		}
 	}(os.Getppid())
-	ch, err := vexec.GetChildHandle()
-	if err == nil {
-		// Only signal that the child is ready if we successfully get
-		// a child handle. We most likely failed to get a child handle
-		// because the subprocess was run directly from the command line.
-		ch.SetReady()
+
+	return m.fn(os.Stdin, os.Stdout, os.Stderr, osEnvironMap(), flag.Args()...)
+}
+
+func (child *childRegistrar) addSubprocesses(sh *Shell, pattern string) error {
+	re, err := regexp.Compile(pattern)
+	if err != nil {
+		return err
 	}
-	return m(os.Stdin, os.Stdout, os.Stderr, osEnvironMap(), flag.Args()...)
+	child.Lock()
+	defer child.Unlock()
+	found := false
+	for name, subproc := range child.mains {
+		if re.MatchString(name) {
+			sh.addSubprocess(name, subproc.help)
+			found = true
+		}
+	}
+	if !found {
+		return fmt.Errorf("patterh %q failed to match any registered commands", pattern)
+	}
+	return nil
+}
+
+// AddRegisteredSubprocesses adds any commands that match the regexp pattern
+// to the supplied shell.
+func AddRegisteredSubprocesses(sh *Shell, pattern string) error {
+	return child.addSubprocesses(sh, pattern)
 }
 
 // WaitForEof returns when a read on its io.Reader parameter returns io.EOF
diff --git a/lib/modules/modules_internal_test.go b/lib/modules/modules_internal_test.go
index c5e686e..f60b2bb 100644
--- a/lib/modules/modules_internal_test.go
+++ b/lib/modules/modules_internal_test.go
@@ -9,7 +9,7 @@
 )
 
 func init() {
-	RegisterChild("echos", Echo)
+	RegisterChild("echos", "[args]*", Echo)
 }
 
 func Echo(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
@@ -24,16 +24,14 @@
 
 func assertNumHandles(t *testing.T, sh *Shell, n int) {
 	if got, want := len(sh.handles), n; got != want {
-		_, file, line, _ := runtime.Caller(2)
+		_, file, line, _ := runtime.Caller(1)
 		t.Errorf("%s:%d: got %d, want %d", filepath.Base(file), line, got, want)
 	}
 }
 
 func TestState(t *testing.T) {
-	sh := NewShell()
-
+	sh := NewShell("echos")
 	sh.AddSubprocess("echonotregistered", "[args]*")
-	sh.AddSubprocess("echos", "[args]*")
 	sh.AddFunction("echof", Echo, "[args]*")
 	assertNumHandles(t, sh, 0)
 
diff --git a/lib/modules/modules_test.go b/lib/modules/modules_test.go
index 7320eb6..e4357ef 100644
--- a/lib/modules/modules_test.go
+++ b/lib/modules/modules_test.go
@@ -5,6 +5,7 @@
 	"bytes"
 	"fmt"
 	"io"
+	"os"
 	"testing"
 	"time"
 
@@ -12,9 +13,9 @@
 )
 
 func init() {
-	modules.RegisterChild("envtest", PrintEnv)
-	modules.RegisterChild("echo", Echo)
-	modules.RegisterChild("errortest", ErrorMain)
+	modules.RegisterChild("envtest", "envtest: <variables to print>...", PrintEnv)
+	modules.RegisterChild("echo", "[args]*", Echo)
+	modules.RegisterChild("errortest", "", ErrorMain)
 }
 
 func Echo(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
@@ -106,27 +107,37 @@
 }
 
 func TestChild(t *testing.T) {
-	sh := modules.NewShell()
+	sh := modules.NewShell("envtest")
 	defer sh.Cleanup(nil, nil)
 	key, val := "simpleVar", "foo & bar"
 	sh.SetVar(key, val)
-	sh.AddSubprocess("envtest", "envtest: <variables to print>...")
 	testCommand(t, sh, "envtest", key, val)
 }
 
+func TestChildNoRegistration(t *testing.T) {
+	sh := modules.NewShell()
+	defer sh.Cleanup(os.Stderr, os.Stderr)
+	key, val := "simpleVar", "foo & bar"
+	sh.SetVar(key, val)
+	testCommand(t, sh, "envtest", key, val)
+	_, err := sh.Start("non-existent-command", "random", "args")
+	if err == nil || err.Error() != `Shell command "non-existent-command" not registered` {
+		t.Fatalf("unexpected error %v", err)
+	}
+}
+
 func TestFunction(t *testing.T) {
-	sh := modules.NewShell()
+	sh := modules.NewShell(".*")
 	defer sh.Cleanup(nil, nil)
 	key, val := "simpleVar", "foo & bar & baz"
 	sh.SetVar(key, val)
-	sh.AddFunction("envtest", PrintEnv, "envtest: <variables to print>...")
-	testCommand(t, sh, "envtest", key, val)
+	sh.AddFunction("envtestf", PrintEnv, "envtest: <variables to print>...")
+	testCommand(t, sh, "envtestf", key, val)
 }
 
 func TestErrorChild(t *testing.T) {
-	sh := modules.NewShell()
+	sh := modules.NewShell("errortest")
 	defer sh.Cleanup(nil, nil)
-	sh.AddSubprocess("errortest", "")
 	h, err := sh.Start("errortest")
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
@@ -160,8 +171,7 @@
 }
 
 func TestShutdownSubprocess(t *testing.T) {
-	sh := modules.NewShell()
-	sh.AddSubprocess("echo", "[args]*")
+	sh := modules.NewShell("echo")
 	testShutdown(t, sh)
 }
 
diff --git a/lib/modules/shell.go b/lib/modules/shell.go
index b467fbc..96ed5d5 100644
--- a/lib/modules/shell.go
+++ b/lib/modules/shell.go
@@ -42,7 +42,6 @@
 
 import (
 	"flag"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -70,18 +69,28 @@
 	help    string
 }
 
-type childRegistrar struct {
-	sync.Mutex
-	mains map[string]Main
+type childEntryPoint struct {
+	fn   Main
+	help string
 }
 
-var child = &childRegistrar{mains: make(map[string]Main)}
+type childRegistrar struct {
+	sync.Mutex
+	mains map[string]*childEntryPoint
+}
+
+var child = &childRegistrar{mains: make(map[string]*childEntryPoint)}
 
 // NewShell creates a new instance of Shell. If this new instance is
 // is a test and no identity has been configured in the environment
 // via VEYRON_IDENTITY then CreateAndUseNewID will be used to configure a new
 // ID for the shell and its children.
-func NewShell() *Shell {
+// NewShell takes optional regexp patterns that can be used to specify
+// subprocess commands that are implemented in the same binary as this shell
+// (i.e. have been registered using modules.RegisterChild) to be
+// automatically added to it. If the patterns fail to match any such command
+// then they have no effect.
+func NewShell(patterns ...string) *Shell {
 	// TODO(cnicolaou): should create a new identity if one doesn't
 	// already exist
 	sh := &Shell{
@@ -94,6 +103,9 @@
 			panic(err)
 		}
 	}
+	for _, pattern := range patterns {
+		child.addSubprocesses(sh, pattern)
+	}
 	return sh
 }
 
@@ -125,10 +137,14 @@
 // as a subprocess. In addition, the child process must call RegisterChild
 // using the same name used here and provide the function to be executed
 // in the child.
-func (sh *Shell) AddSubprocess(name string, help string) {
+func (sh *Shell) AddSubprocess(name, help string) {
 	if !child.hasCommand(name) {
 		vlog.Infof("Warning: %q is not registered with modules.Dispatcher", name)
 	}
+	sh.addSubprocess(name, help)
+}
+
+func (sh *Shell) addSubprocess(name string, help string) {
 	entryPoint := ShellEntryPoint + "=" + name
 	sh.mu.Lock()
 	sh.cmds[name] = &commandDesc{func() command { return newExecHandle(entryPoint) }, help}
@@ -168,15 +184,22 @@
 // Start starts the specified command, it returns a Handle which can be used
 // for interacting with that command. The Shell tracks all of the Handles
 // that it creates so that it can shut them down when asked to.
-func (sh *Shell) Start(command string, args ...string) (Handle, error) {
+// Commands may have already been registered with the Shell using AddFunction
+// or AddSubprocess, but if not, they will treated as subprocess commands
+// and an attempt made to run them. Such 'dynamically' started subprocess
+// commands are not remembered the by Shell and do not provide a 'help'
+// message etc; their handles are remembered and will be acted on by
+// the Cleanup method. If the non-registered subprocess command does not
+// exist then the Start command will return an error.
+func (sh *Shell) Start(name string, args ...string) (Handle, error) {
 	sh.mu.Lock()
-	cmd := sh.cmds[command]
-	if cmd == nil {
-		sh.mu.Unlock()
-		return nil, fmt.Errorf("command %q is not available", command)
-	}
-	expanded := append([]string{command}, sh.expand(args...)...)
+	cmd := sh.cmds[name]
 	sh.mu.Unlock()
+	if cmd == nil {
+		entryPoint := ShellEntryPoint + "=" + name
+		cmd = &commandDesc{func() command { return newExecHandle(entryPoint) }, ""}
+	}
+	expanded := append([]string{name}, sh.expand(args...)...)
 	h, err := cmd.factory().start(sh, expanded...)
 	if err != nil {
 		return nil, err
diff --git a/lib/signals/signals_test.go b/lib/signals/signals_test.go
index 3114304..ade0df3 100644
--- a/lib/signals/signals_test.go
+++ b/lib/signals/signals_test.go
@@ -33,10 +33,10 @@
 }
 
 func init() {
-	modules.RegisterChild("handleDefaults", handleDefaults)
-	modules.RegisterChild("handleCustom", handleCustom)
-	modules.RegisterChild("handleCustomWithStop", handleCustomWithStop)
-	modules.RegisterChild("handleDefaultsIgnoreChan", handleDefaultsIgnoreChan)
+	modules.RegisterChild("handleDefaults", "", handleDefaults)
+	modules.RegisterChild("handleCustom", "", handleCustom)
+	modules.RegisterChild("handleCustomWithStop", "", handleCustomWithStop)
+	modules.RegisterChild("handleDefaultsIgnoreChan", "", handleDefaultsIgnoreChan)
 }
 
 func stopLoop(stdin io.Reader, ch chan<- struct{}) {