[MERGE] Packs. Kthxbye.

Robert Collins robertc at robertcollins.net
Thu Oct 18 07:53:53 BST 2007


On Thu, 2007-10-18 at 15:43 +1000, Ian Clatworthy wrote:
> Robert Collins wrote:
> > I think the moment has come to get the pack repository format reviewed.
> 
> Thanks for the changes pushed earlier today. Some more comments below.
> 
> > -        for version in topo_sort(new_parents.items()):
> > +        for version in TopoSorter(new_parents.items()).iter_topo_order():
> 
> IIUC, this is a performance enhancement quite separate to the pack
> changes. Might be good to put this one up separately to get it through
> faster, given there's a bit of reconciling going on right now.

Well, I've done a huge number of split outs like this; at this point I
just want to get the lot done.

> > +        # TODO: jam 20070210 This should be an assert, not a translate
> > +        revision_id = osutils.safe_revision_id(revision_id)
> 
> I think these two lines can go. I'm guessing this code was copied before
> we went through and removed most of the safe_revision_id calls?

Yah, I'll correct.

> > +        self.count_copied = 0
> 
> This looks unnecessary. It might be harmless but it's not clear why it
> required here given fetch.py is the one place that sets this attribute
> for other InterRepo fetches.

This fetch, unlike the ones using fetch.py, is done directly.


>         target_ids = set(self.target.all_revision_ids())
>         possibly_present_revisions = target_ids.intersection(source_ids_set)
>         actually_present_revisions =
> set(self.target._eliminate_revisions_not_present(possibly_present_revisions))

Each repo format has its own improvements. The knit code may be wrong or
doing to much, but its much more obvious for a pack repo when this is
done.



> >  class InterModel1and2(InterRepository):
> >  
> >      @classmethod
> > @@ -2306,10 +2443,15 @@
> >          """Be compatible with Knit1 source and Knit3 target"""
> >          from bzrlib.repofmt.knitrepo import RepositoryFormatKnit3
> >          try:
> > -            from bzrlib.repofmt.knitrepo import RepositoryFormatKnit1, \
> > -                    RepositoryFormatKnit3
> > -            return (isinstance(source._format, (RepositoryFormatKnit1)) and
> > -                    isinstance(target._format, (RepositoryFormatKnit3)))
> > +            from bzrlib.repofmt.knitrepo import (RepositoryFormatKnit1,
> > +                RepositoryFormatKnit3)
> > +            from bzrlib.repofmt.pack_repo import (RepositoryFormatGraphKnit1,
> > +                RepositoryFormatGraphKnit3)
> > +            return (isinstance(source._format, RepositoryFormatKnit1) and
> > +                    isinstance(target._format, RepositoryFormatKnit3) or
> > +                    isinstance(source._format, (RepositoryFormatGraphKnit1)) and
> > +                    isinstance(target._format, (RepositoryFormatGraphKnit3))
> > +                    )
> 
> Any reason why Knit1->Knit3 is fine and GraphKnit1->GraphKnit3 is fine
> but Knit1->GraphKnit3 (for example) is not?

Hmm, I think I was just being cautious; its probably ok. I'll relax it.

-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/fc4f3178/attachment.pgp 


More information about the bazaar mailing list