[RFC] bzr info improvements, now with locking status

[Martin Pool]

On 30 Apr 2006, Olaf Conradi <oohlaf at gmail.com> wrote:
> OK, upon the previous improvements, there is now initial support to
> query the status of the locks present in a checkout, branch and
> repository.
> 
> On 21/04/06, Olaf Conradi <olaf at conradi.org> wrote:
> >Hi
> >
> >New version of patch after Robert's comments.
> >
> > * Show repository location when it's shared or differs from branch
> >    https://launchpad.net/products/bzr/+bug/38332
> > * Extra testcase for info on non existing branches
> > * Few comments additions
> > * New help text (with help from jblack)
> > * Prefix checkout with light if it's lightweight
> > * Rename bound branch to checkout
> > * Detect construct used by comparing transport root, not bzrdir object
> * Fix upgrade error in test_info test case.
> 
> * Add is_locked() to Branch and WorkingTree, Repository already had
> one. This method is only used to query the (in-memory) lock status of
> the objects.
> * Add get_physical_lock_status to retrieve the lock status on the transport.
> * Show lock status in a new section of bzr info

That sounds good.

> At present this is only implemented for the new LockDir style locks.
> The old transport locks have some problems and I tried to find a way
> to query the locks, but haven't found one. And can't test the windows
> locks as I do not use it.
> 
> For unix, the os style locks use fcntl.lockf. I tried to set a new
> write lock and see if it raises IOError, but unfortunately that does
> not happen. See attached locktest.py. We could add peek() to class
> _fcntl_WriteLock in lock.py and return True, and in _fcntl_ReadLock
> return None. But that does not query the filesystem.

These locks are process-wide - if you have a file locked through one
file handle you cannot fail to get it on another.  I think if you ran
the two tests in separate processes you'd see the desired error.  This
will make it slightly difficult to make your test case pass too.

> So at the moment TransportLock just returns NotImplementedError and
> get_physical_lock_status returns false on it. We could pass that error
> through to the caller.

These formats are deprecated, so it's not really crucial that you
display all information about them.  In any case OS locks are
automatically collected when the process finishes and so information
about stale locks is less interesting.  

I'd be OK just to leave it as notimplemented really.

> The test case for oslocks needs to be adjusted to whatever we decide.
> The test case is now marked skipped.
> 
> >http://deschacht.student.utwente.nl/bzr/bazaar-vcs/bzr.olaf.info
> 
> Cheers
> -Olaf

+1 from me.

> +
> +
> +def _show_locking_info(repository, branch=None, working=None):
> +    """Show locking status of working, branch and repository."""
> +    if (repository.get_physical_lock_status() or
> +       (branch and branch.get_physical_lock_status()) or
> +       (working and working.get_physical_lock_status())):

Small but important point - these should be indented like this:

   if (repository.get_physical_lock_status() or
       (branch and branch.get_physical_lock_status()) or
       (working and working.get_physical_lock_status())):

otherwise it's unclear just how the parenthesis nest.


-- 
Martin