[merge][#295350] fix another "revision not present" in stacked repositories

[John Arbash Meinel]

John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
v- I think it is more general, and should be "Check if this Parent 
Provider has one key". Since it would work for repositories, etc. 
Anything that provides '.get_parent_map()'.

+def _has_key_from_parent_map(self, key):
+    """Check if this index has one key.
+
+    If it's possible to check for multiple keys at once through
+    calling get_parent_map that should be faster.
+    """
+    return (key in self.get_parent_map([key]))
+

-- Extra line here

+def _missing_keys_from_parent_map(self, keys):
+    return set(keys) - set(self.get_parent_map(keys))
+
+


...

+    def _external_references(self):
+        """Return references that are not present in this index.
+        """
+        keys = set()
+        refs = set()
+        for node in self.iter_all_entries():
+            keys.add(node[1])
+            refs.update(node[3][1])
+        return refs - keys
+

^- Should this be guarded for the case where self.reference_lists == 0, 
in which case node[3] doesn't actually exist, IIRC. Which means you'll 
get an IndexError rather than just returning nothing.

You use node[3][1] because that is the "compression parent", right? I'm 
not entirely sure that the Index object itself should know about it. 
That knowledge is currently correct in most cases, but is certainly 
possible to change. It at least feels like this needs to be a 
VersionedFile function, as the VF has the right knowledge about what it 
puts into the index. (For example, if we change to allow multi-parent 
diffs, then we need to check that *all* compression parents are 
present.)

I'm willing consider it a YAGNI, though a comment about what assumptions 
are being made would probably be good.

+                  and (not parents
+                       or not self._index.missing_keys(parents)
+                       or not self._fallback_vfs
+                       or self.missing_keys(parents))):

^- I thought we wanted to put the "or not self._fallback_vfs" before the 
other missing_keys() check.

...

-                    if basis_parent not in 
self.get_parent_map([basis_parent]):
+                    #
+                    # They're required to be physically in this
+                    # KnitVersionedFiles, not in a fallback.
+                    if self.missing_keys(parents):
                          pending = buffered_index_entries.setdefault(
-                            basis_parent, [])
+                            parents[0], [])


^- I know Aaron asked for this, but it doesn't seem like we should 
actually *require* that both parents be present unless we are actually 
using a multi-parent compression format. Anyway, we can do it either way 
for now, and wait until we actually fix the compression_parent bug to 
address this again.

(Note that at least at first, if multi-parent compression was used then 
this part: (record.storage_kind in knit_types) would have already 
failed, as multi-parent isn't a known knit_type storage kind.


=== modified file 'bzrlib/versionedfile.py'
--- bzrlib/versionedfile.py     2008-09-08 13:35:44 +0000
+++ bzrlib/versionedfile.py     2008-11-19 07:44:51 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006 Canonical Ltd
+# Copyright (C) 2005, 2006, 2008 Canonical Ltd

^- I'm rather positive that we modified this file in 2007 as well, just 
forgot to update the copyright line. :)


For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3Ce01316480811200433l1ed1082dr4f1e2c729e973cdb%40mail.gmail.com%3E
Project: Bazaar