[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