ObjectNotLocked. What I should do?

[John Arbash Meinel]

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

Olexandr Byelchenko wrote:
> I have plugin x-bit that help me to control executable bit
> in working tree on win32. This plugin operate with inventory.
> 
> Today (after upgrade one of my branch to WT4) I've got error:
> 

...

> 
> The code in question:
> 
>     def run(self, files_list, set=False, clear=False):
>         from bzrlib.workingtree import WorkingTree
> 
>         tree = WorkingTree.open_containing(files_list[0])[0]
>         inv = tree.read_working_inventory()
> 
> Error raised on last line.
> What I do wrong? And what I need to change to make my plugin works
> with both WT3 and WT4?
> 
> [µ]

Lock the tree. :)

tree = WT.open_containing(...)
tree.lock_tree_write()
try:
  inv = tree.read_working_inventory()
finally:
  tree.unlock()


There are a couple of things, though.

1) WorkingTree3 has a @needs_read_lock() around
'read_working_inventory', should we restore that?

2) You should avoid directly working with inventories, as we are trying
to phase out their existence. If we find that isn't possible, then
certainly we won't, but we want to see if we can.

Certainly if you are changing the executable bits, you need to go
through TreeTransform.set_executability. Otherwise you have to go
through 'WT._write_inventory()' which is certainly being phased out.


Looking at your plugin, I think you could do a few things...

1) You should have a 'lock_tree_write' for the duration. Otherwise
someone else could change the working inventory underneath you, and then
you end up with a bogus inventory. (Say you forget you are doing a merge
in the background, without both sides taking a lock, it will let you try
to modify things).

2) Use 'tree.path2id()' rather than 'inv.path2id()'.

3) tree.is_executable() versus checking the inventory entry itself

4) Rewrite slightly to use TreeTransform, rather than directly
setting/unsetting the IE.executable flag. (This also has the advantage
that it would work on non-win32, effectively just doing a chmod for you).

5) I'm not sure how to get the kind. 0.15 introduced WT._kind(), but as
that is private for now, I wouldn't depend on it (and it didn't exist
before).

I think doing:

  kind = osutils.file_kind(tree.abspath(relpath))

would work.


6) Use 'bzrlib.builtins.tree_files()' to make sure you open the right
working tree, and convert all paths to relative to wt root, so that it
works even when you are in a subdirectory.


Attached is a patch which makes those changes.

John
=:->

PS> The only thing I didn't do was add a test suite... :) Though I
should have tried to do it true TDD style.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGCrceJdeBCYSNAAMRAuFZAJ9hTXPte1DfT6Lg/dAVeHiAs2d4HgCgmIDa
n8xgG+PNr/vTyOimn5i+dvA=
=wmiC
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: x_bit_no_inventory.patch
Type: text/x-patch
Size: 4398 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070328/15f85ee5/attachment-0001.bin