[MERGE] Makeing WorkingTree nicer to derive from.

[John Arbash Meinel]

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

Robert Collins wrote:
> This patch:
>  * factors workingtree.unlock to be cleaner
>  * Adds a trivial test for the unlock method.
>  * Moves unlock for Format2 trees into a new Format2 specific class, 
>    making the base WorkingTree cleaner to derive from.
> 

...

v--- I thought we were doing:
# Copyright (C) Canonical Ltd

And not doing Authors:

I also recently added 'test_locking.py' which already tests lock and
unlock (and that they are in the proper order)

So I think this test is redundant.

> +# (C) 2006 Canonical Ltd
> +# Authors:  Robert Collins <robert.collins at canonical.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> +
> +"""Tests for interface conformance of 'tree.unlock.'"""
> +
> +from bzrlib.tests.workingtree_implementations import TestCaseWithWorkingTree
> +
> +
> +class TestUnlock(TestCaseWithWorkingTree):
> +
> +    def test_unlock_completes(self):
> +        t = self.make_branch_and_tree('.')
> +        t.lock_write()
> +        t.unlock()
> 
> === modified file 'bzrlib/tests/workingtree_implementations/__init__.py'
> --- bzrlib/tests/workingtree_implementations/__init__.py	2006-07-07 15:22:42 +0000
> +++ bzrlib/tests/workingtree_implementations/__init__.py	2006-07-14 05:06:07 +0000
> @@ -60,6 +60,7 @@
>          'bzrlib.tests.workingtree_implementations.test_is_ignored',
>          'bzrlib.tests.workingtree_implementations.test_locking',
>          'bzrlib.tests.workingtree_implementations.test_pull',
> +        'bzrlib.tests.workingtree_implementations.test_unlock',
>          'bzrlib.tests.workingtree_implementations.test_workingtree',
>          ]
>      adapter = WorkingTreeTestProviderAdapter(
> 


...

>  class WorkingTree3(WorkingTree):
>      """This is the Format 3 working tree.
>  
> @@ -1542,6 +1541,15 @@
>              raise ConflictFormatError()
>          return ConflictList.from_stanzas(RioReader(confile))
>  
> +    def unlock(self):
> +        if self._hashcache.needs_write and self._control_files._lock_count==1:
> +            self._hashcache.write()
> +        # reverse order of locking.
> +        try:
> +            return self._control_files.unlock()
> +        finally:
> +            self.branch.unlock()
> +

I like this refactoring, as it does make things cleaner. The only part
here, is that 'self._hashcache.write()' needs a try/except around it, in
the case that we have a readonly working tree.

There is a debian bug open about it.
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=374673

Rather than putting a try/except around self._hashcache.write() twice,
it might be better to have a helper function in the base working tree.

Your refactoring is still +1 to come in. But I wouldn't mind discussing
whether not being able to write the hashcache is a warning/note/or just
a mutter.

The specific use case is running 'bzr status' or 'bzr info' on a
readonly tree. (bzr diff doesn't seem to update the hashcache in my
testing).

John
=:->

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

iD8DBQFEt6cRJdeBCYSNAAMRAh59AJ9uk10z51xBj4AnBRlMcsOVKhnXHACgjhab
fuFieaybyNY8TQ4b9Py8T9Q=
=WeuN
-----END PGP SIGNATURE-----