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

[Andrew Bennetts]

John Arbash Meinel wrote:
[...]
> > 
> > I've added this paragraph to the docstring:
> > 
> >         A token should be passed in if you know that you have locked the object
> >         some other way, and need to synchronise this object's state with that
> >         fact.  For instance, this could happen when accessing the same branch
> >         over bzr+ssh:// and then falling back to do some operations on the same
> >         branch via sftp://.
> >          
> 
> 
> hmm.. falling back to sftp:// after bzr+ssh:// seems a bit weird. (As in
> you aren't ever going to do that). But I could be wrong.

That's true.  But the smart server branch does something very similiar; it falls
back to using a branch object backed on to a vfs transport (i.e. the 0.11
"smart" protocol) if the RemoteBranch doesn't have a smart protocol
implementation for that operation.  Ditto RemoteRepositories.

> One other use-case, though. Is that you might have a repository open,
> and you are opening a new one. And you don't *know* whether it is the
> same repository or not. (You have a heavy checkout and are opening the
> master branch, which may or may not be on the same filesystem, or might
> be a loopback over sftp/bzr+ssh onto the same filesystem).
> 
> So you *might* be passing a token as "I might already have this open,
> but I might not". Is there a reasonable way to do that?

Like this, I suppose:

    try:
        repo.lock_write(token=token)
    except TokenMismatch:
        token = repo.lock_write()

[...]
> >> 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.
> > 
> > As said in an earlier mail, this is a good idea, but for the sake of landing
> > this now I'll leave it as is for now.  We can fairly easily change this later;
> > the token is just an arbitrary string, and if they start being returned as a
> > longer string encoding some extra debug info rather than just the nonce nothing
> > will break.
> > 
> > Deferring this also means I can defer worrying about encoding issues for binary
> > data (like \x01) in the hpss protocol for a little bit longer...
> > 
> > -Andrew.
> 
> Well, if this is going to be changing, it sounds like it needs to change
> before an official release. Since you might be using a data structure
> rather than a "arbitrary string".

The current API is "arbitrary string that uniquely identifies the lock".  The
future API would be a "string that encodes the lock info (e.g. the raw bytes
of the lock/info file) that uniquely identifies the lock."  So code written
with today's token API will still work just fine with the new one, so long as
the new one preserves the properties that 
   1) "type(token) is str" is always true (or "token is None" for objects that
      don't support lock tokens), and
   2) "token_a == token_b" iff they are referring to the same lock.

This is why I say this improvement doesn't need to happen before this can land.

That said, I would definitely like to do this further work in time for 0.16,
which would mean no released bzr would have just arbitrary lock tokens w/o debug
info, but at the moment I want to focus on getting the smart server landed
before working on "nice to have"s like this.

-Andrew.