Conversion to --2a broken when upgrading into a stacked branch

I believe that this is triggered when a 2a repo is used and pygettextpo
is pulled into it - it indicates a bug in the conversion logic.

-Rob

A second example..

ataraxia% bzr branch --no-tree --stacked lp:launchpad /tmp
Created new stacked branch referring to http://bazaar.launchpad.net/~launchpad-pqm/launchpad/db-devel/.
ataraxia% cd launchpad /tmp
ataraxia(bzr)% bzr pull --overwrite lp:~launchpad-pqm/launchpad-cscvs/devel/
bzr: ERROR: bzrlib.errors.NoSuchRevision: KnitPackRepository('bzr+ssh://bazaar.launchpad.net/~launchpad-pqm/launchpad-cscvs/devel/.bzr/repository/') has no revision ('Arch-1:<email address hidden>%cscvs--devel--1.0--patch-16',)

Traceback (most recent call last):
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/commands.py", line 835, in exception_to_return_code
    return the_callable(*args, **kwargs)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/commands.py", line 1030, in run_bzr
    ret = run(*run_argv)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/commands.py", line 647, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/builtins.py", line 1016, in run
    branch_from, overwrite, revision_id, local=local)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/branch.py", line 891, in pull
    possible_transports=possible_transports, *args, **kwargs)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/branch.py", line 3154, in pull
    _override_hook_target=_override_hook_target)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/branch.py", line 3031, in pull
    overwrite=overwrite, graph=graph)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/branch.py", line 839, in update_revisions
    overwrite, graph)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/branch.py", line 2974, in update_revisions
    self.target.fetch(self.source, stop_revision)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/branch.py", line 565, in fetch
    pb=pb)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/repository.py", line 1544, in fetch
    find_ghosts=find_ghosts, fetch_spec=fetch_spec)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/repository.py", line 3735, in fetch
    self._fetch_all_revisions(revision_ids, pb)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/repository.py", line 3689, in _fetch_all_revisions
    basis_id = self._fetch_batch(batch, basis_id, cache)
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/repository.py", line 3651, in _fetch_batch
    for parent_tree in self.source.revision_trees(parent_ids):
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/repository.py", line 2373, in revision_trees
    for inv in inventories:
  File "/home/dsilvers/dev-bzr/bzr.dev/bzrlib/repository....

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.

Fix in associated branch. The IDS code that handles sending parent inventories for stacked branches now also
1) sends correct deltas
2) does not try to fill in parents who are ghosts in the source

The fix has been merged into bzr.dev, I believe. (Launchpad certainly thinks the associated branch is merged)