Make external plugin loading require explicit configuration in project.conf

In addition, it changes the "plugins" and "required-versions" fields,
combining them for plugins and adding a new "required-project-version"
field.

6 files changed, 216 insertions, 142 deletions

diff --git a/buildstream/_elementfactory.py b/buildstream/_elementfactory.py
index deaa13c5e..6cb07a835 100644
--- a/buildstream/_elementfactory.py
+++ b/buildstream/_elementfactory.py
@@ -28,17 +28,13 @@ from .element import Element
 #
 # Args:
 #     plugin_base (PluginBase): The main PluginBase object to work with
-#     searchpath (list):        Search path for external Element plugins
+#     plugin_origins (list):    Data used to search for external Element plugins
 #
 class ElementFactory(PluginContext):
 
-    def __init__(self, plugin_base, searchpath=None):
+    def __init__(self, plugin_base, plugin_origins=None):
 
-        if searchpath is None:
-            searchpath = []
-
-        searchpath.insert(0, _site.element_plugins)
-        super().__init__(plugin_base, Element, searchpath)
+        super().__init__(plugin_base, Element, [_site.element_plugins], plugin_origins)
 
     # create():
     #
diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py
index f182cce3b..cb05fa524 100644
--- a/buildstream/_pipeline.py
+++ b/buildstream/_pipeline.py
@@ -141,8 +141,8 @@ class Pipeline():
 
         # Create the factories after resolving the project
         pluginbase = PluginBase(package='buildstream.plugins')
-        self.element_factory = ElementFactory(pluginbase, project._plugin_element_paths)
-        self.source_factory = SourceFactory(pluginbase, project._plugin_source_paths)
+        self.element_factory = ElementFactory(pluginbase, project._plugin_element_origins)
+        self.source_factory = SourceFactory(pluginbase, project._plugin_source_origins)
 
         # Resolve the real elements now that we've resolved the project
         resolved_elements = [self.resolve(meta_element, ticker=resolve_ticker)
diff --git a/buildstream/_plugincontext.py b/buildstream/_plugincontext.py
index 3b82954ed..f81dff67c 100644
--- a/buildstream/_plugincontext.py
+++ b/buildstream/_plugincontext.py
@@ -31,7 +31,8 @@ from . import utils
 # Args:
 #     plugin_base (PluginBase): The main PluginBase object to work with
 #     base_type (type):         A base object type for this context
-#     searchpath (list):        A list of paths to search for plugins
+#     site_plugin_path (str):   Path to where buildstream keeps plugins
+#     plugin_origins (list):    Data used to search for plugins
 #
 # Since multiple pipelines can be processed recursively
 # within the same interpretor, it's important that we have
@@ -42,23 +43,18 @@ from . import utils
 #
 class PluginContext():
 
-    def __init__(self, plugin_base, base_type, searchpath=None, dependencies=None):
-
-        if not searchpath:
-            raise PluginError("Cannot create plugin context without any searchpath")
+    def __init__(self, plugin_base, base_type, site_plugin_path, plugin_origins=None, dependencies=None):
 
         self.dependencies = dependencies
         self.loaded_dependencies = []
         self.base_type = base_type  # The base class plugins derive from
         self.types = {}             # Plugin type lookup table by kind
-
-        # Raise an error if we have more than one plugin with the same name
-        self.assert_searchpath(searchpath)
+        self.plugin_origins = plugin_origins or []
 
         # The PluginSource object
         self.plugin_base = plugin_base
-        self.source = plugin_base.make_plugin_source(searchpath=searchpath)
-        self.alternate_sources = []
+        self.site_source = plugin_base.make_plugin_source(searchpath=site_plugin_path)
+        self.alternate_sources = {}
 
     # lookup():
     #
@@ -74,55 +70,80 @@ class PluginContext():
     def lookup(self, kind):
         return self.ensure_plugin(kind)
 
+    def _get_local_plugin_source(self, path):
+        if ('local', path) not in self.alternate_sources:
+            # key by a tuple to avoid collision
+            source = self.plugin_base.make_plugin_source(searchpath=[path])
+            # Ensure that sources never get garbage collected,
+            # as they'll take the plugins with them.
+            self.alternate_sources[('local', path)] = source
+        else:
+            source = self.alternate_sources(('local', path))
+        return source
+
+    def _get_pip_plugin_source(self, package_name, kind):
+        defaults = None
+        if ('pip', package_name) not in self.alternate_sources:
+            # key by a tuple to avoid collision
+            try:
+                package = pkg_resources.get_entry_info(package_name,
+                                                       'buildstream.plugins',
+                                                       kind)
+            except pkg_resources.DistributionNotFound as e:
+                raise PluginError("Failed to load {} plugin '{}': {}"
+                                  .format(self.base_type.__name__, kind, e)) from e
+            location = package.dist.get_resource_filename(
+                pkg_resources._manager,
+                package.module_name.replace('.', os.sep) + '.py'
+            )
+
+            # Also load the defaults - required since setuptools
+            # may need to extract the file.
+            defaults = package.dist.get_resource_filename(
+                pkg_resources._manager,
+                package.module_name.replace('.', os.sep) + '.yaml'
+            )
+
+            source = self.plugin_base.make_plugin_source(searchpath=[os.path.dirname(location)])
+            self.alternate_sources[('pip', package_name)] = source
+
+        else:
+            source = self.alternate_sources[('pip', package_name)]
+
+        return source, defaults
+
     def ensure_plugin(self, kind):
 
         if kind not in self.types:
+            # Check whether the plugin is specified in plugins
             source = None
             defaults = None
-            dist, package = self.split_name(kind)
-
-            if dist:
-                # Find the plugin on disk using setuptools - this
-                # potentially unpacks the file and puts it in a
-                # temporary directory, but it is otherwise guaranteed
-                # to exist.
-                try:
-                    plugin = pkg_resources.get_entry_info(dist, 'buildstream.plugins', package)
-                except pkg_resources.DistributionNotFound as e:
-                    raise PluginError("Failed to load {} plugin '{}': {}"
-                                      .format(self.base_type.__name__, kind, e)) from e
-
-                # Missing plugins will return as 'None'
-                if plugin is not None:
-                    location = plugin.dist.get_resource_filename(
-                        pkg_resources._manager,
-                        plugin.module_name.replace('.', os.sep) + '.py'
-                    )
-
-                    # Also load the defaults - required since setuptools
-                    # may need to extract the file.
-                    defaults = plugin.dist.get_resource_filename(
-                        pkg_resources._manager,
-                        plugin.module_name.replace('.', os.sep) + '.yaml'
-                    )
-
-                    # Set the plugin-base source to the setuptools directory
-                    source = self.plugin_base.make_plugin_source(searchpath=[os.path.dirname(location)])
-                    # Ensure the plugin sources aren't garbage
-                    # collected - if they are, they take any loaded
-                    # plugins with them, regardless of whether those
-                    # have remaining references or not.
-                    self.alternate_sources.append(source)
-
-            elif package in self.source.list_plugins():
-                source = self.source
-
+            loaded_dependency = False
+            for origin in self.plugin_origins:
+                if kind not in origin['plugins']:
+                    continue
+
+                if origin['origin'] == 'local':
+                    source = self._get_local_plugin_source(origin['path'])
+                elif origin['origin'] == 'pip':
+                    source, defaults = self._get_pip_plugin_source(origin['package-name'], kind)
+                else:
+                    raise PluginError("Failed to load plugin '{}': "
+                                      "Unexpected plugin origin '{}'"
+                                      .format(kind, origin['origin']))
+                loaded_dependency = True
+                break
+
+            # Fall back to getting the source from site
             if not source:
-                raise PluginError("No {} type registered for kind '{}'"
-                                  .format(self.base_type.__name__, kind))
-            self.types[kind] = self.load_plugin(source, package, defaults)
+                if kind not in self.site_source.list_plugins():
+                    raise PluginError("No {} type registered for kind '{}'"
+                                      .format(self.base_type.__name__, kind))
+
+                source = self.site_source
 
-            if dist:
+            self.types[kind] = self.load_plugin(source, kind, defaults)
+            if loaded_dependency:
                 self.loaded_dependencies.append(kind)
 
         return self.types[kind]
@@ -155,18 +176,6 @@ class PluginContext():
         self.assert_version(kind, plugin_type)
         return (plugin_type, defaults)
 
-    def split_name(self, name):
-        if name.count(':') > 1:
-            raise PluginError("Plugin and package names must not contain ':'")
-
-        try:
-            dist, kind = name.split(':', maxsplit=1)
-        except ValueError:
-            dist = None
-            kind = name
-
-        return dist, kind
-
     def assert_plugin(self, kind, plugin_type):
         if kind in self.types:
             raise PluginError("Tried to register {} plugin for existing kind '{}' "
@@ -197,25 +206,3 @@ class PluginContext():
                                   self.base_type.__name__, kind,
                                   plugin_type.BST_REQUIRED_VERSION_MAJOR,
                                   plugin_type.BST_REQUIRED_VERSION_MINOR))
-
-    # We want a PluginError when trying to create a context
-    # where more than one plugin has the same name
-    def assert_searchpath(self, searchpath):
-        names = []
-        fullnames = []
-        for path in searchpath:
-            for filename in os.listdir(path):
-                basename = os.path.basename(filename)
-                name, extension = os.path.splitext(basename)
-                if extension == '.py' and name != '__init__':
-                    fullname = os.path.join(path, filename)
-
-                    if name in names:
-                        idx = names.index(name)
-                        raise PluginError("Failed to register {} plugin '{}' from: {}\n"
-                                          "{} plugin '{}' is already registered by: {}"
-                                          .format(self.base_type.__name__, name, fullname,
-                                                  self.base_type.__name__, name, fullnames[idx]))
-
-                    names.append(name)
-                    fullnames.append(fullname)
diff --git a/buildstream/_project.py b/buildstream/_project.py
index 957bcf263..eee74a057 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -72,8 +72,8 @@ class Project():
         self._elements = {}      # Element specific configurations
         self._aliases = {}       # Aliases dictionary
         self._workspaces = {}    # Workspaces
-        self._plugin_source_paths = []   # Paths to custom sources
-        self._plugin_element_paths = []  # Paths to custom plugins
+        self._plugin_source_origins = []   # Origins of custom sources
+        self._plugin_element_origins = []  # Origins of custom elements
         self._options = None    # Project options, the OptionPool
         self._cache_key = None
         self._source_format_versions = {}
@@ -128,7 +128,7 @@ class Project():
         config.pop('elements', None)
         _yaml.node_final_assertions(config)
         _yaml.node_validate(config, [
-            'required-versions',
+            'required-project-version',
             'element-path', 'variables',
             'environment', 'environment-nocache',
             'split-rules', 'elements', 'plugins',
@@ -178,12 +178,8 @@ class Project():
         # Workspace configurations
         self._workspaces = self._load_workspace_config()
 
-        # Version requirements
-        versions = _yaml.node_get(config, Mapping, 'required-versions', default_value={})
-        _yaml.node_validate(versions, ['project', 'elements', 'sources'])
-
-        # Assert project version first
-        format_version = _yaml.node_get(versions, int, 'project', default_value=0)
+        # Assert project version
+        format_version = _yaml.node_get(config, int, 'required-project-version', default_value=0)
         if BST_FORMAT_VERSION < format_version:
             major, minor = utils.get_bst_version()
             raise LoadError(
@@ -191,23 +187,68 @@ class Project():
                 "Project requested format version {}, but BuildStream {}.{} only supports up until format version {}"
                 .format(format_version, major, minor, BST_FORMAT_VERSION))
 
-        # The source versions
-        source_versions = _yaml.node_get(versions, Mapping, 'sources', default_value={})
-        for key, _ in _yaml.node_items(source_versions):
-            self._source_format_versions[key] = _yaml.node_get(source_versions, int, key)
-
-        # The element versions
-        element_versions = _yaml.node_get(versions, Mapping, 'elements', default_value={})
-        for key, _ in _yaml.node_items(element_versions):
-            self._element_format_versions[key] = _yaml.node_get(element_versions, int, key)
-
-        # Load the plugin paths
-        plugins = _yaml.node_get(config, Mapping, 'plugins', default_value={})
-        _yaml.node_validate(plugins, ['elements', 'sources'])
-        self._plugin_source_paths = [os.path.join(self.directory, path)
-                                     for path in self._extract_plugin_paths(plugins, 'sources')]
-        self._plugin_element_paths = [os.path.join(self.directory, path)
-                                      for path in self._extract_plugin_paths(plugins, 'elements')]
+        # Plugin origins and versions
+        origins = _yaml.node_get(config, list, 'plugins', default_value=[])
+        for origin in origins:
+            allowed_origin_fields = [
+                'origin', 'sources', 'elements',
+                'package-name', 'path',
+            ]
+            allowed_origins = ['core', 'local', 'pip']
+            _yaml.node_validate(origin, allowed_origin_fields)
+
+            if origin['origin'] not in allowed_origins:
+                raise LoadError(
+                    LoadErrorReason.INVALID_YAML,
+                    "Origin '{}' is not one of the allowed types"
+                    .format(origin['origin']))
+
+            # Store source versions for checking later
+            source_versions = _yaml.node_get(origin, Mapping, 'sources', default_value={})
+            for key, _ in _yaml.node_items(source_versions):
+                if key in self._source_format_versions:
+                    raise LoadError(
+                        LoadErrorReason.INVALID_YAML,
+                        "Duplicate listing of source '{}'".format(key))
+                self._source_format_versions[key] = _yaml.node_get(source_versions, int, key)
+
+            # Store element versions for checking later
+            element_versions = _yaml.node_get(origin, Mapping, 'elements', default_value={})
+            for key, _ in _yaml.node_items(element_versions):
+                if key in self._element_format_versions:
+                    raise LoadError(
+                        LoadErrorReason.INVALID_YAML,
+                        "Duplicate listing of element '{}'".format(key))
+                self._element_format_versions[key] = _yaml.node_get(element_versions, int, key)
+
+            # Store the origins if they're not 'core'.
+            # core elements are loaded by default, so storing is unnecessary.
+            if _yaml.node_get(origin, str, 'origin') != 'core':
+                # Add origins for sources
+                if 'sources' in origin:
+                    source_dict = _yaml.node_copy(origin)
+                    sources = _yaml.node_get(origin, Mapping, 'sources', default_value={})
+                    source_dict['plugins'] = [k for k, _ in _yaml.node_items(sources)]
+                    del source_dict['sources']
+                    if 'elements' in source_dict:
+                        del source_dict['elements']
+                    # paths are passed in relative to the project, but must be absolute
+                    if source_dict['origin'] == 'local':
+                        source_dict['path'] = os.path.join(self.directory,
+                                                           source_dict['path'])
+                    self._plugin_source_origins.append(source_dict)
+                if 'elements' in origin:
+                    element_dict = _yaml.node_copy(origin)
+                    elements = _yaml.node_get(origin, Mapping, 'elements', default_value={})
+                    element_dict['plugins'] = [k for k, _ in _yaml.node_items(elements)]
+                    del element_dict['elements']
+                    if 'sources' in element_dict:
+                        del element_dict['sources']
+                    # paths are passed in relative to the project, but must be absolute
+                    if element_dict['origin'] == 'local':
+                        element_dict['path'] = os.path.join(self.directory,
+                                                            element_dict['path'])
+                    self._plugin_element_origins.append(element_dict)
 
         # Source url aliases
         self._aliases = _yaml.node_get(config, Mapping, 'aliases', default_value={})
diff --git a/buildstream/_sourcefactory.py b/buildstream/_sourcefactory.py
index 3461ed2ff..09bf1b04a 100644
--- a/buildstream/_sourcefactory.py
+++ b/buildstream/_sourcefactory.py
@@ -28,17 +28,13 @@ from .source import Source
 #
 # Args:
 #     plugin_base (PluginBase): The main PluginBase object to work with
-#     searchpath (list):        Search path for external Source plugins
+#     plugin_origins (list):    Data used to search for external Source plugins
 #
 class SourceFactory(PluginContext):
 
-    def __init__(self, plugin_base, searchpath=None):
+    def __init__(self, plugin_base, plugin_origins=None):
 
-        if searchpath is None:
-            searchpath = []
-
-        searchpath.insert(0, _site.source_plugins)
-        super().__init__(plugin_base, Source, searchpath)
+        super().__init__(plugin_base, Source, [_site.source_plugins], plugin_origins)
 
     # create():
     #
diff --git a/doc/source/projectconf.rst b/doc/source/projectconf.rst
index c8bfdeefc..f1fece452 100644
--- a/doc/source/projectconf.rst
+++ b/doc/source/projectconf.rst
@@ -77,30 +77,85 @@ with an artifact share.
     url: https://foo.com/artifacts
 
 
-Plugin Paths
-~~~~~~~~~~~~
+Plugin Origins and Versions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The BuildStream format is guaranteed to be backwards compatible
+with any earlier releases. The core YAML format, the format supported
+by various plugins, and the overall BuildStream release version are
+revisioned separately.
+
 If your project includes any custom *Elements* or *Sources*, then
-the project relative subdirectory where these plugins are stored
-must be specified.
+the origins, names, and minimum version must be defined.
+If your project must use a minimum version of a core plugin, this is
+also specified here.
+
+Note that elements or plugins with the same name from different origins
+are not permitted.
+
+Plugin specification format
+'''''''''''''''''''''''''''
 
 .. code:: yaml
 
    plugins:
-
+   
+   # Core is only listed here as a means to allow project.conf
+   # authors to specify API versioning requirements
+   - origin: core
+   
+     # Here we CAN specify minimal bound API version for each plugin,
+     # if we have such dependencies
+     sources:
+       git: 2
+       local: 1
+   
      elements:
-     - plugins/local-elements
-     - plugins/shared-elements
-
+       script: 2
+   
+   # Specify the "pony" plugins found by pip
+   - origin: pip
+     package-name: pony
+   
+     # Here we MUST specify a minimal bound API version for each
+     # plugin, in order to indicate which plugin is to be discovered
+     # from this particular "pip" origin
+     sources:
+       flying-pony: 0
+   
+   - origin: pip
+     package-name: potato
+   
+     # Here we have the rotten potato element loaded
+     # from the "potato" plugin package loaded via pip,
+     # this is a separate origin as the "flying-pony" source
+     elements:
+       rotten-potato: 0
+   
+   # Specify the plugins defined locally
+   - origin: local
+     path: plugins/sources
+   
+     # Here again we MUST define a minimal bound API version,
+     # even though it's immaterial since it's revisioned with
+     # the project itself, it informs BuildStream that this
+     # source must be loaded in this way
      sources:
-     - plugins/local-sources
+       mysource: 0
 
+Project Version Format
+''''''''''''''''''''''
+
+The project's minimum required version of buildstream is specified in
+``project.conf`` with the ``required-project-version`` field, e.g.
+
+.. code:: yaml
+
+  # The minimum base BuildStream format
+  required-project-version: 0
 
 Versioning
 ~~~~~~~~~~
-The BuildStream format is guaranteed to be backwards compatible
-with any earlier releases. The core YAML format, the format supported
-by various plugins, and the overall BuildStream release version are
-revisioned separately.
 
 The ``project.conf`` allows asserting the minimal required core
 format version and the minimal required version for individual
@@ -110,7 +165,6 @@ plugins.
 
   required-versions:
 
-    # The minimum base BuildStream format
     project: 0
 
     # The minimum version of the autotools element