[RFC] intertree-implementations

[John Arbash Meinel]

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

Robert Collins wrote:
> On Mon, 2006-07-24 at 21:57 +1000, Robert Collins wrote:
>> This is the first cut at InterTree - I've hooked it up to Tree.compare.
>> From here I plan to expand the tests until all the parameters are
>> present and minimally tested. At that point I intend to deprecate
>> compare_trees as the public interface.
>>
>> I'm sending this in now for comments and feedback..
> 
> Heres an update - this changes the api name to be 'Tree.changes_from'
> which is IMO clearer - it tells you what the order of the parameters is.
> 
> Also attached is my branch to make all the functionality of
> compare_trees available as Tree.changes_from(). This builds on the basic
> implementation in the first patch. It splits Johns new delta tests into
> InterTree tests and WorkingTree tests, depending on whether they depend
> on working tree features like 'unknowns' or not, and adds tests for all
> the features of compare_trees.
> 
> I'm seeking a +1 on both of these now.
> 
> Cheers,
> Rob
> 

...

> +class TestCaseWithTwoTrees(TestCaseWithTree):
> +
> +    def make_to_branch_and_tree(self, relpath):
> +        """Make a to_workingtree_format branch and tree."""
> +        made_control = self.make_bzrdir(relpath, 
> +            format=self.workingtree_format_to._matchingbzrdir)
> +        made_control.create_repository()
> +        made_control.create_branch()
> +        return self.workingtree_format_to.initialize(made_control)
> +
> +    def get_to_tree_no_parents_no_content(self, empty_tree):
> +        return super(TestCaseWithTwoTrees, self).get_tree_no_parents_no_content(empty_tree, converter=self.workingtree_to_test_tree_to)

1) This should be line wrapped at 79 characters.
2) Considering how ugly it is to re-wrap every function in the base
class, it seems like it might make more sense to have a default
converter, which can be set per-class, and which gets used by the
base-class implementation.

> +
> +    def get_to_tree_no_parents_abc_content(self, empty_tree):
> +        return super(TestCaseWithTwoTrees, self).get_tree_no_parents_abc_content(empty_tree, converter=self.workingtree_to_test_tree_to)
> +
> +    def get_to_tree_no_parents_abc_content_2(self, empty_tree):
> +        return super(TestCaseWithTwoTrees, self).get_tree_no_parents_abc_content_2(empty_tree, converter=self.workingtree_to_test_tree_to)
> +
> +    def get_to_tree_no_parents_abc_content_3(self, empty_tree):
> +        return super(TestCaseWithTwoTrees, self).get_tree_no_parents_abc_content_3(empty_tree, converter=self.workingtree_to_test_tree_to)
> +
> +    def get_to_tree_no_parents_abc_content_4(self, empty_tree):
> +        return super(TestCaseWithTwoTrees, self).get_tree_no_parents_abc_content_4(empty_tree, converter=self.workingtree_to_test_tree_to)
> +
> +    def get_to_tree_no_parents_abc_content_5(self, empty_tree):
> +        return super(TestCaseWithTwoTrees, self).get_tree_no_parents_abc_content_5(empty_tree, converter=self.workingtree_to_test_tree_to)
> +
> +    def get_to_tree_no_parents_abc_content_6(self, empty_tree):
> +        return super(TestCaseWithTwoTrees, self).get_tree_no_parents_abc_content_6(empty_tree, converter=self.workingtree_to_test_tree_to)

...

> === modified file 'bzrlib/tree.py'
> --- bzrlib/tree.py	2006-07-25 06:21:42 +0000
> +++ bzrlib/tree.py	2006-07-26 04:04:19 +0000
> @@ -25,6 +25,7 @@
>  from bzrlib.errors import BzrError, BzrCheckError
>  from bzrlib import errors
>  from bzrlib.inventory import Inventory
> +from bzrlib.inter import InterObject
>  from bzrlib.osutils import fingerprint_file
>  import bzrlib.revision
>  from bzrlib.trace import mutter, note
> @@ -50,6 +51,18 @@
>      trees or versioned trees.
>      """
>      
> +    def changes_from(self, other):
> +        """Return a TreeDelta of the changes from other to this tree.
> +
> +        :param other: A tree to compare with.
> +        The comparison will be performed by an InterTree object looked up on 
> +        self and other.
> +        """
> +        # Martin observes that Tree.changes_from returns a TreeDelta and this
> +        # may confuse people, because the class name of the returned object is
> +        # a synonym of the object referenced in the method name.
> +        return InterTree.get(other, self).compare()
> +    

I don't really understand the comment here. "the class name of the
returned object is a synonym of the object referenced in the method name".

Is it that you are passing in a 'Tree' and getting back a 'TreeDelta'?
That seems very reasonable to me.



...


> === renamed file 'bzrlib/tests/test_delta.py' => 'bzrlib/tests/workingtree_implementations/test_changes_from.py'

It seems like you moved the test_delta in to
workingtree_implementations, and then removed most of the tests.
It would seem more accurate to move it where the bulk of the content goes.

...

I suppose you already had a file over here. Anyway, no big deal, just
looked odd.

> === modified file 'bzrlib/tests/intertree_implementations/test_compare.py'
> --- bzrlib/tests/intertree_implementations/test_compare.py	2006-07-24 11:26:40 +0000
> +++ bzrlib/tests/intertree_implementations/test_compare.py	2006-07-26 04:05:41 +0000
> @@ -16,7 +16,7 @@

The rest of the tests look good. +1 from me.

John
=:->

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

iD8DBQFEx+YPJdeBCYSNAAMRAoFdAKCzUTpe8lIhvGqVQu6FtB3z8rV2SACdGgT+
OhXTXmmxm/CtDAj86hvyP0c=
=rL5H
-----END PGP SIGNATURE-----