Rev 5330: Further clarify NEWS entry. in file:///home/vila/src/bzr/bugs/525571-lock-bazaar-conf-files/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/bugs/525571-lock-bazaar-conf-files/

------------------------------------------------------------
revno: 5330
revision-id: v.ladeuil+lp at free.fr-20100630102917-6d1nx35hpe7ckl1e
parent: v.ladeuil+lp at free.fr-20100630102855-ks87xtazs5xykfm4
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 525571-lock-bazaar-conf-files
timestamp: Wed 2010-06-30 12:29:17 +0200
message:
  Further clarify NEWS entry.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2010-06-30 09:07:59 +0000
+++ b/NEWS	2010-06-30 10:29:17 +0000
@@ -42,7 +42,8 @@
 * ``bzrlib.config.LockableConfig`` is a base class for config files that
   needs to be protected against multiple writers. All methods that
   change a configuration variable value must be decorated with
-  @needs_write_lock.  (Vincent Ladeuil, #525571)
+  @needs_write_lock (set_option() for example).
+  (Vincent Ladeuil,  #525571)
 
 * Support ``--directory`` option for a number of additional commands:
   conflicts, merge-directive, missing, resolve, shelve, switch,

=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2010-06-30 07:59:52 +0000
+++ b/bzrlib/config.py	2010-06-30 10:29:17 +0000
@@ -519,10 +519,10 @@
         super(LockableConfig, self).__init__(get_filename)
         self.dir = osutils.dirname(osutils.safe_unicode(self._get_filename()))
         self.transport = transport.get_transport(self.dir)
-        self.lock = None
+        self._lock = None
 
     def lock_write(self, token=None):
-        if self.lock is None:
+        if self._lock is None:
             ensure_config_dir_exists(self.dir)
             self._lock = lockdir.LockDir(self.transport, self.lock_name)
         self._lock.lock_write(token)
@@ -534,11 +534,15 @@
     def break_lock(self):
         if not self.transport.has(self.lock_name):
             raise errors.NoLockDir(self.dir)
-        if self.lock is None:
-            self.lock = lockdir.LockDir(self.transport, self.lock_name)
-        self.lock.break_lock()
+        if self._lock is None:
+            self._lock = lockdir.LockDir(self.transport, self.lock_name)
+        self._lock.break_lock()
 
     def _write_config_file(self):
+        if self._lock is None or not self._lock.is_held:
+            # NB: if the following exception is raised it probably means a
+            # missing @needs_write_lock decorator on one of the callers.
+            raise errors.ObjectNotLocked(self)
         fname = self._get_filename()
         f = StringIO()
         p = self._get_parser()

=== modified file 'bzrlib/tests/blackbox/test_break_lock.py'
--- a/bzrlib/tests/blackbox/test_break_lock.py	2010-06-29 15:36:54 +0000
+++ b/bzrlib/tests/blackbox/test_break_lock.py	2010-06-30 10:29:17 +0000
@@ -105,11 +105,24 @@
 
 class TestConfigBreakLock(tests.TestCaseWithTransport):
 
-    def test_break_lock(self):
+    def create_pending_lock(self):
         def config_file_name():
             return './my.conf'
         self.build_tree_contents([(config_file_name(), '[DEFAULT]\none=1\n')])
-        c1 = config.LockableConfig(config_file_name)
-        c1.lock_write()
-        self.run_bzr('break-lock %s' % config_file_name(), stdin="y\n")
-        self.assertRaises(errors.LockBroken, c1.unlock)
+        conf = config.LockableConfig(config_file_name)
+        # We lock the config file but arrange for the test to cleanup
+        # afterwards. The lock is held during the test duration allowing
+        # break-lock to be tested.
+        self.addCleanup(conf.lock_write().unlock)
+        return conf
+
+    def test_create_pending_lock(self):
+        conf = self.create_pending_lock()
+        self.assertTrue(conf._lock.is_held)
+
+    def test_break_lock(self):
+        conf = self.create_pending_lock()
+        import pdb; pdb.set_trace()
+        self.run_bzr('break-lock --config %s' % conf._get_filename(),
+                     stdin="y\n")
+        self.assertRaises(errors.LockBroken, conf.unlock)

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2010-06-30 07:59:52 +0000
+++ b/bzrlib/tests/test_config.py	2010-06-30 10:29:17 +0000
@@ -450,6 +450,10 @@
         c.set_user_option('one', 'one')
         self.assertEquals('one', c.get_user_option('one'))
 
+    def test_unlocked_config(self):
+        c = self.create_config()
+        self.assertRaises(errors.ObjectNotLocked, c._write_config_file)
+
     def test_listen_to_the_last_speaker(self):
         c1 = self.create_config()
         c2 = self.create_config()