Keep core plugin from multiply contributing. Fixes bug 1191537.

Config would call all_plugins(), which includes core, and then TreeConfig would add core again.

core is no longer part of Config's enabled_plugins. The only places core comes from now are a TreeConfig's enabled_plugins attr and plugins_named(), which is always fed a frozen list of names from the catalog. Introduce all_plugins_but_core() and core_plugin() for DRY.

Switch the order of some blocks in dxr.plugins to avoid multiple returns.

Also, make skimmers run in the order that plugins are specified in the config file.

dxr/app.py

@@ -26,7 +26,7 @@
 from dxr.indexers import Ref, Region
 from dxr.lines import html_line, tags_per_line, finished_tags
 from dxr.mime import icon, is_image, is_text
-from dxr.plugins import plugins_named, all_plugins
+from dxr.plugins import plugins_named
 from dxr.query import Query, filter_menu_items
 from dxr.utils import (non_negative_int, decode_es_datetime, DXR_BLUEPRINT,
                        format_number, append_update, append_by_line, cumulative_sum)
@@ -384,13 +384,12 @@ def sidebar_links(sections):
         # file_to_skim class.
         skimmers = [plugin.file_to_skim(path,
                                         contents,
-                                        name,
+                                        plugin.name,
                                         config.trees[tree],
                                         file_doc,
                                         line_docs)
-                    for name, plugin in all_plugins().iteritems()
-                    if plugin in config.trees[tree].enabled_plugins
-                    and plugin.file_to_skim]
+                    for plugin in config.trees[tree].enabled_plugins
+                    if plugin.file_to_skim]
         skim_links, refses, regionses, annotationses = skim_file(skimmers, len(line_docs))
         index_refs = imap(Ref.es_to_triple,
                           chain.from_iterable(doc.get('refs', [])

dxr/config.py

@@ -17,7 +17,7 @@
 from schema import Schema, Optional, Use, And, Schema, SchemaError
 
 from dxr.exceptions import ConfigError
-from dxr.plugins import all_plugins
+from dxr.plugins import all_plugins_but_core, core_plugin
 from dxr.utils import cd, if_raises
 
 
@@ -160,7 +160,7 @@ def __init__(self, input, relative_to=None):
                 # Then explicitly enable anything that isn't explicitly
                 # disabled:
                 self._section['enabled_plugins'] = [
-                        p for p in all_plugins().values()
+                        p for p in all_plugins_but_core().values()
                         if p not in self.disabled_plugins]
 
             # Now that enabled_plugins and the other keys that TreeConfig
@@ -239,7 +239,7 @@ def __init__(self, name, unvalidated_tree, sections, config):
         if tree['enabled_plugins'].is_all:
             tree['enabled_plugins'] = [p for p in config.enabled_plugins
                                        if p not in tree['disabled_plugins']]
-        tree['enabled_plugins'].insert(0, all_plugins()['core'])
+        tree['enabled_plugins'].insert(0, core_plugin())
 
         # Split ignores into paths and filenames:
         tree['ignore_paths'] = [i for i in tree['ignore_patterns']
@@ -257,11 +257,10 @@ def __init__(self, name, unvalidated_tree, sections, config):
             if all(isinstance(k, Optional) for k in p.config_schema.iterkeys()))
         plugin_schema = Schema(merge(
             dict((Optional(name) if plugin in enableds_with_all_optional_config
-                                 or plugin not in tree['enabled_plugins']
-                  else name,
+                                    or plugin not in tree['enabled_plugins']
+                                 else name,
                   plugin.config_schema)
-                 for name, plugin in all_plugins().iteritems()
-                 if name != 'core'),
+                 for name, plugin in all_plugins_but_core().iteritems()),
             # And whatever isn't a plugin section, that we don't care about:
             {object: object}))
         # Insert empty missing sections for enabled plugins with entirely
@@ -294,12 +293,14 @@ def plugin_list(value):
     """Turn a space-delimited series of plugin names into a ListAndAll of
     Plugins.
 
+    Never return the core plugin.
+
     """
     if not isinstance(value, basestring):
         raise SchemaError('"%s" is neither * nor a whitespace-delimited list '
                           'of plugin names.' % (value,))
 
-    plugins = all_plugins()
+    plugins = all_plugins_but_core()
     names = value.strip().split()
     is_all = names == ['*']
     if is_all:

dxr/plugins/__init__.py

@@ -152,6 +152,10 @@ def __getstate__(self):
         copy['direct_searchers'] = []
         return copy
 
+    def __repr__(self):
+        return (('<Plugin %s>' % self.name) if hasattr(self, 'name')
+                else super(Plugin, self).__repr__())
+
 
 def filters_from_namespace(namespace):
     """Return the filters which conform to our suggested naming convention.
@@ -195,7 +199,7 @@ def decorator(searcher):
 
 _plugin_cache = None
 def all_plugins():
-    """Return a dict of plugin name -> Plugin for all registered plugins.
+    """Return a dict of plugin name -> Plugin for all plugins, including core.
 
     Plugins are registered via the ``dxr.plugins`` setuptools entry point,
     which may point to either a module (in which case a Plugin will be
@@ -206,18 +210,11 @@ def all_plugins():
     The core plugin, which provides many of DXR's cross-language, built-in
     features, is always the first plugin when iterating over the returned
     dict. This lets other plugins override bits of its elasticsearch mappings
-    and analyzers.
+    and analyzers when we're building up the schema.
 
     """
     global _plugin_cache
 
-    if _plugin_cache:
-        # Iterating over entrypoints could be kind of expensive, with the FS
-        # reads and all.
-        return _plugin_cache
-
-    import dxr.plugins.core
-
     def name_and_plugin(entry_point):
         """Return the name of an entry point and the Plugin it points to."""
         object = entry_point.load()
@@ -226,16 +223,40 @@ def name_and_plugin(entry_point):
         plugin.name = entry_point.name
         return entry_point.name, plugin
 
-    _plugin_cache = OrderedDict()
-    _plugin_cache['core'] = Plugin.from_namespace(dxr.plugins.core.__dict__)
-    _plugin_cache['core'].name = 'core'
-    _plugin_cache.update(name_and_plugin(point) for point in
-                              iter_entry_points('dxr.plugins'))
+    if _plugin_cache is None:
+        # Iterating over entrypoints could be kind of expensive, with the FS
+        # reads and all.
+        _plugin_cache = OrderedDict([('core', core_plugin())])
+        _plugin_cache.update(name_and_plugin(point) for point in
+                             iter_entry_points('dxr.plugins'))
+
     return _plugin_cache
 
 
+def all_plugins_but_core():
+    """Do like :func:`all_plugins()`, but don't return the core plugin."""
+    ret = all_plugins().copy()
+    del ret['core']
+    return ret
+
+
+_core_plugin = None
+def core_plugin():
+    """Return the core plugin."""
+    # This is a function in order to dodge a circular import.
+    global _core_plugin
+    import dxr.plugins.core
+
+    if _core_plugin is None:
+        _core_plugin = Plugin.from_namespace(dxr.plugins.core.__dict__)
+        _core_plugin.name = 'core'
+
+    return _core_plugin
+
+
 def plugins_named(names):
-    """Return an iterable of Plugins having the given names.
+    """Return an iterable of the core plugin, along with Plugins having the
+    given names.
 
     :arg names: An iterable of plugin names
 

tests/test_config.py

@@ -28,7 +28,11 @@ def test_enabled_star():
             [[xpidl]]
             header_path = /somewhere
         """)
-    ok_('urllink' in (p.name for p in config.trees['some_tree'].enabled_plugins))
+    enabled_plugins = config.trees['some_tree'].enabled_plugins
+    plugin_names = [p.name for p in enabled_plugins]
+    ok_('urllink' in plugin_names)
+    eq_('core', plugin_names[0])
+    ok_('core' not in plugin_names[1:])
 
 
 def test_es_index():