From 4d783cb442a01f8f990861466966950870839ad9 Mon Sep 17 00:00:00 2001
From: Daira Hopwood <daira@jacaranda.org>
Date: Tue, 21 Oct 2014 18:32:58 +0100
Subject: [PATCH] Refactor BasedirOptions to improve accuracy of help messages.
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
---
src/allmydata/scripts/common.py | 17 ++++++++++++++-
src/allmydata/scripts/create_node.py | 14 +++++++------
src/allmydata/scripts/keygen.py | 8 +++----
src/allmydata/scripts/startstop_node.py | 28 ++++++++++++++++++-------
src/allmydata/scripts/stats_gatherer.py | 8 +++----
src/allmydata/test/test_cli.py | 13 +++++++++---
6 files changed, 60 insertions(+), 28 deletions(-)
diff --git a/src/allmydata/scripts/common.py b/src/allmydata/scripts/common.py
index bd78430c..21481e35 100644
--- 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 a27ed827..6663d125 100644
--- 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 0982b0d8..47069e39 100644
--- 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 5f5b1ea9..74c9b4b9 100644
--- 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 7762b2c5..efe4f487 100644
--- 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 e6f1cc6f..8f0ecaa6 100644
--- 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"])
--
2.45.2