[MERGE/RFC] MemoryTree and a TreeBuilder helper

[Robert Collins]

On Thu, 2006-09-07 at 15:32 -0500, John Arbash Meinel wrote:
> ...
> 
> v- Because we aren't testing it as an API, shouldn't it be private
> _MutableTree, which is just an implementation detail?

Its a base class for WorkingTree, I think its distateful to have a base
class of a public facility be private. Also, I can add interface tests
for it if you feel thats better - I was on the fence : on the one side
I'm testing the specific behaviours on wt and on memorytree which could
be consolidated. On the other hand, theres still quite some setup to
make interface tests.
...
> > +    For now, we are not treating MutableTree as an interface to provide
> > +    conformance tests for - rather we are MemoryTree specifically, and 
> > +    interface testing implementations of WorkingTree.
> 
> ^- probably you need "rather we test MemoryTree specifically" ...
> ...

'are testing', but yes - thanks.

> v- I know in some ways it would be nice to not care about the file kind
> until you commit. However, don't you need something so that you can
> handle directories versus files, wrt one can contain more files, and the
> other can't?
> Also, it would change our in-memory Inventory layout, because we would
> need a generic InventoryEntry with a parameterized 'kind', rather than
> the inventory hierarchy that we have. I believe we want
> InventoryDirectory, because we have different ways of doing a snapshot()
> etc based on the file kind.

This API still cares about the file kind, its just allowing explicit
overriding. Aarons patch to allow kind changing is related to this
discussion. The most important thing IMO though, is to remember that the
wt inventory is just the map - the disk is the territory, and we need to
consider the inventory as being just a cache of what we last
observed/what we've been told it should be.

> > +        # fill out file kinds for all files [not needed when we stop 
> > +        # caring about the instantaneous file kind within a uncommmitted tree
> > +        #
> > +        self._gather_kinds(files, kinds)
> > +        self._add(files, ids, kinds)
> > +
> > +    def _add(self, files, ids, kinds):
> > +        """Helper function for add - updates the inventory."""
> > +        raise NotImplementedError(self._add)
> 
> ...
> 
> >      def make_bzrdir(self, relpath, format=None):
> >          try:
> > -            url = self.get_url(relpath)
> > -            mutter('relpath %r => url %r', relpath, url)
> > -            segments = url.split('/')
> > +            # might be a relative or absolute path
> > +            maybe_a_url = self.get_url(relpath)
> > +            segments = maybe_a_url.split('/')
> > +            t = get_transport(maybe_a_url)
> 
> If you are only comparing against the last one, it seems like doing
> "base, last_segment = maybe_a_url.rsplit('/', 1)"
> Is more what you want.

Possibly. I actually cribbed this segment from a fix in the hpss branch,
which I'd rather not conflict against - and its coming in with full
tests.

> The rest, I like.


> > === modified file 'bzrlib/tree.py'
> > --- bzrlib/tree.py	2006-08-30 13:54:51 +0000
> > +++ bzrlib/tree.py	2006-09-06 04:37:53 +0000
> > @@ -119,6 +119,18 @@
> >      def id2path(self, file_id):
> >          return self.inventory.id2path(file_id)
> >  
> > +    def is_control_filename(self, filename):
> > +        """True if filename is the name of a control file in this tree.
> > +        
> > +        :param filename: A filename within the tree. This is a relative path
> > +        from the root of this tree.
> > +
> > +        This is true IF and ONLY IF the filename is part of the meta data
> > +        that bzr controls in this tree. I.E. a random .bzr directory placed
> > +        on disk will not be a control file for this tree.
> > +        """
> > +        return self.bzrdir.is_control_filename(filename)
> > +
> 
> ^- Why did you need this here? RevisionTree doesn't have control files.
> Maybe you would want it for MutableTree, it just doesn't seem quite
> relevant to all Tree objects.

As it takes a filename, not a fileid, it seems to me that all trees,
readonly or not, have a legitimate reason to offer the facility. I think
it belongs on Tree.

> Otherwise is seems good enough to me.

Is that +1 ?

-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: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060908/30ff1130/attachment.pgp