Rev 5320: Refresh the parser before setting a new value. in file:///home/vila/src/bzr/bugs/525571-lock-bazaar-conf-files/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Mon Jun 28 09:51:22 BST 2010
At file:///home/vila/src/bzr/bugs/525571-lock-bazaar-conf-files/
------------------------------------------------------------
revno: 5320
revision-id: v.ladeuil+lp at free.fr-20100628085122-ln3uwaqztf4drt3i
parent: v.ladeuil+lp at free.fr-20100625132334-ruz24bfb9t3drcqb
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 525571-lock-bazaar-conf-files
timestamp: Mon 2010-06-28 10:51:22 +0200
message:
Refresh the parser before setting a new value.
* bzrlib/tests/test_config.py:
(InstrumentedConfigObj): Add reload.
(TestLockableConfig): Start adding tests for concurrent accesses.
(TestLocationConfig.test_branch_calls_read_filenames): Cleanup to
avoid file leaks.
(TestLocationConfig.get_branch_config): Make sure config.reload()
can be called safely.
(TestLocationConfig.test_set_user_setting_sets_and_saves):
Simplify, the config dir has already been created. Add 'reload' to
the calls list.
(TestBranchConfigItems.get_branch_config): Make sure the location
config can be reloaded.
* bzrlib/config.py:
(_write_config_file): Always check for the containing config dir.
(GlobalConfig._set_option): Refresh the parser if needed.
(LocationConfig.set_user_option): Refresh the parser if needed,
rely on _write_config_file to create the config dir.
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2010-06-25 13:23:34 +0000
+++ b/bzrlib/config.py 2010-06-28 08:51:22 +0000
@@ -478,9 +478,12 @@
return self.get_user_option('nickname')
def _write_config_file(self):
- f = file(self._get_filename(), "wb")
+ fname = self._get_filename()
+ conf_dir = os.path.dirname(fname)
+ ensure_config_dir_exists(conf_dir)
+ f = file(fname, "wb")
try:
- osutils.copy_ownership_from_path(f.name)
+ osutils.copy_ownership_from_path(fname)
self._get_parser().write(f)
finally:
f.close()
@@ -523,6 +526,8 @@
# file lock on bazaar.conf.
conf_dir = os.path.dirname(self._get_filename())
ensure_config_dir_exists(conf_dir)
+ if self._parser is not None:
+ self._parser.reload()
self._get_parser().setdefault(section, {})[option] = value
self._write_config_file()
@@ -650,10 +655,10 @@
STORE_LOCATION_APPENDPATH]:
raise ValueError('bad storage policy %r for %r' %
(store, option))
+ if self._parser is not None:
+ self._parser.reload()
# FIXME: RBC 20051029 This should refresh the parser and also take a
# file lock on locations.conf.
- conf_dir = os.path.dirname(self._get_filename())
- ensure_config_dir_exists(conf_dir)
location = self.location
if location.endswith('/'):
location = location[:-1]
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2010-06-25 13:23:34 +0000
+++ b/bzrlib/tests/test_config.py 2010-06-28 08:51:22 +0000
@@ -129,6 +129,9 @@
self._calls.append(('keys',))
return []
+ def reload(self):
+ self._calls.append(('reload',))
+
def write(self, arg):
self._calls.append(('write',))
@@ -394,6 +397,40 @@
self.failUnless(my_config._get_parser() is parser)
+class TestLockableConfig(tests.TestCaseInTempDir):
+
+ config_class = config.GlobalConfig
+
+ def setUp(self):
+ super(TestLockableConfig, self).setUp()
+ config.ensure_config_dir_exists()
+ c = self.config_class()
+ fname = c._get_filename()
+ self.build_tree_contents([(fname, '''[DEFAULT]\none=1\ntwo=2''')])
+ self.config = self.create_config()
+
+ def create_config(self):
+ c = self.config_class()
+ return c
+
+ def test_simple_read_access(self):
+ self.assertEquals('1', self.config._get_user_option('one'))
+
+ def test_simple_write_access(self):
+ self.config.set_user_option('one', 'one')
+ self.assertEquals('one', self.config._get_user_option('one'))
+
+ def test_listen_to_the_last_speaker(self):
+ c1 = self.config
+ c2 = self.create_config()
+ c1.set_user_option('one', 'ONE')
+ c2.set_user_option('two', 'TWO')
+ self.assertEquals('ONE', c1._get_user_option('one'))
+ self.assertEquals('TWO', c2._get_user_option('two'))
+ # The second update respect the first one
+ self.assertEquals('ONE', c2._get_user_option('one'))
+
+
class TestGetUserOptionAs(TestIniConfig):
def test_get_user_option_as_bool(self):
@@ -745,10 +782,11 @@
[('__init__', config.locations_config_filename(),
'utf-8')])
config.ensure_config_dir_exists()
- #os.mkdir(config.config_dir())
f = file(config.branches_config_filename(), 'wb')
- f.write('')
- f.close()
+ try:
+ f.write('')
+ finally:
+ f.close()
oldparserclass = config.ConfigObj
config.ConfigObj = InstrumentedConfigObj
try:
@@ -995,10 +1033,15 @@
else:
global_file = StringIO(global_config.encode('utf-8'))
branches_file = StringIO(sample_branches_text.encode('utf-8'))
+ # Make sure the locations config can be reloaded
+ config.ensure_config_dir_exists()
+ f = file(config.locations_config_filename(), 'wb')
+ try:
+ f.write(branches_file.getvalue())
+ finally:
+ f.close
self.my_config = config.BranchConfig(FakeBranch(location))
- # Force location config to use specified file
self.my_location_config = self.my_config._get_location_config()
- self.my_location_config._get_parser(branches_file)
# Force global config to use specified file
self.my_config._get_global_config()._get_parser(global_file)
@@ -1007,25 +1050,14 @@
record = InstrumentedConfigObj("foo")
self.my_location_config._parser = record
- real_mkdir = os.mkdir
- self.created = False
- def checked_mkdir(path, mode=0777):
- self.log('making directory: %s', path)
- real_mkdir(path, mode)
- self.created = True
-
- os.mkdir = checked_mkdir
- try:
- self.callDeprecated(['The recurse option is deprecated as of '
- '0.14. The section "/a/c" has been '
- 'converted to use policies.'],
- self.my_config.set_user_option,
- 'foo', 'bar', store=config.STORE_LOCATION)
- finally:
- os.mkdir = real_mkdir
-
- self.failUnless(self.created, 'Failed to create ~/.bazaar')
- self.assertEqual([('__contains__', '/a/c'),
+ self.callDeprecated(['The recurse option is deprecated as of '
+ '0.14. The section "/a/c" has been '
+ 'converted to use policies.'],
+ self.my_config.set_user_option,
+ 'foo', 'bar', store=config.STORE_LOCATION)
+
+ self.assertEqual([('reload',),
+ ('__contains__', '/a/c'),
('__contains__', '/a/c/'),
('__setitem__', '/a/c', {}),
('__getitem__', '/a/c'),
@@ -1083,10 +1115,13 @@
if global_config is not None:
global_file = StringIO(global_config.encode('utf-8'))
my_config._get_global_config()._get_parser(global_file)
- self.my_location_config = my_config._get_location_config()
+ lconf = my_config._get_location_config()
if location_config is not None:
location_file = StringIO(location_config.encode('utf-8'))
- self.my_location_config._get_parser(location_file)
+ lconf._get_parser(location_file)
+ # Make sure the config can be reloaded
+ lconf._parser.filename = config.locations_config_filename()
+ self.my_location_config = lconf
if branch_data_config is not None:
my_config.branch.control_files.files['branch.conf'] = \
branch_data_config
More information about the bazaar-commits
mailing list