[MERGE][#144421][#144300] Use write lock in log and status with '-r branch:URL'

John Arbash Meinel john at arbash-meinel.com
Thu Aug 28 15:51:03 BST 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
...

> This patch looks to be in a write direction.

right? :)

...

> I realise that revision specs that pull is not particularly pretty, but
> we made that choice some time ago as a decent approach for some
> problems. Remember that back then we didn't _have_ read locks, only
> write locks. Merging this patch won't make status more complex or error
> prone, and we can always improve things later.
> 
> That said, teaching status to not pull data should be very straight
> forward:
>  - add a method to revisionspec called as_revision_tree
>  - use that method in  status.py rather than as_revision_id.
> 
> (so that
>             try:
>                 rev_id = revision[0].as_revision_id(wt.branch)
>                 old = wt.branch.repository.revision_tree(rev_id)
>             except errors.NoSuchRevision, e:
>                 raise errors.BzrCommandError(str(e))
> becomes
>             try:
>                 old = revision[0].as_revision_tree()
>             except errors.NoSuchRevision, e:
>                 raise errors.BzrCommandError(str(e))
> and similarly immediately after that.
> 
> This is probably a couple of hours work, and I'm happy to offer more
> advice on what tests are needed etc.
> 
> -Rob

The pain problem with "as_revision_tree()" is that we would really like to
have the branch/repository locked *before* we ask for something as involved as
a RevisionTree. This is because RT requires an inventory, etc. So we have to
read a bunch of indexes and extract texts from the repository before we are
able to return a value.

I suppose if "as_revision_tree()" tries to use the local branch/repository
first (which should be locked anyway), and then falls back to using the remote
one. Then you get the functionality, with only a small amount of "waste".

I do agree that it makes the api seem clearer, as it helps push the knowledge
about a different branch/repository down into a lower level.

That said, stuff like Graph.find_unique_ancestry() will fail if you don't give
it the remote repository. So "bzr missing" needs to know about 2 repositories.
I *think* bzr diff only needs to know a snapshot (at least at the moment).

One other possibility... have "-r branch:" only fetch if the revision is *not*
in the local repository. For people with shared repos, or who have merged the
branch before, it will succeed. It is unfortunate that it would still fail
sometimes. And perhaps failing sometimes is worse than failing always.

I'm hesitant to say that we need to update all the code to handle the
possibility of 2 repositories. But that *is* reality.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFItrtXJdeBCYSNAAMRAkDJAKC1kEJOcOExi8cB7FnzOoEKAYM5zACfdbnK
GRu3PNCKZYzm/QyUagoUPAM=
=01J8
-----END PGP SIGNATURE-----



More information about the bazaar mailing list