[Merge][#111665] bzr rm should remove clean subtrees

[Marius Kruger]

hi,

this bundle partially addresses bug #111665
'bzr rm should remove clean subtrees'

the following is fixed:
* bzr rm should remove (without needing --force) subtrees that contain
no files with text changes or modified files.
* With --force we should remove the subtree regardless of text changes
or unknown files.
* remove with renames in and out.

I'll be happy to do the following in a followup bundle (unless you want
it now):
* remove unversioned ignored files  (without force)

regards
marius


Martin Pool has voted resubmit.
Comment:
...
> +        self.assertInWorkingTree(files)
> +        self.failUnlessExists(files)
> +        self.build_tree(['b/unknown_directory/',
> 'b/unknown_directory/ud2/'])
> +        tree.remove('b', keep_files=False)
> +        self.assertNotInWorkingTree('b')
> +        self.failIfExists('b')
>
>  This seems to be saying that we will delete directories which contain
>  unknown non-files under them.  Is that correct?  That seems a bit
>  dangerous; we should probably not delete directories that have any
>  unknown things under them, whether files or directories.   Suppose I
>  have a new directory that I meant to add but have not that contains a
>  lot of precious files.

I misinterpreted a part of the bug, I thought you would like EMPTY
directories
to be removed without needing --force.
Ok, so I'll change it to require force to remove empty unknown directories.
 

>                   if osutils.lexists(abs_path):
>                       if (osutils.isdir(abs_path) and
>                           len(os.listdir(abs_path)) > 0):
> -                        message="%s is not empty directory "\
> -                            "and won't be deleted." % (f,)
> +                        if force:
> +                            osutils.rmtree(abs_path)
> +                        else:
> +                            message = "%s is not an empty directory "\
> +                                "and won't be deleted." % (f,)
>
> How does this work when removing the subtree?  
> Have the contents been
> removed by the time we get here?

Yes, that's why we need to sort the paths in reverse order.

 
> @@ -1834,26 +1839,29 @@
>                        kind, executable) in
> self._iter_changes(self.basis_tree(),
>                            include_unchanged=True,
> require_versioned=False,
>                            want_unversioned=True, specific_files=files):
> -                    # check if it's unknown OR changed but not deleted:
> -                    if (versioned == (False, False)
> -                        or (content_change and kind[1] != None)):
> +                    # check if it's an unknown OR changed file:
> +                    if kind[1] == 'file' and ( versioned == (False,
> False)
> +                        or content_change ):
>                           has_changed_files = True
>                           break
>
> I'm not sure why we're checking the kind?

I thought that only files could really have precious changes.
So I was thinking that if its not a file its not so serious if we lost
changes...
...mm, Ok I'll get it back to complain about changes for any kind.
(It is related to the checking for empty directories in any case)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: rm_should_remove_clean_trees2.patch
Type: text/x-diff
Size: 29434 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070810/d7733294/attachment-0001.bin