[MERGE] Hidden bug in `bzr info` when dirstate tree is write-locked

[John Arbash Meinel]

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

Alexander Belchenko wrote:
> (OS locks drive me crazy)
> 
> There is the hidden bug in `info` command and in
> blackbox.test_info.TestInfo.test_info_locking test as well.
> This test try to check if output of `bzr info` is valid
> for all combination of locked repo/breanch/tree.
> But in this test `bzr info` is not launched as subprocess.
> It's important. And this fact has one BIG side effect,
> that hide real problem with OS locks.
> 

Looking at 'test_info_locking()' it seems to be intentionally locking
the working tree, and making sure that "locking" shows up in the status.

We could do a couple things. One is to have the info code check for
"LockingContention"

Something like this, for example:
=== modified file 'bzrlib/info.py'
- --- bzrlib/info.py      2007-04-04 11:40:26 +0000
+++ bzrlib/info.py      2007-04-19 22:12:48 +0000
@@ -21,6 +21,7 @@

 from bzrlib import (
     diff,
+    errors,
     osutils,
     urlutils,
     )
@@ -275,12 +276,16 @@
     try:
         working = a_bzrdir.open_workingtree(
             recommend_upgrade=False)
- -        working.lock_read()
         try:
- -            show_tree_info(working, verbose)
- -        finally:
- -            working.unlock()
- -        return
+            working.lock_read()
+        except errors.LockContention:
+            print 'Working Tree is write-locked.  Unable to generate
working tree information.'
+        else:
+            try:
+                show_tree_info(working, verbose)
+            finally:
+                working.unlock()
+            return
     except (NoWorkingTree, NotLocalUrl):
         pass


But for now, I think it is reasonable to just fail on win32. Until we
get around to redoing dirstate so it doesn't require an OS lock.

This can be done with something like:
=== modified file 'bzrlib/tests/blackbox/test_info.py'
- --- bzrlib/tests/blackbox/test_info.py  2007-04-04 11:40:26 +0000
+++ bzrlib/tests/blackbox/test_info.py  2007-04-19 22:18:17 +0000
@@ -1108,6 +1108,11 @@
         :param repo_locked: If true, expect the repository to be locked.
         :param verbose: If true, expect verbose output
         """
+        if tree_locked and sys.platform == 'win32':
+            self.knownFailure('Win32 cannot run "bzr info"'
+                              ' when the tree is locked.')
+            # Arguably neither can Linux, but for now OS Locks are
+            # not exclusive in-process.
         out, err = self.runbzr('info %s' % command_string)
         if repo_locked or branch_locked or tree_locked:
             def locked_message(a_bool):

But it means that the rest of the test isn't run (since the test is run
as a big series of tests, rather than only testing 1 thing per
"test_foo()" call.)

So instead I went with the attached diff. It still raises
"knownFailure()" for win32, but only after running all of the tests. It
also makes sure that 'bzr info' is actually failing at the right time.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGJ+0XJdeBCYSNAAMRAm3KAKCoSQz9NERteTECTjopplSKiQ+aNQCePpL1
e/FU4niHDE0b0yzCoPuq5Yo=
=W24Y
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test_info_locking.patch
Type: text/x-patch
Size: 1854 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070419/04a3abb7/attachment.bin