[MERGE] Packs. Kthxbye.
Robert Collins
robertc at robertcollins.net
Wed Oct 17 21:35:59 BST 2007
On Wed, 2007-10-17 at 23:44 +1000, Ian Clatworthy wrote:
> Robert Collins wrote:
> > I think the moment has come to get the pack repository format reviewed.
>
> A big congratulations Robert. Great to see this up for review.
>
> I'm planning to tackle this in pieces. The first bit of feedback is below.
Thanks...
> bb:comment
>
> Most of the code I've read so far in this patch looks fine.
>
> > +class RepositoryFormatPack(MetaDirRepositoryFormat):
> > + """Format logic for pack structured repositories.
> > +
> > + This repository format has:
> > + - a list of packs in pack-names
> > + - packs in packs/NAME.pack
> > + - indices in indices/NAME.{iix,six,tix,rix}
> > + - knit deltas in the packs, knit indices mapped to the indices.
>
> Hooray!
>
> > +class RepositoryFormatGraphKnit1(RepositoryFormatPack):
> > + """Experimental pack based repository with knit1 style data.
> > +
> > + This repository format has:
> > + - knits for file texts and inventory
> > + - hash subdirectory based stores.
> > + - knits for revisions and signatures
> > + - uses a GraphKnitIndex for revisions.knit.
> > + - TextStores for revisions and signatures.
>
> This needs updating - it's a copy directly from RepositoryFormatKnit1.
> You want to copy-and-paste from RepositoryFormatPack instead I think.
I've made it
"""A no-subtrees parameterised Pack repository.
This format was introduced in bzr.dev.
"""
and for Knit3
"""A subtrees parameterised Pack repository.
This repository format uses the xml7 serializer to get:
- support for recording full info about the tree root
- support for recording tree references
This format was introduced in bzr.dev.
"""
> > + def __ne__(self, other):
> > + return self.__class__ is not other.__class__
>
> While this method is directly copied out of RepositoryFormatKnit1, I
> don't like it. Wouldn't it to be better to:
>
> 1. Move it up the class hierarchy to RepositoryFormatPack so that it
> applies to RepositoryFormatGraphKit3 as well as this class?
>
> 2. Use the normal idiom: define an __eq__ method and then
> implement the __ne__ method in terms of it?
I've deleted it, running tests to see what breaks now.
> > === modified file 'bzrlib/index.py'
> > --- bzrlib/index.py 2007-10-15 07:56:04 +0000
> > +++ bzrlib/index.py 2007-10-17 09:39:41 +0000
> > @@ -267,6 +267,17 @@
> > self._nodes_by_key = None
> > self._size = size
> >
> > + def __eq__(self, other):
> > + """Equal when self and otherwere created with the same parameters."""
>
> A space is missing between other and were.
Done.
> So set_writer can now be called with an index of None? I probably ought
> to run annotate and see which commit in your branch made this change and
> why. I'm flagging it here in case it prompts Martin or you to say "That
> was a temporary change." If it's here to stay, perhaps a comment
> wouldn't hurt explaining when it makes sense to call set_writer without
> an index. (If the index can really be None, I'll go through and check
> the lookups of self.indices to confirm that None is handled correctly
> there.)
It always has been able to be called with an index of None, its how the
writer is disabled.
> try/except/finally needs to be nested in Python 2.4.
A bug - was meant to be try:except:else.
-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/20071018/6edd6551/attachment.pgp
More information about the bazaar
mailing list