Rev 2780: Trivial review feedback changes. in http://people.ubuntu.com/~robertc/baz2.0/commit

Robert Collins robertc at robertcollins.net
Mon Sep 3 23:17:33 BST 2007


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

------------------------------------------------------------
revno: 2780
revision-id: robertc at robertcollins.net-20070903221720-9k1y25x0x60rk38b
parent: robertc at robertcollins.net-20070903202305-yj4obhvvw3j7t4jc
parent: robertc at robertcollins.net-20070903015712-oo8vn04vryo75xjb
committer: Robert Collins <robertc at robertcollins.net>
branch nick: commit
timestamp: Tue 2007-09-04 08:17:20 +1000
message:
  Trivial review feedback changes.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/graph.py                graph_walker.py-20070525030359-y852guab65d4wtn0-1
  bzrlib/inventory.py            inventory.py-20050309040759-6648b84ca2005b37
  bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
  bzrlib/tests/test_graph.py     test_graph_walker.py-20070525030405-enq4r60hhi9xrujc-1
  bzrlib/tests/tree_implementations/test_inv.py test_inv.py-20070312023226-0cdvk5uwhutis9vg-1
    ------------------------------------------------------------
    revno: 2776.1.1
    revision-id: robertc at robertcollins.net-20070903015712-oo8vn04vryo75xjb
    parent: pqm at pqm.ubuntu.com-20070901160444-hcr66zejwyy0jezc
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: commit
    timestamp: Mon 2007-09-03 11:57:12 +1000
    message:
      * Deprecated method ``find_previous_heads`` on
        ``bzrlib.inventory.InventoryEntry``. This has been superceded by the use
        of ``parent_candidates`` and a separate heads check via the repository
        API. (Robert Collins)
      
      * New public method ``heads`` on the ``bzrlib.graph.Graph`` class. This is
        a simple rename of a previously internal method which implements the same
        semantics as heads(). (Robert Collins, John A Meinel)
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/graph.py                graph_walker.py-20070525030359-y852guab65d4wtn0-1
      bzrlib/inventory.py            inventory.py-20050309040759-6648b84ca2005b37
      bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
      bzrlib/tests/tree_implementations/test_inv.py test_inv.py-20070312023226-0cdvk5uwhutis9vg-1
=== modified file 'NEWS'
--- a/NEWS	2007-09-03 02:58:58 +0000
+++ b/NEWS	2007-09-03 22:17:20 +0000
@@ -152,6 +152,11 @@
      This is done in order to mix the commit messages (which is a unicode
      string), and the diff which is a raw string. (Goffredo Baroncelli)
 
+    * Deprecated method ``find_previous_heads`` on
+      ``bzrlib.inventory.InventoryEntry``. This has been superseded by the use
+      of ``parent_candidates`` and a separate heads check via the repository
+      API. (Robert Collins)
+
    * New trace function ``mutter_callsite`` will print out a subset of the
      stack to the log, which can be useful for gathering debug details.
      (Robert Collins)
@@ -176,6 +181,10 @@
       efficiently streaming the knit data for a set of versions over the smart
       protocol.
 
+    * New public method ``heads`` on the ``bzrlib.graph.Graph`` class. This is
+      a simple rename of a previously-internal method which implements the same
+      semantics as heads(). (Robert Collins, John A Meinel)
+
   TESTING:
 
     * Use UTF-8 encoded StringIO for log tests to avoid failures on

=== modified file 'bzrlib/graph.py'
--- a/bzrlib/graph.py	2007-08-07 15:26:26 +0000
+++ b/bzrlib/graph.py	2007-09-03 22:17:20 +0000
@@ -134,7 +134,10 @@
            ancestor of all border ancestors.
         """
         border_common, common, sides = self._find_border_ancestors(revisions)
-        return self._filter_candidate_lca(border_common)
+        # We may have common ancestors that can be reached from each other.
+        # - ask for the heads of them to filter it down to only ones that
+        # cannot be reached from each other - phase 2.
+        return self.heads(border_common)
 
     def find_difference(self, left_revision, right_revision):
         """Determine the graph difference between two revisions"""
@@ -212,23 +215,24 @@
                     for searcher in searchers:
                         update_common(searcher, revision)
 
-    def _filter_candidate_lca(self, candidate_lca):
-        """Remove candidates which are ancestors of other candidates.
-
-        This is done by searching the ancestries of each border ancestor.  It
-        is perfomed on the principle that a border ancestor that is not an
-        ancestor of any other border ancestor is a lowest common ancestor.
-
-        Searches are stopped when they find a node that is determined to be a
-        common ancestor of all border ancestors, because this shows that it
-        cannot be a descendant of any border ancestor.
-
-        This will scale with the number of candidate ancestors and the length
-        of the shortest path from a candidate to an ancestor common to all
-        candidates.
+    def heads(self, keys):
+        """Return the heads from amongst keys.
+
+        This is done by searching the ancestries of each key.  Any key that is
+        reachable from another key is not returned; all the others are.
+
+        This operation scales with the relative depth between any two keys. If
+        any two keys are completely disconnected all ancestry of both sides
+        will be retrieved.
+
+        :param keys: An iterable of keys.
+        :return: A set of the heads. Note that as a set there is no ordering
+            information. Callers will need to filter their input to create
+            order if they need it.
         """
+        candidate_heads = set(keys)
         searchers = dict((c, self._make_breadth_first_searcher([c]))
-                          for c in candidate_lca)
+                          for c in candidate_heads)
         active_searchers = dict(searchers)
         # skip over the actual candidate for each searcher
         for searcher in active_searchers.itervalues():
@@ -248,8 +252,8 @@
                     del active_searchers[candidate]
                     continue
                 for ancestor in ancestors:
-                    if ancestor in candidate_lca:
-                        candidate_lca.remove(ancestor)
+                    if ancestor in candidate_heads:
+                        candidate_heads.remove(ancestor)
                         del searchers[ancestor]
                         if ancestor in active_searchers:
                             del active_searchers[ancestor]
@@ -264,7 +268,7 @@
                             seen_ancestors =\
                                 searcher.find_seen_ancestors(ancestor)
                             searcher.stop_searching_any(seen_ancestors)
-        return candidate_lca
+        return candidate_heads
 
     def find_unique_lca(self, left_revision, right_revision):
         """Find a unique LCA.

=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py	2007-08-31 05:54:21 +0000
+++ b/bzrlib/inventory.py	2007-09-03 01:57:12 +0000
@@ -50,6 +50,7 @@
     BzrCheckError,
     BzrError,
     )
+from bzrlib.symbol_versioning import deprecated_method, zero_ninetyone
 from bzrlib.trace import mutter
 
 
@@ -161,33 +162,19 @@
     def _diff(self, text_diff, from_label, tree, to_label, to_entry, to_tree,
              output_to, reverse=False):
         """Perform a diff between two entries of the same kind."""
-
-    def find_previous_heads(self, previous_inventories,
-                            versioned_file_store,
-                            transaction,
-                            entry_vf=None):
-        """Return the revisions and entries that directly precede this.
-
-        Returned as a map from revision to inventory entry.
-
-        This is a map containing the file revisions in all parents
-        for which the file exists, and its revision is not a parent of
-        any other. If the file is new, the set will be empty.
-
-        :param versioned_file_store: A store where ancestry data on this
-                                     file id can be queried.
-        :param transaction: The transaction that queries to the versioned 
-                            file store should be completed under.
-        :param entry_vf: The entry versioned file, if its already available.
+    
+    def parent_candidates(self, previous_inventories):
+        """Find possible per-file graph parents.
+
+        This is currently defined by:
+         - The kind has not changed (* not sure why this is a clause *)
+         - Select the last changed revision in the parent inventory.
+         - Do deal with a short lived bug in bzr 0.8's development two entries
+           that have the same last changed but different 'x' bit settings are
+           changed in-place.
         """
-        def get_ancestors(weave, entry):
-            return set(weave.get_ancestry(entry.revision, topo_sorted=False))
         # revision:ie mapping for each ie found in previous_inventories.
         candidates = {}
-        # revision:ie mapping with one revision for each head.
-        heads = {}
-        # revision: ancestor list for each head
-        head_ancestors = {}
         # identify candidate head revision ids.
         for inv in previous_inventories:
             if self.file_id in inv:
@@ -212,19 +199,52 @@
                 else:
                     # add this revision as a candidate.
                     candidates[ie.revision] = ie
-
+        return candidates
+
+    @deprecated_method(zero_ninetyone)
+    def find_previous_heads(self, previous_inventories,
+                            versioned_file_store,
+                            transaction,
+                            entry_vf=None):
+        """Return the revisions and entries that directly precede this.
+
+        Returned as a map from revision to inventory entry.
+
+        This is a map containing the file revisions in all parents
+        for which the file exists, and its revision is not a parent of
+        any other. If the file is new, the set will be empty.
+
+        :param versioned_file_store: A store where ancestry data on this
+                                     file id can be queried.
+        :param transaction: The transaction that queries to the versioned 
+                            file store should be completed under.
+        :param entry_vf: The entry versioned file, if its already available.
+        """
+        candidates = self.parent_candidates(previous_inventories)
+
+        # revision:ie mapping with one revision for each head.
+        heads = {}
         # common case optimisation
         if len(candidates) == 1:
             # if there is only one candidate revision found
-            # then we can opening the versioned file to access ancestry:
+            # then we can avoid opening the versioned file to access ancestry:
             # there cannot be any ancestors to eliminate when there is 
             # only one revision available.
-            heads[ie.revision] = ie
-            return heads
+            return candidates
+        
+        # --- what follows is now encapsulated in repository.get_graph.heads(), 
+        #     but that is not accessible from here as we have no repository
+        #     pointer. Note that the repository.get_graph.heads() call can return
+        #     different results *at the moment* because of the kind-changing check
+        #     we have in parent_candidates().
 
         # eliminate ancestors amongst the available candidates:
         # heads are those that are not an ancestor of any other candidate
         # - this provides convergence at a per-file level.
+        def get_ancestors(weave, entry):
+            return set(weave.get_ancestry(entry.revision, topo_sorted=False))
+        # revision: ancestor list for each head
+        head_ancestors = {}
         for ie in candidates.values():
             # may be an ancestor of a known head:
             already_present = 0 != len(

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2007-09-03 03:03:26 +0000
+++ b/bzrlib/repository.py	2007-09-03 22:17:20 +0000
@@ -2184,44 +2184,57 @@
         if self._new_revision_id is None:
             self._new_revision_id = self._gen_revision_id()
 
-    def record_entry_contents(self, ie, parent_invs, path, tree):
-        """Record the content of ie from tree into the commit if needed.
-
-        Side effect: sets ie.revision when unchanged
-
-        :param ie: An inventory entry present in the commit.
+    def _check_root(self, ie, parent_invs, tree):
+        """Helper for record_entry_contents.
+
+        :param ie: An entry being added.
         :param parent_invs: The inventories of the parent revisions of the
             commit.
-        :param path: The path the entry is at in the tree.
-        :param tree: The tree which contains this entry and should be used to 
-        obtain content.
+        :param tree: The tree that is being committed.
         """
-        if self.new_inventory.root is None and ie.parent_id is not None:
+        if ie.parent_id is not None:
+            # if ie is not root, add a root automatically.
             symbol_versioning.warn('Root entry should be supplied to'
                 ' record_entry_contents, as of bzr 0.10.',
                  DeprecationWarning, stacklevel=2)
             self.record_entry_contents(tree.inventory.root.copy(), parent_invs,
                                        '', tree)
-        self.new_inventory.add(ie)
-
-        # ie.revision is always None if the InventoryEntry is considered
-        # for committing. ie.snapshot will record the correct revision 
-        # which may be the sole parent if it is untouched.
-        if ie.revision is not None:
-            return
-
-        # In this revision format, root entries have no knit or weave
-        if ie is self.new_inventory.root:
-            # When serializing out to disk and back in
-            # root.revision is always _new_revision_id
+        else:
+            # In this revision format, root entries have no knit or weave When
+            # serializing out to disk and back in root.revision is always
+            # _new_revision_id
             ie.revision = self._new_revision_id
+
+    def record_entry_contents(self, ie, parent_invs, path, tree):
+        """Record the content of ie from tree into the commit if needed.
+
+        Side effect: sets ie.revision when unchanged
+
+        :param ie: An inventory entry present in the commit.
+        :param parent_invs: The inventories of the parent revisions of the
+            commit.
+        :param path: The path the entry is at in the tree.
+        :param tree: The tree which contains this entry and should be used to 
+        obtain content.
+        """
+        if self.new_inventory.root is None:
+            self._check_root(ie, parent_invs, tree)
+        self.new_inventory.add(ie)
+
+        # ie.revision is always None if the InventoryEntry is considered
+        # for committing. ie.snapshot will record the correct revision 
+        # which may be the sole parent if it is untouched.
+        if ie.revision is not None:
             return
-        previous_entries = ie.find_previous_heads(
-            parent_invs,
-            self.repository.weave_store,
-            self.repository.get_transaction())
-        # we are creating a new revision for ie in the history store
-        # and inventory.
+
+        parent_candiate_entries = ie.parent_candidates(parent_invs)
+        heads = self.repository.get_graph().heads(parent_candiate_entries.keys())
+        # XXX: Note that this is unordered - and this is tolerable because 
+        # the previous code was also unordered.
+        previous_entries = dict((head, parent_candiate_entries[head]) for head
+            in heads)
+        # we are creating a new revision for ie in the history store and
+        # inventory.
         ie.snapshot(self._new_revision_id, path, previous_entries, tree, self)
 
     def modified_directory(self, file_id, file_parents):
@@ -2302,34 +2315,16 @@
     
     record_root_entry = True
 
-    def record_entry_contents(self, ie, parent_invs, path, tree):
-        """Record the content of ie from tree into the commit if needed.
-
-        Side effect: sets ie.revision when unchanged
-
-        :param ie: An inventory entry present in the commit.
+    def _check_root(self, ie, parent_invs, tree):
+        """Helper for record_entry_contents.
+
+        :param ie: An entry being added.
         :param parent_invs: The inventories of the parent revisions of the
             commit.
-        :param path: The path the entry is at in the tree.
-        :param tree: The tree which contains this entry and should be used to 
-        obtain content.
+        :param tree: The tree that is being committed.
         """
-        assert self.new_inventory.root is not None or ie.parent_id is None
-        self.new_inventory.add(ie)
-
-        # ie.revision is always None if the InventoryEntry is considered
-        # for committing. ie.snapshot will record the correct revision 
-        # which may be the sole parent if it is untouched.
-        if ie.revision is not None:
-            return
-
-        previous_entries = ie.find_previous_heads(
-            parent_invs,
-            self.repository.weave_store,
-            self.repository.get_transaction())
-        # we are creating a new revision for ie in the history store
-        # and inventory.
-        ie.snapshot(self._new_revision_id, path, previous_entries, tree, self)
+        # ie must be root for this builder
+        assert ie.parent_id is None
 
 
 _unescape_map = {

=== modified file 'bzrlib/tests/test_graph.py'
--- a/bzrlib/tests/test_graph.py	2007-08-07 20:45:47 +0000
+++ b/bzrlib/tests/test_graph.py	2007-09-03 22:17:20 +0000
@@ -373,4 +373,83 @@
         #      c
         graph = self.make_graph({'c': ['b', 'd'], 'd': ['e'], 'b': ['a'],
                                  'a': [NULL_REVISION], 'e': [NULL_REVISION]})
-        self.assertEqual(['c'], graph._filter_candidate_lca(['a', 'c', 'e']))
+        self.assertEqual(set(['c']), graph.heads(['a', 'c', 'e']))
+
+    def test_heads_null(self):
+        graph = self.make_graph(ancestry_1)
+        self.assertEqual(set(['null:']), graph.heads(['null:']))
+        self.assertEqual(set(['rev1']), graph.heads(['null:', 'rev1']))
+        self.assertEqual(set(['rev1']), graph.heads(['rev1', 'null:']))
+        self.assertEqual(set(['rev1']), graph.heads(set(['rev1', 'null:'])))
+        self.assertEqual(set(['rev1']), graph.heads(('rev1', 'null:')))
+
+    def test_heads_one(self):
+        # A single node will alwaya be a head
+        graph = self.make_graph(ancestry_1)
+        self.assertEqual(set(['null:']), graph.heads(['null:']))
+        self.assertEqual(set(['rev1']), graph.heads(['rev1']))
+        self.assertEqual(set(['rev2a']), graph.heads(['rev2a']))
+        self.assertEqual(set(['rev2b']), graph.heads(['rev2b']))
+        self.assertEqual(set(['rev3']), graph.heads(['rev3']))
+        self.assertEqual(set(['rev4']), graph.heads(['rev4']))
+
+    def test_heads_single(self):
+        graph = self.make_graph(ancestry_1)
+        self.assertEqual(set(['rev4']), graph.heads(['null:', 'rev4']))
+        self.assertEqual(set(['rev2a']), graph.heads(['rev1', 'rev2a']))
+        self.assertEqual(set(['rev2b']), graph.heads(['rev1', 'rev2b']))
+        self.assertEqual(set(['rev3']), graph.heads(['rev1', 'rev3']))
+        self.assertEqual(set(['rev4']), graph.heads(['rev1', 'rev4']))
+        self.assertEqual(set(['rev4']), graph.heads(['rev2a', 'rev4']))
+        self.assertEqual(set(['rev4']), graph.heads(['rev2b', 'rev4']))
+        self.assertEqual(set(['rev4']), graph.heads(['rev3', 'rev4']))
+
+    def test_heads_two_heads(self):
+        graph = self.make_graph(ancestry_1)
+        self.assertEqual(set(['rev2a', 'rev2b']),
+                         graph.heads(['rev2a', 'rev2b']))
+        self.assertEqual(set(['rev3', 'rev2b']),
+                         graph.heads(['rev3', 'rev2b']))
+
+    def test_heads_criss_cross(self):
+        graph = self.make_graph(criss_cross)
+        self.assertEqual(set(['rev2a']),
+                         graph.heads(['rev2a', 'rev1']))
+        self.assertEqual(set(['rev2b']),
+                         graph.heads(['rev2b', 'rev1']))
+        self.assertEqual(set(['rev3a']),
+                         graph.heads(['rev3a', 'rev1']))
+        self.assertEqual(set(['rev3b']),
+                         graph.heads(['rev3b', 'rev1']))
+        self.assertEqual(set(['rev2a', 'rev2b']),
+                         graph.heads(['rev2a', 'rev2b']))
+        self.assertEqual(set(['rev3a']),
+                         graph.heads(['rev3a', 'rev2a']))
+        self.assertEqual(set(['rev3a']),
+                         graph.heads(['rev3a', 'rev2b']))
+        self.assertEqual(set(['rev3a']),
+                         graph.heads(['rev3a', 'rev2a', 'rev2b']))
+        self.assertEqual(set(['rev3b']),
+                         graph.heads(['rev3b', 'rev2a']))
+        self.assertEqual(set(['rev3b']),
+                         graph.heads(['rev3b', 'rev2b']))
+        self.assertEqual(set(['rev3b']),
+                         graph.heads(['rev3b', 'rev2a', 'rev2b']))
+        self.assertEqual(set(['rev3a', 'rev3b']),
+                         graph.heads(['rev3a', 'rev3b']))
+        self.assertEqual(set(['rev3a', 'rev3b']),
+                         graph.heads(['rev3a', 'rev3b', 'rev2a', 'rev2b']))
+
+    def test_heads_shortcut(self):
+        graph = self.make_graph(history_shortcut)
+
+        self.assertEqual(set(['rev2a', 'rev2b', 'rev2c']),
+                         graph.heads(['rev2a', 'rev2b', 'rev2c']))
+        self.assertEqual(set(['rev3a', 'rev3b']),
+                         graph.heads(['rev3a', 'rev3b']))
+        self.assertEqual(set(['rev3a', 'rev3b']),
+                         graph.heads(['rev2a', 'rev3a', 'rev3b']))
+        self.assertEqual(set(['rev2a', 'rev3b']),
+                         graph.heads(['rev2a', 'rev3b']))
+        self.assertEqual(set(['rev2c', 'rev3a']),
+                         graph.heads(['rev2c', 'rev3a']))

=== modified file 'bzrlib/tests/tree_implementations/test_inv.py'
--- a/bzrlib/tests/tree_implementations/test_inv.py	2007-04-14 11:55:51 +0000
+++ b/bzrlib/tests/tree_implementations/test_inv.py	2007-09-03 01:57:12 +0000
@@ -25,6 +25,7 @@
 from bzrlib.osutils import (
     has_symlinks,
     )
+from bzrlib.symbol_versioning import zero_ninetyone
 from bzrlib.tests import TestSkipped
 from bzrlib.tests.tree_implementations import TestCaseWithTree
 from bzrlib.uncommit import uncommit
@@ -177,8 +178,9 @@
             self.branch.repository.get_transaction())
         
     def get_previous_heads(self, inventories):
-        return self.file_active.find_previous_heads(
-            inventories, 
+        return self.applyDeprecated(zero_ninetyone,
+            self.file_active.find_previous_heads,
+            inventories,
             self.branch.repository.weave_store,
             self.branch.repository.get_transaction())
         



More information about the bazaar-commits mailing list