Rev 4726: Fix test failure and clean up ftp APPE. in file:///home/vila/src/bzr/bugs/443041-ftp-append-bytes/

Vincent Ladeuil v.ladeuil+lp at free.fr
Mon Oct 5 15:29:46 BST 2009


At file:///home/vila/src/bzr/bugs/443041-ftp-append-bytes/

------------------------------------------------------------
revno: 4726
revision-id: v.ladeuil+lp at free.fr-20091005142946-26vzznan0kgxlq11
parent: pqm at pqm.ubuntu.com-20091002180105-oeomf71t7l9gpv6g
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 443041-ftp-append-bytes
timestamp: Mon 2009-10-05 16:29:46 +0200
message:
  Fix test failure and clean up ftp APPE.
  
  * bzrlib/transport/ftp/__init__.py:
  (FtpTransport._try_append): Bad usage of the library and mixed
  dialogs left the connection is ASCII mode at the wrong time.
  (FtpTransport._try_append, FtpTransport._fallback_append): Use a
  file-like object instead of a str parametre to avoid useless
  buffering.
  (FtpTransport.append_file): Don't read the whole text to append,
  leave the called functions handled that instead.
  
  * bzrlib/tests/ftp_server/pyftpdlib_based.py:
  (FTPTestServer._run_server): Don't let spurious EBADF pollute the
  test outputs, they are rare but it's useless to report them.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2009-10-02 16:48:36 +0000
+++ b/NEWS	2009-10-05 14:29:46 +0000
@@ -101,6 +101,9 @@
   with some combinations of remote and local formats.  This was causing
   "unknown object type identifier 60" errors.  (Andrew Bennetts, #427736)
 
+* ftp APPE command was not cleanly handled in all cases and buffered too
+  much too. (Vincent Ladeuil, #443041)
+
 * Network streams now decode adjacent records of the same type into a
   single stream, reducing layering churn. (Robert Collins)
 

=== modified file 'bzrlib/tests/ftp_server/pyftpdlib_based.py'
--- a/bzrlib/tests/ftp_server/pyftpdlib_based.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/ftp_server/pyftpdlib_based.py	2009-10-05 14:29:46 +0000
@@ -202,7 +202,11 @@
         self._ftpd_running = True
         self._ftpd_starting.release()
         while self._ftpd_running:
-            self._ftp_server.serve_forever(timeout=0.1, count=1)
+            try:
+                self._ftp_server.serve_forever(timeout=0.1, count=1)
+            except select.error, e:
+                if e.args[0] != errno.EBADF:
+                    raise
         self._ftp_server.close_all(ignore_all=True)
 
     def add_user(self, user, password):

=== modified file 'bzrlib/transport/ftp/__init__.py'
--- a/bzrlib/transport/ftp/__init__.py	2009-07-22 06:51:13 +0000
+++ b/bzrlib/transport/ftp/__init__.py	2009-10-05 14:29:46 +0000
@@ -25,17 +25,13 @@
 """
 
 from cStringIO import StringIO
-import errno
 import ftplib
 import getpass
 import os
-import os.path
-import urlparse
 import random
 import socket
 import stat
 import time
-from warnings import warn
 
 from bzrlib import (
     config,
@@ -51,8 +47,6 @@
     register_urlparse_netloc_protocol,
     Server,
     )
-from bzrlib.transport.local import LocalURLServer
-import bzrlib.ui
 
 
 register_urlparse_netloc_protocol('aftp')
@@ -99,8 +93,9 @@
             self.is_active = True
         else:
             self.is_active = False
-        
-        # Most modern FTP servers support the APPE command. If ours doesn't, we (re)set this flag accordingly later.
+
+        # Most modern FTP servers support the APPE command. If ours doesn't, we
+        # (re)set this flag accordingly later.
         self._has_append = True
 
     def _get_FTP(self):
@@ -390,8 +385,6 @@
         """Append the text in the file-like object into the final
         location.
         """
-        text = f.read()
-        
         abspath = self._remote_path(relpath)
         if self.has(relpath):
             ftp = self._get_FTP()
@@ -401,13 +394,13 @@
 
         if self._has_append:
             mutter("FTP appe to %s", abspath)
-            self._try_append(relpath, text, mode)
+            self._try_append(relpath, f, mode)
         else:
-            self._fallback_append(relpath, text, mode)
+            self._fallback_append(relpath, f, mode)
 
         return result
 
-    def _try_append(self, relpath, text, mode=None, retries=0):
+    def _try_append(self, relpath, fp, mode=None, retries=0):
         """Try repeatedly to append the given text to the file at relpath.
 
         This is a recursive function. On errors, it will be called until the
@@ -417,37 +410,37 @@
             abspath = self._remote_path(relpath)
             mutter("FTP appe (try %d) to %s", retries, abspath)
             ftp = self._get_FTP()
-            cmd = "APPE %s" % abspath
-            conn = ftp.transfercmd(cmd)
-            conn.sendall(text)
-            conn.close()
+            starting_at = fp.tell()
+            ftp.storbinary("APPE %s" % abspath, fp)
             self._setmode(relpath, mode)
-            ftp.getresp()
         except ftplib.error_perm, e:
             # Check whether the command is not supported (reply code 502)
             if str(e).startswith('502 '):
-                warning("FTP server does not support file appending natively. " \
+                warning(
+                    "FTP server does not support file appending natively. "
                     "Performance may be severely degraded! (%s)", e)
                 self._has_append = False
-                self._fallback_append(relpath, text, mode)
+                fp.seek(starting_at)
+                self._fallback_append(relpath, fp, mode)
             else:
                 self._translate_perm_error(e, abspath, extra='error appending',
                     unknown_exc=errors.NoSuchFile)
-            
         except ftplib.error_temp, e:
             if retries > _number_of_retries:
-                raise errors.TransportError("FTP temporary error during APPEND %s." \
-                        "Aborting." % abspath, orig_error=e)
+                raise errors.TransportError(
+                    "FTP temporary error during APPEND %s. Aborting."
+                    % abspath, orig_error=e)
             else:
                 warning("FTP temporary error: %s. Retrying.", str(e))
                 self._reconnect()
-                self._try_append(relpath, text, mode, retries+1)
+                fp.seek(starting_at)
+                self._try_append(relpath, fp, mode, retries+1)
 
-    def _fallback_append(self, relpath, text, mode = None):
+    def _fallback_append(self, relpath, fp, mode = None):
         remote = self.get(relpath)
-        remote.seek(0, 2)
-        remote.write(text)
-        remote.seek(0, 0)
+        remote.seek(0, os.SEEK_END)
+        osutils.pumpfile(fp, remote)
+        remote.seek(0)
         return self.put_file(relpath, remote, mode)
 
     def _setmode(self, relpath, mode):



More information about the bazaar-commits mailing list