From: david-sarah <david-sarah@jacaranda.org>
Date: Tue, 3 Aug 2010 08:54:16 +0000 (-0700)
Subject: CLI: further improve consistency of basedir options and add tests. addresses #118
X-Git-Tag: allmydata-tahoe-1.8.0b2~34
X-Git-Url: https://git.rkrishnan.org/pf/content/FOOURL?a=commitdiff_plain;h=a7c474a09893b9aa3d6c20249c32b3150e202d19;p=tahoe-lafs%2Ftahoe-lafs.git

CLI: further improve consistency of basedir options and add tests. addresses #118
---

diff --git a/src/allmydata/scripts/cli.py b/src/allmydata/scripts/cli.py
index 1c01fe4d..bb0f4db2 100644
--- a/src/allmydata/scripts/cli.py
+++ b/src/allmydata/scripts/cli.py
@@ -13,9 +13,9 @@ class VDriveOptions(BaseOptions):
          "Specify which Tahoe node directory should be used. The directory "
          "should either contain a full Tahoe node, or a file named node.url "
          "that points to some other Tahoe node. It should also contain a file "
-         "named private/aliases which contains the mapping from alias name "
-         "to root dirnode URI." + (
-            _default_nodedir and (" [default for most commands: " + quote_output(_default_nodedir) + "]") or "")],
+         "named '" + os.path.join('private', 'aliases') + "' which contains the "
+         "mapping from alias name to root dirnode URI." + (
+            _default_nodedir and (" [default: " + quote_output(_default_nodedir) + "]") or "")],
         ["node-url", "u", None,
          "Specify the URL of the Tahoe gateway node, such as 'http://127.0.0.1:3456'. "
          "This overrides the URL found in the --node-directory ."],
diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py
index 5a7f2527..d12c0285 100644
--- a/src/allmydata/scripts/create_node.py
+++ b/src/allmydata/scripts/create_node.py
@@ -7,7 +7,6 @@ import allmydata
 
 class CreateClientOptions(BasedirMixin, BaseOptions):
     optParameters = [
-        ("basedir", "C", None, "which directory to create the node in"),
         # we provide 'create-node'-time options for the most common
         # configuration knobs. The rest can be controlled by editing
         # tahoe.cfg before node startup.
@@ -26,8 +25,8 @@ class CreateIntroducerOptions(BasedirMixin, BaseOptions):
     default_nodedir = None
 
     optParameters = [
-        ["basedir", "C", None, "which directory to create the introducer in"],
-        ]
+        ["node-directory", "d", None, "Specify which directory the introducer should be created in. [no default]"],
+    ]
 
 client_tac = """
 # -*- python -*-
diff --git a/src/allmydata/scripts/debug.py b/src/allmydata/scripts/debug.py
index 793ee1c3..bd828ee0 100644
--- a/src/allmydata/scripts/debug.py
+++ b/src/allmydata/scripts/debug.py
@@ -770,7 +770,8 @@ def corrupt_share(options):
 
 
 class ReplOptions(usage.Options):
-    pass
+    def getSynopsis(self):
+        return "Usage: tahoe debug repl"
 
 def repl(options):
     import code
@@ -801,6 +802,7 @@ Subcommands:
     tahoe debug find-shares     Locate sharefiles in node directories.
     tahoe debug catalog-shares  Describe all shares in node dirs.
     tahoe debug corrupt-share   Corrupt a share by flipping a bit.
+    tahoe debug repl            Open a Python interpreter.
 
 Please run e.g. 'tahoe debug dump-share --help' for more details on each
 subcommand.
diff --git a/src/allmydata/scripts/keygen.py b/src/allmydata/scripts/keygen.py
index a8fd7fc1..f96b3a76 100644
--- a/src/allmydata/scripts/keygen.py
+++ b/src/allmydata/scripts/keygen.py
@@ -1,14 +1,14 @@
 
 import os, sys
 from allmydata.scripts.common import BasedirMixin, BaseOptions
+from allmydata.util.assertutil import precondition
 from allmydata.util.encodingutil import listdir_unicode, quote_output
 
 class CreateKeyGeneratorOptions(BasedirMixin, BaseOptions):
     default_nodedir = None
-    allow_multiple = False
 
     optParameters = [
-        ["basedir", "C", None, "which directory to create the key-generator in"],
+        ["node-directory", "d", None, "Specify which directory the key-generator should be created in. [no default]"],
     ]
 
 keygen_tac = """
@@ -26,8 +26,10 @@ application = service.Application("allmydata_key_generator")
 k.setServiceParent(application)
 """
 
-def create_key_generator(config, out=sys.stdout, err=sys.stderr):
-    basedir = config['basedirs'][0]
+def create_key_generator(basedir, config, out=sys.stdout, err=sys.stderr):
+    # This should always be called with an absolute Unicode basedir.
+    precondition(isinstance(basedir, unicode), basedir)
+
     if os.path.exists(basedir):
         if listdir_unicode(basedir):
             print >>err, "The base directory %s is not empty." % quote_output(basedir)
diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py
index 1da7059a..4a81b06d 100644
--- a/src/allmydata/scripts/runner.py
+++ b/src/allmydata/scripts/runner.py
@@ -41,6 +41,11 @@ class Options(BaseOptions, usage.Options):
         if not hasattr(self, 'subOptions'):
             raise usage.UsageError("must specify a command")
 
+
+create_dispatch = {}
+for module in (create_node, keygen, stats_gatherer):
+    create_dispatch.update(module.dispatch)
+
 def runner(argv,
            run_by_human=True,
            stdin=None, stdout=None, stderr=None,
@@ -87,9 +92,9 @@ def runner(argv,
     so.stdin = stdin
 
     rc = 0
-    if command in create_node.dispatch:
+    if command in create_dispatch:
+        f = create_dispatch[command]
         for basedir in so.basedirs:
-            f = create_node.dispatch[command]
             rc = f(basedir, so, stdout, stderr) or rc
     elif command in startstop_node.dispatch:
         rc = startstop_node.dispatch[command](so, stdout, stderr)
@@ -97,10 +102,6 @@ def runner(argv,
         rc = debug.dispatch[command](so)
     elif command in cli.dispatch:
         rc = cli.dispatch[command](so)
-    elif command in keygen.dispatch:
-        rc = keygen.dispatch[command](so, stdout, stderr)
-    elif command in stats_gatherer.dispatch:
-        rc = stats_gatherer.dispatch[command](so)
     elif command in ac_dispatch:
         rc = ac_dispatch[command](so, stdout, stderr)
     else:
diff --git a/src/allmydata/scripts/startstop_node.py b/src/allmydata/scripts/startstop_node.py
index 551b92fd..1fe996e2 100644
--- a/src/allmydata/scripts/startstop_node.py
+++ b/src/allmydata/scripts/startstop_node.py
@@ -2,25 +2,19 @@
 import os, sys, signal, time
 from allmydata.scripts.common import BasedirMixin, BaseOptions
 from allmydata.util import fileutil, find_exe
+from allmydata.util.assertutil import precondition
+from allmydata.util.encodingutil import listdir_unicode, quote_output
 
 class StartOptions(BasedirMixin, BaseOptions):
-    optParameters = [
-        ["basedir", "C", None, "which directory to start the node in"],
-        ]
     optFlags = [
         ["profile", "p", "Run under the Python profiler, putting results in 'profiling_results.prof'."],
         ["syslog", None, "Tell the node to log to syslog, not a file."],
         ]
 
 class StopOptions(BasedirMixin, BaseOptions):
-    optParameters = [
-        ["basedir", "C", None, "which directory to stop the node in"],
-        ]
+    pass
 
 class RestartOptions(BasedirMixin, BaseOptions):
-    optParameters = [
-        ["basedir", "C", None, "which directory to restart the node in"],
-        ]
     optFlags = [
         ["profile", "p", "Run under the Python profiler, putting results in 'profiling_results.prof'."],
         ["syslog", None, "Tell the node to log to syslog, not a file."],
@@ -28,22 +22,24 @@ class RestartOptions(BasedirMixin, BaseOptions):
 
 class RunOptions(BasedirMixin, BaseOptions):
     default_nodedir = u"."
+    allow_multiple = False
 
     optParameters = [
-        ["basedir", "C", None, "which directory to run the node in, CWD by default"],
-        ]
+        ["node-directory", "d", None, "Specify the directory of the node to be run. [default, for 'tahoe run' only: current directory]"],
+        ["multiple", "m", None, "['tahoe run' cannot accept multiple node directories]"],
+    ]
 
 def do_start(basedir, opts, out=sys.stdout, err=sys.stderr):
-    print >>out, "STARTING", basedir
+    print >>out, "STARTING", quote_output(basedir)
     if not os.path.isdir(basedir):
-        print >>err, "%s does not look like a directory at all" % basedir
+        print >>err, "%s does not look like a directory at all" % quote_output(basedir)
         return 1
-    for fn in os.listdir(basedir):
-        if fn.endswith(".tac"):
-            tac = fn
+    for fn in listdir_unicode(basedir):
+        if fn.endswith(u".tac"):
+            tac = str(fn)
             break
     else:
-        print >>err, "%s does not look like a node directory (no .tac file)" % basedir
+        print >>err, "%s does not look like a node directory (no .tac file)" % quote_output(basedir)
         return 1
     if "client" in tac:
         nodetype = "client"
@@ -108,10 +104,10 @@ def do_start(basedir, opts, out=sys.stdout, err=sys.stderr):
         return 1
 
 def do_stop(basedir, out=sys.stdout, err=sys.stderr):
-    print >>out, "STOPPING", basedir
+    print >>out, "STOPPING", quote_output(basedir)
     pidfile = os.path.join(basedir, "twistd.pid")
     if not os.path.exists(pidfile):
-        print >>err, "%s does not look like a running node directory (no twistd.pid)" % basedir
+        print >>err, "%s does not look like a running node directory (no twistd.pid)" % quote_output(basedir)
         # we define rc=2 to mean "nothing is running, but it wasn't me who
         # stopped it"
         return 2
@@ -194,11 +190,26 @@ def run(config, stdout, stderr):
     from twisted.python import log, logfile
     from allmydata import client
 
-    basedir = config['basedir']
-    if basedir is None:
-        basedir = '.'
+    basedir = config['basedirs'][0]
+    precondition(isinstance(basedir, unicode), basedir)
+
+    if not os.path.isdir(basedir):
+        print >>stderr, "%s does not look like a directory at all" % quote_output(basedir)
+        return 1
+    for fn in listdir_unicode(basedir):
+        if fn.endswith(u".tac"):
+            tac = str(fn)
+            break
     else:
-        os.chdir(basedir)
+        print >>stderr, "%s does not look like a node directory (no .tac file)" % quote_output(basedir)
+        return 1
+    if "client" not in tac:
+        print >>stderr, ("%s looks like it contains a non-client node (%s).\n"
+                         "Use 'tahoe start' instead of 'tahoe run'."
+                         % (quote_output(basedir), tac))
+        return 1
+
+    os.chdir(basedir)
 
     # set up twisted logging. this will become part of the node rsn.
     logdir = os.path.join(basedir, 'logs')
diff --git a/src/allmydata/scripts/stats_gatherer.py b/src/allmydata/scripts/stats_gatherer.py
index e799d4e8..0764bfc5 100644
--- a/src/allmydata/scripts/stats_gatherer.py
+++ b/src/allmydata/scripts/stats_gatherer.py
@@ -1,16 +1,15 @@
 
-import os
-from twisted.python import usage
+import os, sys
+from allmydata.scripts.common import BasedirMixin, BaseOptions
+from allmydata.util.assertutil import precondition
+from allmydata.util.encodingutil import listdir_unicode, quote_output
 
-class CreateStatsGathererOptions(usage.Options):
-    optParameters = [
-        ["basedir", "C", None, "which directory to create the stats-gatherer in"],
-        ]
+class CreateStatsGathererOptions(BasedirMixin, BaseOptions):
+    default_nodedir = None
 
-    def parseArgs(self, basedir=None):
-        if basedir is not None:
-            assert self["basedir"] is None
-            self["basedir"] = basedir
+    optParameters = [
+        ["node-directory", "d", None, "Specify which directory the stats-gatherer should be created in. [no default]"],
+    ]
 
 
 stats_gatherer_tac = """
@@ -26,15 +25,14 @@ application = service.Application('allmydata_stats_gatherer')
 g.setServiceParent(application)
 """
 
-def create_stats_gatherer(config):
-    err = config.stderr
-    basedir = config['basedir']
-    if not basedir:
-        print >>err, "a basedir was not provided, please use --basedir or -C"
-        return -1
+
+def create_stats_gatherer(basedir, config, out=sys.stdout, err=sys.stderr):
+    # This should always be called with an absolute Unicode basedir.
+    precondition(isinstance(basedir, unicode), basedir)
+
     if os.path.exists(basedir):
-        if os.listdir(basedir):
-            print >>err, "The base directory \"%s\", which is \"%s\" is not empty." % (basedir, os.path.abspath(basedir))
+        if listdir_unicode(basedir):
+            print >>err, "The base directory %s is not empty." % quote_output(basedir)
             print >>err, "To avoid clobbering anything, I am going to quit now."
             print >>err, "Please use a different directory, or empty this one."
             return -1
diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index 62c328ae..42e73a75 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -147,27 +147,33 @@ class CreateNode(unittest.TestCase):
         rc = runner.runner(argv, stdout=out, stderr=err)
         return rc, out.getvalue(), err.getvalue()
 
-    def do_create(self, command, basedir):
-        c1 = os.path.join(basedir, command + "-c1")
-        argv = ["--quiet", command, "--basedir", c1]
+    def do_create(self, kind):
+        basedir = self.workdir("test_" + kind)
+        command = "create-" + kind
+        is_client = kind in ("node", "client")
+        tac = is_client and "tahoe-client.tac" or ("tahoe-" + kind + ".tac")
+
+        n1 = os.path.join(basedir, command + "-n1")
+        argv = ["--quiet", command, "--basedir", n1]
         rc, out, err = self.run_tahoe(argv)
         self.failUnlessEqual(err, "")
         self.failUnlessEqual(out, "")
         self.failUnlessEqual(rc, 0)
-        self.failUnless(os.path.exists(c1))
-        self.failUnless(os.path.exists(os.path.join(c1, "tahoe-client.tac")))
-
-        # tahoe.cfg should exist, and should have storage enabled for
-        # 'create-node', and disabled for 'create-client'.
-        tahoe_cfg = os.path.join(c1, "tahoe.cfg")
-        self.failUnless(os.path.exists(tahoe_cfg))
-        content = open(tahoe_cfg).read()
-        if command == "create-client":
-            self.failUnless("\n[storage]\nenabled = false\n" in content)
-        else:
-            self.failUnless("\n[storage]\nenabled = true\n" in content)
-
-        # creating the client a second time should be rejected
+        self.failUnless(os.path.exists(n1))
+        self.failUnless(os.path.exists(os.path.join(n1, tac)))
+
+        if is_client:
+            # tahoe.cfg should exist, and should have storage enabled for
+            # 'create-node', and disabled for 'create-client'.
+            tahoe_cfg = os.path.join(n1, "tahoe.cfg")
+            self.failUnless(os.path.exists(tahoe_cfg))
+            content = open(tahoe_cfg).read()
+            if kind == "client":
+                self.failUnless("\n[storage]\nenabled = false\n" in content)
+            else:
+                self.failUnless("\n[storage]\nenabled = true\n" in content)
+
+        # creating the node a second time should be rejected
         rc, out, err = self.run_tahoe(argv)
         self.failIfEqual(rc, 0, str((out, err, rc)))
         self.failUnlessEqual(out, "")
@@ -179,138 +185,67 @@ class CreateNode(unittest.TestCase):
             self.failIf(re.search("[\S][^\.!?]$", line), (line,))
 
         # test that the non --basedir form works too
-        c2 = os.path.join(basedir, command + "c2")
-        argv = ["--quiet", command, c2]
-        rc, out, err = self.run_tahoe(argv)
-        self.failUnless(os.path.exists(c2))
-        self.failUnless(os.path.exists(os.path.join(c2, "tahoe-client.tac")))
-
-        # make sure it rejects too many arguments
-        argv = [command, "basedir", "extraarg"]
-        self.failUnlessRaises(usage.UsageError,
-                              runner.runner, argv,
-                              run_by_human=False)
-
-    def test_node(self):
-        basedir = self.workdir("test_node")
-        self.do_create("create-node", basedir)
-
-    def test_client(self):
-        # create-client should behave like create-node --no-storage.
-        basedir = self.workdir("test_client")
-        self.do_create("create-client", basedir)
-
-    def test_introducer(self):
-        basedir = self.workdir("test_introducer")
-        c1 = os.path.join(basedir, "c1")
-        argv = ["--quiet", "create-introducer", "--basedir", c1]
+        n2 = os.path.join(basedir, command + "-n2")
+        argv = ["--quiet", command, n2]
         rc, out, err = self.run_tahoe(argv)
-        self.failUnlessEqual(err, "", err)
+        self.failUnlessEqual(err, "")
         self.failUnlessEqual(out, "")
         self.failUnlessEqual(rc, 0)
-        self.failUnless(os.path.exists(c1))
-        self.failUnless(os.path.exists(os.path.join(c1,"tahoe-introducer.tac")))
-
-        # creating the introducer a second time should be rejected
-        rc, out, err = self.run_tahoe(argv)
-        self.failIfEqual(rc, 0)
-        self.failUnlessEqual(out, "")
-        self.failUnless("is not empty" in err)
-
-        # Fail if there is a non-empty line that doesn't end with a
-        # punctuation mark.
-        for line in err.splitlines():
-            self.failIf(re.search("[\S][^\.!?]$", line), (line,))
+        self.failUnless(os.path.exists(n2))
+        self.failUnless(os.path.exists(os.path.join(n2, tac)))
 
-        # test the non --basedir form
-        c2 = os.path.join(basedir, "c2")
-        argv = ["--quiet", "create-introducer", c2]
+        # test the --node-directory form
+        n3 = os.path.join(basedir, command + "-n3")
+        argv = ["--quiet", command, "--node-directory", n3]
         rc, out, err = self.run_tahoe(argv)
-        self.failUnlessEqual(err, "", err)
+        self.failUnlessEqual(err, "")
         self.failUnlessEqual(out, "")
         self.failUnlessEqual(rc, 0)
-        self.failUnless(os.path.exists(c2))
-        self.failUnless(os.path.exists(os.path.join(c2,"tahoe-introducer.tac")))
-
-        # reject extra arguments
-        argv = ["create-introducer", "basedir", "extraarg"]
-        self.failUnlessRaises(usage.UsageError,
-                              runner.runner, argv,
-                              run_by_human=False)
-        # and require basedir to be provided in some form
-        argv = ["create-introducer"]
-        self.failUnlessRaises(usage.UsageError,
-                              runner.runner, argv,
-                              run_by_human=False)
+        self.failUnless(os.path.exists(n3))
+        self.failUnless(os.path.exists(os.path.join(n3, tac)))
 
-    def test_key_generator(self):
-        basedir = self.workdir("test_key_generator")
-        kg1 = os.path.join(basedir, "kg1")
-        argv = ["--quiet", "create-key-generator", "--basedir", kg1]
+        # test the --multiple form
+        n4 = os.path.join(basedir, command + "-n4")
+        n5 = os.path.join(basedir, command + "-n5")
+        argv = ["--quiet", command, "--multiple", n4, n5]
         rc, out, err = self.run_tahoe(argv)
         self.failUnlessEqual(err, "")
         self.failUnlessEqual(out, "")
         self.failUnlessEqual(rc, 0)
-        self.failUnless(os.path.exists(kg1))
-        self.failUnless(os.path.exists(os.path.join(kg1, "tahoe-key-generator.tac")))
-
-        # creating it a second time should be rejected
-        rc, out, err = self.run_tahoe(argv)
-        self.failIfEqual(rc, 0, str((out, err, rc)))
-        self.failUnlessEqual(out, "")
-        self.failUnlessIn("is not empty.", err)
+        self.failUnless(os.path.exists(n4))
+        self.failUnless(os.path.exists(os.path.join(n4, tac)))
+        self.failUnless(os.path.exists(n5))
+        self.failUnless(os.path.exists(os.path.join(n5, tac)))
 
-        # make sure it rejects too many arguments
-        argv = ["create-key-generator", "basedir", "extraarg"]
+        # make sure it rejects too many arguments without --multiple
+        argv = [command, "basedir", "extraarg"]
         self.failUnlessRaises(usage.UsageError,
                               runner.runner, argv,
                               run_by_human=False)
 
-        # make sure it rejects a missing basedir specification
-        argv = ["create-key-generator"]
-        self.failUnlessRaises(usage.UsageError,
-                              runner.runner, argv,
-                              run_by_human=False)
+        # when creating a non-client, there is no default for the basedir
+        if not is_client:
+            argv = [command]
+            self.failUnlessRaises(usage.UsageError,
+                                  runner.runner, argv,
+                                  run_by_human=False)
 
-    def test_stats_gatherer(self):
-        basedir = self.workdir("test_stats_gatherer")
-        sg1 = os.path.join(basedir, "sg1")
-        argv = ["--quiet", "create-stats-gatherer", "--basedir", sg1]
-        rc, out, err = self.run_tahoe(argv)
-        self.failUnlessEqual(err, "")
-        self.failUnlessEqual(out, "")
-        self.failUnlessEqual(rc, 0)
-        self.failUnless(os.path.exists(sg1))
-        self.failUnless(os.path.exists(os.path.join(sg1, "tahoe-stats-gatherer.tac")))
 
-        # creating it a second time should be rejected
-        rc, out, err = self.run_tahoe(argv)
-        self.failIfEqual(rc, 0, str((out, err, rc)))
-        self.failUnlessEqual(out, "")
-        self.failUnless("is not empty." in err)
+    def test_node(self):
+        self.do_create("node")
 
-        # test the non --basedir form
-        kg2 = os.path.join(basedir, "kg2")
-        argv = ["--quiet", "create-stats-gatherer", kg2]
-        rc, out, err = self.run_tahoe(argv)
-        self.failUnlessEqual(err, "", err)
-        self.failUnlessEqual(out, "")
-        self.failUnlessEqual(rc, 0)
-        self.failUnless(os.path.exists(kg2))
-        self.failUnless(os.path.exists(os.path.join(kg2,"tahoe-stats-gatherer.tac")))
+    def test_client(self):
+        # create-client should behave like create-node --no-storage.
+        self.do_create("client")
 
-        # make sure it rejects too many arguments
-        argv = ["create-stats-gatherer", "basedir", "extraarg"]
-        self.failUnlessRaises(usage.UsageError,
-                              runner.runner, argv,
-                              run_by_human=False)
+    def test_introducer(self):
+        self.do_create("introducer")
 
-        # make sure it rejects a missing basedir specification
-        argv = ["create-stats-gatherer"]
-        rc, out, err = self.run_tahoe(argv)
-        self.failIfEqual(rc, 0, str((out, err, rc)))
-        self.failUnlessEqual(out, "")
-        self.failUnless("a basedir was not provided" in err)
+    def test_key_generator(self):
+        self.do_create("key-generator")
+
+    def test_stats_gatherer(self):
+        self.do_create("stats-gatherer")
 
     def test_subcommands(self):
         # no arguments should trigger a command listing, via UsageError