Rev 6437: (vila) Provide a config section matcher respecting the file order. (Vincent in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

Patch Queue Manager pqm at pqm.ubuntu.com
Mon Jan 9 12:20:43 UTC 2012


At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6437 [merge]
revision-id: pqm at pqm.ubuntu.com-20120109122041-5efxdgkusn0a319w
parent: pqm at pqm.ubuntu.com-20120106201436-ko6esavn69wt1lwt
parent: v.ladeuil+lp at free.fr-20120105161930-bh6bwqnt9tvv902f
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2012-01-09 12:20:41 +0000
message:
  (vila) Provide a config section matcher respecting the file order. (Vincent
   Ladeuil)
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  doc/developers/configuration.txt configuration.txt-20110408142435-korjxxnskvq44sta-1
  doc/en/release-notes/bzr-2.5.txt bzr2.5.txt-20110708125756-587p0hpw7oke4h05-1
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2012-01-05 14:10:32 +0000
+++ b/bzrlib/config.py	2012-01-09 12:20:41 +0000
@@ -3114,10 +3114,6 @@
 class IniFileStore(Store):
     """A config Store using ConfigObj for storage.
 
-    :ivar transport: The transport object where the config file is located.
-
-    :ivar file_name: The config file basename in the transport directory.
-
     :ivar _config_obj: Private member to hold the ConfigObj instance used to
         serialize/deserialize the config file.
     """
@@ -3253,9 +3249,19 @@
             value = self._config_obj._unquote(value)
         return value
 
+    def external_url(self):
+        # Since an IniFileStore can be used without a file (at least in tests),
+        # it's better to provide something than raising a NotImplementedError.
+        # All daughter classes are supposed to provide an implementation
+        # anyway.
+        return 'In-Process Store, no URL'
 
 class TransportIniFileStore(IniFileStore):
     """IniFileStore that loads files from a transport.
+
+    :ivar transport: The transport object where the config file is located.
+
+    :ivar file_name: The config file basename in the transport directory.
     """
 
     def __init__(self, transport, file_name):
@@ -3435,9 +3441,8 @@
 
 class LocationSection(Section):
 
-    def __init__(self, section, length, extra_path):
+    def __init__(self, section, extra_path):
         super(LocationSection, self).__init__(section.id, section.options)
-        self.length = length
         self.extra_path = extra_path
         self.locals = {'relpath': extra_path,
                        'basename': urlutils.basename(extra_path)}
@@ -3465,6 +3470,53 @@
         return value
 
 
+class StartingPathMatcher(SectionMatcher):
+    """Select sections for a given location respecting the Store order."""
+
+    # FIXME: Both local paths and urls can be used for section names as well as
+    # ``location`` to stay consistent with ``LocationMatcher`` which itself
+    # inherited the fuzziness from the previous ``LocationConfig``
+    # implementation. We probably need to revisit which encoding is allowed for
+    # both ``location`` and section names and how we normalize
+    # them. http://pad.lv/85479, http://pad.lv/437009 and http://359320 are
+    # related too. -- vila 2012-01-04
+
+    def __init__(self, store, location):
+        super(StartingPathMatcher, self).__init__(store)
+        if location.startswith('file://'):
+            location = urlutils.local_path_from_url(location)
+        self.location = location
+
+    def get_sections(self):
+        """Get all sections matching ``location`` in the store.
+
+        The most generic sections are described first in the store, then more
+        specific ones can be provided for reduced scopes.
+
+        The returned section are therefore returned in the reversed order so
+        the most specific ones can be found first.
+        """
+        location_parts = self.location.rstrip('/').split('/')
+        store = self.store
+        sections = []
+        # Later sections are more specific, they should be returned first
+        for _, section in reversed(list(store.get_sections())):
+            if section.id is None:
+                # The no-name section is always included if present
+                yield store, LocationSection(section, self.location)
+                continue
+            section_path = section.id
+            if section_path.startswith('file://'):
+                # the location is already a local path or URL, convert the
+                # section id to the same format
+                section_path = urlutils.local_path_from_url(section_path)
+            if (self.location.startswith(section_path)
+                or fnmatch.fnmatch(self.location, section_path)):
+                section_parts = section_path.rstrip('/').split('/')
+                extra_path = '/'.join(location_parts[len(section_parts):])
+                yield store, LocationSection(section, extra_path)
+
+
 class LocationMatcher(SectionMatcher):
 
     def __init__(self, store, location):
@@ -3494,7 +3546,7 @@
         matching_sections = []
         if no_name_section is not None:
             matching_sections.append(
-                LocationSection(no_name_section, 0, self.location))
+                (0, LocationSection(no_name_section, self.location)))
         for section_id, extra_path, length in filtered_sections:
             # a section id is unique for a given store so it's safe to take the
             # first matching section while iterating. Also, all filtered
@@ -3504,7 +3556,7 @@
                 section = iter_all_sections.next()
                 if section_id == section.id:
                     matching_sections.append(
-                        LocationSection(section, length, extra_path))
+                        (length, LocationSection(section, extra_path)))
                     break
         return matching_sections
 
@@ -3513,10 +3565,10 @@
         matching_sections = self._get_matching_sections()
         # We want the longest (aka more specific) locations first
         sections = sorted(matching_sections,
-                          key=lambda section: (section.length, section.id),
+                          key=lambda (length, section): (length, section.id),
                           reverse=True)
         # Sections mentioning 'ignore_parents' restrict the selection
-        for section in sections:
+        for _, section in sections:
             # FIXME: We really want to use as_bool below -- vila 2011-04-07
             ignore = section.get('ignore_parents', None)
             if ignore is not None:

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2012-01-05 10:39:49 +0000
+++ b/bzrlib/tests/test_config.py	2012-01-09 12:20:41 +0000
@@ -3336,8 +3336,7 @@
 
     def get_section(self, options, extra_path):
         section = config.Section('foo', options)
-        # We don't care about the length so we use '0'
-        return config.LocationSection(section, 0, extra_path)
+        return config.LocationSection(section, extra_path)
 
     def test_simple_option(self):
         section = self.get_section({'foo': 'bar'}, '')
@@ -3380,9 +3379,7 @@
                            '/quux/quux'],
                           [section.id for _, section in store.get_sections()])
         matcher = config.LocationMatcher(store, '/foo/bar/quux')
-        sections = [section for s, section in matcher.get_sections()]
-        self.assertEquals([3, 2],
-                          [section.length for section in sections])
+        sections = [section for _, section in matcher.get_sections()]
         self.assertEquals(['/foo/bar', '/foo'],
                           [section.id for section in sections])
         self.assertEquals(['quux', 'bar/quux'],
@@ -3399,9 +3396,7 @@
         self.assertEquals(['/foo', '/foo/bar'],
                           [section.id for _, section in store.get_sections()])
         matcher = config.LocationMatcher(store, '/foo/bar/baz')
-        sections = [section for s, section in matcher.get_sections()]
-        self.assertEquals([3, 2],
-                          [section.length for section in sections])
+        sections = [section for _, section in matcher.get_sections()]
         self.assertEquals(['/foo/bar', '/foo'],
                           [section.id for section in sections])
         self.assertEquals(['baz', 'bar/baz'],
@@ -3432,6 +3427,90 @@
         self.assertEquals(expected_location, matcher.location)
 
 
+class TestStartingPathMatcher(TestStore):
+
+    def setUp(self):
+        super(TestStartingPathMatcher, self).setUp()
+        # Any simple store is good enough
+        self.store = config.IniFileStore()
+
+    def assertSectionIDs(self, expected, location, content):
+        self.store._load_from_string(content)
+        matcher = config.StartingPathMatcher(self.store, location)
+        sections = list(matcher.get_sections())
+        self.assertLength(len(expected), sections)
+        self.assertEqual(expected, [section.id for _, section in sections])
+        return sections
+
+    def test_empty(self):
+        self.assertSectionIDs([], self.get_url(), '')
+
+    def test_url_vs_local_paths(self):
+        # The matcher location is an url and the section names are local paths
+        sections = self.assertSectionIDs(['/foo/bar', '/foo'],
+                                         'file:///foo/bar/baz', '''\
+[/foo]
+[/foo/bar]
+''')
+
+    def test_local_path_vs_url(self):
+        # The matcher location is a local path and the section names are urls
+        sections = self.assertSectionIDs(['file:///foo/bar', 'file:///foo'],
+                                         '/foo/bar/baz', '''\
+[file:///foo]
+[file:///foo/bar]
+''')
+
+
+    def test_no_name_section_included_when_present(self):
+        # Note that other tests will cover the case where the no-name section
+        # is empty and as such, not included.
+        sections = self.assertSectionIDs(['/foo/bar', '/foo', None],
+                                         '/foo/bar/baz', '''\
+option = defined so the no-name section exists
+[/foo]
+[/foo/bar]
+''')
+        self.assertEquals(['baz', 'bar/baz', '/foo/bar/baz'],
+                          [s.locals['relpath'] for _, s in sections])
+
+    def test_order_reversed(self):
+        self.assertSectionIDs(['/foo/bar', '/foo'], '/foo/bar/baz', '''\
+[/foo]
+[/foo/bar]
+''')
+
+    def test_unrelated_section_excluded(self):
+        self.assertSectionIDs(['/foo/bar', '/foo'], '/foo/bar/baz', '''\
+[/foo]
+[/foo/qux]
+[/foo/bar]
+''')
+
+    def test_glob_included(self):
+        sections = self.assertSectionIDs(['/foo/*/baz', '/foo/b*', '/foo'],
+                                         '/foo/bar/baz', '''\
+[/foo]
+[/foo/qux]
+[/foo/b*]
+[/foo/*/baz]
+''')
+        # Note that 'baz' as a relpath for /foo/b* is not fully correct, but
+        # nothing really is... as far using {relpath} to append it to something
+        # else, this seems good enough though.
+        self.assertEquals(['', 'baz', 'bar/baz'],
+                          [s.locals['relpath'] for _, s in sections])
+
+    def test_respect_order(self):
+        self.assertSectionIDs(['/foo', '/foo/b*', '/foo/*/baz'],
+                              '/foo/bar/baz', '''\
+[/foo/*/baz]
+[/foo/qux]
+[/foo/b*]
+[/foo]
+''')
+
+
 class TestNameMatcher(TestStore):
 
     def setUp(self):

=== modified file 'doc/developers/configuration.txt'
--- a/doc/developers/configuration.txt	2011-12-22 18:37:21 +0000
+++ b/doc/developers/configuration.txt	2012-01-09 12:20:41 +0000
@@ -301,12 +301,15 @@
 
 ``bzrlib`` provides:
 
-* ``LocationMatcher(store, location)``: To select all sections that match
-  ``location``.
-
 * ``NameMatcher(store, unique_id)``: To select a single section matching
   ``unique_id``.
 
+* ``LocationMatcher(store, location)``: To select all sections that match
+  ``location`` sorted by decreasing number of path components.
+
+* ``StartingPathMatcher(store, location)``: To select all sections that
+  match ``location`` in the order they appear in the ``store``.
+
 Stacks
 ------
 

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- a/doc/en/release-notes/bzr-2.5.txt	2012-01-06 14:09:04 +0000
+++ b/doc/en/release-notes/bzr-2.5.txt	2012-01-09 12:20:41 +0000
@@ -82,6 +82,11 @@
   of revisions whose ancestry is not obviously on the same developement
   line. (Vincent Ladeuil, #904744)
 
+* Configuration stacks can now use ``StartingPathMatcher`` to select the
+  sections matching a location while respecting the order chosen by the user
+  in the configuration file: from generic sections to specific
+  sections. (Vincent Ladeuil, #832046).
+
 * Make lazy imports resilient when resolved concurrently from multiple
   threads. Now the stand-in object will behave as a proxy for the real object
   after the initial access, rather than throwing. Assigning the object to




More information about the bazaar-commits mailing list