Improve error reporting and help for start/stop/etc. commands.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

diff --git a/src/allmydata/scripts/common.py b/src/allmydata/scripts/common.py
index bd78430cc9d8e10a57bb5ea9069762fa5fcdfc86..21481e35d6a7248aaa328fa14aba422263bd49d5 100644 (file)
--- a/src/allmydata/scripts/common.py
+++ b/src/allmydata/scripts/common.py
@@ -39,7 +39,7 @@ class BasedirOptions(BaseOptions):
     default_nodedir = _default_nodedir
 
     optParameters = [
-        ["basedir", "C", None, "Same as --node-directory (default %s)."
+        ["basedir", "C", None, "Specify which Tahoe base directory should be used. [default: %s]"
          % get_default_nodedir()],
     ]
 
@@ -67,6 +67,21 @@ class BasedirOptions(BaseOptions):
         if not self['basedir']:
             raise usage.UsageError("A base directory for the node must be provided.")
 
+class NoDefaultBasedirOptions(BasedirOptions):
+    default_nodedir = None
+
+    optParameters = [
+        ["basedir", "C", None, "Specify which Tahoe base directory should be used."],
+    ]
+
+    # This is overridden in order to ensure we get a "Wrong number of arguments."
+    # error when more than one argument is given.
+    def parseArgs(self, basedir=None):
+        BasedirOptions.parseArgs(self, basedir)
+
+    def getSynopsis(self):
+        return "Usage:  %s [global-opts] %s [options] NODEDIR" % (self.command_name, self.subcommand_name)
+
 
 DEFAULT_ALIAS = u"tahoe"
 

diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py
index a27ed82719570fdf6785ea9aaf77a63e67e8f6b0..6663d125d2d5278dd6240c82c77d1a85b845f34e 100644 (file)
--- a/src/allmydata/scripts/create_node.py
+++ b/src/allmydata/scripts/create_node.py
@@ -1,6 +1,6 @@
 
 import os, sys
-from allmydata.scripts.common import BasedirOptions
+from allmydata.scripts.common import BasedirOptions, NoDefaultBasedirOptions
 from allmydata.util.assertutil import precondition
 from allmydata.util.encodingutil import listdir_unicode, argv_to_unicode, quote_output
 import allmydata
@@ -19,6 +19,11 @@ class CreateClientOptions(BasedirOptions):
     def getSynopsis(self):
         return "Usage:  %s [global-opts] create-client [options] [NODEDIR]" % (self.command_name,)
 
+    # This is overridden in order to ensure we get a "Wrong number of arguments."
+    # error when more than one argument is given.
+    def parseArgs(self, basedir=None):
+        BasedirOptions.parseArgs(self, basedir)
+
 
 class CreateNodeOptions(CreateClientOptions):
     optFlags = [
@@ -29,11 +34,8 @@ class CreateNodeOptions(CreateClientOptions):
         return "Usage:  %s [global-opts] create-node [options] [NODEDIR]" % (self.command_name,)
 
 
-class CreateIntroducerOptions(BasedirOptions):
-    default_nodedir = None
-
-    def getSynopsis(self):
-        return "Usage:  %s [global-opts] create-introducer [options] NODEDIR" % (self.command_name,)
+class CreateIntroducerOptions(NoDefaultBasedirOptions):
+    subcommand_name = "create-introducer"
 
 
 client_tac = """

diff --git a/src/allmydata/scripts/keygen.py b/src/allmydata/scripts/keygen.py
index 0982b0d80b858d458a18df9e0328295be75fb305..47069e3970d6f01eb4c5dae1e2599d6e61ebab06 100644 (file)
--- a/src/allmydata/scripts/keygen.py
+++ b/src/allmydata/scripts/keygen.py
@@ -1,14 +1,12 @@
 
 import os, sys
-from allmydata.scripts.common import BasedirOptions
+from allmydata.scripts.common import NoDefaultBasedirOptions
 from allmydata.util.assertutil import precondition
 from allmydata.util.encodingutil import listdir_unicode, quote_output
 
-class CreateKeyGeneratorOptions(BasedirOptions):
-    default_nodedir = None
 
-    def getSynopsis(self):
-        return "Usage:  %s [global-opts] create-key-generator [options] NODEDIR" % (self.command_name,)
+class CreateKeyGeneratorOptions(NoDefaultBasedirOptions):
+    subcommand_name = "create-key-generator"
 
 
 keygen_tac = """

diff --git a/src/allmydata/scripts/startstop_node.py b/src/allmydata/scripts/startstop_node.py
index e5cb49424ff02455ea05e754ea4569cf7ab1fa2f..09cdc9373a46b12baccaf967ca82948c7fef469e 100644 (file)
--- a/src/allmydata/scripts/startstop_node.py
+++ b/src/allmydata/scripts/startstop_node.py
@@ -8,30 +8,45 @@ from allmydata.util.encodingutil import listdir_unicode, quote_output
 
 
 class StartOptions(BasedirOptions):
+    subcommand_name = "start"
+
     def parseArgs(self, basedir=None, *twistd_args):
-        # this can't handle e.g. 'tahoe start --nodaemon', since then
-        # --nodaemon looks like a basedir. So you can either use 'tahoe
-        # start' or 'tahoe start BASEDIR --TWISTD-OPTIONS'.
+        # This can't handle e.g. 'tahoe start --nodaemon', since '--nodaemon'
+        # looks like an option to the tahoe subcommand, not to twistd.
+        # So you can either use 'tahoe start' or 'tahoe start NODEDIR --TWISTD-OPTIONS'.
+        # Note that 'tahoe --node-directory=NODEDIR start --TWISTD-OPTIONS' also
+        # isn't allowed, unfortunately.
+
         BasedirOptions.parseArgs(self, basedir)
         self.twistd_args = twistd_args
 
     def getSynopsis(self):
-        return "Usage:  %s [global-opts] start [options] [NODEDIR]" % (self.command_name,)
+        return "Usage:  %s [global-opts] %s [options] [NODEDIR [twistd-options]]" % (self.command_name, self.subcommand_name)
+
+    def getUsage(self, width=None):
+        t = BasedirOptions.getUsage(self, width) + "\n"
+        twistd_options = str(MyTwistdConfig()).partition("\n")[2].partition("\n\n")[0]
+        t += twistd_options.replace("Options:", "twistd-options:", 1)
+        t += """
 
+Note that if any twistd-options are used, NODEDIR must be specified explicitly
+(not by default or using -C/--basedir or -d/--node-directory), and followed by
+the twistd-options.
+"""
+        return t
 
 class StopOptions(BasedirOptions):
+    def parseArgs(self, basedir=None):
+        BasedirOptions.parseArgs(self, basedir)
+
     def getSynopsis(self):
         return "Usage:  %s [global-opts] stop [options] [NODEDIR]" % (self.command_name,)
 
-
 class RestartOptions(StartOptions):
-    def getSynopsis(self):
-        return "Usage:  %s [global-opts] restart [options] [NODEDIR]" % (self.command_name,)
-
+    subcommand_name = "restart"
 
 class RunOptions(StartOptions):
-    def getSynopsis(self):
-        return "Usage:  %s [global-opts] run [options] [NODEDIR]" % (self.command_name,)
+    subcommand_name = "run"
 
 
 class MyTwistdConfig(twistd.ServerOptions):
@@ -103,8 +118,8 @@ def start(config, out=sys.stdout, err=sys.stderr):
         twistd_config.parseOptions(twistd_args)
     except usage.error, ue:
         # these arguments were unsuitable for 'twistd'
-        print >>err, twistd_config
-        print >>err, "tahoe start: %s" % (config.subCommand, ue)
+        print >>err, config
+        print >>err, "tahoe %s: usage error from twistd: %s\n" % (config.subcommand_name, ue)
         return 1
     twistd_config.loadedPlugins = {"StartTahoeNode": StartTahoeNodePlugin(nodetype, basedir)}
 

diff --git a/src/allmydata/scripts/stats_gatherer.py b/src/allmydata/scripts/stats_gatherer.py
index 7762b2c50236a4e535bfea09fca89218568b2845..efe4f48743edd16fed3b4ad07f29aa5c129e0df0 100644 (file)
--- a/src/allmydata/scripts/stats_gatherer.py
+++ b/src/allmydata/scripts/stats_gatherer.py
@@ -1,14 +1,12 @@
 
 import os, sys
-from allmydata.scripts.common import BasedirOptions
+from allmydata.scripts.common import NoDefaultBasedirOptions
 from allmydata.util.assertutil import precondition
 from allmydata.util.encodingutil import listdir_unicode, quote_output
 
-class CreateStatsGathererOptions(BasedirOptions):
-    default_nodedir = None
 
-    def getSynopsis(self):
-        return "Usage:  %s [global-opts] create-stats-gatherer [options] NODEDIR" % (self.command_name,)
+class CreateStatsGathererOptions(NoDefaultBasedirOptions):
+    subcommand_name = "create-stats-gatherer"
 
 
 stats_gatherer_tac = """

diff --git a/src/allmydata/test/test_cli.py b/src/allmydata/test/test_cli.py
index e6f1cc6f3fd4f0ee0184493cf354f0d185918793..8f0ecaa646b03582b525f28be5f16615afd39ac9 100644 (file)
--- a/src/allmydata/test/test_cli.py
+++ b/src/allmydata/test/test_cli.py
@@ -646,7 +646,7 @@ class Help(unittest.TestCase):
 
     def test_start(self):
         help = str(startstop_node.StartOptions())
-        self.failUnlessIn(" [global-opts] start [options] [NODEDIR]", help)
+        self.failUnlessIn(" [global-opts] start [options] [NODEDIR [twistd-options]]", help)
 
     def test_stop(self):
         help = str(startstop_node.StopOptions())
@@ -654,11 +654,11 @@ class Help(unittest.TestCase):
 
     def test_restart(self):
         help = str(startstop_node.RestartOptions())
-        self.failUnlessIn(" [global-opts] restart [options] [NODEDIR]", help)
+        self.failUnlessIn(" [global-opts] restart [options] [NODEDIR [twistd-options]]", help)
 
     def test_run(self):
         help = str(startstop_node.RunOptions())
-        self.failUnlessIn(" [global-opts] run [options] [NODEDIR]", help)
+        self.failUnlessIn(" [global-opts] run [options] [NODEDIR [twistd-options]]", help)
 
     def test_create_client(self):
         help = str(create_node.CreateClientOptions())
@@ -3869,6 +3869,9 @@ class Options(unittest.TestCase):
         o = self.parse(["--node-directory", "there", "start"])
         self.failUnlessEqual(o["basedir"], os.path.abspath("there"))
 
+        o = self.parse(["start", "here", "--nodaemon"])
+        self.failUnlessEqual(o["basedir"], os.path.abspath("here"))
+
         self.failUnlessRaises(usage.UsageError, self.parse,
                               ["--basedir", "there", "start"])
         self.failUnlessRaises(usage.UsageError, self.parse,
@@ -3886,3 +3889,7 @@ class Options(unittest.TestCase):
                               ["--node-directory=there",
                                "start", "--basedir=here", "anywhere"])
 
+        self.failUnlessRaises(usage.UsageError, self.parse,
+                              ["--node-directory=there", "start", "--nodaemon"])
+        self.failUnlessRaises(usage.UsageError, self.parse,
+                              ["start", "--basedir=here", "--nodaemon"])