indexerror in Knit._get_components_positions pulling in pack repo

I'm not following this closely but I thought I'd mention a bit of diagnosis. I think the lines in question are:
                method = self._index.get_method(cursor)
                if method == 'fulltext':
                    next = None
                else:
                    next = self.get_parents(cursor)[0]

Which would indicate that we have a record with no parents that is not marked as a fulltext. This shouldn't be possible because we don't have anything to delta against if we don't have any parents. (So it should *have* to be a fulltext).

It may be that Packs change that logic, though I'm not sure how. We certainly could do:

parents = self.get_parents(cursor)
if not parents:
  next = parents[0]

However, that would miss the case where we have the fulltext cached *and* parents. And it seems like a data corruption to have no parents but no fulltext.

Could it just be that http://people.ubuntu.com/~robertc/baz2.0/repository is broken?

On Fri, 2007-10-19 at 19:45 +0000, John A Meinel wrote:
>
> Could it just be that
> http://people.ubuntu.com/~robertc/baz2.0/repository is broken?

Is broken. And this has been documented on the list many times:).

bzr.dev was broken when I started the work on packs. Aaron wrote a patch
for check and reconcile to detect this, and I later found more index
issues which Andrew added to the patch. This patch has been merged, but
I have not regenerated my repository yet.

To play with packs:
pull bzr.dev to a normal new branch.
./bzr reconcile
./bzr upgrade --experimental

Cheers,
Rob
--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

For other people following along, the recipe robert gives will let you play with packs, but it's apparently not enough to let you successfully pull from his baz2.0 repository.

I've written a smaller/faster version of check in https://code.edge.launchpad.net/~bzr/bzr-check-knits/trunk which will let you know if a particular repo is affected by this problem.

Robert suggests that pull should do a check after all the data has been brought in, but before finishing the write group, and this should make sure that all the text dependencies for new knit revisions being added are present.

When this fails, it looks like this:

checking knits of GraphKnitRepository('file:///home/mbp/newbzr/robertc-repository-packs-unrec/.bzr/repository/')
in store GraphKnitTextStore('file:///home/mbp/newbzr/robertc-repository-packs-unrec/.bzr/repository/knits/')
  BAD
  file: join-branches.txt-20050309044946-f635037a889c0388
  verson: <email address hidden>
Knit KnitVersionedFile(file:///home/mbp/newbzr/robertc-repository-packs-unrec/.bzr/repository/text%3Ajoin-branches.txt-20050309044946-f635037a889c0388) corrupt:
version <email address hidden>
    has compression method 'line-delta' but index parents ()

so if this is the case, just checking that the dependencies are there won't be enough.

and that message is with this patch applied

=== modified file 'bzrlib/knit.py'
--- bzrlib/knit.py 2007-10-22 01:23:51 +0000
+++ bzrlib/knit.py 2007-10-22 06:50:29 +0000
@@ -815,7 +815,17 @@
                 if method == 'fulltext':
                     next = None
                 else:
- next = self.get_parents(cursor)[0]
+ parents = self.get_parents(cursor)
+ try:
+ next = parents[0]
+ except IndexError:
+ raise KnitCorrupt(
+ self,
+ "\nversion %s\n"
+ " has compression method %r " \
+ "but index parents %r " % (
+ cursor, method, parents))
+ next = parents[0]
                 index_memo = self._index.get_position(cursor)
                 component_data[cursor] = (method, index_memo, next)
                 cursor = next

Looking with Robert at the knit indices: the version that's raising this error *does* have a pointer to a compression parent, but that parent is marked 'a' for absent in the index, and that may be because it is not referenced by the revision graph or inventory graph.

_get_components_positions should actually be calling get_parents_with_ghosts, or (better) get_compression_parents, which can return both the parent(s) and the method. If we didn't exclude ghosts, then we'd get a more obvious error about not being able to reconstruct the parent.

+1 to the patch which at least gives a better error.

Closing this bug if it was due to a quirky/invalid source repository.