[MERGE] filtered-deltas - basis for log DIR

Aaron Bentley aaron at aaronbentley.com
Fri Mar 20 14:00:52 GMT 2009


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
> Aaron Bentley wrote:
> 
>>         if self.root is None:
>>> +            return Inventory(root_id=None)
>>> +        other = Inventory(entries.next()[1].file_id)
>>
>>> +        other.root.revision = self.root.revision
>>> +        other.revision_id = self.revision_id
>>> +        directories_to_expand = set()
>>> +        for path, entry in entries:
>>> +            file_id = entry.file_id
>>> +            if (file_id in specific_fileids
>>> +                or entry.parent_id in directories_to_expand):
>>> +                if entry.kind == 'directory':
>>> +                    directories_to_expand.add(file_id)
>> I don't think this is correct.  How are you ensuring that grandparents
>> and great-grandparents, etc. are being retained?
>>
>> bb:comment until this issue is resolved.
> 
> I collect those in the interesting_parents set before starting
> the iteration. The code immediately after the snippet above is ...

Sorry for missing that.  I was unfamiliar with get_idpath.

bb:approve

It would be nice if the tests included a test where a file was moved
from one directory to another.

>> It seems strange for an optimization to be doing a copy.
> 
> Non-chk inventories are mutable so I'm doing the copying to
> be safe.

You mean, because someone might mutate the inventory of the returned
RevisionTree?  That's definitely not acceptable behaviour for using a
RevisionTree.

Or do you mean someone using filter?  That could be addressed by
rejigging filter so it used the current Inventory instead of returning a
new one.

> We could change the semantics of the method though so
> that it returned a 'view' rather than something safe to mutate.
> That would be fine given the way I'm using it in the layer above.
> What do you think? Maybe do that and use a more explicit method
> name?

I don't have a firm grasp on views, so that's up to you.  At this point,
I think we should take the shortest distance from A to B, because this
approach can't have the kind of wins that CHK has anyway.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAknDoZEACgkQ0F+nu1YWqI1UqwCeL1g54LocwWy634lanr0Mf+oZ
vgcAn0yg2X0423q9W5Bqor54T3oUs/ym
=Qlix
-----END PGP SIGNATURE-----



More information about the bazaar mailing list