[MERGE][0.8] break-lock command

[Martin Pool]

On 05/05/2006, at 12:47 PM, Robert Collins wrote:

>> Given the danger of cStringIO with Unicode string I'd like to make  
>> some
>> effort in the future to remove or reduce use of it.  Perhaps we  
>> could go
>> through a decorator class that will prevent any attempt to put  
>> Unicode
>> in.
>
> mmm, sure I guess. Is this a general comment or a request that the  
> patch
> be changed ?

It's a general comment; I've added it to my list and you needn't do  
anything now.

>> Shouldn't this be calling the super implementation?
>>
>> To avoid this kind of bug I'd rather have call addCleanup from the
>> setUp method - it puts Same Things Together.
>
> Meep, yes it should. addCleanup still doesn't fit in my head in casual
> programming, I need to look at why.

Let's try to work out why, or what would be a better fit.

>>> +    def test_unlocked_repo_locked(self):
>>> +        # break lock on the branch should try on the repository  
>>> even
>>> +        # if the branch isn't locked
>>> +        self.branch.repository.lock_write()
>>> +        bzrlib.ui.ui_factory.stdin = StringIO("y\n")
>>> +        try:
>>> +            self.unused_branch.break_lock()
>>> +        except NotImplementedError:
>>> +            # branch does not support break_lock
>>> +            self.branch.repository.unlock()
>>> +            return
>>> +        self.assertRaises(errors.LockBroken,  
>>> self.branch.repository.unlock)
>>
>> q: would it be better to say 'skipped' if it's unimplemented, rather
>> than just return?
>
> No: Being unimplemented is valid for this interface : it means that  
> the
> branch does not suffer from broken locks. . There is nothing 'Wrong' -
> and our definition for 'skipped' is 'There is something you could  
> do to
> fix this.' Constrast with format 4 branches, where we can fix it by
> restoring the format 4 write logic.

General discussion:

But as John said, isn't it possible that the sftp-only locks used by  
old formats could be left behind?

Also we might want to distinguish "This is not implemented yet" from  
"This is not supported by this format".  I think of  
NotImplementedError as more of the former.
>
> (Can I call again for the nuking with extrem prejuidice of the SFTP
> connection cache ?)

Yes, please do.  It seems to me that using a single sftp connection  
for all operations needed for a user-level command is not just an  
optimisation but a hard requirement: the user should not be required  
to enter their passphrase twice just because it happens to fall out  
of the cache.  Therefore, we should actually keep around a Transport  
that holds the reference.

-- 
Martin