Fixed bug with unicode handling reported by Nicolas Alvarez.

Date: Mon, 16 Nov 2009 20:34:50 -0300
From: Nicolas Alvarez <nicolas.alvarez@gmail.com>
Subject: [Be-devel] Mercurial + BE + Unicode doesn't work

My username in ~/.hgrc contains a Unicode character. When I run "be new" on
a Mercurial repository, I get an unhandled Python exception:

UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 13:
ordinal not in range(128)

The following shell script should reproduce the error:

#!/bin/sh

repo=/tmp/`mktemp -d bug-repro.XXXX`
hg init $repo
cd $repo
/usr/bin/printf "[ui]\nusername = Nicol\u00e1s\n" > $repo/.hg/hgrc
be set-root $repo
be new "Testing"
rm -rf /tmp/$repo


[WTK:
Note that the
  be set-root
usage is out of date, it is now
  be init
]

9 files changed, 48 insertions, 34 deletions

diff --git a/libbe/arch.py b/libbe/arch.py
index daa8ac6..98c4edd 100644
--- a/libbe/arch.py
+++ b/libbe/arch.py
@@ -83,7 +83,7 @@ class Arch(vcs.VCS):
         self._archive_dir = "/tmp/%s" % trailer
         self._tmp_archive = True
         self._u_invoke_client("make-archive", self._archive_name,
-                              self._archive_dir, directory=path)
+                              self._archive_dir, cwd=path)
     def _invoke_client(self, *args, **kwargs):
         """
         Invoke the client on our archive.
@@ -121,7 +121,7 @@ class Arch(vcs.VCS):
         version = "0.1"
         self._project_name = "%s--%s--%s" % (category, branch, version)
         self._invoke_client("archive-setup", self._project_name,
-                            directory=path)
+                            cwd=path)
         self._tmp_project = True
     def _remove_project(self):
         assert self._tmp_project == True
@@ -164,10 +164,10 @@ class Arch(vcs.VCS):
         # http://regexps.srparish.net/tutorial-tla/new-source.html
         # http://regexps.srparish.net/tutorial-tla/importing-first.html
         self._invoke_client("init-tree", self._project_name,
-                              directory=path)
+                            cwd=path)
         self._adjust_naming_conventions(path)
         self._invoke_client("import", "--summary", "Began versioning",
-                            directory=path)
+                            cwd=path)
     def _vcs_cleanup(self):
         if self._tmp_project == True:
             self._remove_project()
@@ -198,7 +198,7 @@ class Arch(vcs.VCS):
         assert self._archive_name != None
     def _get_archive_project_name(self, root):
         # get project names
-        status,output,error = self._u_invoke_client("tree-version", directory=root)
+        status,output,error = self._u_invoke_client("tree-version", cwd=root)
         # e.g output
         # jdoe@example.com--bugs-everywhere-auto-2008.22.24.52/be--mainline--0.1
         archive_name,project_name = output.rstrip('\n').split('/')
@@ -266,7 +266,7 @@ class Arch(vcs.VCS):
             vcs.VCS._vcs_duplicate_repo(self, directory, revision)
         else:
             status,output,error = \
-                self._u_invoke_client("get", revision,directory)
+                self._u_invoke_client("get", revision, directory)
     def _vcs_commit(self, commitfile, allow_empty=False):
         if allow_empty == False:
             # arch applies empty commits without complaining, so check first
diff --git a/libbe/bug.py b/libbe/bug.py
index fd30ff7..6633ab7 100644
--- a/libbe/bug.py
+++ b/libbe/bug.py
@@ -267,7 +267,9 @@ class Bug(settings_object.SavedSettingsObject):
         value = getattr(self, setting)
         if value == None:
             return ""
-        return str(value)
+        if type(value) not in types.StringTypes:
+            return str(value)
+        return value
 
     def xml(self, show_comments=False):
         if self.bugdir == None:
diff --git a/libbe/bzr.py b/libbe/bzr.py
index ed9e032..8e91d0c 100644
--- a/libbe/bzr.py
+++ b/libbe/bzr.py
@@ -49,7 +49,7 @@ class Bzr(vcs.VCS):
         status,output,error = self._u_invoke_client("root", path)
         return output.rstrip('\n')
     def _vcs_init(self, path):
-        self._u_invoke_client("init", directory=path)
+        self._u_invoke_client("init", cwd=path)
     def _vcs_get_user_id(self):
         status,output,error = self._u_invoke_client("whoami")
         return output.rstrip('\n')
diff --git a/libbe/comment.py b/libbe/comment.py
index 02bcc93..17daf62 100644
--- a/libbe/comment.py
+++ b/libbe/comment.py
@@ -308,7 +308,9 @@ class Comment(Tree, settings_object.SavedSettingsObject):
         value = getattr(self, setting)
         if value == None:
             return ""
-        return str(value)
+        if type(value) not in types.StringTypes:
+            return str(value)
+        return value
 
     def xml(self, indent=0, shortname=None):
         """
diff --git a/libbe/darcs.py b/libbe/darcs.py
index 9115886..6bf0119 100644
--- a/libbe/darcs.py
+++ b/libbe/darcs.py
@@ -60,7 +60,7 @@ class Darcs(vcs.VCS):
             return None
         return os.path.dirname(darcs_dir)
     def _vcs_init(self, path):
-        self._u_invoke_client("init", directory=path)
+        self._u_invoke_client("init", cwd=path)
     def _vcs_get_user_id(self):
         # following http://darcs.net/manual/node4.html#SECTION00410030000000000000
         # as of June 29th, 2009
@@ -107,10 +107,12 @@ class Darcs(vcs.VCS):
                 # Darcs versions < 2.0.0pre2 lack the "show contents" command
 
                 status,output,error = self._u_invoke_client( \
-                    "diff", "--unified", "--from-patch", revision, path)
+                    "diff", "--unified", "--from-patch", revision, path,
+                    unicode_output=False)
                 major_patch = output
                 status,output,error = self._u_invoke_client( \
-                    "diff", "--unified", "--patch", revision, path)
+                    "diff", "--unified", "--patch", revision, path,
+                    unicode_output=False)
                 target_patch = output
                 
                 # "--output -" to be supported in GNU patch > 2.5.9
diff --git a/libbe/git.py b/libbe/git.py
index 628f9b9..781a278 100644
--- a/libbe/git.py
+++ b/libbe/git.py
@@ -50,12 +50,12 @@ class Git(vcs.VCS):
         if os.path.isdir(path) != True:
             path = os.path.dirname(path)
         status,output,error = self._u_invoke_client("rev-parse", "--git-dir",
-                                                    directory=path)
+                                                    cwd=path)
         gitdir = os.path.join(path, output.rstrip('\n'))
         dirname = os.path.abspath(os.path.dirname(gitdir))
         return dirname
     def _vcs_init(self, path):
-        self._u_invoke_client("init", directory=path)
+        self._u_invoke_client("init", cwd=path)
     def _vcs_get_user_id(self):
         status,output,error = \
             self._u_invoke_client("config", "user.name", expect=(0,1))
@@ -102,9 +102,8 @@ class Git(vcs.VCS):
         if revision==None:
             vcs.VCS._vcs_duplicate_repo(self, directory, revision)
         else:
-            #self._u_invoke_client("archive", revision, directory) # makes tarball
-            self._u_invoke_client("clone", "--no-checkout",".",directory)
-            self._u_invoke_client("checkout", revision, directory=directory)
+            self._u_invoke_client("clone", "--no-checkout", ".", directory)
+            self._u_invoke_client("checkout", revision, cwd=directory)
     def _vcs_commit(self, commitfile, allow_empty=False):
         args = ['commit', '--all', '--file', commitfile]
         if allow_empty == True:
diff --git a/libbe/hg.py b/libbe/hg.py
index 7cd4c2f..40739b6 100644
--- a/libbe/hg.py
+++ b/libbe/hg.py
@@ -45,10 +45,10 @@ class Hg(vcs.VCS):
             return True
         return False
     def _vcs_root(self, path):
-        status,output,error = self._u_invoke_client("root", directory=path)
+        status,output,error = self._u_invoke_client("root", cwd=path)
         return output.rstrip('\n')
     def _vcs_init(self, path):
-        self._u_invoke_client("init", directory=path)
+        self._u_invoke_client("init", cwd=path)
     def _vcs_get_user_id(self):
         status,output,error = self._u_invoke_client("showconfig","ui.username")
         return output.rstrip('\n')
diff --git a/libbe/mapfile.py b/libbe/mapfile.py
index 4d69601..d0e16fe 100644
--- a/libbe/mapfile.py
+++ b/libbe/mapfile.py
@@ -75,7 +75,7 @@ def generate(map):
             assert(len(key) > 0)
         except AssertionError:
             raise IllegalKey(unicode(key).encode('unicode_escape'))
-        if "\n" in map[key]:
+        if '\n' in map[key]:
             raise IllegalValue(unicode(map[key]).encode('unicode_escape'))
 
     lines = []
@@ -83,7 +83,7 @@ def generate(map):
         lines.append(yaml.safe_dump({key: map[key]},
                                     default_flow_style=False,
                                     allow_unicode=True))
-        lines.append("")
+        lines.append('')
     return '\n'.join(lines)
 
 def parse(contents):
@@ -101,16 +101,21 @@ def parse(contents):
     'd'
     >>> dict["e"]
     'f'
+    >>> contents = generate({"q":u"Fran\u00e7ais"})
+    >>> dict = parse(contents)
+    >>> dict["q"]
+    u'Fran\\xe7ais'
     """
     return yaml.load(contents) or {}
 
 def map_save(vcs, path, map, allow_no_vcs=False):
     """Save the map as a mapfile to the specified path"""
     contents = generate(map)
-    vcs.set_file_contents(path, contents, allow_no_vcs)
+    vcs.set_file_contents(path, contents, allow_no_vcs, binary=True)
 
 def map_load(vcs, path, allow_no_vcs=False):
-    contents = vcs.get_file_contents(path, allow_no_vcs=allow_no_vcs)
+    contents = vcs.get_file_contents(path, allow_no_vcs=allow_no_vcs,
+                                     binary=True)
     return parse(contents)
 
 suite = doctest.DocTestSuite()
diff --git a/libbe/vcs.py b/libbe/vcs.py
index 7484660..ba23858 100644
--- a/libbe/vcs.py
+++ b/libbe/vcs.py
@@ -384,7 +384,7 @@ class VCS(object):
         revision==None specifies the current revision.
         Return the path to the arbitrary directory at the base of the new repo.
         """
-        # Dirname in Baseir to protect against simlink attacks.
+        # Dirname in Basedir to protect against simlink attacks.
         if self._duplicateBasedir == None:
             self._duplicateBasedir = tempfile.mkdtemp(prefix='BEvcs')
             self._duplicateDirname = \
@@ -456,10 +456,13 @@ class VCS(object):
             if list_string in string:
                 return True
         return False
-    def _u_invoke(self, args, stdin=None, expect=(0,), cwd=None):
+    def _u_invoke(self, args, stdin=None, expect=(0,), cwd=None,
+                  unicode_output=True):
         """
         expect should be a tuple of allowed exit codes.  cwd should be
-        the directory from which the command will be executed.
+        the directory from which the command will be executed.  When
+        unicode_output == True, convert stdout and stdin strings to
+        unicode before returing them.
         """
         if cwd == None:
             cwd = self.rootdir
@@ -474,20 +477,20 @@ class VCS(object):
                           shell=True, cwd=cwd)
         except OSError, e :
             raise CommandError(args, status=e.args[0], stdout="", stderr=e)
-        output,error = q.communicate(input=stdin)
+        stdout,stderr = q.communicate(input=stdin)
         status = q.wait()
+        if unicode_output == True:
+            stdout = unicode(stdout, self.encoding)
+            stderr = unicode(stderr, self.encoding)
         if self.verboseInvoke == True:
-            print >> sys.stderr, "%d\n%s%s" % (status, output, error)
+            print >> sys.stderr, "%d\n%s%s" % (status, stdout, stderr)
         if status not in expect:
-            raise CommandError(args, status, output, error)
-        return status, output, error
+            raise CommandError(args, status, stdout, stderr)
+        return status, stdout, stderr
     def _u_invoke_client(self, *args, **kwargs):
-        directory = kwargs.get('directory',None)
-        expect = kwargs.get('expect', (0,))
-        stdin = kwargs.get('stdin', None)
         cl_args = [self.client]
         cl_args.extend(args)
-        return self._u_invoke(cl_args, stdin=stdin,expect=expect,cwd=directory)
+        return self._u_invoke(cl_args, **kwargs)
     def _u_search_parent_directories(self, path, filename):
         """
         Find the file (or directory) named filename in path or in any
@@ -678,6 +681,7 @@ class VCSTestCase(unittest.TestCase):
 
     def tearDown(self):
         self.vcs.cleanup()
+        self.dir.cleanup()
         super(VCSTestCase, self).tearDown()
 
     def full_path(self, rel_path):