upgrading branch on launchpad to 2a resulted in missing revision error

Here's the repo that failed conversion.

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

Monty Taylor wrote:
> Public bug reported:
>
>
> $ bzr upgrade --2a lp:~mordred/libcpuinfo/add-bindings
> starting upgrade of bzr+ssh://bazaar.launchpad.net/%7Emordred/libcpuinfo/add-bindings/
> making backup of bzr+ssh://bazaar.launchpad.net/%7Emordred/libcpuinfo/add-bindings/.bzr
> to bzr+ssh://bazaar.launchpad.net/%7Emordred/libcpuinfo/add-bindings/backup.bzr
> starting repository conversion
> ERROR: Revision {('autotools-20090613010730-o7cmjnev3rcn0yjz-1', '<email address hidden>')} not present in "<bzrlib.groupcompress.GroupCompressVersionedFiles object at 0x2cf8f10>".
>
> ** Affects: bzr
> Importance: Undecided
> Status: New
>

So that was traditionally caused in the past by converting with knit
deltas into the gc format and not either sorting topologically, or
requesting fulltexts.

Is it possible to get the full traceback (should be in ~/.bzr.log)

When I grab your backup.bzr file it turns out that this is a stacked branch.

I'm not sure we support upgrading a stacked branch without upgrading the
stacked-on location (yet).

Anyway, I'm pretty sure that is fairly relevant to what is going on.

John
=:->

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

iEYEARECAAYFAkpcdf8ACgkQJdeBCYSNAAPqbACg08CMiyGFDOHFaf8Ro/TQBJ6I
9uEAn03KG5u6/JGCjDyigokx6w8sTCAh
=HAhi
-----END PGP SIGNATURE-----

Here's my .bzr.log file...

On Tue, 2009-07-14 at 12:11 +0000, John A Meinel wrote:
>
> I'm not sure we support upgrading a stacked branch without upgrading
> the
> stacked-on location (yet).

We do; we even have tests that we do. Doesn't mean its flaw-free ;P

I upgraded the trunk from 1.9-rr to 2a, monty went to upgrade his branch
and it went boom.

-Rob

I'm pretty sure I'm hitting this with bzr-hookless-email, upgrading a
stacked-upon branch from 1.9 to 2a works fine, but then upgrading the stacked
branch fails.

Sat 2009-07-25 21:28:21 +0200
0.131 bzr arguments: [u'upgrade', u'--2a', u'lp:~larstiq/+junk/1.9-stacked-on-trunk']
0.203 looking for plugins in /home/larstiq/.bazaar/plugins/dev
0.417 looking for plugins in /media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/plugins
0.451 looking for plugins in /usr/lib/python2.5/site-packages/bzrlib/plugins
0.578 encoding stdout as sys.stdout encoding 'UTF-8'
1.056 bzr-svn: using Subversion 1.5.1 ()
1.168 opening SVN RA connection to 'bzr+ssh://bazaar.launchpad.net/~larstiq/%2Bjunk/1.9-stacked-on-trunk'
1.199 ssh implementation is OpenSSH
6.092 opening SVN RA connection to 'bzr+ssh://bazaar.launchpad.net/~larstiq/%2Bjunk/hookless-trunk'
11.689 creating repository in bzr+ssh://bazaar.launchpad.net/~larstiq/%2Bjunk/1.9-stacked-on-trunk/.bzr/.
13.858 Traceback (most recent call last):
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/commands.py", line 835, in exception_to_return_code
    return the_callable(*args, **kwargs)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/commands.py", line 1030, in run_bzr
    ret = run(*run_argv)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/commands.py", line 647, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/builtins.py", line 3145, in run
    upgrade(url, format)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/upgrade.py", line 85, in upgrade
    Convert(url, format)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/upgrade.py", line 40, in __init__
    self.convert()
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/upgrade.py", line 79, in convert
    self.bzrdir = converter.convert(self.bzrdir, self.pb)
  File "bzrlib/bzrdir.py", line 3010, in convert
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/repository.py", line 3831, in convert
    self.source_repo.copy_content_into(converted)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/repository.py", line 1413, in copy_content_into
    return InterRepository.get(self, destination).copy_content(revision_id)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/repository.py", line 3190, in copy_content
    self.target.fetch(self.source, revision_id=revision_id)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/repository.py", line 1566, in fetch
    find_ghosts=find_ghosts, fetch_spec=fetch_spec)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/repository.py", line 3757, in fetch
    self._fetch_all_revisions(revision_ids, pb)
  File "/media/links/home/larstiq/src/bzr/bzr.dev/bzrlib/repository.py", line 3711, in _fetch_all_revisions
    basis_id = self._fetch_batch(batch, basis_id, cache)
  File "/media/links/home/larstiq/s...

John, have your recent changes to InterDifferingSerializer fixed this?

If I grab Monty's branch by SFTP and try upgrading it locally, I get the same error.

However, if I use bzr.dev, and add the -DIDS_never flag, it appears to upgrade successfully (including passing bzr check). So using bzr.dev with -DIDS_never may be a viable workaround.

This is a bug in the way InterDifferingSerializer deals with stacked sources. The main issue here is that the way InterDifferingSerializer calculates the text keys to send isn't quite right. It generates a probably-shortest delta, and then assumes every text key referenced in that needs to be transferred. That doesn't align with reality when the source is a stacked branch... there can be many text keys in the delta that logic generates that legitimately do not appear in the source repository.

In fact, with a stacked source opened by upgrade there will be no fallbacks configured. So the first revision InterDifferingSerializer tries to fetch will have no parent revision present, so it will generate a delta from the null revision for the first revision it wants to transfer (even though a parent inventory is present), and thus it will attempt to send every text in the complete inventory for that revision!

The actual rule for which text keys must be present for a given revision , as described by Robert elsewhere, is: "all text versions not present in any parent".

So, for each batch IDS processes, I think what we want to do is something more like:

 - check for present parent inventories for absent parent revisions, and transfer those first (as these are probably there to maintain the stacking invariants)
 - build up a set of text_keys from the deltas we calculate, but filter out any text_key that we can find in any basis inventory for this revision

I have a slightly hackish branch that does this, and it successfully upgrades the libcpuinfo branch of the original bug report. (Upgrades succeeds; check passes cleanly, and all inventory, revision and text keys in the original repo are present after upgrade).

(A perhaps simpler approach conceptually might be to change InterDifferingSerializer's logic when revision_ids=None (i.e. we are copying the entire repository) to stop calculating the set of text keys to transfer etc, and simply unconditionally transfer every inventory and every text present in the source (while still taking care to generate rich roots as needed, of course). This would probably be faster and might be more robust, but would be a larger departure from the current design. Or maybe the "fetch entire repository" case could be safely delegated to the generic StreamSource code? Something to look into later...)