Fixed libbe.command.diff + ugly BugDir.duplicate_bugdir implementation

duplicate_bugdir() works, but for the vcs backends, it could require
shelling out for _every_ file read.  This could, and probably will, be
horribly slow.  Still it works ;).

I'm not sure what a better implementation would be.  The old
implementation checked out the entire earlier state into a temporary
directory
  pros: single shell out, simple upgrade implementation
  cons: wouldn't work well for HTTP backens

I think a good solution would run along the lines of the currently
commented out code in duplicate_bugdir(), where a
  VersionedStorage.changed_since(revision)
call would give you a list of changed files.  diff could work off of
that directly, without the need to generate a whole duplicate bugdir.
I'm stuck on how to handle upgrades though...

Also removed trailing whitespace from all python files.

11 files changed, 73 insertions, 55 deletions

diff --git a/libbe/storage/base.py b/libbe/storage/base.py
index dd35586..97c8b29 100644
--- a/libbe/storage/base.py
+++ b/libbe/storage/base.py
@@ -256,8 +256,10 @@ class Storage (object):
         else:
             decode = False
         value = self._get(*args, **kwargs)
-        if decode == True:
+        if decode == True and type(value) != types.UnicodeType:
             return unicode(value, self.encoding)
+        if decode == False and type(value) != types.StringType:
+            return value.encode(self.encoding)
         return value
 
     def _get(self, id, default=InvalidObject, revision=None):
@@ -673,7 +675,7 @@ if TESTING == True:
             self.failUnless(s == val,
                     "%s.get() returned %s not %s"
                     % (vars(self.Class)['name'], s, self.val))
-            
+
 
     class Storage_persistence_TestCase (StorageTestCase):
         """Test cases for Storage.disconnect and .connect methods."""
@@ -767,7 +769,7 @@ if TESTING == True:
                 revs.append(self.s.commit('%s: %d' % (self.commit_msg, i),
                                           self.commit_body))
             for i in range(10):
-                rev = self.s.revision_id(i+1) 
+                rev = self.s.revision_id(i+1)
                 self.failUnless(rev == revs[i],
                                 "%s.revision_id(%d) returned %s not %s"
                                 % (vars(self.Class)['name'], i+1, rev, revs[i]))
@@ -794,7 +796,7 @@ if TESTING == True:
                 self.failUnless(ret == val(i),
                                 "%s.get() returned %s not %s for revision %s"
                                 % (vars(self.Class)['name'], ret, val(i), revs[i]))
-        
+
     def make_storage_testcase_subclasses(storage_class, namespace):
         """Make StorageTestCase subclasses for storage_class in namespace."""
         storage_testcase_classes = [
diff --git a/libbe/storage/util/config.py b/libbe/storage/util/config.py
index a0a252e..9f95d14 100644
--- a/libbe/storage/util/config.py
+++ b/libbe/storage/util/config.py
@@ -46,7 +46,7 @@ def set_val(name, value, section="DEFAULT", encoding=None):
     if encoding == None:
         encoding = default_encoding
     config = ConfigParser.ConfigParser()
-    if os.path.exists(path()) == False: # touch file or config 
+    if os.path.exists(path()) == False: # touch file or config
         open(path(), 'w').close()       # read chokes on missing file
     f = codecs.open(path(), 'r', encoding)
     config.readfp(f, path())
diff --git a/libbe/storage/util/mapfile.py b/libbe/storage/util/mapfile.py
index a8d5516..35ae1a0 100644
--- a/libbe/storage/util/mapfile.py
+++ b/libbe/storage/util/mapfile.py
@@ -24,6 +24,7 @@ independent/conflicting changes.
 
 import errno
 import os.path
+import types
 import yaml
 
 import libbe
@@ -39,32 +40,37 @@ class IllegalKey(Exception):
 class IllegalValue(Exception):
     def __init__(self, value):
         Exception.__init__(self, 'Illegal value "%s"' % value)
-        self.value = value 
+        self.value = value
+
+class InvalidMapfileContents(Exception):
+    def __init__(self, contents):
+        Exception.__init__(self, 'Invalid YAML contents')
+        self.contents = contents
 
 def generate(map):
     """Generate a YAML mapfile content string.
-    >>> generate({"q":"p"})
+    >>> generate({'q':'p'})
     'q: p\\n\\n'
-    >>> generate({"q":u"Fran\u00e7ais"})
+    >>> generate({'q':u'Fran\u00e7ais'})
     'q: Fran\\xc3\\xa7ais\\n\\n'
-    >>> generate({"q":u"hello"})
+    >>> generate({'q':u'hello'})
     'q: hello\\n\\n'
-    >>> generate({"q=":"p"})
+    >>> generate({'q=':'p'})
     Traceback (most recent call last):
     IllegalKey: Illegal key "q="
-    >>> generate({"q:":"p"})
+    >>> generate({'q:':'p'})
     Traceback (most recent call last):
     IllegalKey: Illegal key "q:"
-    >>> generate({"q\\n":"p"})
+    >>> generate({'q\\n':'p'})
     Traceback (most recent call last):
     IllegalKey: Illegal key "q\\n"
-    >>> generate({"":"p"})
+    >>> generate({'':'p'})
     Traceback (most recent call last):
     IllegalKey: Illegal key ""
-    >>> generate({">q":"p"})
+    >>> generate({'>q':'p'})
     Traceback (most recent call last):
     IllegalKey: Illegal key ">q"
-    >>> generate({"q":"p\\n"})
+    >>> generate({'q':'p\\n'})
     Traceback (most recent call last):
     IllegalValue: Illegal value "p\\n"
     """
@@ -97,20 +103,28 @@ def parse(contents):
     'p'
     >>> parse('q: \\'p\\'\\n\\n')['q']
     'p'
-    >>> contents = generate({"a":"b", "c":"d", "e":"f"})
+    >>> contents = generate({'a':'b', 'c':'d', 'e':'f'})
     >>> dict = parse(contents)
-    >>> dict["a"]
+    >>> dict['a']
     'b'
-    >>> dict["c"]
+    >>> dict['c']
     'd'
-    >>> dict["e"]
+    >>> dict['e']
     'f'
-    >>> contents = generate({"q":u"Fran\u00e7ais"})
+    >>> contents = generate({'q':u'Fran\u00e7ais'})
     >>> dict = parse(contents)
-    >>> dict["q"]
+    >>> dict['q']
     u'Fran\\xe7ais'
+    >>> dict = parse('a!')
+    Traceback (most recent call last):
+      ...
+    InvalidMapfileContents: Invalid YAML contents
     """
-    return yaml.load(contents) or {}
+    c = yaml.load(contents)
+    if type(c) == types.StringType:
+        raise InvalidMapfileContents(
+            'Unable to parse YAML (BE format missmatch?):\n\n%s' % contents)
+    return c or {}
 
 if libbe.TESTING == True:
     suite = doctest.DocTestSuite()
diff --git a/libbe/storage/util/settings_object.py b/libbe/storage/util/settings_object.py
index 760df03..8b86829 100644
--- a/libbe/storage/util/settings_object.py
+++ b/libbe/storage/util/settings_object.py
@@ -197,9 +197,8 @@ class SavedSettingsObject(object):
         settings as primed.
         """
         for property in self.settings_properties:
-            if property not in self.settings:
-                self.settings[property] = EMPTY
-            elif self.settings[property] == UNPRIMED:
+            if property not in self.settings \
+                    or self.settings[property] == UNPRIMED:
                 self.settings[property] = EMPTY
         if flag_as_loaded == True:
             self._settings_loaded = True
diff --git a/libbe/storage/util/upgrade.py b/libbe/storage/util/upgrade.py
index 7ef760e..c94f171 100644
--- a/libbe/storage/util/upgrade.py
+++ b/libbe/storage/util/upgrade.py
@@ -24,17 +24,16 @@ import sys
 
 import libbe
 import libbe.bug as bug
-import libbe.util.encoding as encoding
 import libbe.storage.util.mapfile as mapfile
+import libbe.util.encoding as encoding
+import libbe.util.id
 
-if libbe.TESTING == True:
-    import doctest
 
 # a list of all past versions
-BUGDIR_DISK_VERSIONS = ["Bugs Everywhere Tree 1 0",
-                        "Bugs Everywhere Directory v1.1",
-                        "Bugs Everywhere Directory v1.2",
-                        "Bugs Everywhere Directory v1.3"]
+BUGDIR_DISK_VERSIONS = ['Bugs Everywhere Tree 1 0',
+                        'Bugs Everywhere Directory v1.1',
+                        'Bugs Everywhere Directory v1.2',
+                        'Bugs Everywhere Directory v1.3']
 
 # the current version
 BUGDIR_DISK_VERSION = BUGDIR_DISK_VERSIONS[-1]
@@ -43,13 +42,17 @@ class Upgrader (object):
     "Class for converting between different on-disk BE storage formats."
     initial_version = None
     final_version = None
-    def __init__(self, root):
-        self.root = root
+    def __init__(self, repo):
+        self.repo = repo
 
-    def get_path(self, *args):
+    def get_path(self, id):
         """
-        Return a path relative to .root.
+        Return a path relative to .repo.
         """
+        if id == 'version':
+            return os.path.join(self.repo, id)
+
+TODO
         dir = os.path.join(self.root, '.be')
         if len(args) == 0:
             return dir
@@ -58,15 +61,15 @@ class Upgrader (object):
 
     def check_initial_version(self):
         path = self.get_path('version')
-        version = self.vcs.get_file_contents(path).rstrip('\n')
+        version = encoding.get_file_contents(path).rstrip('\n')
         assert version == self.initial_version, version
 
     def set_version(self):
-        path = self.get_path("version")
-        self.vcs.set_file_contents(path, self.final_version+"\n")
+        path = self.get_path('version')
+        encoding.set_file_contents(path, self.final_version+'\n')
 
     def upgrade(self):
-        print >> sys.stderr, "upgrading bugdir from '%s' to '%s'" \
+        print >> sys.stderr, 'upgrading bugdir from "%s" to "%s"' \
             % (self.initial_version, self.final_version)
         self.check_initial_version()
         self.set_version()
@@ -237,6 +240,3 @@ def upgrade(path, current_version,
             if version_b == target_version:
                 break
             i += 1
-
-if libbe.TESTING == True:
-    suite = doctest.DocTestSuite()
diff --git a/libbe/storage/vcs/arch.py b/libbe/storage/vcs/arch.py
index 8afdca9..f1b5b7b 100644
--- a/libbe/storage/vcs/arch.py
+++ b/libbe/storage/vcs/arch.py
@@ -167,7 +167,7 @@ class Arch(base.VCS):
           http://regexps.srparish.net/tutorial-tla/naming-conventions.html
         Since our bug directory '.be' doesn't satisfy these conventions,
         we need to adjust them.
-        
+
         The conventions are specified in
           project-root/{arch}/=tagging-method
         """
@@ -211,7 +211,7 @@ class Arch(base.VCS):
             dirname = path
         status,output,error = self._u_invoke_client('tree-root', dirname)
         root = output.rstrip('\n')
-        
+
         self._get_archive_project_name(root)
 
         return root
diff --git a/libbe/storage/vcs/base.py b/libbe/storage/vcs/base.py
index 768a85f..3bdb4ac 100644
--- a/libbe/storage/vcs/base.py
+++ b/libbe/storage/vcs/base.py
@@ -195,6 +195,9 @@ class CachedPathID (object):
                 id = self.id(dirpath)
                 relpath = dirpath[len(self._root)+1:]
                 if id.count('/') == 0:
+                    if id in self._cache:
+                        import sys
+                        print >> sys.stderr, 'Multiple paths for %s: \n  %s\n  %s' % (id, self._cache[id], relpath)
                     self._cache[id] = relpath
             except InvalidPath:
                 pass
@@ -521,7 +524,7 @@ os.listdir(self.get_path("bugs")):
         dumping VCS-specific files into the .be directory.
 
         If you do need to implement this method (e.g. Arch), set
-          self.interspersed_vcs_files = True 
+          self.interspersed_vcs_files = True
         """
         assert self.interspersed_vcs_files == False
         raise NotImplementedError
diff --git a/libbe/storage/vcs/bzr.py b/libbe/storage/vcs/bzr.py
index 04cc6c1..6f3e840 100644
--- a/libbe/storage/vcs/bzr.py
+++ b/libbe/storage/vcs/bzr.py
@@ -49,7 +49,7 @@ class Bzr(base.VCS):
 
     def _vcs_version(self):
         status,output,error = self._u_invoke_client('--version')
-        return output        
+        return output
 
     def _vcs_get_user_id(self):
         status,output,error = self._u_invoke_client('whoami')
@@ -88,7 +88,7 @@ class Bzr(base.VCS):
             return base.VCS._vcs_get_file_contents(self, path, revision)
         else:
             status,output,error = \
-                self._u_invoke_client('cat', '-r', revision,path)
+                self._u_invoke_client('cat', '-r', revision, path)
             return output
 
     def _vcs_commit(self, commitfile, allow_empty=False):
@@ -123,7 +123,7 @@ class Bzr(base.VCS):
             return str(index) # bzr commit 0 is the empty tree.
         return str(current_revision+index+1)
 
-    
+
 if libbe.TESTING == True:
     base.make_vcs_testcase_subclasses(Bzr, sys.modules[__name__])
 
diff --git a/libbe/storage/vcs/darcs.py b/libbe/storage/vcs/darcs.py
index 97e31ff..9a371d9 100644
--- a/libbe/storage/vcs/darcs.py
+++ b/libbe/storage/vcs/darcs.py
@@ -76,7 +76,7 @@ class Darcs(base.VCS):
     def _vcs_detect(self, path):
         if self._u_search_parent_directories(path, "_darcs") != None :
             return True
-        return False 
+        return False
 
     def _vcs_root(self, path):
         """Find the root of the deepest repository containing path."""
@@ -129,7 +129,7 @@ class Darcs(base.VCS):
                     'diff', '--unified', '--patch', revision, path,
                     unicode_output=False)
                 target_patch = output
-                
+
                 # '--output -' to be supported in GNU patch > 2.5.9
                 # but that hasn't been released as of June 30th, 2009.
 
@@ -206,7 +206,7 @@ class Darcs(base.VCS):
         except IndexError:
             return None
 
-    
+
 if libbe.TESTING == True:
     base.make_vcs_testcase_subclasses(Darcs, sys.modules[__name__])
 
diff --git a/libbe/storage/vcs/git.py b/libbe/storage/vcs/git.py
index 29abda7..8d1b529 100644
--- a/libbe/storage/vcs/git.py
+++ b/libbe/storage/vcs/git.py
@@ -76,7 +76,7 @@ class Git(base.VCS):
     def _vcs_detect(self, path):
         if self._u_search_parent_directories(path, '.git') != None :
             return True
-        return False 
+        return False
 
     def _vcs_root(self, path):
         """Find the root of the deepest repository containing path."""
@@ -154,7 +154,7 @@ class Git(base.VCS):
         except IndexError:
             return None
 
-    
+
 if libbe.TESTING == True:
     base.make_vcs_testcase_subclasses(Git, sys.modules[__name__])
 
diff --git a/libbe/storage/vcs/hg.py b/libbe/storage/vcs/hg.py
index 7e0643b..d2d3281 100644
--- a/libbe/storage/vcs/hg.py
+++ b/libbe/storage/vcs/hg.py
@@ -124,7 +124,7 @@ class Hg(base.VCS):
             return id
         return None
 
-    
+
 if libbe.TESTING == True:
     base.make_vcs_testcase_subclasses(Hg, sys.modules[__name__])