Bazaar

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

  1. 224373-2.2-ftp-response
  2. Merge into 2.2
Proposed by Martin Pool
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: 5078
Proposed branch: lp:~mbp/bzr/224373-2.2-ftp-response
Merge into: lp:bzr/2.2
Diff against target: 36 lines (+14/-1)
2 files modified
NEWS (+3/-0)
bzrlib/transport/ftp/__init__.py (+11/-1)
To merge this branch: bzr merge lp:~mbp/bzr/224373-2.2-ftp-response
Reviewer Review Type Date Requested Status
John A Meinel Approve
Martin Packman (community) Approve
Review via email: mp+32731@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Martin Packman (gz) wrote :

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
Revision history for this message
John A Meinel (jameinel) wrote :

-----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
Revision history for this message
Martin Pool (mbp) wrote :

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

Revision history for this message
Martin Pool (mbp) wrote :

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

Revision history for this message
Martin Pool (mbp) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-08-13 19:08:57 +0000
3+++ NEWS 2010-08-17 00:51:41 +0000
4@@ -20,6 +20,9 @@
5 * CommitBuilder now uses the committer instead of _config.username to generate
6 the revision-id. (Aaron Bentley, #614404)
7
8+* Cope with Microsoft FTP server that returns reply '250 Directory
9+ created' when mkdir succeeds. (Martin Pool, #224373)
10+
11 Improvements
12 ************
13
14
15=== modified file 'bzrlib/transport/ftp/__init__.py'
16--- bzrlib/transport/ftp/__init__.py 2010-02-27 01:37:54 +0000
17+++ bzrlib/transport/ftp/__init__.py 2010-08-17 00:51:41 +0000
18@@ -350,7 +350,17 @@
19 try:
20 mutter("FTP mkd: %s", abspath)
21 f = self._get_FTP()
22- f.mkd(abspath)
23+ try:
24+ f.mkd(abspath)
25+ except ftplib.error_reply, e:
26+ # <https://bugs.launchpad.net/bzr/+bug/224373> Microsoft FTP
27+ # server returns "250 Directory created." which is kind of
28+ # reasonable, 250 meaning "requested file action OK", but not what
29+ # Python's ftplib expects.
30+ if e[0][:3] == '250':
31+ pass
32+ else:
33+ raise
34 self._setmode(relpath, mode)
35 except ftplib.error_perm, e:
36 self._translate_ftp_error(e, abspath,

Subscribers

People subscribed via source and target branches