#1) this may be papered over by Andrew's inventory-delta fixes for streaming over the network, so I think it is prudent that any work being done in this direction takes his patch into account. (wait for it to land or test it on his branch)
#2) It only happens when the target branch is stacked and has ghosts. If you look at the code it is specifically:
if self.target._fallback_repositories:
... parent_ids = set() revision_ids = set()
for revision in pending_revisions: revision_ids.add(revision.revision_id) parent_ids.update(revision.parent_ids) parent_ids.difference_update(revision_ids) parent_ids.discard(_mod_revision.NULL_REVISION) parent_map = self.source.get_parent_map(parent_ids)
for parent_tree in self.source.revision_trees(parent_ids):
... self.target.add_inventory_by_delta( basis_id, delta, current_revision_id, parents_parents)
This wasn't part of my original "IDS" code, but was added by Andrew in:
4257.3.9 Andrew Bennetts 2009-04-09
Add fix to InterDifferingSerializer too, although it's pretty ugly.
Which is why I didn't notice it right away.
Looking closely, it is actually both broken and incorrect. Specifically:
1) We can just use the results of "parent_map" so that we don't try to fetch anything that is considered a ghost.
2) It does something really bad, namely it does:
for parent_tree in self.source.revision_trees(parent_map): basis_id, delta = self._get_delta_for_revision(tree, parent_ids, basis_id, cache)
... self.target.add_inventory_by_delta( basis_id, delta, current_revision_id, parents_parents)
^- Note that it loops over "parent_tree" but generates the delta for *tree*, which it then adds as "current_revision_id" which is actually the parent_tree's revision..... yikes.
So definitely critical, and I should investigate whatever other fixes happened, in case there are Copy & Paste bugs present elsewhere.
So
#1) this may be papered over by Andrew's inventory-delta fixes for streaming over the network, so I think it is prudent that any work being done in this direction takes his patch into account. (wait for it to land or test it on his branch) _fallback_ repositories:
parent_ ids = set()
revision_ ids = set()
revision_ ids.add( revision. revision_ id)
parent_ ids.update( revision. parent_ ids)
parent_ ids.difference_ update( revision_ ids)
parent_ ids.discard( _mod_revision. NULL_REVISION)
parent_ map = self.source. get_parent_ map(parent_ ids) revision_ trees(parent_ ids):
self. target. add_inventory_ by_delta(
basis_ id, delta, current_ revision_ id, parents_parents)
#2) It only happens when the target branch is stacked and has ghosts. If you look at the code it is specifically:
if self.target.
...
for revision in pending_revisions:
for parent_tree in self.source.
...
This wasn't part of my original "IDS" code, but was added by Andrew in: erializer too, although it's pretty ugly.
4257.3.9 Andrew Bennetts 2009-04-09
Add fix to InterDifferingS
Which is why I didn't notice it right away.
Looking closely, it is actually both broken and incorrect. Specifically: revision_ trees(parent_ map):
basis_ id, delta = self._get_ delta_for_ revision( tree, parent_ids, basis_id, cache)
self. target. add_inventory_ by_delta(
basis_ id, delta, current_ revision_ id, parents_parents)
1) We can just use the results of "parent_map" so that we don't try to fetch anything that is considered a ghost.
2) It does something really bad, namely it does:
for parent_tree in self.source.
...
^- Note that it loops over "parent_tree" but generates the delta for *tree*, which it then adds as "current_ revision_ id" which is actually the parent_tree's revision..... yikes.
So definitely critical, and I should investigate whatever other fixes happened, in case there are Copy & Paste bugs present elsewhere.