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

[Alexander Belchenko]

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

John Arbash Meinel пишет:
> 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.

I wonder if we need to check format of WT in addition to checking platform,
i.e.

@@ -1285,6 +1291,10 @@
             lco_tree.branch.repository.lock_write()
             lco_tree.branch.unlock()

+        if sys.platform == 'win32':
+            self.knownFailure('Win32 cannot run "bzr info"'
+                              ' when the tree is locked.')
+

^-- here add check for tree format before generate knownFailure?

[µ]

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

iD8DBQFGKF6dzYr338mxwCURAqo0AJwIz0HA5NJfUuMdMDYHkimk+PcV4QCdHlw5
WPFQuLz2LVoewIC+D5jqfnU=
=F5go
-----END PGP SIGNATURE-----