Refactor BasedirOptions to improve accuracy of help messages.

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 5f5b1ea9e8b26a7fb705b83ca102b26b3d6a9ee3..74c9b4b9b0bf72df73a682f4254aed76e179186a 100644 (file)
--- a/src/allmydata/scripts/startstop_node.py
+++ b/src/allmydata/scripts/startstop_node.py
@@ -7,28 +7,40 @@ from allmydata.util import fileutil
 from allmydata.util.encodingutil import listdir_unicode, quote_output
 
 
-def format_twisted_options(config):
-    return "twistd %s\n" % (str(config).partition("\n")[2].partition("\n\n")[0],)
-
 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] %s [options] [NODEDIR] [twistd_options]" % (self.command_name, self.subcommand_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"
-        t += format_twisted_options(MyTwistdConfig())
+        twistd_options = str(MyTwistdConfig()).partition("\n")[2].partition("\n\n")[0]
+        if twistd_options.startswith("Options:"):
+            twistd_options = "twistd-o" + twistd_options[1 :]
+        t += twistd_options
+        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,)
 

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"])