Rev 6062: (mbp) tolerate empty limbo and pending-deletion directories (bug 427773) in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/2.4/

[Patch Queue Manager]

At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/2.4/

------------------------------------------------------------
revno: 6062 [merge]
revision-id: pqm at pqm.ubuntu.com-20111201013001-346ixnbkd9n7d6be
parent: pqm at pqm.ubuntu.com-20111118010254-bmlsxny8e3xo7pj0
parent: mbp at canonical.com-20111201010306-lwk9pd34y1uk09nd
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.4
timestamp: Thu 2011-12-01 01:30:01 +0000
message:
  (mbp) tolerate empty limbo and pending-deletion directories (bug 427773)
   (Martin Pool)
modified:
  bzrlib/osutils.py              osutils.py-20050309040759-eeaff12fbf77ac86
  bzrlib/tests/per_merger.py     per_merger.py-20091216002111-bzeo6wx2tcfpuj67-1
  bzrlib/transform.py            transform.py-20060105172343-dd99e54394d91687
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py	2011-11-18 00:32:50 +0000
+++ b/bzrlib/osutils.py	2011-11-29 20:20:02 +0000
@@ -2532,3 +2532,18 @@
     fn = getattr(os, 'fdatasync', getattr(os, 'fsync', None))
     if fn is not None:
         fn(fileno)
+
+
+def ensure_empty_directory_exists(path, exception_class):
+    """Make sure a local directory exists and is empty.
+    
+    If it does not exist, it is created.  If it exists and is not empty, an
+    instance of exception_class is raised.
+    """
+    try:
+        os.mkdir(path)
+    except OSError, e:
+        if e.errno != errno.EEXIST:
+            raise
+        if os.listdir(path) != []:
+            raise exception_class(path)

=== modified file 'bzrlib/tests/per_merger.py'
--- a/bzrlib/tests/per_merger.py	2011-01-12 01:01:53 +0000
+++ b/bzrlib/tests/per_merger.py	2011-12-01 01:03:06 +0000
@@ -175,17 +175,33 @@
         transform.finalize()
         return (limbodir, deletiondir)
 
-    def test_merge_with_existing_limbo(self):
-        wt = self.make_branch_and_tree('this')
-        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
-        os.mkdir(limbodir)
+    def test_merge_with_existing_limbo_empty(self):
+        """Empty limbo dir is just cleaned up - see bug 427773"""
+        wt = self.make_branch_and_tree('this')
+        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
+        os.mkdir(limbodir)
+        self.do_merge(wt, wt)
+
+    def test_merge_with_existing_limbo_non_empty(self):
+        wt = self.make_branch_and_tree('this')
+        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
+        os.mkdir(limbodir)
+        os.mkdir(os.path.join(limbodir, 'something'))
         self.assertRaises(errors.ExistingLimbo, self.do_merge, wt, wt)
         self.assertRaises(errors.LockError, wt.unlock)
 
-    def test_merge_with_pending_deletion(self):
-        wt = self.make_branch_and_tree('this')
-        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
-        os.mkdir(deletiondir)
+    def test_merge_with_pending_deletion_empty(self):
+        wt = self.make_branch_and_tree('this')
+        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
+        os.mkdir(deletiondir)
+        self.do_merge(wt, wt)
+
+    def test_merge_with_pending_deletion_non_empty(self):
+        """Also see bug 427773"""
+        wt = self.make_branch_and_tree('this')
+        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
+        os.mkdir(deletiondir)
+        os.mkdir(os.path.join(deletiondir, 'something'))
         self.assertRaises(errors.ExistingPendingDeletion, self.do_merge, wt, wt)
         self.assertRaises(errors.LockError, wt.unlock)
 

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2011-11-14 09:37:40 +0000
+++ b/bzrlib/transform.py	2011-11-29 20:20:02 +0000
@@ -1562,18 +1562,14 @@
         try:
             limbodir = urlutils.local_path_from_url(
                 tree._transport.abspath('limbo'))
-            try:
-                os.mkdir(limbodir)
-            except OSError, e:
-                if e.errno == errno.EEXIST:
-                    raise ExistingLimbo(limbodir)
+            osutils.ensure_empty_directory_exists(
+                limbodir,
+                errors.ExistingLimbo)
             deletiondir = urlutils.local_path_from_url(
                 tree._transport.abspath('pending-deletion'))
-            try:
-                os.mkdir(deletiondir)
-            except OSError, e:
-                if e.errno == errno.EEXIST:
-                    raise errors.ExistingPendingDeletion(deletiondir)
+            osutils.ensure_empty_directory_exists(
+                deletiondir,
+                errors.ExistingPendingDeletion)
         except:
             tree.unlock()
             raise

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-11-14 09:37:40 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-11-29 21:37:27 +0000
@@ -35,6 +35,12 @@
 * Cope with Unix filesystems, such as smbfs, where chmod gives 'permission
   denied'.  (Martin Pool, #606537)
 
+* When the ``limbo`` or ``pending-deletion`` directories exist, typically
+  because of an interrupted tree update, but are empty, bzr no longer
+  errors out, because there is nothing for the user to clean up.  Also,
+  errors in creation of these directories are no longer squelched.
+  (Martin Pool, #427773)
+
 * During merges, when two entries end up using the same path for two
   different file-ids (the same file being 'bzr added' in two different
   branches) , 'duplicate' conflicts are created instead of 'content'