Merge lp:~mbp/bzr/224373-2.2-ftp-response into lp:bzr/2.2

[Martin Pool]

cope with ftp servers giving 250 reply for mkd (bug 224373)

Here's a workaround for MS FTP server's habit of returning '250' for successful mkd calls.

This is done 'blind' without a test and without testing it against a Microsoft ftp server, just based on the tracebacks and interactively creating an error that looks like them. If someone could actually test this, that would be good.

[Martin Packman (gz)]

Looked at this bug yesterday as well and this change is the same conclusion I'd drawn. Are you considering _setmode redundant as it's an win ftp server bug, or should that be called on this path as well?

Had looked at the ftp tests as well and bar subclassing both the medusa and pyftpdlib paths it didn't seem like this 'd be properly testable so just the fix is okay I think.

review: Approve

[John A Meinel (jameinel)]

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

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/224373-2.2-ftp-response into lp:bzr/2.2.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #224373 Bzr ftp support does not handle 250 response from Windows 2003 server for mkdir
> https://bugs.launchpad.net/bugs/224373
>
>
> Here's a workaround for MS FTP server's habit of returning '250' for successful mkd calls.
>
> This is done 'blind' without a test and without testing it against a Microsoft ftp server, just based on the tracebacks and interactively creating an error that looks like them. If someone could actually test this, that would be good.

Seems reasonable to me. Obviously I'd rather this was tested by someone
actually using it.

I'd also like to ask why you targeted 2.2, but not 2.0 or 2.1? Anyway,
in general:

  merge: approve

Though if you know someone that can actually test it, it would be nice
to wait to land until then. (As *I* don't know anyone, and can't do it
myself, I wouldn't block on this.)

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkxpaGsACgkQJdeBCYSNAAPNGgCdFTEc3mgXD49GTVX06StxS4Ox
U6oAoJ1rocNHchw0GbNZyTZbBhZRXw+w
=Da49
-----END PGP SIGNATURE-----

review: Approve

[Martin Pool (mbp)]

I had mixed feelings about whether something that's a workaround for a
bug elsewhere, and not automatically tested, ought to go into stable
series. 2.2 was kind of a compromise. On the whole I might put it
only into 2.3. One of the affected users offered me access to an ftp
server to at least test it interactively.
--
Martin

[Martin Pool (mbp)]

@mgz good catch re setmode; I would presume it probably will fail but we should probably set it anyhow.

[lp:~mbp/bzr/224373-2.2-ftp-response]

5078. By Martin Pool on 2010-08-17

Narrower try/except scope for ftp reply 250 from mkd

[Martin Pool (mbp)]

sent to pqm by email

=== modified file 'NEWS'
--- NEWS	2010-08-13 19:08:57 +0000
+++ NEWS	2010-08-17 00:51:41 +0000
@@ -20,6 +20,9 @@
 * CommitBuilder now uses the committer instead of _config.username to generate
   the revision-id.  (Aaron Bentley, #614404)
 
+* Cope with Microsoft FTP server that returns reply '250 Directory
+  created' when mkdir succeeds.  (Martin Pool, #224373)
+
 Improvements
 ************
 

=== modified file 'bzrlib/transport/ftp/__init__.py'
--- bzrlib/transport/ftp/__init__.py	2010-02-27 01:37:54 +0000
+++ bzrlib/transport/ftp/__init__.py	2010-08-17 00:51:41 +0000
@@ -350,7 +350,17 @@
         try:
             mutter("FTP mkd: %s", abspath)
             f = self._get_FTP()
-            f.mkd(abspath)
+            try:
+                f.mkd(abspath)
+            except ftplib.error_reply, e:
+                # <https://bugs.launchpad.net/bzr/+bug/224373> Microsoft FTP
+                # server returns "250 Directory created." which is kind of
+                # reasonable, 250 meaning "requested file action OK", but not what
+                # Python's ftplib expects.
+                if e[0][:3] == '250':
+                    pass
+                else:
+                    raise
             self._setmode(relpath, mode)
         except ftplib.error_perm, e:
             self._translate_ftp_error(e, abspath,