[MERGE] Replace VersionedFile.get_parents with get_parent_map

John Arbash Meinel john at arbash-meinel.com
Thu Mar 20 15:47:35 GMT 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
| This may seem large for a trivial change, but its not really that
| trivial.
|
| get_parents was a very old API from back with weaves and did work to
| hide ghosts. So, removing it meant finding and fixing many places ghosts
| were not handled correctly.
|
| This patch then:
|  - deprecates VersionedFile.get_parents
|  - prohibits pulling a ghost containing repo into a non-ghost-capable
| repo
|  - adds VersionedFile.get_parent_map
|  - tunes the internals of KnitVersionedFile to reduce friction and use
| this api where possible
|
| This is the first of many patches that will give us a much leaner
| VersionedFile api, which I will then port across via a thunk layer to
| VersionedFiles.
|
| -Rob
|


Your change to iter_reverse_revision_history doesn't handle if a mainline parent
is a ghost:
~     def iter_reverse_revision_history(self, revision_id):
~         """Iterate backwards through revision ids in the lefthand history

~         :param revision_id: The revision id to start with.  All its lefthand
~             ancestors will be traversed.
~         """
- -        if revision_id in (None, _mod_revision.NULL_REVISION):
- -            return
+        graph = self.get_graph()
~         next_id = revision_id
- -        versionedfile = self._get_history_vf()
~         while True:
+            if next_id in (None, _mod_revision.NULL_REVISION):
+                return
~             yield next_id
- -            parents = versionedfile.get_parents(next_id)
+            parents = graph.get_parent_map([next_id])[next_id]
~             if len(parents) == 0:
~                 return
~             else:

Specifically you assume that 'graph.get_parent_map([next_id])' will always
return an entry for 'next_id'.

I would also do:

revision_id = ensure_null(revision_id) # Since it should not be None
and then you can just do:
while next_id != _mod_revision.NULL_REVISION:
~  yield next_id
~  ...



This seems wrong:
@@ -758,8 +768,11 @@
~         version_ids = set(other.versions()).intersection(set(version_ids))
~         # pull in the referenced graph.
~         version_ids = other.get_ancestry(version_ids)
- -        pending_graph = [(version, other.get_parents(version)) for
- -                         version in version_ids]
+        pending_parents = other.get_parent_map(version_ids)
+        pending_graph = pending_parents.items()
+        if len(pending_graph) != len(version_ids):
+            raise RevisionNotPresent(
+                set(version_ids) - pending_parents.keys(), self)

When I try it:

|>> set([1, 2,3]) - [2, 3]
TypeError: unsupported operand type(s) for -: 'set' and 'list'

Which may indicate we have a missing test.

Otherwise:

BB:tweak

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFH4ocWJdeBCYSNAAMRAhDTAKCnUKkIDV377tVessasPqvKNrLgawCgw5k+
DqXbeK6tK9inxxlXhbnk1YU=
=KYhl
-----END PGP SIGNATURE-----



More information about the bazaar mailing list