Comment 3 for bug 402778

Revision history for this message
John A Meinel (jameinel) wrote : Re: NoSuchRevision error when branching with rocketfuel-get

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)
#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.