Rev 5064: (mbp) handle directory rename failed from ftp #528722 in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Mon Mar 1 03:43:21 GMT 2010


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5064 [merge]
revision-id: pqm at pqm.ubuntu.com-20100301034320-01vhbnw7b8mxrmx5
parent: pqm at pqm.ubuntu.com-20100301003010-5gcgeng1xom1f0rh
parent: mbp at canonical.com-20100227013754-2z5alzyils3oh8lx
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2010-03-01 03:43:20 +0000
message:
  (mbp) handle directory rename failed from ftp #528722
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/test_ftp_transport.py test_aftp_transport.-20060823221619-98mwjzxtwtkt527k-1
  bzrlib/transport/ftp/__init__.py ftp.py-20051116161804-58dc9506548c2a53
=== modified file 'NEWS'
--- a/NEWS	2010-02-28 19:53:48 +0000
+++ b/NEWS	2010-03-01 03:43:20 +0000
@@ -84,6 +84,10 @@
   ``add``.
   (Parth Malwankar, #335033, #300001)
 
+* Correctly interpret "451 Rename/move failure: Directory not empty" from
+  ftp servers while trying to take a lock.
+  (Martin Pool, #528722)
+
 * Network transfer amounts and rates are now displayed in SI units according
   to the Ubuntu Units Policy <https://wiki.ubuntu.com/UnitsPolicy>.
   (Gordon Tyler, #514399)

=== modified file 'bzrlib/tests/test_ftp_transport.py'
--- a/bzrlib/tests/test_ftp_transport.py	2009-07-22 07:48:27 +0000
+++ b/bzrlib/tests/test_ftp_transport.py	2010-02-27 01:34:49 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 Canonical Ltd
+# Copyright (C) 2006, 2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -15,16 +15,20 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
 from cStringIO import StringIO
+import ftplib
 import getpass
 import sys
 
 from bzrlib import (
     config,
+    errors,
     tests,
     transport,
     ui,
     )
 
+from bzrlib.transport import ftp
+
 from bzrlib.tests import ftp_server
 
 
@@ -130,3 +134,17 @@
         # stdin should be empty (the provided password have been consumed),
         # even if the password is empty, it's followed by a newline.
         ui.ui_factory.assert_all_input_consumed()
+
+
+class TestFTPErrorTranslation(tests.TestCase):
+
+    def test_translate_directory_not_empty(self):
+        # https://bugs.launchpad.net/bugs/528722
+        
+        t = ftp.FtpTransport("ftp://none/")
+
+        try:
+            raise ftplib.error_temp("Rename/move failure: Directory not empty")
+        except Exception, e:
+            e = self.assertRaises(errors.DirectoryNotEmpty,
+                t._translate_ftp_error, e, "/path")

=== modified file 'bzrlib/transport/ftp/__init__.py'
--- a/bzrlib/transport/ftp/__init__.py	2010-02-17 17:11:16 +0000
+++ b/bzrlib/transport/ftp/__init__.py	2010-02-27 01:37:54 +0000
@@ -13,6 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
 """Implementation of Transport over ftp.
 
 Written by Daniel Silverstone <dsilvers at digital-scurf.org> with serious
@@ -163,9 +164,9 @@
         connection, credentials = self._create_connection(credentials)
         self._set_connection(connection, credentials)
 
-    def _translate_perm_error(self, err, path, extra=None,
+    def _translate_ftp_error(self, err, path, extra=None,
                               unknown_exc=FtpPathError):
-        """Try to translate an ftplib.error_perm exception.
+        """Try to translate an ftplib exception to a bzrlib exception.
 
         :param err: The error to translate into a bzr error
         :param path: The path which had problems
@@ -173,6 +174,9 @@
         :param unknown_exc: If None, we will just raise the original exception
                     otherwise we raise unknown_exc(path, extra=extra)
         """
+        # ftp error numbers are very generic, like "451: Requested action aborted,
+        # local error in processing" so unfortunately we have to match by
+        # strings.
         s = str(err).lower()
         if not extra:
             extra = str(err)
@@ -189,10 +193,12 @@
             or (s.startswith('550 ') and 'unable to rename to' in extra)
             ):
             raise errors.NoSuchFile(path, extra=extra)
-        if ('file exists' in s):
+        elif ('file exists' in s):
             raise errors.FileExists(path, extra=extra)
-        if ('not a directory' in s):
+        elif ('not a directory' in s):
             raise errors.PathError(path, extra=extra)
+        elif 'directory not empty' in s:
+            raise errors.DirectoryNotEmpty(path, extra=extra)
 
         mutter('unable to understand error for path: %s: %s', path, err)
 
@@ -318,7 +324,7 @@
                     raise e
                 raise
         except ftplib.error_perm, e:
-            self._translate_perm_error(e, abspath, extra='could not store',
+            self._translate_ftp_error(e, abspath, extra='could not store',
                                        unknown_exc=errors.NoSuchFile)
         except ftplib.error_temp, e:
             if retries > _number_of_retries:
@@ -347,7 +353,7 @@
             f.mkd(abspath)
             self._setmode(relpath, mode)
         except ftplib.error_perm, e:
-            self._translate_perm_error(e, abspath,
+            self._translate_ftp_error(e, abspath,
                 unknown_exc=errors.FileExists)
 
     def open_write_stream(self, relpath, mode=None):
@@ -373,7 +379,7 @@
             f = self._get_FTP()
             f.rmd(abspath)
         except ftplib.error_perm, e:
-            self._translate_perm_error(e, abspath, unknown_exc=errors.PathError)
+            self._translate_ftp_error(e, abspath, unknown_exc=errors.PathError)
 
     def append_file(self, relpath, f, mode=None):
         """Append the text in the file-like object into the final
@@ -419,7 +425,7 @@
                 self._has_append = False
                 self._fallback_append(relpath, text, mode)
             else:
-                self._translate_perm_error(e, abspath, extra='error appending',
+                self._translate_ftp_error(e, abspath, extra='error appending',
                     unknown_exc=errors.NoSuchFile)
         except ftplib.error_temp, e:
             if retries > _number_of_retries:
@@ -472,8 +478,8 @@
     def _rename(self, abs_from, abs_to, f):
         try:
             f.rename(abs_from, abs_to)
-        except ftplib.error_perm, e:
-            self._translate_perm_error(e, abs_from,
+        except (ftplib.error_temp, ftplib.error_perm), e:
+            self._translate_ftp_error(e, abs_from,
                 ': unable to rename to %r' % (abs_to))
 
     def move(self, rel_from, rel_to):
@@ -485,7 +491,7 @@
             f = self._get_FTP()
             self._rename_and_overwrite(abs_from, abs_to, f)
         except ftplib.error_perm, e:
-            self._translate_perm_error(e, abs_from,
+            self._translate_ftp_error(e, abs_from,
                 extra='unable to rename to %r' % (rel_to,),
                 unknown_exc=errors.PathError)
 
@@ -509,7 +515,7 @@
             mutter("FTP rm: %s", abspath)
             f.delete(abspath)
         except ftplib.error_perm, e:
-            self._translate_perm_error(e, abspath, 'error deleting',
+            self._translate_ftp_error(e, abspath, 'error deleting',
                 unknown_exc=errors.NoSuchFile)
 
     def external_url(self):
@@ -530,7 +536,7 @@
             try:
                 paths = f.nlst(basepath)
             except ftplib.error_perm, e:
-                self._translate_perm_error(e, relpath,
+                self._translate_ftp_error(e, relpath,
                                            extra='error with list_dir')
             except ftplib.error_temp, e:
                 # xs4all's ftp server raises a 450 temp error when listing an
@@ -579,7 +585,7 @@
             f = self._get_FTP()
             return FtpStatResult(f, abspath)
         except ftplib.error_perm, e:
-            self._translate_perm_error(e, abspath, extra='error w/ stat')
+            self._translate_ftp_error(e, abspath, extra='error w/ stat')
 
     def lock_read(self, relpath):
         """Lock the given file for shared (read) access.




More information about the bazaar-commits mailing list