[MERGE, 165306] Use the per-file graph during commits of pack's

John Arbash Meinel john at arbash-meinel.com
Tue Nov 27 22:15:08 GMT 2007


John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
=== modified file 'bzrlib/index.py'
...

+    def get_parents(self, revision_ids):
+        """See StackedParentsProvider.get_parents.
+
+        This implementation thunks the graph.Graph.get_parents api 
across to
+        GraphIndex.
+
+        :param revision_ids: An iterable of graph keys for this graph.

^- This shouldn't be called revision_ids if it is actually graph keys.

I found it hard to sort out how it was working when you were later on
using (file_id, revision_id) tuples.

Maybe call it:

def get_parents(self, node_keys):
...
     :param node_keys: An iterable of graph keys for this graph.
         (eg. for a simple revision graph, this will be revision ids, for 
a
         CombinedGraph this will be (file_id, revision_id) tuples.)


This also means that your later code:
         search_keys = set(revision_ids)
         search_keys.discard(NULL_REVISION)
         found_parents = {NULL_REVISION:[]}

Is probably bogus. Because (file_id, NULL_REVISION) will not be found.

I also wonder about the logic of returning (NULL_REVISION,) as the 
parents
of a (file_id, revision_id) pair.


I also notice that "Combined_Graph.iter_keys says:"
         :return: An iterable of (index, key, reference_lists, value). 
There is no
             defined order for the result iteration - it will be in the 
most
             efficient order for the index.
but you are looping on:
         for index, key, value, refs in self.iter_entries(search_keys):

I'm guessing the comment for Combined_Graph.iter_entries is wrong.


Is there a way to reduce the duplication in __init__ for 
PackCommitBuilder
and PackRootCommitBuilder? They both have the same __init__ and the same
implementation of heads().


I don't see any direct tests for CommitBuilder.heads(). Which seems like
they should be in
bzrlib/tests/repository_implementations/test_commit_builder.py
I only see heads tests for test_graph.py and test_inv.py



For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C1196129886.32300.243.camel%40lifeless-64%3E



More information about the bazaar mailing list