[MERGE] Add an optional 'token' keyword argument to LockableFiles.lock_write

[Martin Pool]

On 12 Feb 2007, Andrew Bennetts <andrew at canonical.com> wrote:
> +class TokenMismatch(LockError):
> +
> +    _fmt = "The lock token %(given_token)r does not match lock token %(lock_token)r."
> +
> +    internal_error = True
> +
> +    def __init__(self, given_token, lock_token):
> +        self.given_token = given_token
> +        self.lock_token = lock_token
> +
> +
>  class PointlessCommit(BzrError):

I think you can be more clear here and in the other docstrings about
what has happened in this case: does it mean that someone else broke the
lock between when you got the initial token and when you tried to
continue the lock?

>      _fmt = "No changes to commit"
> 
> === modified file bzrlib/lockable_files.py
> --- bzrlib/lockable_files.py
> +++ bzrlib/lockable_files.py
> @@ -212,21 +212,33 @@
>              raise errors.BzrBadParameterNotString(a_string)
>          self.put(path, StringIO(a_string.encode('utf-8')))
>  
> -    def lock_write(self):
> +    def lock_write(self, token=None):
> +        """Lock this group of files for writing.
> +        
> +        :param token: if this is already locked, then lock_write will fail
> +            unless the token matches the existing lock.
> +        :returns: a token if this instance supports tokens, otherwise None.
> +        :raises TokenLockingNotSupported: when a token is given but this
> +            instance doesn't support using token locks.
> +        :raises MismatchedToken: if the specified token doesn't match the token
> +            of the existing lock.
> +        """

Could do with a couple of sentences about why you might want to pass the
token and where it comes from.

I note in passing that having to pass this through seems like additional
reason to remove LockableFiles and just talk to either the transport or
the lock directly.

> @@ -457,3 +479,14 @@
>              'locked %s' % (format_delta(delta),),
>              ]
>  
> +    def validate_token(self, token):
> +        if token is not None:
> +            info = self.peek()
> +            if info is None:
> +                # Lock isn't held
> +                lock_token = None
> +            else:
> +                lock_token = info.get('nonce')
> +            if token != lock_token:
> +                raise errors.TokenMismatch(token, lock_token)
> +

Why not use the whole info object as the token?  Doing so makes it a
little more sure we won't collide and might help debug any problems.

-- 
Martin