[MERGE] Fix WorkingTree.remove for tree-references and non-cwd trees

[Aaron Bentley]

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

Splitting this into bugfix discussion and general Tree.remove discussion...

John Arbash Meinel wrote:
>> Since recursion is always desired, I changed it to use osutils.walkdirs,
>> which *always* operates in terms of disk kinds.  This means that tree
>> references have the kind "directory", and are recursed into.
> 
> It would seem that that would mean you should be opening up the sub-tree
> and calling subtree.walkdirs().

That's not my approach to nested trees.  I think it would be confusing
if removing a file from tree "foo" could actually remove it from tree
"bar" just because "bar" is a subtree of "foo".

Where possible, I do recursion into subtrees at a high level.  (This
works quite well for "bzr pull", for example.)  Where not possible (e.g.
"bzr mv"), I create a CompositeTree out of foo and bar, and perform the
operation on the CompositeTree.

> Also, unless I miss my guess, using osutils.walkdirs() means that we
> will walk into unversioned directories.

Yes, but that behavior is specified by the test cases.  They will fail
if you don't traverse unversioned directories.

And I don't see how you could delete an directory that contained
unversioned directories unless you recursed into unversioned directories.

> +    def test_remove_subtree(self):
> +        tree = self.make_branch_and_tree('.')
> +        subtree = self.make_branch_and_tree('subtree')
> +        tree.add('subtree', 'subtree-id')
> +        tree.remove('subtree')
> +        self.assertIs(None, tree.path2id('subtree'))
> +
> 
> Do all working tree implementations allow you to add a subtree?

Yes.   The test would fail otherwise.

> Certainly I agree that adding and removing it should end up with it
> unversioned in the final tree, but it seems like some trees should be
> complaining about trying to add a subtree.

Well, they don't now.  If someone wants to start doing that, they can
change this test at that time.

> Other than that, I'm very much +1 on your actual patch. There is just
> much to discuss about what you proposed.

Do my explanations satisfy you, or are there changes you still want?

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

iD8DBQFGYHxZ0F+nu1YWqI0RArrTAJ9o2wGXXSoUqnkg8Tbh7LnncByNtACcD4rd
LsLyl6mfrnGKonlt6MH++WA=
=8ETl
-----END PGP SIGNATURE-----