Rev 310: Optimizations, split out tests. in http://people.samba.org/bzr/jelmer/bzr-svn/bzr.dev

Jelmer Vernooij jelmer at samba.org
Tue Dec 26 22:05:29 GMT 2006


------------------------------------------------------------
revno: 310
revision-id: jelmer at samba.org-20061226220429-85up7e6w2qw90pwb
parent: jelmer at samba.org-20061226210852-oo9vf8m883pb5wka
committer: Jelmer Vernooij <jelmer at samba.org>
branch nick: main
timestamp: Tue 2006-12-26 23:04:29 +0100
message:
  Optimizations, split out tests.
modified:
  logwalker.py                   logwalker.py-20060621215743-c13fhfnyzh1xzwh2-1
  repository.py                  repository.py-20060306123302-1f8c5069b3fe0265
  tests/test_logwalker.py        test_logwalker.py-20060622141944-pkocc3rj8g62ukbi-1
  tests/test_scheme.py           test_scheme.py-20060621221855-va2xabhlxpmc9llx-1
=== modified file 'logwalker.py'
--- a/logwalker.py	2006-12-26 21:08:52 +0000
+++ b/logwalker.py	2006-12-26 22:04:29 +0000
@@ -143,55 +143,44 @@
         self.db.commit()
         pool.destroy()
 
-    def follow_history(self, branch_path, revnum):
+    def follow_path(self, path, revnum):
         """Return iterator over all the revisions between revnum and 
-        0 names branch_path or inside branch_path.
+        0 named path or inside path.
 
-        If branch_path is None, all changes will be reported.
-        
-        :param branch_path:   Branch path to start reporting (in revnum)
+        :param path:   Branch path to start reporting (in revnum)
         :param revnum:        Start revision.
 
-        :return: An iterators that yields tuples with (branch_path, paths, revnum)
-        where paths is a dictionary with all changes that happened in branch_path 
+        :return: An iterators that yields tuples with (path, paths, revnum)
+        where paths is a dictionary with all changes that happened in path 
         in revnum.
         """
         assert revnum >= 0
 
-        if revnum == 0 and branch_path in (None, ""):
+        if revnum == 0 and path == "":
             return
 
-        if branch_path:
-            branch_path = branch_path.strip("/")
-
-        continue_revnum = None
-        for i in range(revnum+1):
-            i = revnum - i
-
-            if i == 0:
-                continue
-
-            if not (continue_revnum is None or continue_revnum == i):
-                continue
-
-            continue_revnum = None
-
-            revpaths = self.get_revision_paths(i, branch_path)
-            yield (branch_path, revpaths, i)
-
-            if (not branch_path is None and 
-                revpaths.has_key(branch_path) and
-                revpaths[branch_path][0] in ('A', 'R') and
-                revpaths[branch_path][1] is None):
+        path = path.strip("/")
+
+        i = revnum
+        while i > 0:
+            revpaths = self.get_revision_paths(i, path)
+            yield (path, revpaths, i)
+
+            if (revpaths.has_key(path) and
+                revpaths[path][0] in ('A', 'R') and
+                revpaths[path][1] is None):
+               # this path didn't exist before this revision
                return
 
-            if (not branch_path is None and 
-                branch_path in revpaths and 
-                not revpaths[branch_path][1] is None):
-                # In this revision, this branch was copied from 
+            if (not path is None and 
+                path in revpaths and 
+                not revpaths[path][1] is None):
+                # In this revision, this path was copied from 
                 # somewhere else
-                continue_revnum = revpaths[branch_path][2]
-                branch_path = revpaths[branch_path][1]
+                i = revpaths[path][2]
+                path = revpaths[path][1]
+            else:
+                i-=1
 
     def get_revision_paths(self, revnum, path=None):
         """Obtain dictionary with all the changes in a particular revision.
@@ -201,6 +190,9 @@
         :returns: dictionary with paths as keys and 
                   (action, copyfrom_path, copyfrom_rev) as values.
         """
+
+        if revnum == 0:
+            return {'': ('A', None, -1)}
                 
         if revnum > self.saved_revnum:
             self.fetch_revisions(revnum)
@@ -209,8 +201,6 @@
         if path is not None and path != "":
             query += " and (path='%s' or path like '%s/%%')" % (path, path)
 
-        mutter('query: %r' % query)
-
         paths = {}
         for p, act, cf, cr in self.db.execute(query):
             paths[p] = (act, cf, cr)
@@ -222,6 +212,7 @@
         :param revnum: Revision number.
         :returns: Tuple with author, log message and date of the revision.
         """
+        assert revnum >= 1
         if revnum > self.saved_revnum:
             self.fetch_revisions(revnum, pb)
         (author, message, date) = self.db.execute("select author, message, date from revision where revno="+ str(revnum)).fetchone()
@@ -288,6 +279,7 @@
         :param path:  Path to check
         :param revnum:  Revision to check
         """
+        assert revnum >= 0
         if revnum > self.saved_revnum:
             self.fetch_revisions(revnum)
         if revnum == 0:

=== modified file 'repository.py'
--- a/repository.py	2006-12-26 21:08:52 +0000
+++ b/repository.py	2006-12-26 22:04:29 +0000
@@ -500,7 +500,8 @@
             yield (bp, rev)
 
     def follow_history(self, revnum):
-        for (branch_path, paths, revnum) in self._log.follow_history(None, revnum):
+        while revnum > 0:
+            paths = self._log.get_revision_paths(revnum)
             changed_paths = {}
             for p in paths:
                 try:
@@ -517,7 +518,7 @@
                     not self.scheme.is_branch(changed_paths[bp][bp][1])):
                     # FIXME: if copyfrom_path is not a branch path, 
                     # should simulate a reverse "split" of a branch
-                    # for now, just make it look like the branch ended here
+                    # For now, just make it look like the branch originated here.
                     for c in self._log.find_children(changed_paths[bp][bp][1], changed_paths[bp][bp][2]):
                         path = c.replace(changed_paths[bp][bp][1], bp+"/", 1).replace("//", "/")
                         changed_paths[bp][path] = ('A', None, -1)
@@ -525,33 +526,29 @@
 
                 yield (bp, changed_paths[bp], revnum)
 
+            revnum-=1
+
     def follow_branch_history(self, branch_path, revnum):
         assert branch_path is not None
         if not self.scheme.is_branch(branch_path):
             raise errors.NotSvnBranchPath(branch_path, revnum)
 
-        for (bp, paths, revnum) in self._log.follow_history(branch_path, revnum):
-            changed_paths = {}
-            for p in paths:
-                (bp_, _) = self.scheme.unprefix(p)
-                assert bp_ == bp
-                changed_paths[p] = paths[p]
-
-            if (changed_paths.has_key(bp) and 
-                changed_paths[bp][1] is not None and
-                not self.scheme.is_branch(changed_paths[bp][1])):
+        for (bp, paths, revnum) in self._log.follow_path(branch_path, revnum):
+            if (paths.has_key(bp) and 
+                paths[bp][1] is not None and
+                not self.scheme.is_branch(paths[bp][1])):
                 # FIXME: if copyfrom_path is not a branch path, 
                 # should simulate a reverse "split" of a branch
                 # for now, just make it look like the branch ended here
-                for c in self._log.find_children(changed_paths[bp][1], changed_paths[bp][2]):
-                    path = c.replace(changed_paths[bp][1], bp+"/", 1).replace("//", "/")
-                    changed_paths[path] = ('A', None, -1)
-                changed_paths[bp] = ('A', None, -1)
+                for c in self._log.find_children(paths[bp][1], paths[bp][2]):
+                    path = c.replace(paths[bp][1], bp+"/", 1).replace("//", "/")
+                    paths[path] = ('A', None, -1)
+                paths[bp] = ('A', None, -1)
 
-                yield (bp, changed_paths, revnum)
+                yield (bp, paths, revnum)
                 return
                      
-            yield (bp, changed_paths, revnum)
+            yield (bp, paths, revnum)
 
     def has_signature_for_revision_id(self, revision_id):
         # TODO: Retrieve from SVN_PROP_BZR_SIGNATURE 
@@ -592,10 +589,7 @@
         created_branches = {}
 
         for i in range(revnum+1):
-            if i == 0:
-                paths = {'': ('A', None, None)}
-            else:
-                paths = self._log.get_revision_paths(i)
+            paths = self._log.get_revision_paths(i)
             for p in paths:
                 if self.scheme.is_branch(p):
                     if paths[p][0] in ('R', 'D'):

=== modified file 'tests/test_logwalker.py'
--- a/tests/test_logwalker.py	2006-12-26 17:00:19 +0000
+++ b/tests/test_logwalker.py	2006-12-26 22:04:29 +0000
@@ -42,13 +42,28 @@
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(1, len(list(walker.follow_history("", 1))))
+        self.assertEqual(1, len(list(walker.follow_path("", 1))))
+
+    def test_get_revision_paths(self):
+        repos_url = self.make_client("a", "dc")
+        self.build_tree({'dc/foo': "data"})
+        self.client_add("dc/foo")
+        self.client_commit("dc", "My Message")
+        walker = logwalker.LogWalker(SvnRaTransport(repos_url))
+        self.assertEqual({"foo": ('A', None, -1)}, walker.get_revision_paths(1))
+        self.assertEqual({"foo": ('A', None, -1)}, walker.get_revision_paths(1, "foo"))
+        self.assertEqual({"": ('A', None, -1)}, walker.get_revision_paths(0, "foo"))
+
+    def test_get_revision_paths_zero(self):
+        repos_url = self.make_client("a", "dc")
+        walker = logwalker.LogWalker(SvnRaTransport(repos_url))
+        self.assertEqual({'': ('A', None, -1)}, walker.get_revision_paths(0))
 
     def test_get_branch_invalid_revision(self):
         repos_url = self.make_client("a", "dc")
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
         self.assertRaises(NoSuchRevision, list, 
-                          walker.follow_history("/", 20))
+                          walker.follow_path("/", 20))
 
 
     def test_branch_log_all(self):
@@ -60,7 +75,7 @@
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(1, len(list(walker.follow_history(None, 1))))
+        self.assertEqual(1, len(list(walker.follow_path("", 1))))
 
     def test_branch_log_specific(self):
         repos_url = self.make_client("a", "dc")
@@ -75,7 +90,7 @@
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(1, len(list(walker.follow_history("branches/brancha",
+        self.assertEqual(1, len(list(walker.follow_path("branches/brancha",
             1))))
 
     def test_find_latest_none(self):
@@ -173,7 +188,7 @@
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
         self.assertEqual([("trunk", {"trunk/data": ('A', None, -1),
                                      "trunk": ('A', None, -1)}, 3)], 
-                list(walker.follow_history("trunk", 3)))
+                list(walker.follow_path("trunk", 3)))
 
     def test_follow_history(self):
         repos_url = self.make_client("a", "dc")
@@ -183,7 +198,7 @@
         self.client_add("dc/foo")
         self.client_commit("dc", "My Message")
 
-        for (branch, paths, rev) in walker.follow_history("", 1):
+        for (branch, paths, rev) in walker.follow_path("", 1):
            self.assertEqual(branch, "")
            self.assertTrue(paths.has_key("foo"))
            self.assertEqual(rev, 1)
@@ -192,7 +207,7 @@
         repos_url = self.make_client("a", "dc")
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual([], list(walker.follow_history("", 0)))
+        self.assertEqual([], list(walker.follow_path("", 0)))
 
     def test_later_update(self):
         repos_url = self.make_client("a", "dc")
@@ -203,12 +218,12 @@
         self.client_add("dc/foo")
         self.client_commit("dc", "My Message")
 
-        for (branch, paths, rev) in walker.follow_history("", 1):
+        for (branch, paths, rev) in walker.follow_path("", 1):
            self.assertEqual(branch, "")
            self.assertTrue(paths.has_key("foo"))
            self.assertEqual(rev, 1)
 
-        iter = walker.follow_history("", 2)
+        iter = walker.follow_path("", 2)
         self.assertRaises(NoSuchRevision, list, iter)
 
     def test_get_branch_log_follow(self):
@@ -223,7 +238,7 @@
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        items = list(walker.follow_history("branches/abranch", 2))
+        items = list(walker.follow_path("branches/abranch", 2))
         self.assertEqual(2, len(items))
 
     def test_touches_path(self):

=== modified file 'tests/test_scheme.py'
--- a/tests/test_scheme.py	2006-12-26 00:43:54 +0000
+++ b/tests/test_scheme.py	2006-12-26 22:04:29 +0000
@@ -77,55 +77,111 @@
 
     def test_unprefix(self):
         self.assertEqual(NoBranchingScheme().unprefix(""), ("", ""))
+
+    def test_unprefix_slash(self):
         self.assertEqual(NoBranchingScheme().unprefix("/"), ("", ""))
+
+    def test_unprefix_nested(self):
         self.assertEqual(NoBranchingScheme().unprefix("foo/foo"), ("", "foo/foo"))
+
+    def test_unprefix_slash_nested(self):
         self.assertEqual(NoBranchingScheme().unprefix("/foo/foo"), ("", "foo/foo"))
 
 class ListScheme(TestCase):
-    def test_is_branch(self):
-        scheme = ListBranchingScheme(["foo", "bar/bloe"])
-        self.assertFalse(scheme.is_branch(""))
-        self.assertFalse(scheme.is_branch("/"))
-        self.assertTrue(scheme.is_branch("/foo"))
-        self.assertTrue(scheme.is_branch("foo"))
-        self.assertFalse(scheme.is_branch("/foo/foo"))
-        self.assertFalse(scheme.is_branch("foo/bar"))
-        self.assertFalse(scheme.is_branch("foobla"))
-        self.assertTrue(scheme.is_branch("//foo/"))
-        self.assertTrue(scheme.is_branch("bar/bloe"))
-
-    def test_unprefix(self):
-        scheme = ListBranchingScheme(["foo", "bar/bloe"])
-        self.assertRaises(NotBranchError, scheme.unprefix, "")
-        self.assertRaises(NotBranchError, scheme.unprefix, "/")
-        self.assertRaises(NotBranchError, scheme.unprefix, "blie/bloe/bla")
-        self.assertEqual(scheme.unprefix("/foo"), ("foo", ""))
-        self.assertEqual(scheme.unprefix("foo"), ("foo", ""))
-        self.assertEqual(scheme.unprefix("/foo/foo"), ("foo", "foo"))
-        self.assertEqual(scheme.unprefix("foo/bar"), ("foo", "bar"))
-        self.assertEqual(scheme.unprefix("foo/bar/bla"), ("foo", "bar/bla"))
-        self.assertEqual(scheme.unprefix("//foo/"), ("foo", ""))
-        self.assertEqual(scheme.unprefix("bar/bloe"), ("bar/bloe", ""))
+    def setUp(self):
+        self.scheme = ListBranchingScheme(["foo", "bar/bloe"])
+
+    def test_is_branch_empty(self):
+        self.assertFalse(self.scheme.is_branch(""))
+
+    def test_is_branch_slash(self):
+        self.assertFalse(self.scheme.is_branch("/"))
+        self.assertTrue(self.scheme.is_branch("/foo"))
+        self.assertTrue(self.scheme.is_branch("foo"))
+        self.assertFalse(self.scheme.is_branch("/foo/foo"))
+        self.assertFalse(self.scheme.is_branch("foo/bar"))
+        self.assertFalse(self.scheme.is_branch("foobla"))
+        self.assertTrue(self.scheme.is_branch("//foo/"))
+        self.assertTrue(self.scheme.is_branch("bar/bloe"))
+
+    def test_unprefix_notbranch_empty(self):
+        self.assertRaises(NotBranchError, self.scheme.unprefix, "")
+
+    def test_unprefix_notbranch_slash(self):
+        self.assertRaises(NotBranchError, self.scheme.unprefix, "/")
+
+    def test_unprefix_notbranch_unknown(self):
+        self.assertRaises(NotBranchError, self.scheme.unprefix, "blie/bloe/bla")
+
+    def test_unprefix_branch_slash(self):
+        self.assertEqual(self.scheme.unprefix("/foo"), ("foo", ""))
+
+    def test_unprefix_branch(self):
+        self.assertEqual(self.scheme.unprefix("foo"), ("foo", ""))
+
+    def test_unprefix_nested_slash(self):
+        self.assertEqual(self.scheme.unprefix("/foo/foo"), ("foo", "foo"))
+
+    def test_unprefix_nested(self):
+        self.assertEqual(self.scheme.unprefix("foo/bar"), ("foo", "bar"))
+
+    def test_unprefix_double_nested(self):
+        self.assertEqual(self.scheme.unprefix("foo/bar/bla"), ("foo", "bar/bla"))
+
+    def test_unprefix_double_slash(self):
+        self.assertEqual(self.scheme.unprefix("//foo/"), ("foo", ""))
+
+    def test_unprefix_nested_branch(self):
+        self.assertEqual(self.scheme.unprefix("bar/bloe"), ("bar/bloe", ""))
 
 class TrunkScheme(TestCase):
-    def test_is_branch(self):
-        scheme = TrunkBranchingScheme()
-        self.assertFalse(scheme.is_branch(""))
-        self.assertFalse(scheme.is_branch("/"))
-        self.assertFalse(scheme.is_branch("/foo"))
-        self.assertFalse(scheme.is_branch("foo"))
-        self.assertFalse(scheme.is_branch("/foo/foo"))
-        self.assertFalse(scheme.is_branch("foo/bar"))
-        self.assertFalse(scheme.is_branch("foobla"))
-        self.assertTrue(scheme.is_branch("/trunk/"))
-        self.assertTrue(scheme.is_branch("////trunk"))
-        self.assertTrue(scheme.is_branch("/branches/foo"))
-        self.assertFalse(scheme.is_branch("/branche/foo"))
-        self.assertFalse(scheme.is_branch("/branchesfoo"))
-        self.assertTrue(scheme.is_branch("/branches/foo/"))
-        self.assertFalse(scheme.is_branch("/trunkfoo"))
-        self.assertFalse(scheme.is_branch("/trunk/foo"))
-        self.assertFalse(scheme.is_branch("/branches"))
+    def test_is_branch_empty(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch(""))
+
+    def test_is_branch_slash(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("/"))
+
+    def test_is_branch_unknown_slash(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("/foo"))
+
+    def test_is_branch_unknown(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("foo"))
+
+    def test_is_branch_unknown_nested_slash(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("/foo/foo"))
+
+    def test_is_branch_unknown_nested(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("foo/bar"))
+
+    def test_is_branch_unknown2(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("foobla"))
+
+    def test_is_branch_trunk(self):
+        self.assertTrue(TrunkBranchingScheme().is_branch("/trunk/"))
+
+    def test_is_branch_trunk_slashes(self):
+        self.assertTrue(TrunkBranchingScheme().is_branch("////trunk"))
+
+    def test_is_branch_branch(self):
+        self.assertTrue(TrunkBranchingScheme().is_branch("/branches/foo"))
+
+    def test_is_branch_typo(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("/branche/foo"))
+
+    def test_is_branch_missing_slash(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("/branchesfoo"))
+
+    def test_is_branch_branch_slash(self):
+        self.assertTrue(TrunkBranchingScheme().is_branch("/branches/foo/"))
+
+    def test_is_branch_trunk_missing_slash(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("/trunkfoo"))
+
+    def test_is_branch_trunk_file(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("/trunk/foo"))
+
+    def test_is_branch_branches(self):
+        self.assertFalse(TrunkBranchingScheme().is_branch("/branches"))
 
     def test_is_branch_level(self):
         scheme = TrunkBranchingScheme(2)




More information about the bazaar-commits mailing list