Rev 5370: Move the '_save' parameter from '__init__' to 'from_bytes', fix fallouts. in file:///home/vila/src/bzr/bugs/525571-lock-bazaar-conf-files/

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Aug 24 10:01:23 BST 2010


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

------------------------------------------------------------
revno: 5370 [merge]
revision-id: v.ladeuil+lp at free.fr-20100824090123-tsa07sygd9etn6hu
parent: v.ladeuil+lp at free.fr-20100823165752-uu7kdj3p69nn7q7z
parent: v.ladeuil+lp at free.fr-20100824081006-8bqbtcdoyx0kvfwr
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: remove-gratuitous-ensure-config-dir-exist-calls
timestamp: Tue 2010-08-24 11:01:23 +0200
message:
  Move the '_save' parameter from '__init__' to 'from_bytes', fix fallouts.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/plugins/launchpad/test_account.py test_account.py-20071011033320-50y6vfftywf4yllw-2
  bzrlib/tests/blackbox/test_alias.py test_alias.py-20080425112253-fbt0yz1c1834jriz-1
  bzrlib/tests/blackbox/test_aliases.py test_aliases.py-20060210230318-f0c08c9294dbfae1
  bzrlib/tests/blackbox/test_help.py test_help.py-20060216004358-4ee8a2a338f75a62
  bzrlib/tests/per_branch/test_branch.py testbranch.py-20050711070244-121d632bc37d7253
  bzrlib/tests/test_branch.py    test_branch.py-20060116013032-97819aa07b8ab3b5
  bzrlib/tests/test_commands.py  test_command.py-20051019190109-3b17be0f52eaa7a8
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  bzrlib/tests/test_debug.py     test_debug.py-20090303053802-01e8mlv24odmpgix-1
  bzrlib/tests/test_msgeditor.py test_msgeditor.py-20051202041359-920315ec6011ee51
  bzrlib/tests/test_smtp_connection.py test_smtp_connection-20070618204509-wuyxc0r0ztrecv7e-1
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2010-08-23 16:38:45 +0000
+++ b/NEWS	2010-08-24 09:01:23 +0000
@@ -145,12 +145,11 @@
 API Changes
 ***********
 
-* Configuration files should now use the ``_content`` parameter in the
-  constructor rather than the ``file`` parameter of the ``_get_parser``
-  method. The later has been deprecated. They can also specify
-  ``_save=True`` at build time to have the configuration file immediately
-  written to disk. (Vincent Ladeuil)
-
+* Configuration files should now use the ``from_bytes`` constructor the
+  rather than the ``file`` parameter of the ``_get_parser`` method. The
+  later has been deprecated. ``from_bytes`` also accept a ``save=True``
+  parameter to have the configuration file immediately written to
+  disk. (Vincent Ladeuil)
 
 * InventoryEntry instances now raise AttributeError if you try to assign
   to attributes that are irrelevant to that kind of entry.  e.g. setting

=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2010-08-23 16:57:52 +0000
+++ b/bzrlib/config.py	2010-08-24 09:01:23 +0000
@@ -356,16 +356,10 @@
     """A configuration policy that draws from ini files."""
 
     def __init__(self, get_filename=symbol_versioning.DEPRECATED_PARAMETER,
-                 file_name=None, _content=None, _save=False):
+                 file_name=None):
         """Base class for configuration files using an ini-like syntax.
 
         :param file_name: The configuration file path.
-
-        :param _content: For tests only, a string representing the file
-            content. This will be utf-8 encoded.
-
-         :param _save: For tests only, whether the file should be saved upon
-            creation.
         """
         super(IniBasedConfig, self).__init__()
         self.file_name = file_name
@@ -377,14 +371,31 @@
                 stacklevel=2)
             if get_filename is not None:
                 self.file_name = get_filename()
-        if _content is not None:
-            # wrap the content as a file-like object
-            _content = StringIO(_content.encode('utf-8'))
-        self._content = _content
+        else:
+            self.file_name = file_name
+        self._content = None
         self._parser = None
+
+    @classmethod
+    def from_bytes(cls, unicode_bytes, file_name=None, save=False):
+        """Create a config object from bytes.
+
+        :param unicode_bytes: A string representing the file content. This will
+            be utf-8 encoded.
+
+        :param file_name: The configuration file path.
+
+        :param _save: Whether the file should be saved upon creation.
+        """
+        conf = cls(file_name=file_name)
+        conf._create_from_bytes(unicode_bytes, save)
+        return conf
+
+    def _create_from_bytes(self, unicode_bytes, save):
+        self._content = StringIO(unicode_bytes.encode('utf-8'))
         # Some tests use in-memory configs, some other always need the config
         # file to exist on disk.
-        if _save:
+        if save:
             self._write_config_file()
 
     def _get_parser(self, file=symbol_versioning.DEPRECATED_PARAMETER):
@@ -564,13 +575,15 @@
 
     lock_name = 'lock'
 
-    def __init__(self, file_name, _content=None, _save=False):
-        super(LockableConfig, self).__init__(file_name=file_name,
-                                             _content=_content, _save=False)
+    def __init__(self, file_name):
+        super(LockableConfig, self).__init__(file_name=file_name)
         self.dir = osutils.dirname(osutils.safe_unicode(self.file_name))
         self.transport = transport.get_transport(self.dir)
         self._lock = lockdir.LockDir(self.transport, 'lock')
-        if _save:
+
+    def _create_from_bytes(self, unicode_bytes, save):
+        super(LockableConfig, self)._create_from_bytes(unicode_bytes, False)
+        if save:
             # We need to handle the saving here (as opposed to IniBasedConfig)
             # to be able to lock
             self.lock_write()
@@ -602,9 +615,21 @@
 class GlobalConfig(LockableConfig):
     """The configuration that should be used for a specific location."""
 
-    def __init__(self, _content=None, _save=False):
-        super(GlobalConfig, self).__init__(file_name=config_filename(),
-                                           _content=_content, _save=_save)
+    def __init__(self):
+        super(GlobalConfig, self).__init__(file_name=config_filename())
+
+    @classmethod
+    def from_bytes(cls, unicode_bytes, save=False):
+        """Create a config object from bytes.
+
+        :param unicode_bytes: A string representing the file content. This will
+            be utf-8 encoded.
+
+        :param save: Whether the file should be saved upon creation.
+        """
+        conf = cls()
+        conf._create_from_bytes(unicode_bytes, save)
+        return conf
 
     def get_editor(self):
         return self._get_user_option('editor')
@@ -645,10 +670,9 @@
 class LocationConfig(LockableConfig):
     """A configuration object that gives the policy for a location."""
 
-    def __init__(self, location, _content=None, _save=False):
+    def __init__(self, location):
         super(LocationConfig, self).__init__(
-            file_name=locations_config_filename(),
-            _content=_content, _save=_save)
+            file_name=locations_config_filename())
         # local file locations are looked up by local path, rather than
         # by file url. This is because the config file is a user
         # file, and we would rather not expose the user to file urls.
@@ -656,6 +680,21 @@
             location = urlutils.local_path_from_url(location)
         self.location = location
 
+    @classmethod
+    def from_bytes(cls, unicode_bytes, location, save=False):
+        """Create a config object from bytes.
+
+        :param unicode_bytes: A string representing the file content. This will
+            be utf-8 encoded.
+
+        :param location: The location url to filter the configuration.
+
+        :param save: Whether the file should be saved upon creation.
+        """
+        conf = cls(location)
+        conf._create_from_bytes(unicode_bytes, save)
+        return conf
+
     def _get_matching_sections(self):
         """Return an ordered list of section names matching this location."""
         sections = self._get_parser()

=== modified file 'bzrlib/plugins/launchpad/test_account.py'
--- a/bzrlib/plugins/launchpad/test_account.py	2010-07-19 12:54:54 +0000
+++ b/bzrlib/plugins/launchpad/test_account.py	2010-08-24 08:05:12 +0000
@@ -26,7 +26,7 @@
 class LaunchpadAccountTests(TestCaseInTempDir):
 
     def setup_config(self, text):
-        my_config = config.GlobalConfig(_content=text)
+        my_config = config.GlobalConfig.from_bytes(text)
         return my_config
 
     def test_get_lp_login_unconfigured(self):

=== modified file 'bzrlib/tests/blackbox/test_alias.py'
--- a/bzrlib/tests/blackbox/test_alias.py	2010-07-22 07:12:45 +0000
+++ b/bzrlib/tests/blackbox/test_alias.py	2010-08-24 09:01:23 +0000
@@ -53,8 +53,8 @@
         tree.add(file_name)
         tree.commit('added')
 
-        config.GlobalConfig(_save=True,
-                            _content=u'[ALIASES]\nust=st %s\n' % (file_name,))
+        config.GlobalConfig.from_bytes(u'[ALIASES]\nust=st %s\n' % (file_name,),
+                                       save=True)
 
         out, err = self.run_bzr('ust')
         self.assertEquals(err, '')

=== modified file 'bzrlib/tests/blackbox/test_aliases.py'
--- a/bzrlib/tests/blackbox/test_aliases.py	2010-07-22 07:12:45 +0000
+++ b/bzrlib/tests/blackbox/test_aliases.py	2010-08-24 09:01:23 +0000
@@ -35,11 +35,11 @@
             return self.run_bzr(args, **kwargs)[1]
 
 
-        conf = config.GlobalConfig(_save=True, _content='''[ALIASES]
+        conf = config.GlobalConfig.from_bytes('''[ALIASES]
 c=cat
 c1=cat -r 1
 c2=cat -r 1 -r2
-''')
+''', save=True)
 
         str1 = 'foo\n'
         str2 = 'bar\n'

=== modified file 'bzrlib/tests/blackbox/test_help.py'
--- a/bzrlib/tests/blackbox/test_help.py	2010-07-22 06:53:02 +0000
+++ b/bzrlib/tests/blackbox/test_help.py	2010-08-24 09:01:23 +0000
@@ -161,10 +161,10 @@
     def test_help_with_aliases(self):
         original = self.run_bzr('help cat')[0]
 
-        conf = config.GlobalConfig(_save=True, _content='''[ALIASES]
+        conf = config.GlobalConfig.from_bytes('''[ALIASES]
 c=cat
 cat=cat
-''')
+''', save=True)
 
         expected = original + "'bzr cat' is an alias for 'bzr cat'.\n"
         self.assertEqual(expected, self.run_bzr('help cat')[0])

=== modified file 'bzrlib/tests/per_branch/test_branch.py'
--- a/bzrlib/tests/per_branch/test_branch.py	2010-07-22 07:20:38 +0000
+++ b/bzrlib/tests/per_branch/test_branch.py	2010-08-24 09:01:23 +0000
@@ -619,8 +619,8 @@
 
     def test_get_push_location_exact(self):
         b = self.get_branch()
-        config.LocationConfig(b.base, _save=True,
-                              _content='[%s]\npush_location=foo\n' % (b.base,))
+        config.LocationConfig.from_bytes(
+            '[%s]\npush_location=foo\n' % (b.base,), b.base, save=True)
         self.assertEqual("foo", self.get_branch().get_push_location())
 
     def test_set_push_location(self):

=== modified file 'bzrlib/tests/test_branch.py'
--- a/bzrlib/tests/test_branch.py	2010-08-23 09:34:22 +0000
+++ b/bzrlib/tests/test_branch.py	2010-08-24 09:01:23 +0000
@@ -86,7 +86,7 @@
         self.assertIsDirectory('.bzr/branch/lock/held', t)
 
     def test_set_push_location(self):
-        conf = config.LocationConfig('.', _content='# comment\n', _save=True)
+        conf = config.LocationConfig.from_bytes('# comment\n', '.', save=True)
 
         branch = self.make_branch('.', format='knit')
         branch.set_push_location('foo')

=== modified file 'bzrlib/tests/test_commands.py'
--- a/bzrlib/tests/test_commands.py	2010-08-23 09:32:00 +0000
+++ b/bzrlib/tests/test_commands.py	2010-08-24 08:05:12 +0000
@@ -95,7 +95,7 @@
 class TestGetAlias(tests.TestCase):
 
     def _get_config(self, config_text):
-        my_config = config.GlobalConfig(_content=config_text)
+        my_config = config.GlobalConfig.from_bytes(config_text)
         return my_config
 
     def test_simple(self):

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2010-08-23 16:57:52 +0000
+++ b/bzrlib/tests/test_config.py	2010-08-24 09:01:23 +0000
@@ -395,7 +395,7 @@
 class TestIniConfig(tests.TestCaseInTempDir):
 
     def make_config_parser(self, s):
-        conf = config.IniBasedConfig(_content=s)
+        conf = config.IniBasedConfig.from_bytes(s)
         return conf, conf._get_parser()
 
 
@@ -405,11 +405,11 @@
         my_config = config.IniBasedConfig()
 
     def test_from_fp(self):
-        my_config = config.IniBasedConfig(_content=sample_config_text)
+        my_config = config.IniBasedConfig.from_bytes(sample_config_text)
         self.assertIsInstance(my_config._get_parser(), configobj.ConfigObj)
 
     def test_cached(self):
-        my_config = config.IniBasedConfig(_content=sample_config_text)
+        my_config = config.IniBasedConfig.from_bytes(sample_config_text)
         parser = my_config._get_parser()
         self.failUnless(my_config._get_parser() is parser)
 
@@ -436,7 +436,7 @@
 
     def test_get_parser_file_parameter_is_deprecated_(self):
         config_file = StringIO(sample_config_text.encode('utf-8'))
-        conf = config.IniBasedConfig(_content=sample_config_text)
+        conf = config.IniBasedConfig.from_bytes(sample_config_text)
         conf = self.callDeprecated([
             'IniBasedConfig._get_parser(file=xxx) was deprecated in 2.3.'
             ' Use IniBasedConfig(_content=xxx) instead.'],
@@ -450,23 +450,23 @@
 
     def test_saved_with_content(self):
         content = 'foo = bar\n'
-        conf = config.IniBasedConfig(file_name='./test.conf',
-                                     _content=content,_save=True)
+        conf = config.IniBasedConfig.from_bytes(
+            content, file_name='./test.conf', save=True)
         self.assertFileEqual(content, 'test.conf')
 
 
 class TestIniBaseConfigOnDisk(tests.TestCaseInTempDir):
 
     def test_cannot_reload_without_name(self):
-        conf = config.IniBasedConfig(_content=sample_config_text)
+        conf = config.IniBasedConfig.from_bytes(sample_config_text)
         self.assertRaises(AssertionError, conf.reload)
 
     def test_reload_see_new_value(self):
-        c1 = config.IniBasedConfig(file_name='./test/conf',
-                                   _content='editor=vim\n')
+        c1 = config.IniBasedConfig.from_bytes('editor=vim\n',
+                                              file_name='./test/conf')
         c1._write_config_file()
-        c2 = config.IniBasedConfig(file_name='./test/conf',
-                                   _content='editor=emacs\n')
+        c2 = config.IniBasedConfig.from_bytes('editor=emacs\n',
+                                              file_name='./test/conf')
         c2._write_config_file()
         self.assertEqual('vim', c1.get_user_option('editor'))
         self.assertEqual('emacs', c2.get_user_option('editor'))
@@ -491,7 +491,7 @@
         return self.config_class(*self.config_args)
 
     def create_config(self, content):
-        c = self.config_class(*self.config_args, _content=content, _save=True)
+        c = self.config_class.from_bytes(content, *self.config_args, save=True)
         return c
 
     def test_simple_read_access(self):
@@ -711,9 +711,9 @@
         self.assertEqual('branch', branch.nick)
 
         local_url = urlutils.local_path_to_url('branch')
-        conf = config.LocationConfig(
-            local_url, _save=True,
-            _content=('[%s]\nnickname = foobar' % (local_url,)))
+        conf = config.LocationConfig.from_bytes(
+            '[%s]\nnickname = foobar' % (local_url,),
+            local_url, save=True)
         self.assertEqual('foobar', branch.nick)
 
     def test_config_local_path(self):
@@ -722,9 +722,9 @@
         self.assertEqual('branch', branch.nick)
 
         local_path = osutils.getcwd().encode('utf8')
-        conf = config.LocationConfig(
-            'branch',  _save=True,
-            _content='[%s/branch]\nnickname = barry' % (local_path,))
+        conf = config.LocationConfig.from_bytes(
+            '[%s/branch]\nnickname = barry' % (local_path,),
+            'branch',  save=True)
         self.assertEqual('barry', branch.nick)
 
     def test_config_creates_local(self):
@@ -780,7 +780,7 @@
 class TestGlobalConfigItems(tests.TestCase):
 
     def test_user_id(self):
-        my_config = config.GlobalConfig(_content=sample_config_text)
+        my_config = config.GlobalConfig.from_bytes(sample_config_text)
         self.assertEqual(u"Erik B\u00e5gfors <erik at bagfors.nu>",
                          my_config._get_user_id())
 
@@ -789,11 +789,11 @@
         self.assertEqual(None, my_config._get_user_id())
 
     def test_configured_editor(self):
-        my_config = config.GlobalConfig(_content=sample_config_text)
+        my_config = config.GlobalConfig.from_bytes(sample_config_text)
         self.assertEqual("vim", my_config.get_editor())
 
     def test_signatures_always(self):
-        my_config = config.GlobalConfig(_content=sample_always_signatures)
+        my_config = config.GlobalConfig.from_bytes(sample_always_signatures)
         self.assertEqual(config.CHECK_NEVER,
                          my_config.signature_checking())
         self.assertEqual(config.SIGN_ALWAYS,
@@ -801,7 +801,7 @@
         self.assertEqual(True, my_config.signature_needed())
 
     def test_signatures_if_possible(self):
-        my_config = config.GlobalConfig(_content=sample_maybe_signatures)
+        my_config = config.GlobalConfig.from_bytes(sample_maybe_signatures)
         self.assertEqual(config.CHECK_NEVER,
                          my_config.signature_checking())
         self.assertEqual(config.SIGN_WHEN_REQUIRED,
@@ -809,7 +809,7 @@
         self.assertEqual(False, my_config.signature_needed())
 
     def test_signatures_ignore(self):
-        my_config = config.GlobalConfig(_content=sample_ignore_signatures)
+        my_config = config.GlobalConfig.from_bytes(sample_ignore_signatures)
         self.assertEqual(config.CHECK_ALWAYS,
                          my_config.signature_checking())
         self.assertEqual(config.SIGN_NEVER,
@@ -817,7 +817,7 @@
         self.assertEqual(False, my_config.signature_needed())
 
     def _get_sample_config(self):
-        my_config = config.GlobalConfig(_content=sample_config_text)
+        my_config = config.GlobalConfig.from_bytes(sample_config_text)
         return my_config
 
     def test_gpg_signing_command(self):
@@ -1165,10 +1165,10 @@
         if global_config is None:
             global_config = sample_config_text
 
-        my_global_config = config.GlobalConfig(_content=global_config,
-                                               _save=True)
-        my_location_config = config.LocationConfig(
-            my_branch.base, _content=sample_branches_text, _save=True)
+        my_global_config = config.GlobalConfig.from_bytes(global_config,
+                                                          save=True)
+        my_location_config = config.LocationConfig.from_bytes(
+            sample_branches_text, my_branch.base, save=True)
         my_config = config.BranchConfig(my_branch)
         self.my_config = my_config
         self.my_location_config = my_config._get_location_config()
@@ -1239,12 +1239,11 @@
                           location_config=None, branch_data_config=None):
         my_branch = FakeBranch(location)
         if global_config is not None:
-            my_global_config = config.GlobalConfig(_content=global_config,
-                                                   _save=True)
+            my_global_config = config.GlobalConfig.from_bytes(global_config,
+                                                              save=True)
         if location_config is not None:
-            my_location_config = config.LocationConfig(my_branch.base,
-                                                       _content=location_config,
-                                                       _save=True)
+            my_location_config = config.LocationConfig.from_bytes(
+                location_config, my_branch.base, save=True)
         my_config = config.BranchConfig(my_branch)
         if branch_data_config is not None:
             my_config.branch.control_files.files['branch.conf'] = \

=== modified file 'bzrlib/tests/test_debug.py'
--- a/bzrlib/tests/test_debug.py	2010-07-22 06:53:02 +0000
+++ b/bzrlib/tests/test_debug.py	2010-08-24 09:01:23 +0000
@@ -33,7 +33,7 @@
         self.try_debug_flags(['hpss'], 'debug_flags = hpss\n')
 
     def try_debug_flags(self, expected_flags, conf_bytes):
-        conf = config.GlobalConfig(_content=conf_bytes, _save=True)
+        conf = config.GlobalConfig.from_bytes(conf_bytes, save=True)
         self.overrideAttr(debug, 'debug_flags', set())
         debug.set_debug_flags_from_config()
         self.assertEqual(set(expected_flags), debug.debug_flags)

=== modified file 'bzrlib/tests/test_msgeditor.py'
--- a/bzrlib/tests/test_msgeditor.py	2010-07-22 06:53:02 +0000
+++ b/bzrlib/tests/test_msgeditor.py	2010-08-24 09:01:23 +0000
@@ -241,8 +241,8 @@
         os.environ['VISUAL'] = 'visual'
         os.environ['EDITOR'] = 'editor'
 
-        conf = config.GlobalConfig(_content='editor = config_editor\n',
-                                   _save=True)
+        conf = config.GlobalConfig.from_bytes('editor = config_editor\n',
+                                              save=True)
 
         editors = list(msgeditor._get_editor())
         editors = [editor for (editor, cfg_src) in editors]

=== modified file 'bzrlib/tests/test_smtp_connection.py'
--- a/bzrlib/tests/test_smtp_connection.py	2010-07-19 12:54:54 +0000
+++ b/bzrlib/tests/test_smtp_connection.py	2010-08-24 08:05:12 +0000
@@ -91,7 +91,7 @@
 class TestSMTPConnection(tests.TestCaseInTempDir):
 
     def get_connection(self, text, smtp_factory=None):
-        my_config = config.GlobalConfig(_content=text)
+        my_config = config.GlobalConfig.from_bytes(text)
         return smtp_connection.SMTPConnection(my_config,
                                               _smtp_factory=smtp_factory)
 



More information about the bazaar-commits mailing list