Rev 5327: Revert the lock scope to a sane value. 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: 5327
revision-id: v.ladeuil+lp at free.fr-20100630075952-hjvnjup19p1ixc07
parent: v.ladeuil+lp at free.fr-20100629154808-lhd7xgkocj5xbl18
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 525571-lock-bazaar-conf-files
timestamp: Wed 2010-06-30 09:59:52 +0200
message:
  Revert the lock scope to a sane value.
  
  * bzrlib/tests/test_config.py:
  (TestLockableConfig.test_writes_are_serialized)
  (TestLockableConfig.test_read_while_writing): Fix the fallouts.
  
  * bzrlib/config.py:
  (LockableConfig): Wrong idea, the lock needs to be taken arond the
  whole value update call, reducing the lock scope to
  _write_config_file exclude the config file re-read.
  (GlobalConfig.set_user_option, GlobalConfig.set_alias)
  (GlobalConfig.unset_alias, LocationConfig.set_user_option): These
  methods needs to be decorated with needs_write_lock to enforce the
  design constraints (lock, re-read config, set new value, write
  config, unlock).
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2010-06-29 15:48:08 +0000
+++ b/bzrlib/config.py	2010-06-30 07:59:52 +0000
@@ -512,6 +512,7 @@
     If several processes try to write the config file, the accesses need to be
     serialized.
     """
+
     lock_name = 'lock'
 
     def __init__(self, get_filename):
@@ -537,13 +538,7 @@
             self.lock = lockdir.LockDir(self.transport, self.lock_name)
         self.lock.break_lock()
 
-    @needs_write_lock
     def _write_config_file(self):
-        # For tests purposes, we separate the decorated method from the
-        # implementation one.
-        self._write_config_file_atomic()
-
-    def _write_config_file_atomic(self):
         fname = self._get_filename()
         f = StringIO()
         p = self._get_parser()
@@ -560,6 +555,7 @@
     def __init__(self):
         super(GlobalConfig, self).__init__(config_filename)
 
+    @needs_write_lock
     def set_user_option(self, option, value):
         """Save option and its value in the configuration."""
         self._set_option(option, value, 'DEFAULT')
@@ -574,10 +570,12 @@
         else:
             return {}
 
+    @needs_write_lock
     def set_alias(self, alias_name, alias_command):
         """Save the alias in the configuration."""
         self._set_option(alias_name, alias_command, 'ALIASES')
 
+    @needs_write_lock
     def unset_alias(self, alias_name):
         """Unset an existing alias."""
         aliases = self._get_parser().get('ALIASES')
@@ -709,6 +707,7 @@
             if policy_key in self._get_parser()[section]:
                 del self._get_parser()[section][policy_key]
 
+    @needs_write_lock
     def set_user_option(self, option, value, store=STORE_LOCATION):
         """Save option and its value in the configuration."""
         if store not in [STORE_LOCATION,

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2010-06-29 15:36:54 +0000
+++ b/bzrlib/tests/test_config.py	2010-06-30 07:59:52 +0000
@@ -482,14 +482,14 @@
         before_writing = threading.Event()
         after_writing = threading.Event()
         writing_done = threading.Event()
-        c1_orig = c1._write_config_file_atomic
-        def c1_write_config_file_atomic():
+        c1_orig = c1._write_config_file
+        def c1_write_config_file():
             before_writing.set()
             c1_orig()
             # The lock is held we wait for the main thread to decide when to
             # continue
             after_writing.wait()
-        c1._write_config_file_atomic = c1_write_config_file_atomic
+        c1._write_config_file = c1_write_config_file
         def c1_set_option():
             c1.set_user_option('one', 'c1')
             writing_done.set()
@@ -516,15 +516,15 @@
         ready_to_write = threading.Event()
         do_writing = threading.Event()
         writing_done = threading.Event()
-        c1_orig = c1._write_config_file_atomic
-        def c1_write_config_file_atomic():
+        c1_orig = c1._write_config_file
+        def c1_write_config_file():
             ready_to_write.set()
             # The lock is held we wait for the main thread to decide when to
             # continue
             do_writing.wait()
             c1_orig()
             writing_done.set()
-        c1._write_config_file_atomic = c1_write_config_file_atomic
+        c1._write_config_file = c1_write_config_file
         def c1_set_option():
             c1.set_user_option('one', 'c1')
         t1 = threading.Thread(target=c1_set_option)