[MERGE] 'knit' repository format [to reduce size of versioned-file merge]

[Robert Collins]

On Tue, 2006-02-21 at 23:23 -0600, John Arbash Meinel wrote:
> Robert Collins wrote:
> > This patch adds the new repository format and upgrade logic that will be
> > needed to activate knits when the versioned file merge is complete. It
> > adds the UI to upgrade to knits, the logic to convert repositories
> > within a meta-dir and matching tests.
> > 
> > Rob
> > 
> 
> 
> ...
> 
> Rather than continually carry on this semi-hack. Why not just create a
> Repository4, Repository5, Repository6 formats.
> 
> They don't really share a lot of code. (6 could inherit from 5 and just
> set self._prefixed=True.
> 
> It just seems like this code chunk keeps getting moved around. And it
> isn't all that great to start with. It just won't die. (And yes, I wrote
> the initial version :)

We have such formats and the code is being moved out into a cleaner
environment. Note the delegation to concrete formats to assemble the
needed revision store. So the answer is 'yes, when?'

Specifically the three-line conditional blocks are now two-line
conditional blocks. For the MetaDirRepository class they will go
completely as the versioned-file branch merge continues.

> Yipes! Now it has even spawned a friend.
> And why is RepositoryFormatKnit creating a WeaveStore. I thought it had
> to create a KnitStore, or something.

Knits are not yet present in bzr.dev. This work here is orthogonal to
the action of having the format using knits: it tests the cross-format
fetching selector logic, the conversion logic. It can of course be
deferred until we have everything, but it seemed to me you didn't want a
30K line merge request.

> > +
> >  class RepositoryFormat(object):
> >      """A repository format.
> >  
> > @@ -562,6 +602,35 @@
> >          """
> >          raise NotImplementedError(self.get_format_string)
> >  
> 
> something isn't right here. You have '_get_revision_store' raising
> NotImplemented for _get_rev_store. 

typo, thanks.

> And if you have _get_rev_store() it
> seems like the other code should use it, rather than using get_weave()
> and get_store().

_get_rev_store is likely to be quite different for each of format
4/5/6/7 and knit repositories. But _get_revision_store will not be very
different.

get_store and get_weave also vary differently in different ways and I
can't predict exactly how they will look until I have code working with
them. versioned-file adds a revision store api which is different to
TextStore, and implements that for all formats. I could simply move the
code from place A to place B, but that wouldn't clean anything up. The
new goal specific function is part of making this cleaner.


> ...
> 
> This seems like it should at least have a comment that we will be
> replacing the 'from bzrlib.weave import Weave' to importing something else.

Done.
...


> Is this supposed to be a TODO, or just that get_format_string() fails
> for everything that isn't a RepoMetaDir format?
> Again, it seems like using side-effects like this is sub-optimal.
> 
> > +        # this is only useful with metadir layouts - separated repo content.
> > +        # trigger an assertion if not such
> > +        repo._format.get_format_string()

I can add an explicit test if you like. I'm not sure what to call it
though, or if it will be a headache in the future :|. Possibly just the
sound of my brain fizzling out my ears.


> Is the 'has been moved to .bzr.backup' a true statement? At least it
> looks like you are using 'delete_tree()' which would remove
> repository.backup.

Yes.  At the top level we take a full backup. This move and copy is done
within the ./bzr content and will not affect bzr.backup.

> I also thought we were trying to switch to upgrading into '.bzr.new' and
> then renaming into place, rather than renaming out of the way, and then
> upgrading into place.

Uhm, we can do something like that, but I was not working towards that
as part of this. We need the previous format repository to migrate from,
but that may not == the .bzr.backup one if many conversions are
happening at once. So we use the current one, and remove it when we are
done.


> 
> Otherwise, I think it looks good. And I assume this is prep work so that
> you can actually change over to knits in the future. So +1 from me.

Indeed it is. Thanks

Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060222/6842a08b/attachment.pgp