[BUG] sftp push usage

Martin Pool mbp at canonical.com
Thu Jun 1 05:50:29 BST 2006


On 26 May 2006, Robert Collins <robertc at robertcollins.net> wrote:
> On Thu, 2006-05-25 at 09:50 -0500, John Arbash Meinel wrote:
> > Alexander Belchenko wrote:
> > > I think it's a bug.
> > > 
> > > Today I'm testing how paramiko works on win32.
> > > I try to push paramiko's own branch to linux machine.
> > > I specify wrong path but bzr behaviour is buggy.
> > 
> > Well, there are 2 bugs here, but I can confirm both of them.
> > 
> > 1: using 'bzr push sftp://localhost/test' tries to create 'test' rather
> > than '/test'. because it is able to split the url down to
> > 'sftp://localhost' which it thinks is a directory.
> > My encodings branch had a lot of work to switch the url handling, which
> > may fix this bug.
> > 
> > 2: LockableFiles doesn't create _lock_count early enough. So a failure
> > during creation causes the bogus error. I could have sworn I've
> > committed a fix for this (maybe also in my encodings branch).
> > The issue is that _find_modes() can raise an exception, and the
> > deconstructor expects _lock_count to exist.
> > Putting 'self._lock_count = 0' before 'self._find_modes' in
> > LockableFiles.__init__ should fix this.
> > Alternatively, we could add '_lock_count = 0' to the Class declaration,
> > though my understanding is we were avoiding class declared variables.

If people approve I'll commit this patch to tidy it up while we mention
it.

Incidentally is there any good reason why we call object.__init__?  I
don't see one, so am going to remove it for consistency.

> Yup, specifically we are avoiding using 
> class Foo:
> 
>     instancevariable = value
> 
> because its confusing : even experienced python programmers sometimes
> have to look twice to see what you have a class variable being treated
> as a instance variable.

Yes, please avoid it unless either you really need a class variable, or
there is some truly compelling (performance?) reason why it's
appropriate to have the instance shadow the class.  In either case there
should probably be a comment.

-- 
Martin
-------------- next part --------------
=== modified file 'bzrlib/lockable_files.py'
--- bzrlib/lockable_files.py	
+++ bzrlib/lockable_files.py	
@@ -73,13 +73,12 @@
         :param lock_class: Class of lock strategy to use: typically
             either LockDir or TransportLock.
         """
-        object.__init__(self)
         self._transport = transport
         self.lock_name = lock_name
         self._transaction = None
-        self._find_modes()
         self._lock_mode = None
         self._lock_count = 0
+        self._find_modes()
         esc_name = self._escape(lock_name)
         self._lock = lock_class(transport, esc_name,
                                 file_modebits=self._file_mode,



More information about the bazaar mailing list