[MERGE] TreeTransform rolls back on exception (#67699)

[Aaron Bentley]

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

Ian Clatworthy wrote:
> The only change required to merge is being more defensive about the
> pending-deletions directory already existing.

Sure.

> Suggestion: delete 2 items in one or more of these tests. That would
> confirm that more than 1 item is handled correctly, particularly if you
> make the deletion order important. (The code does the right thing in
> terms of processing deletions in reverse order but the tests don't
> exercise that logic.)

The code does not guarantee deletion in any particular order.
pending-deletions has no nesting-- all the items are in
pending-deletions, not in subdirectories.  This means that delete-order
is not important.  In fact, they are deleted in *forward* order, not
reverse order.

>> +    def test_file_mover_rollback(self):
>> +        self.build_tree(['a/', 'a/b', 'c/', 'c/d/', 'c/e/'])
>> +        mover = _FileMover()
>> +        mover.rename('c/d', 'c/f')
>> +        mover.rename('c/e', 'c/d')
>> +        try:
>> +            mover.rename('a', 'c')
>> +        except OSError, e:
>> +            mover.rollback()
>> +        self.failUnlessExists('a')
>> +        self.failUnlessExists('c/d')
> 
> Suggestion: test that c/f doesn't exist on completion.

>> +    def test_rollback_rename(self):
>> +        tree = self.make_branch_and_tree('.')
>> +        self.build_tree(['a/', 'a/b'])

> Suggestion: check c and c/d don't exist post the unsuccessful apply; a
> and a/b don't exist on completion.

We shouldn't be testing everything in every test.  They should build on
one another.  The unit tests for _FileMover should establish what a
rollback entails.  The purpose of this test is to establish that
tt.apply will do a rollback.

> 
> Suggestion: I think past_renames and pending_deletions should be cleared
> on completion of both rollback() and apply_deletions(). Perhaps add a
> _clear_queues() method that gets called from __init__() and those
> routines?

It's supposed to be a run-once operation.  I'd rather set them to None.

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

iD8DBQFG1OUr0F+nu1YWqI0RAsnkAJ4kDzOEzMH5PlQKWcuyWWQp7+nIIwCeNFnY
uvOpzIFQwqFFcSuKgRJJves=
=L9Dy
-----END PGP SIGNATURE-----