[MERGE] don't invoke os.rename on children with no contents

[Robert Collins]

+1 conditional

On Tue, 2007-06-05 at 18:34 -0400, Aaron Bentley wrote:

I think this test method could have a clearer name - perhaps

test_cancel_with_deleted_child_should_succeed

> +    def test_cancel_no_content_child(self):
> +        transform, root = self.get_transform()
> +        parent = transform.new_directory('parent', root)
> +        child = transform.new_directory('child', parent)
> +        transform.cancel_creation(child)
> +        try:
> +            transform.cancel_creation(parent)
> +        except OSError:
> +            self.fail('Transform tried to move a deleted child')

I am guessing the try:except: here is to provide a clear error when the
test fails, but perhaps just
    transform.cancel_creation(child)
    transform.cancel_creation(parent)

is clear enough? It could after all raise many different errors if
various bugs are added, and catching all OSError and interpreting them
as this specific defect seems error prone in the case of other future
changes. I'd rather see the original exception for debugging.

> +        transform.finalize()
> +
>  
>  class TransformGroup(object):
>      def __init__(self, dirname, root_id):
> 
> === modified file bzrlib/transform.py
> --- bzrlib/transform.py
> +++ bzrlib/transform.py
> @@ -208,7 +208,8 @@
>          for trans_id in trans_ids:
>              old_path = self._limbo_files[trans_id]
>              new_path = self._limbo_name(trans_id, from_scratch=True)
> -            os.rename(old_path, new_path)
> +            if trans_id in self._new_contents:
> +                os.rename(old_path, new_path)

Wouldn't:
    for trans_id in trans_ids:
        if trans_id not in self._new_contents:
            continue
        old_path = self._limbo_files[trans_id]
        new_path = self._limbo_name(trans_id, from_scratch=True)
        os.rename(old_path, new_path)

be better? Less calculations for the its-missing-case, and the same
number of checks for the its-present-case.

-Rob
 

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070606/9e0df392/attachment.pgp