Code review comment for lp:~zyga/bzr/find_ancestors

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

On 10/10/2012 10:33 PM, Zygmunt Krynicki wrote:
>>> I did see that branch but I recall reading that the solution is
>>> incorrect. I am unable to judge that myself. Could you please
>>> suggest if skipping BTreeBuilder instances makes sense? If not,
>>> should the (can they) implement the same _find_ancestors()
>>> method?
>>
>> The bug is not terribly clear, but as I read it:
>
> This bug can be triggered in a very simple way using
> bzr-fastimport. If this helps I can prepare reproducible
> instructions. This prevents one from using bzr-fastimport to
> consume a patch.

Right, bzr-fastimport is the only code that uses find_ancestry before
the BTreeBuilder content is actually committed to the repository, and
it becomes a normal BTreeIndex. Which is why it only fails for
bzr-fastimport, and we don't have any tests inside bzr that fail (bzr
itself never needs to search the ancestry that it is now adding).

I believe the reason for searching the ancestry in bzr-fastimport is
genuine, and the find_ancestry call is reasonable to implement. I
thought I looked at Wouter's branch and made a comment on it, but it
has been quite a while since I looked at that.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (Cygwin)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iEYEARECAAYFAlB2R3EACgkQJdeBCYSNAAOwkgCcDh63D0ttLH7esE0/e/fXF/0b
i3gAniNjj07MLTiJgK3L62ro31eBdJlU
=YITc
-----END PGP SIGNATURE-----