[brisbane-core/MERGE] fix create_by_apply_delta() when rename+modify in same delta

ian.clatworthy at internode.on.net ian.clatworthy at internode.on.net
Thu Mar 5 23:27:10 GMT 2009



 On Fri 06/03/09  1:02 AM , John Arbash Meinel john at arbash-meinel.com sent:

> ian.clatworthy at internode.on.net wrote:
> > Yet another create_by_apply_delta() fix. A good inventory delta was generating
> > a bad chkmap delta for the parent_id_basename_to_file_id CHKMap.

> I'm trying to understand this change. I can sort of see where it is
> coming from, but it feels like it is covering up a different bug.

> Consider, though, what happens if you have a rename into a new
> directory, rather than just a rename within a directory:
> 
> ('foo/bar', 'bla/did', 'bar-id', IFile(sha1='1234', p_id='bla-id'))
> ('bla/did', 'bla/did', 'bar-id', IFile(sha1='5678', p_id='bla-id'))
> 
> In that case, with your new code it would be:
> 
> (('foo-id', 'bar'), ('bla-id', 'did'), 'bar-id')
> (('foo-id', 'did'), ('bla-id', 'did'), 'bar-id')
> 
> ^^^^^^^^ bad mojo, as ('foo-id', 'did') never existed.
> 
> So my gut feeling is that an inventory-delta needs to only talk about a
> given file_id 1 time. If that isn't how it is defined, then we need to
> be more careful. Because in that case 'old_path' is no longer the 'path
> in the previous inventory', it is now the 'path in the inventory just
> until this change occurs'. So we actually need to be looking things up
> in the *new* inventory, not the old one.

John,

Thanks for the review and detailed analysis.  I agree.

So, I'll tighten up the documentation for inventory detlas, at least as
used by create_by_apply_delta(). I need to do that anyway to
explain exactly when the root needs to be included.

Ian C.




More information about the bazaar mailing list