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

[Aaron Bentley]

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

John Arbash Meinel wrote:
>> Even if I agreed that it made sense for WorkingTree.remove to delete
>> files, I don't think this is a good implementation.
> 
> Where would you recommend removing them at? Especially from a "if I'm
> not removing them on disk, then I shouldn't remove them from the
> inventory" sense (without --force or --keep).

If I agreed that it was a good idea for "bzr remove" to delete files,
I'd suggest a WorkingTree.smart_remove, to parallel the WorkingTree.add
/ smart_add split.  Keep WorkingTree.remove simple.  It's used
frequently in tests, and changing its behavior has had nasty
side-effects on tests in the nested-trees branch.

>> - Instead of taking to_file, it should take a reporter object, so we
>>   aren't stuck with a particular output format.
> 
> Seems reasonable, though we may want to be aware of getting too many
> custom reporters.

We could use the delta._ChangeReporter interface.

> Passing in 'to_file' at least prevents us from forcing
> it to sys.stdout.

I agree, in a "half a loaf is better than nothing" sense.

>> - It uses "unknown" where it means "unversioned", e.g. the
>>   "unknown_files_in_directory" set.
> 
> Well, are you saying "unversioned" as in "they were versioned but now
> they are not", or just that they are not versioned files.

To me, "unversioned" has always meant not-versioned, i.e. does not have
a file-id.

> I'm pretty sure we use the term 'unknown' for that, with 'ignored' being
> a subset of 'unknowns'.

The definition in "bzr help status" and the behavior of "bzr unknowns"
support my understanding of the terms, which is:

- - versioned: has a file-id
- - unversioned: has no file-id
- - ignored: has no file-id and matches an ignore pattern
- - unknown: has no file-id and does not match any ignore pattern

>> - It uses the format '"%s does not exist." % (f,)', when our house style
>>   is to omit the tuple, and use f directly.
> 
> Actually, I always use the tuple. Especially in any circumstance where
> we may not know what the object is. (because if it ends up being a
> tuple, then you get a python error, rather than just a slightly
> different formatting).

Ah.  I hadn't noticed you were doing that.  It's really the least of my
concerns, and you may have a point about failure modes.

>> I spent all afternoon yesterday trying to reimplement it from scratch,
>> but the desired behavior specified in the test cases is just too weird
>> for it to come easily.
> 
> Any particular behavior that seems odd to you?

Listing the children of unversioned directories was one of them.  The
circumstances in which it errors, noisily avoids removing a directory,
or successfully removes it seemed pretty tangled also.

> I don't think the
> behavior (as defined by the tests) is strictly set in stone. But there
> are probably some pieces that we do want to keep.

Well, you guys are the ones who want this behavior there, so you can
decide how it should work.  Just so long as you don't break other things.

> 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.

I think smart_add just won't do it.  But WorkingTree.add has no problem
with it.

> Oh, and "recurse_directory_to_add_files()" seems like it should be
> including sub-directories into its "to-process" list, and should be
> removing unversioned directories from the list that it continues to
> walk. (That would prevent us from recursing into unversioned directories).

You need to be able to recurse into unversioned directories
- - to do bzr rm --force on a directory containing unversioned directories
- - to list the children of unversioned directories as "unversioned files".

If you decide you don't want that behavior, then you can certainly
change recurse_directory_to_add_files the way you suggest.

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

iD8DBQFGYIGE0F+nu1YWqI0RAmC4AJ43HZ7x6J0SetkvHZvRQsaAqWCREQCfQt5Y
hBjeb77iKoA9OwyIUw76DXY=
=fMA+
-----END PGP SIGNATURE-----