[bug] Repository.copy() copies inventory first

[John A Meinel]

Robert Collins wrote:
> On Wed, 2006-02-15 at 23:24 -0600, John A Meinel wrote:
>> I was trying to track down why 'bzr get' takes so long to display what
>> is going on. I think I found the reason, and at the same time, I found a
>> bug.
>>
>> Here is the code for Repository.copy():
>>
>>     @needs_read_lock
>>     def copy(self, destination):
>>         destination.lock_write()
>>         try:
>>             destination.control_weaves.copy_multi(self.control_weaves,
>>                 ['inventory'])
>>             copy_all(self.weave_store, destination.weave_store)
>>             copy_all(self.revision_store, destination.revision_store)
>>         finally:
>>             destination.unlock()
>>
>> You can see that it copies inventory.weave first, which can take a long
>> time, and also shouldn't be done before it copies the rest of the weave
>> store. So I think this needs to be changed to put the inventory copy
>> after the weave copy.
> 
> Definately a bug.
> 
>> And the other reason why nothing is displayed for far too long is that
>> we have the delay of listing the remote repository, and then the delay
>> of copying the repository weaves. (WeaveStore.copy_multi() has no
>> progress bar).
> 
> I think it should.
> 
> 
> +1.
> 
> 
> Rob

The code is a little bit different after merging your bzrdir work. I
eventually tracked the location where we actually create stuff. The path
seems a little bit convoluted, and we go through 3+ indirections, but it
is possible to follow. (It isn't helped by calling
BzrBranch.copy_content_into which doesn't copy any content, it just sets
the revision history, and parent, and it sets the parent to
source.get_parent() rather than setting it to source. That seems like a
bug to me.
But then again on closer inspection 'sprout()' calls copy_content_into,
and then afterwards overrides the fact that copy_content sets the
parent, by setting the parent itself. So I must ask 'why does
copy_content_into' set parent?
I suppose BzrBranch.clone() uses it. But if nobody else does, it seems
like setting the parent should be done inside clone() and sprout().


Anyway, I can easily add the progress bars. I'm just a little concerned
because I'm not 100% positive when the inventory.weave is being copied
across....

Found it!!! And the bug is still present. So I'll fix it. (revision.py
line 270, we still are copying the inventory weave before we copy the
rest of the weaves).

So attached is the revised version, which I would like to commit to
jam-integration. Can someone look it over?

John
=:->

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: branch-progress.diff
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20060216/18941789/attachment.diff 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060216/18941789/attachment.pgp