Ticket #715: basedir-option-improvements.dpatch

Thu Jul 22 00:15:07 GMT Daylight Time 2010  david-sarah@jacaranda.org
  * util.fileutil, test.test_util: add abspath_expanduser_unicode function, to work around <http://bugs.python.org/issue3426>. util.encodingutil: add a convenience function argv_to_abspath.

Thu Jul 22 00:48:34 GMT Daylight Time 2010  david-sarah@jacaranda.org
  * Basedir/node directory option improvements. addresses #188, #706, #715, #772, #890

New patches:

[util.fileutil, test.test_util: add abspath_expanduser_unicode function, to work around <http://bugs.python.org/issue3426>. util.encodingutil: add a convenience function argv_to_abspath.
david-sarah@jacaranda.org**20100721231507
 Ignore-this: eee6904d1f65a733ff35190879844d08
] {
hunk ./src/allmydata/test/test_util.py 4
 
 def foo(): pass # keep the line number constant
 
-import os, time
+import os, time, sys
 from StringIO import StringIO
 from twisted.trial import unittest
 from twisted.internet import defer, reactor
hunk ./src/allmydata/test/test_util.py 473
         used = fileutil.du(basedir)
         self.failUnlessEqual(10+11+12+13, used)
 
+    def test_abspath_expanduser_unicode(self):
+        self.failUnlessRaises(AssertionError, fileutil.abspath_expanduser_unicode, "bytestring")
+
+        saved_cwd = os.path.normpath(os.getcwdu())
+        abspath_cwd = fileutil.abspath_expanduser_unicode(u".")
+        self.failUnless(isinstance(saved_cwd, unicode), saved_cwd)
+        self.failUnless(isinstance(abspath_cwd, unicode), abspath_cwd)
+        self.failUnlessEqual(abspath_cwd, saved_cwd)
+
+        # adapted from <http://svn.python.org/view/python/branches/release26-maint/Lib/test/test_posixpath.py?view=markup&pathrev=78279#test_abspath>
+
+        self.failUnlessIn(u"foo", fileutil.abspath_expanduser_unicode(u"foo"))
+        self.failIfIn(u"~", fileutil.abspath_expanduser_unicode(u"~"))
+
+        cwds = ['cwd']
+        try:
+            cwds.append(u'\xe7w\xf0'.encode(sys.getfilesystemencoding()
+                                            or 'ascii'))
+        except UnicodeEncodeError:
+            pass # the cwd can't be encoded -- test with ascii cwd only
+
+        for cwd in cwds:
+            try:
+                os.mkdir(cwd)
+                os.chdir(cwd)
+                for upath in (u'', u'fuu', u'f\xf9\xf9', u'/fuu', u'U:\\', u'~'):
+                    uabspath = fileutil.abspath_expanduser_unicode(upath)
+                    self.failUnless(isinstance(uabspath, unicode), uabspath)
+            finally:
+                os.chdir(saved_cwd)
+
 class PollMixinTests(unittest.TestCase):
     def setUp(self):
         self.pm = pollmixin.PollMixin()
hunk ./src/allmydata/util/encodingutil.py 13
 from twisted.python import usage
 import locale
 from allmydata.util import log
+from allmydata.util.fileutil import abspath_expanduser_unicode
 
 
 def _canonical_encoding(encoding):
hunk ./src/allmydata/util/encodingutil.py 95
         raise usage.UsageError("Argument %s cannot be decoded as %s." %
                                (quote_output(s), argv_encoding))
 
+def argv_to_abspath(s):
+    """
+    Convenience function to decode an argv element to an absolute path, with ~ expanded.
+    If this fails, raise a UsageError.
+    """
+    return abspath_expanduser_unicode(argv_to_unicode(s))
+
 def unicode_to_url(s):
     """
     Encode an unicode object used in an URL.
hunk ./src/allmydata/util/fileutil.py 276
     finally:
         outf.close()
 
+
+# Work around <http://bugs.python.org/issue3426>. This code is adapted from
+# <http://svn.python.org/view/python/trunk/Lib/ntpath.py?revision=78247&view=markup>
+# with some simplifications.
+
+_getfullpathname = None
+try:
+    from nt import _getfullpathname
+except ImportError:
+    pass
+
+def abspath_expanduser_unicode(path):
+    """Return the absolute version of a path."""
+    assert isinstance(path, unicode), path
+
+    path = os.path.expanduser(path)
+
+    if _getfullpathname:
+        # On Windows, os.path.isabs will return True for paths without a drive letter,
+        # e.g. "\\". See <http://bugs.python.org/issue1669539>.
+        try:
+            path = _getfullpathname(path or u".")
+        except WindowsError:
+            pass
+
+    if not os.path.isabs(path):
+        path = os.path.join(os.getcwdu(), path)
+
+    # We won't hit <http://bugs.python.org/issue5827> because
+    # there is always at least one Unicode path component.
+    return os.path.normpath(path)
+
}
[Basedir/node directory option improvements. addresses #188, #706, #715, #772, #890
david-sarah@jacaranda.org**20100721234834
 Ignore-this: 92d52f3af4acb0d659cb49e3306fef6c
] {
hunk ./src/allmydata/scripts/cli.py 3
 import os.path, re, sys, fnmatch
 from twisted.python import usage
-from allmydata.scripts.common import BaseOptions, get_aliases
-from allmydata.util.encodingutil import argv_to_unicode
+from allmydata.scripts.common import BaseOptions, get_aliases, get_default_nodedir, DEFAULT_ALIAS
+from allmydata.util.encodingutil import argv_to_unicode, argv_to_abspath, quote_output
 
 NODEURL_RE=re.compile("http(s?)://([^:]*)(:([1-9][0-9]*))?")
 
hunk ./src/allmydata/scripts/cli.py 8
-class VDriveOptions(BaseOptions, usage.Options):
+_default_nodedir = get_default_nodedir()
+
+class VDriveOptions(BaseOptions):
     optParameters = [
hunk ./src/allmydata/scripts/cli.py 12
-        ["node-directory", "d", "~/.tahoe",
-         "Look here to find out which Tahoe node should be used for all "
-         "operations. The directory should either contain a full Tahoe node, "
-         "or a file named node.url which 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."
-         ],
+        ["node-directory", "d", None,
+         "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 "")],
         ["node-url", "u", None,
          "URL of the tahoe node to use, a URL like \"http://127.0.0.1:3456\". "
          "This overrides the URL found in the --node-directory ."],
hunk ./src/allmydata/scripts/cli.py 27
         ]
 
     def postOptions(self):
-        # TODO: allow Unicode node-dir
-        # compute a node-url from the existing options, put in self['node-url']
         if self['node-directory']:
hunk ./src/allmydata/scripts/cli.py 28
-            if sys.platform == 'win32' and self['node-directory'] == '~/.tahoe':
-                from allmydata.windows import registry
-                self['node-directory'] = registry.get_base_dir_path()
-            else:
-                self['node-directory'] = os.path.expanduser(self['node-directory'])
+            self['node-directory'] = argv_to_abspath(self['node-directory'])
+        else:
+            self['node-directory'] = _default_nodedir
+
+        # compute a node-url from the existing options, put in self['node-url']
         if self['node-url']:
             if (not isinstance(self['node-url'], basestring)
                 or not NODEURL_RE.match(self['node-url'])):
hunk ./src/allmydata/scripts/cli.py 48
 
         aliases = get_aliases(self['node-directory'])
         if self['dir-cap']:
-            aliases["tahoe"] = self['dir-cap']
+            aliases[DEFAULT_ALIAS] = self['dir-cap']
         self.aliases = aliases # maps alias name to dircap
 
 
hunk ./src/allmydata/scripts/common.py 5
 import os, sys, urllib
 import codecs
 from twisted.python import usage
-from allmydata.util.encodingutil import unicode_to_url, quote_output
 from allmydata.util.assertutil import precondition
hunk ./src/allmydata/scripts/common.py 6
+from allmydata.util.encodingutil import unicode_to_url, quote_output, argv_to_abspath
+from allmydata.util.fileutil import abspath_expanduser_unicode
 
hunk ./src/allmydata/scripts/common.py 9
-class BaseOptions:
+
+_default_nodedir = None
+if sys.platform == 'win32':
+    from allmydata.windows import registry
+    path = registry.get_base_dir_path()
+    if path:
+        precondition(isinstance(path, unicode), path)
+        _default_nodedir = abspath_expanduser_unicode(path)
+
+if _default_nodedir is None:
+    path = abspath_expanduser_unicode(u"~/.tahoe")
+    precondition(isinstance(path, unicode), path)
+    _default_nodedir = path
+
+def get_default_nodedir():
+    return _default_nodedir
+
+
+class BaseOptions(usage.Options):
     # unit tests can override these to point at StringIO instances
     stdin = sys.stdin
     stdout = sys.stdout
hunk ./src/allmydata/scripts/common.py 37
         ["quiet", "q", "Operate silently."],
         ["version", "V", "Display version numbers and exit."],
         ["version-and-path", None, "Display version numbers and paths to their locations and exit."],
-        ]
-
+    ]
+    optParameters = [
+        ["node-directory", "d", None, "Specify which Tahoe node directory should be used." + (
+            _default_nodedir and (" [default for most commands: " + quote_output(_default_nodedir) + "]") or "")],
+    ]
+    
     def opt_version(self):
         import allmydata
         print >>self.stdout, allmydata.get_package_versions_string()
hunk ./src/allmydata/scripts/common.py 55
 
 
 class BasedirMixin:
-    optFlags = [
-        ["multiple", "m", "allow multiple basedirs to be specified at once"],
-        ]
+    default_nodedir = _default_nodedir
+    allow_multiple = True
 
hunk ./src/allmydata/scripts/common.py 58
-    def postOptions(self):
-        if not self.basedirs:
-            raise usage.UsageError("<basedir> parameter is required")
-        if self['basedir']:
-            del self['basedir']
-        self['basedirs'] = [os.path.abspath(os.path.expanduser(b)) for b in self.basedirs]
+    optParameters = [
+        ["basedir", "C", None, "Same as --node-directory."],
+    ]
+    optFlags = [
+        ["multiple", "m", "Specify multiple node directories at once"],
+    ]
 
     def parseArgs(self, *args):
hunk ./src/allmydata/scripts/common.py 66
-        self.basedirs = []
-        if self['basedir']:
-            precondition(isinstance(self['basedir'], (str, unicode)), self['basedir'])
-            self.basedirs.append(self['basedir'])
-        if self['multiple']:
-            precondition(not [x for x in args if not isinstance(x, (str, unicode))], args)
-            self.basedirs.extend(args)
+        if self['node-directory'] and self['basedir']:
+            raise usage.UsageError("The --node-directory (or -d) and --basedir (or -C) "
+                                   "options cannot both be used.")
+
+        if self['node-directory'] or self['basedir']:
+            self.basedirs = [argv_to_abspath(self['node-directory'] or self['basedir'])]
         else:
hunk ./src/allmydata/scripts/common.py 73
-            if len(args) == 0 and not self.basedirs:
-                if sys.platform == 'win32':
-                    from allmydata.windows import registry
-                    rbdp = registry.get_base_dir_path()
-                    if rbdp:
-                        precondition(isinstance(registry.get_base_dir_path(), (str, unicode)), registry.get_base_dir_path())
-                        self.basedirs.append(rbdp)
-                else:
-                    precondition(isinstance(os.path.expanduser("~/.tahoe"), (str, unicode)), os.path.expanduser("~/.tahoe"))
-                    self.basedirs.append(os.path.expanduser("~/.tahoe"))
-            if len(args) > 0:
-                precondition(isinstance(args[0], (str, unicode)), args[0])
-                self.basedirs.append(args[0])
-            if len(args) > 1:
-                raise usage.UsageError("I wasn't expecting so many arguments")
+            self.basedirs = []
 
hunk ./src/allmydata/scripts/common.py 75
-class NoDefaultBasedirMixin(BasedirMixin):
-    def parseArgs(self, *args):
-        # create-client won't default to --basedir=~/.tahoe
-        self.basedirs = []
-        if self['basedir']:
-            precondition(isinstance(self['basedir'], (str, unicode)), self['basedir'])
-            self.basedirs.append(self['basedir'])
-        if self['multiple']:
-            precondition(not [x for x in args if not isinstance(x, (str, unicode))], args)
-            self.basedirs.extend(args)
+        if self.allow_multiple and self['multiple']:
+            self.basedirs.extend(map(argv_to_abspath, args))
         else:
hunk ./src/allmydata/scripts/common.py 78
-            if len(args) > 0:
-                precondition(isinstance(args[0], (str, unicode)), args[0])
-                self.basedirs.append(args[0])
             if len(args) > 1:
hunk ./src/allmydata/scripts/common.py 79
-                raise usage.UsageError("I wasn't expecting so many arguments")
+                raise usage.UsageError("I wasn't expecting so many arguments." +
+                    (self.allow_multiple and
+                     " Use the --multiple option to specify more than one node directory." or ""))
+
+            if len(args) == 0 and self.default_nodedir and not self.basedirs:
+                self.basedirs.append(self.default_nodedir)
+            elif len(args) > 0:
+                self.basedirs.append(argv_to_abspath(args[0]))
+
+    def postOptions(self):
         if not self.basedirs:
hunk ./src/allmydata/scripts/common.py 90
-            raise usage.UsageError("--basedir must be provided")
+            raise usage.UsageError("A base directory for the node must be provided.")
+        del self['basedir']
+        self['basedirs'] = self.basedirs
+
 
 DEFAULT_ALIAS = u"tahoe"
 
hunk ./src/allmydata/scripts/common.py 107
         f = open(rootfile, "r")
         rootcap = f.read().strip()
         if rootcap:
-            aliases[u"tahoe"] = uri.from_string_dirnode(rootcap).to_string()
+            aliases[DEFAULT_ALIAS] = uri.from_string_dirnode(rootcap).to_string()
     except EnvironmentError:
         pass
     try:
hunk ./src/allmydata/scripts/create_node.py 3
 
 import os, sys
-from twisted.python import usage
-from allmydata.scripts.common import BasedirMixin, NoDefaultBasedirMixin
+from allmydata.scripts.common import BasedirMixin, BaseOptions
+from allmydata.util.assertutil import precondition
+from allmydata.util.encodingutil import listdir_unicode, argv_to_unicode, quote_output
 
hunk ./src/allmydata/scripts/create_node.py 7
-class CreateClientOptions(BasedirMixin, usage.Options):
+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
hunk ./src/allmydata/scripts/create_node.py 24
         ("no-storage", None, "do not offer storage service to other nodes"),
         ]
 
-class CreateIntroducerOptions(NoDefaultBasedirMixin, usage.Options):
+class CreateIntroducerOptions(BasedirMixin, BaseOptions):
+    default_nodedir = None
+
     optParameters = [
         ["basedir", "C", None, "which directory to create the introducer in"],
         ]
hunk ./src/allmydata/scripts/create_node.py 73
     c.write("\n\n")
 
     c.write("[node]\n")
-    c.write("nickname = %s\n" % config.get("nickname", "")) #TODO: utf8 in argv?
-    webport = config.get("webport", "none")
+    nickname = argv_to_unicode(config.get("nickname") or "")
+    c.write("nickname = %s\n" % (nickname.encode('utf-8'),))
+
+    # TODO: validate webport
+    webport = argv_to_unicode(config.get("webport") or "none")
     if webport.lower() == "none":
         webport = ""
hunk ./src/allmydata/scripts/create_node.py 80
-    c.write("web.port = %s\n" % webport)
+    c.write("web.port = %s\n" % (webport.encode('utf-8'),))
     c.write("web.static = public_html\n")
     c.write("#tub.port =\n")
     c.write("#tub.location = \n")
hunk ./src/allmydata/scripts/keygen.py 3
 
 import os, sys
-from twisted.python import usage
-#from allmydata.scripts.common import BasedirMixin, NoDefaultBasedirMixin
+from allmydata.scripts.common import BasedirMixin, BaseOptions
+from allmydata.util.encodingutil import listdir_unicode, quote_output
+
+class CreateKeyGeneratorOptions(BasedirMixin, BaseOptions):
+    default_nodedir = None
+    allow_multiple = False
 
hunk ./src/allmydata/scripts/keygen.py 10
-class CreateKeyGeneratorOptions(usage.Options):
     optParameters = [
         ["basedir", "C", None, "which directory to create the key-generator in"],
hunk ./src/allmydata/scripts/keygen.py 12
-        ]
+    ]
 
 keygen_tac = """
 # -*- python -*-
hunk ./src/allmydata/scripts/keygen.py 30
 """
 
 def create_key_generator(config, out=sys.stdout, err=sys.stderr):
-    basedir = config['basedir']
-    if not basedir:
-        print >>err, "a basedir was not provided, please use --basedir or -C"
-        return -1
+    basedir = config['basedirs'][0]
     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))
hunk ./src/allmydata/scripts/startstop_node.py 3
 
 import os, sys, signal, time
-from twisted.python import usage
-from allmydata.scripts.common import BasedirMixin
+from allmydata.scripts.common import BasedirMixin, BaseOptions
 from allmydata.util import fileutil, find_exe
 
hunk ./src/allmydata/scripts/startstop_node.py 6
-class StartOptions(BasedirMixin, usage.Options):
+class StartOptions(BasedirMixin, BaseOptions):
     optParameters = [
         ["basedir", "C", None, "which directory to start the node in"],
         ]
hunk ./src/allmydata/scripts/startstop_node.py 15
         ["syslog", None, "tell the node to log to syslog, not a file"],
         ]
 
-class StopOptions(BasedirMixin, usage.Options):
+class StopOptions(BasedirMixin, BaseOptions):
     optParameters = [
         ["basedir", "C", None, "which directory to stop the node in"],
         ]
hunk ./src/allmydata/scripts/startstop_node.py 20
 
-class RestartOptions(BasedirMixin, usage.Options):
+class RestartOptions(BasedirMixin, BaseOptions):
     optParameters = [
         ["basedir", "C", None, "which directory to restart the node in"],
         ]
hunk ./src/allmydata/scripts/startstop_node.py 29
         ["syslog", None, "tell the node to log to syslog, not a file"],
         ]
 
-class RunOptions(usage.Options):
+class RunOptions(BasedirMixin, BaseOptions):
+    default_nodedir = u"."
+
     optParameters = [
         ["basedir", "C", None, "which directory to run the node in, CWD by default"],
         ]
hunk ./src/allmydata/test/test_runner.py 206
 
         # make sure it rejects a missing basedir specification
         argv = ["create-key-generator"]
-        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)
+        self.failUnlessRaises(usage.UsageError,
+                              runner.runner, argv,
+                              run_by_human=False)
 
     def test_stats_gatherer(self):
         basedir = self.workdir("test_stats_gatherer")
}

Context:

[__init__.py: silence DeprecationWarning about BaseException.message globally. fixes #1129
david-sarah@jacaranda.org**20100720011939
 Ignore-this: 38808986ba79cb2786b010504a22f89
] 
[test_runner: test that 'tahoe --version' outputs no noise (e.g. DeprecationWarnings).
david-sarah@jacaranda.org**20100720011345
 Ignore-this: dd358b7b2e5d57282cbe133e8069702e
] 
[TAG allmydata-tahoe-1.7.1
zooko@zooko.com**20100719131352
 Ignore-this: 6942056548433dc653a746703819ad8c
] 
Patch bundle hash:
8eb45d45724a0668578ea30a8f2ca4699bf50008