Rev 4343: Move tree and back callbacks into the repository check core. in http://people.ubuntu.com/~robertc/baz2.0/check

Robert Collins robertc at robertcollins.net
Mon May 11 04:48:54 BST 2009


At http://people.ubuntu.com/~robertc/baz2.0/check

------------------------------------------------------------
revno: 4343
revision-id: robertc at robertcollins.net-20090511034849-vxl36avre2zd4bod
parent: robertc at robertcollins.net-20090511023745-7620jhu2tb9s0bnz
committer: Robert Collins <robertc at robertcollins.net>
branch nick: check
timestamp: Mon 2009-05-11 13:48:49 +1000
message:
  Move tree and back callbacks into the repository check core.
=== modified file 'bzrlib/check.py'
--- a/bzrlib/check.py	2009-05-11 02:37:45 +0000
+++ b/bzrlib/check.py	2009-05-11 03:48:49 +0000
@@ -63,7 +63,7 @@
 
     # The Check object interacts with InventoryEntry.check, etc.
 
-    def __init__(self, repository):
+    def __init__(self, repository, check_repo=True):
         self.repository = repository
         self.checked_text_cnt = 0
         self.checked_rev_cnt = 0
@@ -79,28 +79,74 @@
         self.inconsistent_parents = []
         self.rich_roots = repository.supports_rich_root()
         self.text_key_references = {}
+        self.check_repo = check_repo
+        self.other_results = []
 
-    def check(self):
+    def check(self, callback_refs=None, check_repo=True):
+        if callback_refs is None:
+            callback_refs = {}
         self.repository.lock_read()
         self.progress = bzrlib.ui.ui_factory.nested_progress_bar()
         try:
-            self.progress.update('retrieving inventory', 0, 2)
-            # do not put in init, as it should be done with progess,
-            # and inside the lock.
-            self.inventory_weave = self.repository.inventories
-            self.progress.update('checking revision graph', 1)
-            self.check_revision_graph()
-            self.plan_revisions()
-            revno = 0
-            while revno < len(self.planned_revisions):
-                rev_id = self.planned_revisions[revno]
-                self.progress.update('checking revision', revno,
-                                     len(self.planned_revisions))
-                revno += 1
-                self.check_one_rev(rev_id)
-            # check_weaves is done after the revision scan so that
-            # revision index is known to be valid.
-            self.check_weaves()
+            if self.check_repo:
+                self.progress.update('retrieving inventory', 0, 2)
+                # do not put in init, as it should be done with progess,
+                # and inside the lock.
+                self.inventory_weave = self.repository.inventories
+                self.progress.update('checking revision graph', 1)
+                self.check_revision_graph()
+                self.plan_revisions()
+                revno = 0
+                while revno < len(self.planned_revisions):
+                    rev_id = self.planned_revisions[revno]
+                    self.progress.update('checking revision', revno,
+                                         len(self.planned_revisions))
+                    revno += 1
+                    self.check_one_rev(rev_id)
+                # check_weaves is done after the revision scan so that
+                # revision index is known to be valid.
+                self.check_weaves()
+            if callback_refs:
+                repo = self.repository
+                # calculate all refs, and callback the objects requesting them.
+                refs = {}
+                wanting_items = set()
+                # Current crude version calculates everything and calls
+                # everything at once. Doing a queue and popping as things are
+                # satisfied would be cheaper on memory [but few people have
+                # huge numbers of working trees today. TODO: fix before
+                # landing].
+                distances = set()
+                existences = set()
+                for ref, wantlist in callback_refs.iteritems():
+                    wanting_items.update(wantlist)
+                    kind, value = ref
+                    if kind == 'trees':
+                        refs[ref] = repo.revision_tree(value)
+                    elif kind == 'lefthand-distance':
+                        distances.add(value)
+                    elif kind == 'revision-existence':
+                        existences.add(value)
+                    else:
+                        raise AssertionError(
+                            'unknown ref kind for ref %s' % ref)
+                node_distances = repo.get_graph().find_lefthand_distances(distances)
+                for key, distance in node_distances.iteritems():
+                    refs[('lefthand-distance', key)] = distance
+                    if key in existences and distance > 0:
+                        refs[('revision-existence', key)] = True
+                        existences.remove(key)
+                parent_map = repo.get_graph().get_parent_map(existences)
+                for key in parent_map:
+                    refs[('revision-existence', key)] = True
+                    existences.remove(key)
+                for key in existences:
+                    refs[('revision-existence', key)] = False
+                for item in wanting_items:
+                    if isinstance(item, WorkingTree):
+                        item._check(refs)
+                    if isinstance(item, Branch):
+                        self.other_results.append(item.check(refs))
         finally:
             self.progress.finished()
             self.repository.unlock()
@@ -124,6 +170,12 @@
                 '{%s}' % ','.join([f for f in awol]))
 
     def report_results(self, verbose):
+        if self.check_repo:
+            self._report_repo_results(verbose)
+        for result in self.other_results:
+            result.report_results(verbose)
+
+    def _report_repo_results(self, verbose):
         note('checked repository %s format %s',
              self.repository.bzrdir.root_transport,
              self.repository._format)
@@ -284,6 +336,7 @@
     check_branch(branch, verbose)
 
 
+ at deprecated_function(deprecated_in((1,16,0)))
 def check_branch(branch, verbose):
     """Run consistency checks on a branch.
 
@@ -293,33 +346,11 @@
     """
     branch.lock_read()
     try:
-        needed_refs = branch._get_check_refs()
-        refs = {}
-        distances = set()
-        existences = set()
-        for ref in needed_refs:
-            kind, value = ref
-            if kind == 'lefthand-distance':
-                distances.add(value)
-            elif kind == 'revision-existence':
-                existences.add(value)
-            else:
-                raise AssertionError(
-                    'unknown ref kind for ref %s' % ref)
-        node_distances = branch.repository.get_graph().find_lefthand_distances(
-            distances)
-        for key, distance in node_distances.iteritems():
-            refs[('lefthand-distance', key)] = distance
-            if key in existences and distance > 0:
-                refs[('revision-existence', key)] = True
-                existences.remove(key)
-        parent_map = branch.repository.get_graph().get_parent_map(existences)
-        for key in parent_map:
-            refs[('revision-existence', key)] = True
-            existences.remove(key)
-        for key in existences:
-            refs[('revision-existence', key)] = False
-        branch_result = branch.check(refs)
+        needed_refs = {}
+        for ref in branch._get_check_refs():
+            needed_refs.setdefault(ref, []).append(branch)
+        result = branch.repository.check([branch.last_revision()], needed_refs)
+        branch_result = result.other_results[0]
     finally:
         branch.unlock()
     branch_result.report_results(verbose)
@@ -332,6 +363,7 @@
     :param needed_refs: Refs we are accumulating.
     :param to_unlock: The unlock list accumulating.
     """
+    note("Checking branch at '%s'." % (branch.base,))
     branch.lock_read()
     to_unlock.append(branch)
     branch_refs = branch._get_check_refs()
@@ -351,6 +383,7 @@
     """
     if base_tree is not None and tree.basedir == base_tree.basedir:
         return
+    note("Checking working tree at '%s'." % (tree.basedir,))
     tree.lock_read()
     to_unlock.append(tree)
     tree_refs = tree._get_check_refs()
@@ -401,54 +434,13 @@
                 log_error("No branch found at specified location.")
             if do_tree and base_tree is None and not saw_tree:
                 log_error("No working tree found at specified location.")
-            if do_repo:
-                note("Checking repository at '%s'."
-                     % (repo.bzrdir.root_transport.base,))
-                result = repo.check()
+            if do_repo or do_branch or do_tree:
+                if do_repo:
+                    note("Checking repository at '%s'."
+                         % (repo.bzrdir.root_transport.base,))
+                result = repo.check(None, callback_refs=needed_refs,
+                    check_repo=do_repo)
                 result.report_results(verbose)
-            if needed_refs:
-                # calculate all refs, and callback the objects requesting them.
-                refs = {}
-                wanting_items = set()
-                # Current crude version calculates everything and calls
-                # everything at once. Doing a queue and popping as things are
-                # satisfied would be cheaper on memory [but few people have
-                # huge numbers of working trees today. TODO: fix before
-                # landing].
-                distances = set()
-                existences = set()
-                for ref, wantlist in needed_refs.iteritems():
-                    wanting_items.update(wantlist)
-                    kind, value = ref
-                    if kind == 'trees':
-                        refs[ref] = repo.revision_tree(value)
-                    elif kind == 'lefthand-distance':
-                        distances.add(value)
-                    elif kind == 'revision-existence':
-                        existences.add(value)
-                    else:
-                        raise AssertionError(
-                            'unknown ref kind for ref %s' % ref)
-                node_distances = repo.get_graph().find_lefthand_distances(distances)
-                for key, distance in node_distances.iteritems():
-                    refs[('lefthand-distance', key)] = distance
-                    if key in existences and distance > 0:
-                        refs[('revision-existence', key)] = True
-                        existences.remove(key)
-                parent_map = repo.get_graph().get_parent_map(existences)
-                for key in parent_map:
-                    refs[('revision-existence', key)] = True
-                    existences.remove(key)
-                for key in existences:
-                    refs[('revision-existence', key)] = False
-                for item in wanting_items:
-                    if isinstance(item, WorkingTree):
-                        note("Checking working tree at '%s'." % (item.basedir,))
-                        item._check(refs)
-                    if isinstance(item, Branch):
-                        note("Checking branch at '%s'." % (branch.base,))
-                        branch_result = item.check(refs)
-                        branch_result.report_results(verbose)
         else:
             if do_tree:
                 log_error("No working tree found at specified location.")

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2009-04-28 06:53:29 +0000
+++ b/bzrlib/remote.py	2009-05-11 03:48:49 +0000
@@ -1400,9 +1400,10 @@
         return self._real_repository.get_revision_reconcile(revision_id)
 
     @needs_read_lock
-    def check(self, revision_ids=None):
+    def check(self, revision_ids=None, callback_refs=None):
         self._ensure_real()
-        return self._real_repository.check(revision_ids=revision_ids)
+        return self._real_repository.check(revision_ids=revision_ids,
+            callback_refs=callback_refs)
 
     def copy_content_into(self, destination, revision_id=None):
         self._ensure_real()

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2009-05-11 01:49:00 +0000
+++ b/bzrlib/repository.py	2009-05-11 03:48:49 +0000
@@ -2454,13 +2454,18 @@
         return record.get_bytes_as('fulltext')
 
     @needs_read_lock
-    def check(self, revision_ids=None):
+    def check(self, revision_ids=None, callback_refs=None, check_repo=True):
         """Check consistency of all history of given revision_ids.
 
         Different repository implementations should override _check().
 
         :param revision_ids: A non-empty list of revision_ids whose ancestry
              will be checked.  Typically the last revision_id of a branch.
+        :param callback_refs: A dict of check-refs to resolve and callback
+            the check/_check method on the items listed as wanting the ref.
+            see bzrlib.check.
+        :param check_repo: If False do not check the repository contents, just 
+            calculate the data callback_refs requires and call them back.
         """
         # TODO: Reinstate or confirm its obsolescence.
         # from Branch.check - a cross check that the parents index
@@ -2487,11 +2492,12 @@
         #                                "parents of {%s}"
         #                                % (mainline_parent_id, revision_id))
         #            mainline_parent_id = revision_id
-        return self._check(revision_ids)
+        return self._check(revision_ids, callback_refs=callback_refs,
+            check_repo=check_repo)
 
-    def _check(self, revision_ids):
-        result = check.Check(self)
-        result.check()
+    def _check(self, revision_ids, callback_refs, check_repo):
+        result = check.Check(self, check_repo=check_repo)
+        result.check(callback_refs)
         return result
 
     def _warn_if_deprecated(self):

=== modified file 'bzrlib/tests/blackbox/test_check.py'
--- a/bzrlib/tests/blackbox/test_check.py	2009-05-11 02:37:45 +0000
+++ b/bzrlib/tests/blackbox/test_check.py	2009-05-11 03:48:49 +0000
@@ -35,15 +35,15 @@
         tree.commit('hallelujah')
         out, err = self.run_bzr('check')
         self.assertContainsRe(err, r"Checking working tree at '.*'\.\n")
-        self.assertContainsRe(err, r"Checking repository at '.*'\.\n"
-                                   r"checked repository.*\n"
+        self.assertContainsRe(err, r"Checking repository at '.*'\.\n")
+        self.assertContainsRe(err, r"checked repository.*\n"
                                    r"     1 revisions\n"
                                    r"     0 file-ids\n"
                                    r"     0 unique file texts\n"
                                    r"     0 repeated file texts\n"
                                    r"     0 unreferenced text versions\n")
-        self.assertContainsRe(err, r"Checking branch at '.*'\.\n"
-                                   r"checked branch.*")
+        self.assertContainsRe(err, r"Checking branch at '.*'\.\n")
+        self.assertContainsRe(err, r"checked branch.*")
 
     def test_check_branch(self):
         tree = self.make_branch_and_tree('.')
@@ -87,8 +87,8 @@
         branch = self.make_branch('.')
         out, err = self.run_bzr('check --tree --branch')
         self.assertContainsRe(err,
-            r"^No working tree found at specified location\.\n"
             r"Checking branch at '.*'\.\n"
+            r"No working tree found at specified location\.\n"
             r"checked branch.*")
 
     def test_check_missing_branch_in_shared_repo(self):

=== modified file 'bzrlib/tests/per_repository/test_check.py'
--- a/bzrlib/tests/per_repository/test_check.py	2009-04-09 20:23:07 +0000
+++ b/bzrlib/tests/per_repository/test_check.py	2009-05-11 03:48:49 +0000
@@ -36,13 +36,11 @@
         tree = self.make_branch_and_tree('.')
         self.build_tree(['foo'])
         tree.smart_add(['.'])
-        tree.commit('1')
+        revid1 = tree.commit('1')
         self.build_tree(['bar'])
         tree.smart_add(['.'])
-        tree.commit('2')
-        # XXX: check requires a non-empty revision IDs list, but it ignores the
-        # contents of it!
-        check_object = tree.branch.repository.check(['ignored'])
+        revid2 = tree.commit('2')
+        check_object = tree.branch.repository.check([revid1, revid2])
         check_object.report_results(verbose=True)
         log = self._get_log(keep_log_file=True)
         self.assertContainsRe(
@@ -100,3 +98,32 @@
             "revision-id has wrong parents in index: "
             r"\('incorrect-parent',\) should be \(\)")
 
+
+class TestCallbacks(TestCaseWithRepository):
+
+    def test_callback_tree_and_branch(self):
+        # use a real tree to get actual refs that will work
+        tree = self.make_branch_and_tree('foo')
+        revid = tree.commit('foo')
+        tree.lock_read()
+        self.addCleanup(tree.unlock)
+        needed_refs = {}
+        for ref in tree._get_check_refs():
+            needed_refs.setdefault(ref, []).append(tree)
+        for ref in tree.branch._get_check_refs():
+            needed_refs.setdefault(ref, []).append(tree.branch)
+        self.tree_check = tree._check
+        self.branch_check = tree.branch.check
+        tree._check = self.tree_callback
+        tree.branch.check = self.branch_callback
+        self.callbacks = []
+        tree.branch.repository.check([revid], callback_refs=needed_refs)
+        self.assertNotEqual([], self.callbacks)
+
+    def tree_callback(self, refs):
+        self.callbacks.append(('tree', refs))
+        return self.tree_check(refs)
+
+    def branch_callback(self, refs):
+        self.callbacks.append(('branch', refs))
+        return self.branch_check(refs)




More information about the bazaar-commits mailing list