[BUG] bug in weave.join() code

[John Arbash Meinel]

Martin Pool wrote:
> On 24 Jan 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
> 
>>I found a pretty large bug in the Weave.join() code, which causes the
>>code to freak out quite a bit. Specifically the code:
>>
>>if name in self._names:
>>    idx = self.lookup(name)
>>    n1 = map(other.idx_to_name, other._parents[other_idx] )
>>    n2 = map(self.idx_to_name, self._parents[other_idx] )
>>    if sha1 ==  self._sha1s[idx] and n1 == n2:
>>        continue
>>
>>Notice that it uses other_idx instead of 'idx' so this should be:
>>    n2 = map(self.idx_to_name, self._parents[idx] )
> 
> 
> Yes, that does seem to be a bug.
> 
> We should probably change it to ask the two weaves for e.g.
> parent_names() and not use indexes at this level.
> 
> It seems likely it will almost always just cause it not to use the
> optimization but there's some chance it will fail to add the new
> revision when it should.

Well, the chance of not adding a revision is extremely small. Because it
still does the name check. (Though it should probably use _name_map and
not names, since that is a hash lookup rather than a sequential search
through a list).


> 
> 
>>Also, in testing, I found that we still have a lot of mismatched
>>parents. Specifically we have inverted parent heirarchy. For example:
>>
>> inventory merge 3189/3202 0:00:01
>>version {robertc at robertcollins.net-20060121104246-604e9a6a3eb8e10b}
>>present, but different parents:
>>['robertc at robertcollins.net-20060121095439-042130d88768fcb2',
>>'john at arbash-meinel.com-20060120193409-1466a4908863d28f'] vs
>>['john at arbash-meinel.com-20060120193409-1466a4908863d28f',
>>'robertc at robertcollins.net-20060121095439-042130d88768fcb2']
>>All changes applied successfully.
> 
> 
> I don't think this message is emitted by my tree - does it come out when
> trying to add an inventory to the weave file?   I think we should treat
> the order in the revision object as definitive.  I don't know if the
> weave at the moment cares to enforce that ordering, but it probably
> should.  And check should probably make sure that is the case.
> 

No, this is an extra message I added, because I was finding that bzr was
reweaving my inventory. I tracked it down to one of Aaron's commits that
had 2 parents with the same id. (So one weave thought of it as only
having 1 parent, and the other thought it was 2 which happened to be
identical).
As I searched further, I fixed that problem, and eventually found that
the reweave code is using a set() and thus causing ordering of parents
to be lost.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060124/d7a986f2/attachment.pgp