veyron/lib/modules: Make modules parse flags using the veyron2.Init
flag parsing mechanism.
Now the user of modules need not call veyron2.Init to parse the flags,
whereas before the flags had to be parsed before calling dispatch.
The profiles can now use the flags passed to a subcommand whereas before
they could not.
Upcoming changes:
(1) Make the rest of the callers to testutil.Init use go1.4's TestMain to avoid
flags being parsed in init functions.
(2) Get rid of modules.DispatchInTest and use go1.4's TestMain instead.
Change-Id: I71e31929a674dfc11310d203aba452a69c10b2b2
diff --git a/lib/filelocker/locker_test.go b/lib/filelocker/locker_test.go
index b58bfe2..cf483a7 100644
--- a/lib/filelocker/locker_test.go
+++ b/lib/filelocker/locker_test.go
@@ -42,7 +42,7 @@
func testLockChild(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
// Lock the file
- unlocker, err := Lock(args[1])
+ unlocker, err := Lock(args[0])
if err != nil {
return fmt.Errorf("Lock failed: %v", err)
}
diff --git a/lib/flags/flags.go b/lib/flags/flags.go
index b2c767e..241420c 100644
--- a/lib/flags/flags.go
+++ b/lib/flags/flags.go
@@ -5,6 +5,7 @@
"fmt"
"os"
"strings"
+ "sync"
"v.io/core/veyron/lib/flags/consts"
)
@@ -236,12 +237,40 @@
return f
}
+var (
+ defaultProtocol = "wsh" // GUARDED_BY listenMu
+ defaultHostPort = ":0" // GUARDED_BY listenMu
+ listenMu sync.RWMutex
+)
+
+// SetDefaultProtocol sets the default protocol used when --veyron.tcp.protocol is
+// not provided. It must be called before flags are parsed for it to take effect.
+func SetDefaultProtocol(protocol string) {
+ listenMu.Lock()
+ defaultProtocol = protocol
+ listenMu.Unlock()
+}
+
+// SetDefaultHostPort sets the default host and port used when --veyron.tcp.address
+// is not provided. It must be called before flags are parsed for it to take effect.
+func SetDefaultHostPort(s string) {
+ listenMu.Lock()
+ defaultHostPort = s
+ listenMu.Unlock()
+}
+
// createAndRegisterListenFlags creates and registers the ListenFlags
// group with the supplied flag.FlagSet.
func createAndRegisterListenFlags(fs *flag.FlagSet) *ListenFlags {
+ listenMu.RLock()
+ defer listenMu.RUnlock()
+ var ipHostPortFlag IPHostPortFlag
+ if err := ipHostPortFlag.Set(defaultHostPort); err != nil {
+ panic(err)
+ }
f := &ListenFlags{
- protocol: TCPProtocolFlag{"wsh"},
- addresses: ipHostPortFlagVar{validator: IPHostPortFlag{Port: "0"}},
+ protocol: TCPProtocolFlag{defaultProtocol},
+ addresses: ipHostPortFlagVar{validator: ipHostPortFlag},
}
f.addresses.flags = f
diff --git a/lib/modules/core/core_test.go b/lib/modules/core/core_test.go
index a0e03e8..9f2b0b5 100644
--- a/lib/modules/core/core_test.go
+++ b/lib/modules/core/core_test.go
@@ -55,7 +55,7 @@
}
func testArgs(args ...string) []string {
- var targs = []string{"--", "--veyron.tcp.address=127.0.0.1:0"}
+ var targs = []string{"--veyron.tcp.address=127.0.0.1:0"}
return append(targs, args...)
}
diff --git a/lib/modules/core/echo.go b/lib/modules/core/echo.go
index 92160b9..8ba8f5f 100644
--- a/lib/modules/core/echo.go
+++ b/lib/modules/core/echo.go
@@ -50,13 +50,6 @@
ctx, shutdown := veyron2.Init()
defer shutdown()
- fl, args, err := parseListenFlags(args)
- if err != nil {
- return fmt.Errorf("failed to parse args: %s", err)
- }
- if err := checkArgs(args, 2, "<message> <name>"); err != nil {
- return err
- }
id, mp := args[0], args[1]
disp := &treeDispatcher{id: id}
server, err := veyron2.NewServer(ctx)
@@ -64,7 +57,7 @@
return err
}
defer server.Stop()
- eps, err := server.Listen(initListenSpec(fl))
+ eps, err := server.Listen(veyron2.GetListenSpec(ctx))
if err != nil {
return err
}
@@ -83,7 +76,6 @@
ctx, shutdown := veyron2.Init()
defer shutdown()
- args = args[1:]
name := args[0]
args = args[1:]
client := veyron2.GetClient(ctx)
diff --git a/lib/modules/core/mounttable.go b/lib/modules/core/mounttable.go
index 6046c91..00acb04 100644
--- a/lib/modules/core/mounttable.go
+++ b/lib/modules/core/mounttable.go
@@ -34,11 +34,7 @@
ctx, shutdown := veyron2.Init()
defer shutdown()
- fl, args, err := parseListenFlags(args)
- if err != nil {
- return fmt.Errorf("failed to parse args: %s", err)
- }
- lspec := initListenSpec(fl)
+ lspec := veyron2.GetListenSpec(ctx)
server, err := veyron2.NewServer(ctx, options.ServesMountTable(true))
if err != nil {
return fmt.Errorf("root failed: %v", err)
@@ -74,7 +70,6 @@
defer shutdown()
details := false
- args = args[1:] // skip over command name
if len(args) > 0 && args[0] == "-l" {
details = true
args = args[1:]
diff --git a/lib/modules/core/proxy.go b/lib/modules/core/proxy.go
index 79d826b..691d1ca 100644
--- a/lib/modules/core/proxy.go
+++ b/lib/modules/core/proxy.go
@@ -22,18 +22,17 @@
ctx, shutdown := veyron2.Init()
defer shutdown()
- fl, args, err := parseListenFlags(args)
- if err != nil {
- return fmt.Errorf("failed to parse args: %s", err)
- }
expected := len(args)
rid, err := naming.NewRoutingID()
if err != nil {
return err
}
- lf := fl.ListenFlags()
- proxy, err := proxy.New(rid, veyron2.GetPrincipal(ctx), lf.Addrs[0].Protocol, lf.Addrs[0].Address, "")
+ listenSpec := veyron2.GetListenSpec(ctx)
+ protocol := listenSpec.Addrs[0].Protocol
+ addr := listenSpec.Addrs[0].Address
+
+ proxy, err := proxy.New(rid, veyron2.GetPrincipal(ctx), protocol, addr, "")
if err != nil {
return err
}
diff --git a/lib/modules/core/test_identityd.go b/lib/modules/core/test_identityd.go
index f1bc19a..793824a 100644
--- a/lib/modules/core/test_identityd.go
+++ b/lib/modules/core/test_identityd.go
@@ -10,7 +10,6 @@
"v.io/core/veyron2"
- "v.io/core/veyron/lib/flags"
"v.io/core/veyron/lib/modules"
"v.io/core/veyron/services/identity/auditor"
@@ -23,46 +22,41 @@
)
var (
- ifs *flag.FlagSet = flag.NewFlagSet("test_identityd", flag.ContinueOnError)
-
- googleDomain = ifs.String("google_domain", "", "An optional domain name. When set, only email addresses from this domain are allowed to authenticate via Google OAuth")
- host = ifs.String("host", "localhost", "Hostname the HTTP server listens on. This can be the name of the host running the webserver, but if running behind a NAT or load balancer, this should be the host name that clients will connect to. For example, if set to 'x.com', Veyron identities will have the IssuerName set to 'x.com' and clients can expect to find the root name and public key of the signer at 'x.com/blessing-root'.")
- httpaddr = ifs.String("httpaddr", "localhost:0", "Address on which the HTTP server listens on.")
- tlsconfig = ifs.String("tlsconfig", "", "Comma-separated list of TLS certificate and private key files. This must be provided.")
-
- ifl *flags.Flags = flags.CreateAndRegister(ifs, flags.Listen)
+ googleDomain = flag.CommandLine.String("google_domain", "", "An optional domain name. When set, only email addresses from this domain are allowed to authenticate via Google OAuth")
+ host = flag.CommandLine.String("host", "localhost", "Hostname the HTTP server listens on. This can be the name of the host running the webserver, but if running behind a NAT or load balancer, this should be the host name that clients will connect to. For example, if set to 'x.com', Veyron identities will have the IssuerName set to 'x.com' and clients can expect to find the root name and public key of the signer at 'x.com/blessing-root'.")
+ httpaddr = flag.CommandLine.String("httpaddr", "localhost:0", "Address on which the HTTP server listens on.")
+ tlsconfig = flag.CommandLine.String("tlsconfig", "", "Comma-separated list of TLS certificate and private key files. This must be provided.")
)
func init() {
- modules.RegisterChild(TestIdentitydCommand, usage(ifs), startTestIdentityd)
+ modules.RegisterChild(TestIdentitydCommand, usage(flag.CommandLine), startTestIdentityd)
}
func startTestIdentityd(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
- if err := parseFlags(ifl, args); err != nil {
- return fmt.Errorf("failed to parse args: %s", err)
- }
-
// Duration to use for tls cert and blessing duration.
duration := 365 * 24 * time.Hour
+ ctx, shutdown := veyron2.Init()
+ defer shutdown()
+
// If no tlsconfig has been provided, generate new cert and key and use them.
- if ifs.Lookup("tlsconfig").Value.String() == "" {
+ if flag.CommandLine.Lookup("tlsconfig").Value.String() == "" {
certFile, keyFile, err := util.WriteCertAndKey(*host, duration)
if err != nil {
return fmt.Errorf("Could not write cert and key: %v", err)
}
- if err := ifs.Set("tlsconfig", certFile+","+keyFile); err != nil {
+ if err := flag.CommandLine.Set("tlsconfig", certFile+","+keyFile); err != nil {
return fmt.Errorf("Could not set tlsconfig: %v", err)
}
}
// Pick a free port if httpaddr flag is not set.
- // We can't use :0 here, because the identity server calles
- // http.ListenAndServeTLS, which block, leaving us with no way to tell
+ // We can't use :0 here, because the identity server calls
+ // http.ListenAndServeTLS, which blocks, leaving us with no way to tell
// what port the server is running on. Hence, we must pass in an
// actual port so we know where the server is running.
- if ifs.Lookup("httpaddr").Value.String() == ifs.Lookup("httpaddr").DefValue {
- if err := ifs.Set("httpaddr", "localhost:"+freePort()); err != nil {
+ if flag.CommandLine.Lookup("httpaddr").Value.String() == flag.CommandLine.Lookup("httpaddr").DefValue {
+ if err := flag.CommandLine.Set("httpaddr", "localhost:"+freePort()); err != nil {
return fmt.Errorf("Could not set httpaddr: %v", err)
}
}
@@ -86,10 +80,7 @@
params,
caveats.NewMockCaveatSelector())
- l := initListenSpec(ifl)
-
- ctx, shutdown := veyron2.Init()
- defer shutdown()
+ l := veyron2.GetListenSpec(ctx)
_, veyronEPs, externalHttpaddress := s.Listen(ctx, &l, *host, *httpaddr, *tlsconfig)
diff --git a/lib/modules/core/util.go b/lib/modules/core/util.go
index 3d16bb7..08c53f3 100644
--- a/lib/modules/core/util.go
+++ b/lib/modules/core/util.go
@@ -4,36 +4,8 @@
"flag"
"fmt"
"strings"
-
- "v.io/core/veyron2/ipc"
-
- "v.io/core/veyron/lib/flags"
)
-func parseFlags(fl *flags.Flags, args []string) error {
- if len(args) == 0 {
- return nil
- }
- return fl.Parse(args[1:], nil)
-}
-
-// parseListenFlags parses the given args using just the flags and env vars
-// defined in the veyron/lib/flags package.
-func parseListenFlags(args []string) (*flags.Flags, []string, error) {
- fs := flag.NewFlagSet("modules/core", flag.ContinueOnError)
- fl := flags.CreateAndRegister(fs, flags.Listen)
- err := parseFlags(fl, args)
- return fl, fl.Args(), err
-}
-
-func initListenSpec(fl *flags.Flags) ipc.ListenSpec {
- lf := fl.ListenFlags()
- return ipc.ListenSpec{
- Addrs: ipc.ListenAddrs(lf.Addrs),
- Proxy: lf.ListenProxy,
- }
-}
-
// checkArgs checks for the expected number of args in args. A negative
// value means at least that number of args are expected.
func checkArgs(args []string, expected int, usage string) error {
diff --git a/lib/modules/core/wspr.go b/lib/modules/core/wspr.go
index 42db213..8e38a84 100644
--- a/lib/modules/core/wspr.go
+++ b/lib/modules/core/wspr.go
@@ -5,7 +5,6 @@
"fmt"
"io"
- "v.io/core/veyron/lib/flags"
"v.io/core/veyron/lib/modules"
"v.io/wspr/veyron/services/wsprd/wspr"
@@ -13,29 +12,19 @@
)
var (
- // TODO(sadovsky): We should restructure things so that we can avoid
- // duplicating code between subprocess command impls and actual main()'s.
- fs *flag.FlagSet = flag.NewFlagSet("wspr", flag.ContinueOnError)
-
- port *int = fs.Int("port", 0, "Port to listen on.")
- identd *string = fs.String("identd", "", "identd server name. Must be set.")
-
- fl *flags.Flags = flags.CreateAndRegister(fs, flags.Listen)
+ port *int = flag.CommandLine.Int("port", 0, "Port to listen on.")
+ identd *string = flag.CommandLine.String("identd", "", "identd server name. Must be set.")
)
func init() {
- modules.RegisterChild(WSPRCommand, usage(fs), startWSPR)
+ modules.RegisterChild(WSPRCommand, usage(flag.CommandLine), startWSPR)
}
func startWSPR(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
- if err := parseFlags(fl, args); err != nil {
- return fmt.Errorf("failed to parse args: %s", err)
- }
-
ctx, shutdown := veyron2.Init()
defer shutdown()
- l := initListenSpec(fl)
+ l := veyron2.GetListenSpec(ctx)
proxy := wspr.NewWSPR(ctx, *port, &l, *identd, nil)
defer proxy.Shutdown()
diff --git a/lib/modules/examples_test.go b/lib/modules/examples_test.go
index cb97b64..e4ab42a 100644
--- a/lib/modules/examples_test.go
+++ b/lib/modules/examples_test.go
@@ -36,9 +36,8 @@
h, _ := sh.Start("echo", nil, "a", "b")
h.Shutdown(os.Stdout, os.Stderr)
// Output:
- // 0: echo
- // 1: a
- // 2: b
+ // 0: a
+ // 1: b
}
func ExampleDispatchAndExit() {
@@ -51,7 +50,6 @@
h, _ := sh.Start("echo", nil, "c", "d")
h.Shutdown(os.Stdout, os.Stderr)
// Output:
- // 0: echo
- // 1: c
- // 2: d
+ // 0: c
+ // 1: d
}
diff --git a/lib/modules/func.go b/lib/modules/func.go
index 5c5bc4c..24f1fde 100644
--- a/lib/modules/func.go
+++ b/lib/modules/func.go
@@ -88,7 +88,7 @@
cenv := envSliceToMap(env)
vlog.VI(1).Infof("Start: %q args: %v", fh.name, args)
vlog.VI(2).Infof("Start: %q env: %v", fh.name, cenv)
- err := main(stdin, stdout, stderr, cenv, args...)
+ err := main(stdin, stdout, stderr, cenv, args[1:]...)
if err != nil {
fmt.Fprintf(stderr, "%s\n", err)
}
diff --git a/lib/modules/modules_test.go b/lib/modules/modules_test.go
index 277fb2f..a5c989d 100644
--- a/lib/modules/modules_test.go
+++ b/lib/modules/modules_test.go
@@ -62,7 +62,7 @@
}
func PrintFromEnv(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
- for _, a := range args[1:] {
+ for _, a := range args {
if v := env[a]; len(v) > 0 {
fmt.Fprintf(stdout, "%s\n", a+"="+v)
} else {
@@ -286,7 +286,7 @@
var stdoutBuf bytes.Buffer
var stderrBuf bytes.Buffer
sh.Cleanup(&stdoutBuf, &stderrBuf)
- stdoutOutput, stderrOutput := "stdout: "+command+"\n", "stderr: "+command+"\n"
+ var stdoutOutput, stderrOutput string
for _, a := range args {
stdoutOutput += fmt.Sprintf("stdout: %s\n", a)
stderrOutput += fmt.Sprintf("stderr: %s\n", a)
diff --git a/lib/modules/registry.go b/lib/modules/registry.go
index 8331f9c..f473d14 100644
--- a/lib/modules/registry.go
+++ b/lib/modules/registry.go
@@ -200,8 +200,8 @@
}
}(os.Getppid())
- args := append([]string{command}, flag.Args()...)
- return m.main(os.Stdin, os.Stdout, os.Stderr, envSliceToMap(os.Environ()), args...)
+ flag.Parse()
+ return m.main(os.Stdin, os.Stdout, os.Stderr, envSliceToMap(os.Environ()), flag.Args()...)
}
// WaitForEOF returns when a read on its io.Reader parameter returns io.EOF
diff --git a/lib/testutil/glob.go b/lib/testutil/glob.go
index 84d7ec7..0c6fd23 100644
--- a/lib/testutil/glob.go
+++ b/lib/testutil/glob.go
@@ -35,5 +35,6 @@
if ferr := call.Finish(&err); ferr != nil {
err = ferr
}
+
return results, err
}
diff --git a/lib/testutil/integration/util.go b/lib/testutil/integration/util.go
index da7db76..82870e2 100644
--- a/lib/testutil/integration/util.go
+++ b/lib/testutil/integration/util.go
@@ -548,7 +548,7 @@
// returns a handle for the started command along with the object name
// of the mount table.
func startRootMT(shell *modules.Shell) (modules.Handle, string, error) {
- handle, err := shell.Start(core.RootMTCommand, nil, "--", "--veyron.tcp.address=127.0.0.1:0")
+ handle, err := shell.Start(core.RootMTCommand, nil, "--veyron.tcp.address=127.0.0.1:0")
if err != nil {
return nil, "", err
}
