[patch] lockdir contention detection on sftp

[Martin Pool]

At present sftp operations raise NoSuchFile if they get a generic
failure message.  This means that lock contention over sftp isn't
properly detected as such, and in general it seems a bit strange to
assume NoSuchFile is best.

This patch is one way to fix it: raise a generic PathError if we get a
generic failure from sftp.  Another way would be to treat any
transport-related error while trying to take the lock as possibly
indicating contention.

-- 
Martin
-------------- next part --------------
=== modified file 'a/bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py	
+++ b/bzrlib/lockdir.py	
@@ -109,9 +109,11 @@
         LockError,
         LockNotHeld,
         NoSuchFile,
+        PathError,
         ResourceBusy,
         UnlockableTransport,
         )
+from bzrlib.trace import mutter
 from bzrlib.transport import Transport
 from bzrlib.osutils import rand_chars
 from bzrlib.rio import RioWriter, read_stanza, Stanza
@@ -201,11 +203,9 @@
             self.transport.rename(tmpname, self._held_dir)
             self._lock_held = True
             self.confirm()
-            return
-        except (DirectoryNotEmpty, FileExists, ResourceBusy), e:
-            pass
-        # fall through to here on contention
-        raise LockContention(self)
+        except (PathError, DirectoryNotEmpty, FileExists, ResourceBusy), e:
+            mutter("contention on %r: %s", self, e)
+            raise LockContention(self)
 
     def unlock(self):
         """Release a held lock

=== modified file 'a/bzrlib/transport/sftp.py'
--- a/bzrlib/transport/sftp.py	
+++ b/bzrlib/transport/sftp.py	
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Robey Pointer <robey at lag.net>, Canonical Ltd
+# Copyrighr (C) 2005 Robey Pointer <robey at lag.net>, 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
@@ -34,7 +34,9 @@
                            FileExists, 
                            TransportNotPossible, NoSuchFile, PathNotChild,
                            TransportError,
-                           LockError, ParamikoNotPresent
+                           LockError, 
+                           PathError,
+                           ParamikoNotPresent,
                            )
 from bzrlib.osutils import pathjoin, fancy_rename
 from bzrlib.trace import mutter, warning, error
@@ -470,7 +472,8 @@
             self._translate_io_exception(e, path, ': unable to mkdir',
                 failure_exc=FileExists)
 
-    def _translate_io_exception(self, e, path, more_info='', failure_exc=NoSuchFile):
+    def _translate_io_exception(self, e, path, more_info='', 
+                                failure_exc=PathError):
         """Translate a paramiko or IOError into a friendlier exception.
 
         :param e: The original exception
@@ -480,8 +483,8 @@
         :param failure_exc: Paramiko has the super fun ability to raise completely
                            opaque errors that just set "e.args = ('Failure',)" with
                            no more information.
-                           This sometimes means FileExists, but it also sometimes
-                           means NoSuchFile
+                           If this parameter is set, it defines the exception 
+                           to raise in these cases.
         """
         # paramiko seems to generate detailless errors.
         self._translate_error(e, path, raise_generic=False)