Rev 2569: Better handling in LockDir of rename that moves one directory within another in http://sourcefrog.net/bzr/dlock

[Martin Pool]

At http://sourcefrog.net/bzr/dlock

------------------------------------------------------------
revno: 2569
revision-id: mbp at sourcefrog.net-20070703072442-y3pwex52rrtsa8gg
parent: mbp at sourcefrog.net-20070703070030-vayxt8668a4nappy
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: dlock
timestamp: Tue 2007-07-03 17:24:42 +1000
message:
  Better handling in LockDir of rename that moves one directory within another
modified:
  bzrlib/lockdir.py              lockdir.py-20060220222025-98258adf27fbdda3
  bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
  bzrlib/transport/brokenrename.py brokenrename.py-20070628050843-mbwebk50srn93rut-1
=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py	2007-07-03 07:00:30 +0000
+++ b/bzrlib/lockdir.py	2007-07-03 07:24:42 +0000
@@ -216,7 +216,7 @@
             self._trace("lock_write...")
             start_time = time.time()
             tmpname = self._create_pending_dir()
-
+    
             self.transport.rename(tmpname, self._held_dir)
             # We must check we really got the lock, because Launchpad's sftp
             # server at one time had a bug were the rename would successfully
@@ -258,7 +258,6 @@
             self.create(mode=self._dir_modebits)
             # After creating the lock directory, try again
             self.transport.mkdir(tmpname)
-        
         self.nonce = rand_chars(20)
         info_bytes = self._prepare_info()
         # We use put_file_non_atomic because we just created a new unique
@@ -291,7 +290,19 @@
             self.transport.rename(self._held_dir, tmpname)
             self._lock_held = False
             self.transport.delete(tmpname + self.__INFO_NAME)
-            self.transport.rmdir(tmpname)
+            try:
+                self.transport.rmdir(tmpname)
+            except DirectoryNotEmpty, e:
+                # There might have been junk left over by a rename that moved
+                # another locker within the 'held' directory.  do a slower
+                # deletion where we list the directory and remove everything
+                # within it.
+                #
+                # Maybe this should be broader to allow for ftp servers with
+                # non-specific error messages?
+                self._trace("doing recursive deletion of non-empty directory "
+                        "%s", tmpname)
+                self.transport.delete_tree(tmpname)
             self._trace("... unlock succeeded after %dms",
                     (time.time() - start_time) * 1000)
 

=== modified file 'bzrlib/tests/test_lockdir.py'
--- a/bzrlib/tests/test_lockdir.py	2007-06-28 08:09:01 +0000
+++ b/bzrlib/tests/test_lockdir.py	2007-07-03 07:24:42 +0000
@@ -641,13 +641,16 @@
         t = transport.get_transport('brokenrename+' + self.get_url())
         ld1 = LockDir(t, 'test_lock')
         ld1.create()
+
+        import pdb;pdb.set_trace()
         ld1.attempt_lock()
         ld2 = LockDir(t, 'test_lock')
-        # we should notice that we failed to lock, even though the transport
-        # has the wrong rename behaviour
-        e = self.assertRaises(errors.LockError, ld2.attempt_lock)
-        self.assertContainsRe(str(e),
-                "rename succeeded, but lock is still held by someone else")
+        # we should fail to lock
+        e = self.assertRaises(errors.LockContention, ld2.attempt_lock)
+        # now the original caller should succeed in unlocking
+        ld1.unlock()
+        # and there should be nothing left over
+        self.assertEquals([], t.list_dir('test_lock'))
 
     def test_failed_lock_leaves_no_trash(self):
         # if we fail to acquire the lock, we don't leave pending directories

=== modified file 'bzrlib/transport/brokenrename.py'
--- a/bzrlib/transport/brokenrename.py	2007-06-28 06:00:52 +0000
+++ b/bzrlib/transport/brokenrename.py	2007-07-03 07:24:42 +0000
@@ -19,7 +19,10 @@
 
 from stat import S_ISDIR
 
-import bzrlib.errors as errors
+from bzrlib import (
+        errors,
+        urlutils,
+        )
 from bzrlib.transport.decorator import TransportDecorator, DecoratorServer
 
 
@@ -35,6 +38,8 @@
         """See Transport.rename().
         """
         try:
+            if self._decorated.has(rel_to):
+                rel_to = urlutils.join(rel_to, urlutils.basename(rel_from))
             self._decorated.rename(rel_from, rel_to)
         except (errors.DirectoryNotEmpty, errors.FileExists), e:
             # absorb the error