Fixed libbe.storage.util.upgrade

Note that it only upgrades on-disk versions, so you can't use a
non-VCS storage backend whose version isn't your command's current
storage version.  See #bea/110/bd1# for reasoning.  To see the on-disk
storage version, look at
  .be/version
To see your command's supported storage version, look at
  be --full-version

I added test_upgrade.sh to exercise the upgrade mechanism on BE's own
repository.

9 files changed, 264 insertions, 78 deletions

diff --git a/.be/bea86499-824e-4e77-b085-2d581fa9ccab/bugs/1100c966-9671-4bc6-8b68-6d408a910da1/comments/bd1207ef-f97e-4078-8c5d-046072012082/body b/.be/bea86499-824e-4e77-b085-2d581fa9ccab/bugs/1100c966-9671-4bc6-8b68-6d408a910da1/comments/bd1207ef-f97e-4078-8c5d-046072012082/body
new file mode 100644
index 0000000..21170a2
--- /dev/null
+++ b/.be/bea86499-824e-4e77-b085-2d581fa9ccab/bugs/1100c966-9671-4bc6-8b68-6d408a910da1/comments/bd1207ef-f97e-4078-8c5d-046072012082/body
@@ -0,0 +1,45 @@
+Some additional thoughts, as I've been developing this idea:
+
+Different BE storage versions will be difficult to handle.
+We currently do disk upgrades via
+  libbe.storage.util.upgrade
+which browses through the .be/ directory, making appropriate changes.
+
+The new formats know very little about paths, which brought on the
+whole libbe.storage.vcs.base.CachedPathID bit.  Still, most VCSs
+seem to be able to handle renames, e.g.
+  $ bzr cat -r 200 ./libbe/command/new.py
+works, when as of revision 200, the file was
+  ./becommands/new.py
+In fact, bzr recognizes both names:
+  $ diff <(bzr cat -r 200 ./becommands/new.py) \
+         <(bzr cat -r 200 ./libbe/commands/new.py)
+returns nothing.  Still, I'm not sure this is something we should
+require in a storage backend.  Which means we'd need to have a
+version-dependent id-to-path(version) function.
+
+We also have the unfortunate situation of duplicate UUIDs from the old
+  be merge
+implemtation.  This means that id-to-path is not a well defined
+mapping with single-uuid ids.  That's ok though, we get a bit uglier
+and send the long_user() id into the storage backend instead.  While
+not so elegant, this will avoid the need for the cached id/path table.
+
+Ok, you say, we're fine if we have the compound bugdir/bug/comment ids
+going out to storage, with the upgrader upgrading the file
+appropriately for each file type.  Almost.  You'll still run into
+trouble with upgrades like dir format v1.2 to 1.3 where targets
+moved from a per-bug string to a seperate-bugs-with-dependencies.
+Now you need to create virtual-target-bugs on the fly when you're
+loading the old bugs.  Yuck.
+
+All of this makes me wonder how much we care about being able to
+see bug diffs for any repository format older than the current one.
+I think that we don't really care ;).  After all, the on-disk
+format should settle down as BE matures :p.  When you _do_ want
+to see the long-term history of a particular bug, there's always
+  bzr log .be/123/bugs/456/values
+or the equivalent for your VCS.  If access to the raw log ends
+up being important, it should be very easy to add
+  libbe.storage.base.VersionedStorage.log(id)
+  libbe.command.log
diff --git a/.be/bea86499-824e-4e77-b085-2d581fa9ccab/bugs/1100c966-9671-4bc6-8b68-6d408a910da1/comments/bd1207ef-f97e-4078-8c5d-046072012082/values b/.be/bea86499-824e-4e77-b085-2d581fa9ccab/bugs/1100c966-9671-4bc6-8b68-6d408a910da1/comments/bd1207ef-f97e-4078-8c5d-046072012082/values
new file mode 100644
index 0000000..f0af48d
--- /dev/null
+++ b/.be/bea86499-824e-4e77-b085-2d581fa9ccab/bugs/1100c966-9671-4bc6-8b68-6d408a910da1/comments/bd1207ef-f97e-4078-8c5d-046072012082/values
@@ -0,0 +1,11 @@
+Author: W. Trevor King <wking@drexel.edu>
+
+
+Content-type: text/plain
+
+
+Date: Tue, 15 Dec 2009 12:21:11 +0000
+
+
+In-reply-to: bb406a33-92b6-46dd-950c-c7cfb5440e7b
+
diff --git a/.be/version b/.be/version
index 29baa0e..e7aade4 100644
--- a/.be/version
+++ b/.be/version
@@ -1 +1 @@
-Bugs Everywhere Directory v1.3
+Bugs Everywhere Directory v1.4
diff --git a/libbe/bug.py b/libbe/bug.py
index 6ab4d78..1186ad4 100644
--- a/libbe/bug.py
+++ b/libbe/bug.py
@@ -628,7 +628,7 @@ class Bug(settings_object.SavedSettingsObject):
     def load_settings(self, settings_mapfile=None):
         if settings_mapfile == None:
             settings_mapfile = \
-                self.storage.get(self.id.storage("values"), default="\n")
+                self.storage.get(self.id.storage('values'), default='\n')
         try:
             self.settings = mapfile.parse(settings_mapfile)
         except mapfile.InvalidMapfileContents, e:
@@ -638,7 +638,7 @@ class Bug(settings_object.SavedSettingsObject):
 
     def save_settings(self):
         mf = mapfile.generate(self._get_saved_settings())
-        self.storage.set(self.id.storage("values"), mf)
+        self.storage.set(self.id.storage('values'), mf)
 
     def save(self):
         """
diff --git a/libbe/storage/util/upgrade.py b/libbe/storage/util/upgrade.py
index c94f171..ce6831d 100644
--- a/libbe/storage/util/upgrade.py
+++ b/libbe/storage/util/upgrade.py
@@ -23,8 +23,11 @@ import os, os.path
 import sys
 
 import libbe
-import libbe.bug as bug
+import libbe.bug
 import libbe.storage.util.mapfile as mapfile
+#import libbe.storage.vcs # delay import to avoid cyclic dependency
+import libbe.ui.util.editor
+import libbe.util
 import libbe.util.encoding as encoding
 import libbe.util.id
 
@@ -33,7 +36,9 @@ import libbe.util.id
 BUGDIR_DISK_VERSIONS = ['Bugs Everywhere Tree 1 0',
                         'Bugs Everywhere Directory v1.1',
                         'Bugs Everywhere Directory v1.2',
-                        'Bugs Everywhere Directory v1.3']
+                        'Bugs Everywhere Directory v1.3',
+                        'Bugs Everywhere Directory v1.4',
+                        ]
 
 # the current version
 BUGDIR_DISK_VERSION = BUGDIR_DISK_VERSIONS[-1]
@@ -43,30 +48,37 @@ class Upgrader (object):
     initial_version = None
     final_version = None
     def __init__(self, repo):
+        import libbe.storage.vcs
+
         self.repo = repo
+        vcs_name = self._get_vcs_name()
+        if vcs_name == None:
+            vcs_name = 'None'
+        self.vcs = libbe.storage.vcs.vcs_by_name(vcs_name)
+        self.vcs.repo = self.repo
+        self.vcs.root()
 
-    def get_path(self, id):
+    def get_path(self, *args):
         """
-        Return a path relative to .repo.
+        Return the absolute path using args relative to .be.
         """
-        if id == 'version':
-            return os.path.join(self.repo, id)
-
-TODO
-        dir = os.path.join(self.root, '.be')
+        dir = os.path.join(self.repo, '.be')
         if len(args) == 0:
             return dir
-        assert args[0] in ['version', 'settings', 'bugs'], str(args)
         return os.path.join(dir, *args)
 
+    def _get_vcs_name(self):
+        return None
+
     def check_initial_version(self):
         path = self.get_path('version')
-        version = encoding.get_file_contents(path).rstrip('\n')
-        assert version == self.initial_version, version
+        version = encoding.get_file_contents(path, decode=True).rstrip('\n')
+        assert version == self.initial_version, '%s: %s' % (path, version)
 
     def set_version(self):
         path = self.get_path('version')
         encoding.set_file_contents(path, self.final_version+'\n')
+        self.vcs._vcs_update(path)
 
     def upgrade(self):
         print >> sys.stderr, 'upgrading bugdir from "%s" to "%s"' \
@@ -82,11 +94,20 @@ TODO
 class Upgrade_1_0_to_1_1 (Upgrader):
     initial_version = "Bugs Everywhere Tree 1 0"
     final_version = "Bugs Everywhere Directory v1.1"
+    def _get_vcs_name(self):
+        path = self.get_path('settings')
+        settings = encoding.get_file_contents(path)
+        for line in settings.splitlines(False):
+            fields = line.split('=')
+            if len(fields) == 2 and fields[0] == 'rcs_name':
+                return fields[1]
+        return None
+            
     def _upgrade_mapfile(self, path):
-        contents = self.vcs.get_file_contents(path)
+        contents = encoding.get_file_contents(path, decode=True)
         old_format = False
         for line in contents.splitlines():
-            if len(line.split("=")) == 2:
+            if len(line.split('=')) == 2:
                 old_format = True
                 break
         if old_format == True:
@@ -105,43 +126,56 @@ class Upgrade_1_0_to_1_1 (Upgrader):
             contents = '\n'.join(newlines)
             # load the YAML and save
             map = mapfile.parse(contents)
-            mapfile.map_save(self.vcs, path, map)
+            contents = mapfile.generate(map)
+            encoding.set_file_contents(path, contents)
+            self.vcs._vcs_update(path)
 
     def _upgrade(self):
         """
         Comment value field "From" -> "Author".
         Homegrown mapfile -> YAML.
         """
-        path = self.get_path("settings")
+        path = self.get_path('settings')
         self._upgrade_mapfile(path)
-        for bug_uuid in os.listdir(self.get_path("bugs")):
-            path = self.get_path("bugs", bug_uuid, "values")
+        for bug_uuid in os.listdir(self.get_path('bugs')):
+            path = self.get_path('bugs', bug_uuid, 'values')
             self._upgrade_mapfile(path)
-            c_path = ["bugs", bug_uuid, "comments"]
+            c_path = ['bugs', bug_uuid, 'comments']
             if not os.path.exists(self.get_path(*c_path)):
                 continue # no comments for this bug
             for comment_uuid in os.listdir(self.get_path(*c_path)):
-                path_list = c_path + [comment_uuid, "values"]
+                path_list = c_path + [comment_uuid, 'values']
                 path = self.get_path(*path_list)
                 self._upgrade_mapfile(path)
-                settings = mapfile.map_load(self.vcs, path)
-                if "From" in settings:
-                    settings["Author"] = settings.pop("From")
-                    mapfile.map_save(self.vcs, path, settings)
+                settings = mapfile.parse(
+                    encoding.get_file_contents(path))
+                if 'From' in settings:
+                    settings['Author'] = settings.pop('From')
+                    encoding.set_file_contents(
+                        path, mapfile.generate(settings))
+                    self.vcs._vcs_update(path)
 
 
 class Upgrade_1_1_to_1_2 (Upgrader):
     initial_version = "Bugs Everywhere Directory v1.1"
     final_version = "Bugs Everywhere Directory v1.2"
+    def _get_vcs_name(self):
+        path = self.get_path('settings')
+        settings = mapfile.parse(encoding.get_file_contents(path))
+        if 'rcs_name' in settings:
+            return settings['rcs_name']
+        return None
+            
     def _upgrade(self):
         """
         BugDir settings field "rcs_name" -> "vcs_name".
         """
-        path = self.get_path("settings")
-        settings = mapfile.map_load(self.vcs, path)
-        if "rcs_name" in settings:
-            settings["vcs_name"] = settings.pop("rcs_name")
-            mapfile.map_save(self.vcs, path, settings)
+        path = self.get_path('settings')
+        settings = mapfile.parse(encoding.get_file_contents(path))
+        if 'rcs_name' in settings:
+            settings['vcs_name'] = settings.pop('rcs_name')
+            encoding.set_file_contents(path, mapfile.generate(settings))
+            self.vcs._vcs_update(path)
 
 class Upgrade_1_2_to_1_3 (Upgrader):
     initial_version = "Bugs Everywhere Directory v1.2"
@@ -149,42 +183,64 @@ class Upgrade_1_2_to_1_3 (Upgrader):
     def __init__(self, *args, **kwargs):
         Upgrader.__init__(self, *args, **kwargs)
         self._targets = {} # key: target text,value: new target bug
+
+    def _get_vcs_name(self):
         path = self.get_path('settings')
-        settings = mapfile.map_load(self.vcs, path)
+        settings = mapfile.parse(encoding.get_file_contents(path))
         if 'vcs_name' in settings:
-            old_vcs = self.vcs
-            self.vcs = vcs.vcs_by_name(settings['vcs_name'])
-            self.vcs.root(self.root)
-            self.vcs.encoding = old_vcs.encoding
+            return settings['vcs_name']
+        return None
+
+    def _save_bug_settings(self, bug):
+        # The target bugs don't have comments
+        path = self.get_path('bugs', bug.uuid, 'values')
+        if not os.path.exists(path):
+            self.vcs._add_path(path, directory=False)
+        path = self.get_path('bugs', bug.uuid, 'values')
+        mf = mapfile.generate(bug._get_saved_settings())
+        encoding.set_file_contents(path, mf)
+        self.vcs._vcs_update(path)
 
     def _target_bug(self, target_text):
         if target_text not in self._targets:
-            _bug = bug.Bug(bugdir=self, summary=target_text)
-            # note: we're not a bugdir, but all Bug.save() needs is
-            # .root, .vcs, and .get_path(), which we have.
-            _bug.severity = 'target'
-            self._targets[target_text] = _bug
+            bug = libbe.bug.Bug(summary=target_text)
+            bug.severity = 'target'
+            self._targets[target_text] = bug
         return self._targets[target_text]
 
     def _upgrade_bugdir_mapfile(self):
         path = self.get_path('settings')
-        settings = mapfile.map_load(self.vcs, path)
+        mf = encoding.get_file_contents(path)
+        if mf == libbe.util.InvalidObject:
+            return # settings file does not exist
+        settings = mapfile.parse(mf)
         if 'target' in settings:
             settings['target'] = self._target_bug(settings['target']).uuid
-            mapfile.map_save(self.vcs, path, settings)
+            mf = mapfile.generate(settings)
+            encoding.set_file_contents(path, mf)
+            self.vcs._vcs_update(path)
 
     def _upgrade_bug_mapfile(self, bug_uuid):
-        import becommands.depend
+        import libbe.command.depend as dep
         path = self.get_path('bugs', bug_uuid, 'values')
-        settings = mapfile.map_load(self.vcs, path)
+        mf = encoding.get_file_contents(path)
+        if mf == libbe.util.InvalidObject:
+            return # settings file does not exist
+        settings = mapfile.parse(mf)
         if 'target' in settings:
             target_bug = self._target_bug(settings['target'])
-            _bug = bug.Bug(bugdir=self, uuid=bug_uuid, from_disk=True)
-            # note: we're not a bugdir, but all Bug.load_settings()
-            # needs is .root, .vcs, and .get_path(), which we have.
-            becommands.depend.add_block(target_bug, _bug)
-            _bug.settings.pop('target')
-            _bug.save()
+
+            blocked_by_string = '%s%s' % (dep.BLOCKED_BY_TAG, bug_uuid)
+            dep._add_remove_extra_string(target_bug, blocked_by_string, add=True)
+            blocks_string = dep._generate_blocks_string(target_bug)
+            estrs = settings.get('extra_strings', [])
+            estrs.append(blocks_string)
+            settings['extra_strings'] = sorted(estrs)
+
+            settings.pop('target')
+            mf = mapfile.generate(settings)
+            encoding.set_file_contents(path, mf)
+            self.vcs._vcs_update(path)
 
     def _upgrade(self):
         """
@@ -194,12 +250,55 @@ class Upgrade_1_2_to_1_3 (Upgrader):
         for bug_uuid in os.listdir(self.get_path('bugs')):
             self._upgrade_bug_mapfile(bug_uuid)
         self._upgrade_bugdir_mapfile()
-        for _bug in self._targets.values():
-            _bug.save()
+        for bug in self._targets.values():
+            self._save_bug_settings(bug)
+
+class Upgrade_1_3_to_1_4 (Upgrader):
+    initial_version = "Bugs Everywhere Directory v1.3"
+    final_version = "Bugs Everywhere Directory v1.4"
+    def _get_vcs_name(self):
+        path = self.get_path('settings')
+        settings = mapfile.parse(encoding.get_file_contents(path))
+        if 'vcs_name' in settings:
+            return settings['vcs_name']
+        return None
+
+    def _upgrade(self):
+        """
+        add new directory "./be/BUGDIR-UUID"
+        "./be/bugs" -> "./be/BUGDIR-UUID/bugs"
+        "./be/settings" -> "./be/BUGDIR-UUID/settings"
+        """
+        self.repo = os.path.abspath(self.repo)
+        basenames = [p for p in os.listdir(self.get_path())]
+        if not 'bugs' in basenames and not 'settings' in basenames \
+                and len([p for p in basenames if len(p)==36]) == 1:
+            return # the user has upgraded the directory.
+        basenames = [p for p in basenames if p in ['bugs','settings']]
+        uuid = libbe.util.id.uuid_gen()
+        add = [self.get_path(uuid)]
+        move = [(self.get_path(p), self.get_path(uuid, p)) for p in basenames]
+        msg = ['Upgrading BE directory version v1.3 to v1.4',
+               '',
+               "Because BE's VCS drivers don't support 'move',",
+               'please make the following changes with your VCS',
+               'and re-run BE.  Note that you can choose a different',
+               'bugdir UUID to preserve uniformity across branches',
+               'of a distributed repository.'
+               '',
+               'add',
+               '  ' + '\n  '.join(add),
+               'move',
+               '  ' + '\n  '.join(['%s %s' % (a,b) for a,b in move]),
+               ]
+        self.vcs._cached_path_id.destroy()
+        raise Exception('Need user assistance\n%s' % '\n'.join(msg))
+
 
 upgraders = [Upgrade_1_0_to_1_1,
              Upgrade_1_1_to_1_2,
-             Upgrade_1_2_to_1_3]
+             Upgrade_1_2_to_1_3,
+             Upgrade_1_3_to_1_4]
 upgrade_classes = {}
 for upgrader in upgraders:
     upgrade_classes[(upgrader.initial_version,upgrader.final_version)]=upgrader
@@ -213,10 +312,10 @@ def upgrade(path, current_version,
     """
     if current_version not in BUGDIR_DISK_VERSIONS:
         raise NotImplementedError, \
-            "Cannot handle version '%s' yet." % version
+            "Cannot handle version '%s' yet." % current_version
     if target_version not in BUGDIR_DISK_VERSIONS:
         raise NotImplementedError, \
-            "Cannot handle version '%s' yet." % version
+            "Cannot handle version '%s' yet." % current_version
 
     if (current_version, target_version) in upgrade_classes:
         # direct conversion
diff --git a/libbe/storage/vcs/base.py b/libbe/storage/vcs/base.py
index 3bdb4ac..a45f1fe 100644
--- a/libbe/storage/vcs/base.py
+++ b/libbe/storage/vcs/base.py
@@ -40,7 +40,7 @@ from libbe.storage.base import EmptyCommit, InvalidRevision
 from libbe.util.utility import Dir, search_parent_directories
 from libbe.util.subproc import CommandError, invoke
 from libbe.util.plugin import import_by_name
-#import libbe.storage.util.upgrade as upgrade
+import libbe.storage.util.upgrade as upgrade
 
 if libbe.TESTING == True:
     import unittest
@@ -657,8 +657,7 @@ os.listdir(self.get_path("bugs")):
     def disconnect(self):
         self._cached_path_id.disconnect()
 
-    def _add(self, id, parent=None, directory=False):
-        path = self._cached_path_id.add_id(id, parent)
+    def _add_path(self, path, directory=False):
         relpath = self._u_rel_path(path)
         reldirs = relpath.split(os.path.sep)
         if directory == False:
@@ -676,6 +675,10 @@ os.listdir(self.get_path("bugs")):
                 open(path, 'w').close()
             self._vcs_add(self._u_rel_path(path))
 
+    def _add(self, id, parent=None, **kwargs):
+        path = self._cached_path_id.add_id(id, parent)
+        self._add_path(path, **kwargs)
+
     def _remove(self, id):
         path = self._cached_path_id.path(id)
         if os.path.exists(path):
@@ -877,27 +880,17 @@ os.listdir(self.get_path("bugs")):
         return (summary, body)
 
     def check_disk_version(self):
-        version = self.version()
-        #if version != upgrade.BUGDIR_DISK_VERSION:
-        #    upgrade.upgrade(self.repo, version)
+        version = self.disk_version()
+        if version != upgrade.BUGDIR_DISK_VERSION:
+            upgrade.upgrade(self.repo, version)
 
     def disk_version(self, path=None):
         """
         Requires disk access.
         """
         if path == None:
-            path = self.get_path('version')
-        return self.get(path).rstrip('\n')
-
-    def set_disk_version(self):
-        """
-        Requires disk access.
-        """
-        if self.sync_with_disk == False:
-            raise DiskAccessRequired('set version')
-        self.vcs.mkdir(self.get_path())
-        #self.vcs.set_file_contents(self.get_path("version"),
-        #                           upgrade.BUGDIR_DISK_VERSION+"\n")
+            path = os.path.join(self.repo, '.be', 'version')
+        return libbe.util.encoding.get_file_contents(path).rstrip('\n')
 
 
 
diff --git a/libbe/util/id.py b/libbe/util/id.py
index adc827c..6b6b51d 100644
--- a/libbe/util/id.py
+++ b/libbe/util/id.py
@@ -172,7 +172,6 @@ class ID (object):
         assert self._type in HIERARCHY, self._type
 
     def storage(self, *args):
-        import libbe.comment
         return _assemble(self._object.uuid, *args)
 
     def _ancestors(self):
diff --git a/libbe/version.py b/libbe/version.py
index f8eebbd..1214b3e 100644
--- a/libbe/version.py
+++ b/libbe/version.py
@@ -23,7 +23,10 @@ be bothered setting version strings" and the "I want complete control
 over the version strings" workflows.
 """
 
+import copy
+
 import libbe._version as _version
+import libbe.storage.util.upgrade as upgrade
 
 # Manually set a version string (optional, defaults to bzr revision id)
 #_VERSION = "1.2.3"
@@ -39,11 +42,14 @@ def version(verbose=False):
     else:
         string = _version.version_info["revision_id"]
     if verbose == True:
+        info = copy.copy(_version.version_info)
+        info['storage'] = upgrade.BUGDIR_DISK_VERSION
         string += ("\n"
                    "revision: %(revno)d\n"
                    "nick: %(branch_nick)s\n"
-                   "revision id: %(revision_id)s"
-                   % _version.version_info)
+                   "revision id: %(revision_id)s\n"
+                   "storage version: %(storage)s"
+                   % info)
     return string
 
 if __name__ == "__main__":
diff --git a/test_upgrade.py b/test_upgrade.py
new file mode 100755
index 0000000..40db42a
--- /dev/null
+++ b/test_upgrade.py
@@ -0,0 +1,33 @@
+#!/bin/bash
+#
+# Test upgrade functionality by checking out revisions with the
+# various initial on-disk versions and running `be list` on them to
+# force an auto-upgrade.
+#
+# usage: test_upgrade.sh
+
+REVS='revid:wking@drexel.edu-20090831063121-85p59rpwoi1mzk3i
+revid:wking@drexel.edu-20090831171945-73z3wwt4lrm7zbmu
+revid:wking@drexel.edu-20091205224008-z4fed13sd80bj4fe
+revid:wking@drexel.edu-20091207123614-okq7i0ahciaupuy9'
+
+ROOT=$(bzr root)
+BE="$ROOT/be"
+cd "$ROOT"
+
+echo "$REVS" | while read REV; do
+    TMPDIR=$(mktemp --directory --tmpdir "BE-upgrade.XXXXXXXXXX")
+    REPO="$TMPDIR/repo"
+    echo "Testing revision: $REV"
+    echo "  Test directory: $REPO"
+    bzr checkout --lightweight --revision="$REV" "$ROOT" "$TMPDIR/repo"
+    VERSION=$(cat "$REPO/.be/version")
+    echo "  Version: $VERSION"
+    $BE --repo "$REPO" list > /dev/null
+    RET="$?"
+    rm -rf "$TMPDIR"
+    if [ $RET -ne 0 ]; then
+	echo "Error! ($RET)"
+	exit $RET
+    fi
+done