Comment 5 for bug 507566

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

...

> I'm missing something. The state I see right now is that:
>
> self._names => 7 items
> but
> self._diff_pack_names returns
>
> disk_nodes => 6 items (missing 75b...)
> new_nodes => empty
> deleted_nodes => empty
>
> I really don't understand how we can read from disk, and then add
> everything in self._names, and come up with less than 7 items...
>

The issue is in the comparisons in _diff_pack_names

It reads the actual disk index {which does not include the new file}
It creates 'new_nodes' from self._names {which does}
It then computes
        deleted_nodes = self._packs_at_load - current_nodes
        new_nodes = current_nodes - self._packs_at_load

However self._packs_at_load *also* contains the new file.
And thus _packs_at_load - current_nodes == empty() and current - at_load
= empty().

And thus nothing gets added to, or removed from, the disk index.

A potential fix is this, I think:
=== modified file 'bzrlib/repofmt/pack_repo.py'
- --- bzrlib/repofmt/pack_repo.py 2010-01-04 06:56:46 +0000
+++ bzrlib/repofmt/pack_repo.py 2010-01-16 03:20:50 +0000
@@ -1991,8 +1991,8 @@
         if first_read:
             return True
         # out the new value.
- - disk_nodes, _, _ = self._diff_pack_names()
- - self._packs_at_load = disk_nodes
+ disk_nodes, _, new_nodes = self._diff_pack_names()
+ self._packs_at_load = disk_nodes - new_nodes
         (removed, added,
          modified) =
self._syncronize_pack_names_from_disk_nodes(disk_nodes)
         if removed or added or modified:

Namely, when we are reloading the pack names, *only* consider names that
are actually in the on-disk pack-names file to be part of
"_packs_at_load", don't include packs that are on disk but not yet
referenced.

On the plus side, if anyone actually hit this, the pack file is still on
disk, and just needs to be referenced from the pack-names file.

Robert, do you think you can follow this and confirm the logic? I'll try
to get a test written, etc.

Note that this isn't specific to 2a, as this is generic packing logic.
So it is true for --pack-0.92 => --2a. (Older versions of bzr probably
didn't suffer this because they didn't support concurrent autopacking
anyway. :)

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

iEYEARECAAYFAktRMT0ACgkQJdeBCYSNAAMnlACfSy5DuVcsmT/wFWzczAYnLty+
fR0AoKLyaJExEx8lnV8DQYu27TzTskxG
=arss
-----END PGP SIGNATURE-----