_pluginfactory: Delegating the work of locating plugins to the PluginOrigin

This way we split up the logic of how to load plugins from different
origins into their respective classes.

This commit also:

  o Introduces PluginType (which is currently either SOURCE or ELEMENT)

  o Reduces the complexity of the PluginFactory constructor

  o Kills the loaded_dependencies list and the all_loaded_plugins API,
    and replaces both of these with a new list_plugins() API.

    Consequently the jobpickler.py from the scheduler, and the
    widget.py from the frontend, are updated to use list_plugins().

  o Split up the PluginOrigin implementations into separate files

    Instead of having all PluginOrigin classes in pluginorigin.py, split
    it up into one base class and separate files for each implementation,
    which is more inline with BuildStream coding style.

    This has the unfortunate side effect of adding load_plugin_origin()
    into the __init__.py file, because keeping new_from_node() as
    a PluginOrigin class method cannot be done without introducing a
    cyclic dependency with PluginOrigin and it's implementations.

10 files changed, 429 insertions, 237 deletions

diff --git a/src/buildstream/_frontend/widget.py b/src/buildstream/_frontend/widget.py
index fcc951d00..81ca2f7b5 100644
--- a/src/buildstream/_frontend/widget.py
+++ b/src/buildstream/_frontend/widget.py
@@ -481,12 +481,13 @@ class LogLine(Widget):
 
         # Plugins
         text += self._format_plugins(
-            project.first_pass_config.element_factory.loaded_dependencies,
-            project.first_pass_config.source_factory.loaded_dependencies,
+            [p for p, _, _ in project.first_pass_config.element_factory.list_plugins()],
+            [p for p, _, _ in project.first_pass_config.source_factory.list_plugins()],
         )
         if project.config.element_factory and project.config.source_factory:
             text += self._format_plugins(
-                project.config.element_factory.loaded_dependencies, project.config.source_factory.loaded_dependencies
+                [p for p, _, _ in project.config.element_factory.list_plugins()],
+                [p for p, _, _ in project.config.source_factory.list_plugins()],
             )
 
         # Pipeline state
diff --git a/src/buildstream/_pluginfactory/__init__.py b/src/buildstream/_pluginfactory/__init__.py
index fe69b6e77..e724d48af 100644
--- a/src/buildstream/_pluginfactory/__init__.py
+++ b/src/buildstream/_pluginfactory/__init__.py
@@ -15,6 +15,33 @@
 #  License along with this library. If not, see <http://www.gnu.org/licenses/>.
 #
 
-from .pluginorigin import PluginOrigin, PluginOriginType
+from .pluginorigin import PluginOrigin, PluginOriginType, PluginType
+from .pluginoriginlocal import PluginOriginLocal
+from .pluginoriginpip import PluginOriginPip
 from .sourcefactory import SourceFactory
 from .elementfactory import ElementFactory
+
+
+# load_plugin_origin()
+#
+# Load a PluginOrigin from the YAML in project.conf
+#
+# Args:
+#    project (Project): The project from whence this origin is loaded
+#    origin_node (MappingNode): The node defining this origin
+#
+# Returns:
+#    (PluginOrigin): The newly created PluginOrigin
+#
+def load_plugin_origin(project, origin_node):
+
+    origin_type = origin_node.get_enum("origin", PluginOriginType)
+
+    if origin_type == PluginOriginType.LOCAL:
+        origin = PluginOriginLocal()
+    elif origin_type == PluginOriginType.PIP:
+        origin = PluginOriginPip()
+
+    origin.initialize(project, origin_node)
+
+    return origin
diff --git a/src/buildstream/_pluginfactory/elementfactory.py b/src/buildstream/_pluginfactory/elementfactory.py
index da6e8ac56..9854c1a5c 100644
--- a/src/buildstream/_pluginfactory/elementfactory.py
+++ b/src/buildstream/_pluginfactory/elementfactory.py
@@ -17,10 +17,8 @@
 #  Authors:
 #        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
 
-from .. import _site
-from ..element import Element
-
 from .pluginfactory import PluginFactory
+from .pluginorigin import PluginType
 
 
 # A ElementFactory creates Element instances
@@ -31,10 +29,7 @@ from .pluginfactory import PluginFactory
 #
 class ElementFactory(PluginFactory):
     def __init__(self, plugin_base):
-
-        super().__init__(
-            plugin_base, Element, [_site.element_plugins], "buildstream.plugins.elements",
-        )
+        super().__init__(plugin_base, PluginType.ELEMENT)
 
     # create():
     #
diff --git a/src/buildstream/_pluginfactory/pluginfactory.py b/src/buildstream/_pluginfactory/pluginfactory.py
index 042d0f565..22f62427e 100644
--- a/src/buildstream/_pluginfactory/pluginfactory.py
+++ b/src/buildstream/_pluginfactory/pluginfactory.py
@@ -18,27 +18,28 @@
 #        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
 
 import os
-import inspect
-from typing import Tuple, Type
+from typing import Tuple, Type, Iterator
+from pluginbase import PluginSource
 
 from .. import utils
+from .. import _site
 from ..plugin import Plugin
+from ..source import Source
+from ..element import Element
 from ..node import ProvenanceInformation
 from ..utils import UtilError
 from .._exceptions import PluginError
 from .._messenger import Messenger
 from .._message import Message, MessageType
 
-from .pluginorigin import PluginOrigin, PluginOriginType
+from .pluginorigin import PluginOrigin, PluginType
 
 
 # A Context for loading plugin types
 #
 # Args:
 #     plugin_base (PluginBase): The main PluginBase object to work with
-#     base_type (type):         A base object type for this context
-#     site_plugin_path (str):   Path to where buildstream keeps plugins
-#     entrypoint_group (str):   Name of the entry point group that provides plugins
+#     plugin_type (PluginType): The type of plugin to load
 #
 # Since multiple pipelines can be processed recursively
 # within the same interpretor, it's important that we have
@@ -48,14 +49,7 @@ from .pluginorigin import PluginOrigin, PluginOriginType
 # Pipelines.
 #
 class PluginFactory:
-    def __init__(self, plugin_base, base_type, site_plugin_path, entrypoint_group):
-
-        # The plugin kinds which were loaded
-        self.loaded_dependencies = []
-
-        #
-        # Private members
-        #
+    def __init__(self, plugin_base, plugin_type):
 
         # For pickling across processes, make sure this context has a unique
         # identifier, which we prepend to the identifier of each PluginSource.
@@ -63,22 +57,39 @@ class PluginFactory:
         # from eachother.
         self._identifier = str(id(self))
 
-        self._base_type = base_type  # The base class plugins derive from
+        self._plugin_type = plugin_type  # The kind of plugins this factory loads
         self._types = {}  # Plugin type lookup table by kind
         self._origins = {}  # PluginOrigin lookup table by kind
         self._allow_deprecated = {}  # Lookup table to check if a plugin is allowed to be deprecated
 
-        # The PluginSource object
-        self._plugin_base = plugin_base
-        self._site_plugin_path = site_plugin_path
-        self._entrypoint_group = entrypoint_group
-        self._alternate_sources = {}
+        self._plugin_base = plugin_base  # The PluginBase object
+
+        # The PluginSource objects need to be kept in scope for the lifetime
+        # of the loaded plugins, otherwise the PluginSources delete the plugin
+        # modules when they go out of scope.
+        #
+        # FIXME: Instead of keeping this table, we can call:
+        #
+        #            PluginBase.make_plugin_source(..., persist=True)
+        #
+        #        The persist attribute avoids this behavior. This is not currently viable
+        #        because the BuildStream data model (projects and elements) does not properly
+        #        go out of scope when the CLI completes, causing errors to occur when
+        #        invoking BuildStream multiple times during tests.
+        #
+        self._sources = {}  #  A mapping of (location, kind) -> PluginSource objects
 
         self._init_site_source()
 
+    # Initialize the PluginSource object for core plugins
     def _init_site_source(self):
+        if self._plugin_type == PluginType.SOURCE:
+            self._site_plugins_path = _site.source_plugins
+        elif self._plugin_type == PluginType.ELEMENT:
+            self._site_plugins_path = _site.element_plugins
+
         self._site_source = self._plugin_base.make_plugin_source(
-            searchpath=self._site_plugin_path, identifier=self._identifier + "site",
+            searchpath=[self._site_plugins_path], identifier=self._identifier + "site",
         )
 
     def __getstate__(self):
@@ -91,8 +102,6 @@ class PluginFactory:
         # get rid of those. It is only a cache - we will automatically recreate
         # them on demand.
         #
-        # Similarly we must clear out the `_alternate_sources` cache.
-        #
         # Note that this method of referring to members is error-prone in that
         # a later 'search and replace' renaming might miss these. Guard against
         # this by making sure we are not creating new members, only clearing
@@ -101,8 +110,8 @@ class PluginFactory:
         del state["_site_source"]
         assert "_types" in state
         state["_types"] = {}
-        assert "_alternate_sources" in state
-        state["_alternate_sources"] = {}
+        assert "_sources" in state
+        state["_sources"] = {}
 
         return state
 
@@ -116,6 +125,29 @@ class PluginFactory:
         # BuildStream, so the identifier is not restored here.
         self._init_site_source()
 
+    ######################################################
+    #                  Public Methods                    #
+    ######################################################
+
+    # register_plugin_origin():
+    #
+    # Registers the PluginOrigin to use for the given plugin kind
+    #
+    # Args:
+    #    kind (str): The kind identifier of the Plugin
+    #    origin (PluginOrigin): The PluginOrigin providing the plugin
+    #    allow_deprecated (bool): Whether this plugin kind is allowed to be used in a deprecated state
+    #
+    def register_plugin_origin(self, kind: str, origin: PluginOrigin, allow_deprecated: bool):
+        if kind in self._origins:
+            raise PluginError(
+                "More than one {} plugin registered as kind '{}'".format(self._plugin_type, kind),
+                reason="duplicate-plugin",
+            )
+
+        self._origins[kind] = origin
+        self._allow_deprecated[kind] = allow_deprecated
+
     # lookup():
     #
     # Fetches a type loaded from a plugin in this plugin context
@@ -158,195 +190,197 @@ class PluginFactory:
 
         return plugin_type, defaults
 
-    # register_plugin_origin():
+    # list_plugins():
     #
-    # Registers the PluginOrigin to use for the given plugin kind
+    # A generator which yields all of the plugins which have been loaded
     #
-    # Args:
-    #    kind (str): The kind identifier of the Plugin
-    #    origin (PluginOrigin): The PluginOrigin providing the plugin
-    #    allow_deprecated (bool): Whether this plugin kind is allowed to be used in a deprecated state
+    # Yields:
+    #    (str): The plugin kind
+    #    (type): The loaded plugin type
+    #    (str): The default yaml file, if any
     #
-    def register_plugin_origin(self, kind: str, origin: PluginOrigin, allow_deprecated: bool):
-        if kind in self._origins:
-            raise PluginError(
-                "More than one {} plugin registered as kind '{}'".format(self._base_type.__name__, kind),
-                reason="duplicate-plugin",
-            )
-
-        self._origins[kind] = origin
-        self._allow_deprecated[kind] = allow_deprecated
+    def list_plugins(self) -> Iterator[Tuple[str, Type[Plugin], str]]:
+        for kind, (plugin_type, defaults) in self._types.items():
+            yield kind, plugin_type, defaults
 
-    # all_loaded_plugins():
+    # get_plugin_paths():
     #
-    # Returns: an iterable over all the loaded plugins.
+    # Gets the directory on disk where the plugin itself is located,
+    # and a full path to the plugin's accompanying YAML file for
+    # it's defaults (if any).
     #
-    def all_loaded_plugins(self):
-        return self._types.values()
-
-    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], identifier=self._identifier + 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:
-            import pkg_resources
-
+    # Args:
+    #    kind (str): The plugin kind
+    #
+    # Returns:
+    #    (str): The full path to the directory containing the plugin
+    #    (str): The full path to the accompanying .yaml file containing
+    #           the plugin's preferred defaults.
+    #
+    def get_plugin_paths(self, kind: str):
+        try:
             origin = self._origins[kind]
+        except KeyError:
+            return None, None
 
-            # key by a tuple to avoid collision
-            try:
-                package = pkg_resources.get_entry_info(package_name, self._entrypoint_group, kind)
-            except pkg_resources.DistributionNotFound as e:
-                raise PluginError(
-                    "{}: Failed to load {} plugin '{}': {}".format(
-                        origin.provenance, self._base_type.__name__, kind, e
-                    ),
-                    reason="package-not-found",
-                ) from e
-            except pkg_resources.VersionConflict as e:
-                raise PluginError(
-                    "{}: Version conflict encountered while loading {} plugin '{}'".format(
-                        origin.provenance, self._base_type.__name__, kind
-                    ),
-                    detail=e.report(),
-                    reason="package-version-conflict",
-                ) from e
-            except pkg_resources.RequirementParseError as e:
-                raise PluginError(
-                    "{}: Malformed package-name '{}' encountered: {}".format(origin.provenance, package_name, e),
-                    reason="package-malformed-requirement",
-                ) from e
-
-            if package is None:
-                raise PluginError(
-                    "{}: Pip package {} does not contain a plugin named '{}'".format(
-                        origin.provenance, package_name, kind
-                    ),
-                    reason="plugin-not-found",
-                )
+        return origin.get_plugin_paths(kind, self._plugin_type)
 
-            location = package.dist.get_resource_filename(
-                pkg_resources._manager, package.module_name.replace(".", os.sep) + ".py"
-            )
+    ######################################################
+    #                 Private Methods                    #
+    ######################################################
 
-            # Also load the defaults - required since setuptools
-            # may need to extract the file.
-            try:
-                defaults = package.dist.get_resource_filename(
-                    pkg_resources._manager, package.module_name.replace(".", os.sep) + ".yaml"
-                )
-            except KeyError:
-                # The plugin didn't have an accompanying YAML file
-                defaults = None
+    # _ensure_plugin():
+    #
+    # Ensures that a plugin is loaded, delegating the work of getting
+    # the plugin materials from the respective PluginOrigin
+    #
+    # Args:
+    #    kind (str): The plugin kind to load
+    #    provenance (str): The provenance of whence the plugin was referred to in the project
+    #
+    # Returns:
+    #    (type): The loaded type
+    #    (str): The full path the the yaml file containing defaults, or None
+    #
+    # Raises:
+    #    (PluginError): In case something went wrong loading the plugin
+    #
+    def _ensure_plugin(self, kind: str, provenance: ProvenanceInformation) -> Tuple[Type[Plugin], str]:
 
-            source = self._plugin_base.make_plugin_source(
-                searchpath=[os.path.dirname(location)], identifier=self._identifier + os.path.dirname(location),
-            )
-            self._alternate_sources[("pip", package_name)] = source
+        if kind not in self._types:
 
-        else:
-            source = self._alternate_sources[("pip", package_name)]
+            # Get the directory on disk where the plugin exists, and
+            # the optional accompanying .yaml file for the plugin, should
+            # one have been provided.
+            #
+            location, defaults = self.get_plugin_paths(kind)
 
-        return source, defaults
+            if location:
 
-    def _ensure_plugin(self, kind: str, provenance: ProvenanceInformation) -> Tuple[Type[Plugin], str]:
+                # Make the PluginSource object
+                #
+                source = self._plugin_base.make_plugin_source(
+                    searchpath=[location], identifier=self._identifier + location + kind,
+                )
 
-        if kind not in self._types:
-            source = None
-            defaults = None
-
-            origin = self._origins.get(kind, None)
-            if origin:
-                # Try getting the plugin source from a registered origin
-                if origin.origin_type == PluginOriginType.LOCAL:
-                    source = self._get_local_plugin_source(origin.path)
-                elif origin.origin_type == PluginOriginType.PIP:
-                    source, defaults = self._get_pip_plugin_source(origin.package_name, kind)
-                else:
-                    assert False, "Encountered invalid plugin origin type"
+                # Keep a reference on the PluginSources (see comment in __init__)
+                #
+                self._sources[(location, kind)] = source
             else:
                 # Try getting it from the core plugins
                 if kind not in self._site_source.list_plugins():
                     raise PluginError(
-                        "{}: No {} type registered for kind '{}'".format(provenance, self._base_type.__name__, kind),
+                        "{}: No {} plugin registered for kind '{}'".format(provenance, self._plugin_type, kind),
                         reason="plugin-not-found",
                     )
 
                 source = self._site_source
+                defaults = os.path.join(self._site_plugins_path, "{}.yaml".format(kind))
+                if not os.path.exists(defaults):
+                    defaults = None
 
-            self._types[kind] = self._load_plugin(source, kind, defaults)
-            self.loaded_dependencies.append(kind)
+            self._types[kind] = (self._load_plugin(source, kind), defaults)
 
         return self._types[kind]
 
-    def _load_plugin(self, source, kind, defaults):
+    # _load_plugin():
+    #
+    # Loads the actual plugin type from the PluginSource
+    #
+    # Args:
+    #    source (PluginSource): The PluginSource
+    #    kind (str): The plugin kind to load
+    #
+    # Returns:
+    #    (type): The loaded type
+    #
+    # Raises:
+    #    (PluginError): In case something went wrong loading the plugin
+    #
+    def _load_plugin(self, source: PluginSource, kind: str) -> Type[Plugin]:
 
         try:
             plugin = source.load_plugin(kind)
 
-            if not defaults:
-                plugin_file = inspect.getfile(plugin)
-                plugin_dir = os.path.dirname(plugin_file)
-                plugin_conf_name = "{}.yaml".format(kind)
-                defaults = os.path.join(plugin_dir, plugin_conf_name)
-
         except ImportError as e:
-            raise PluginError("Failed to load {} plugin '{}': {}".format(self._base_type.__name__, kind, e)) from e
+            raise PluginError("Failed to load {} plugin '{}': {}".format(self._plugin_type, kind, e)) from e
 
         try:
             plugin_type = plugin.setup()
         except AttributeError as e:
             raise PluginError(
-                "{} plugin '{}' did not provide a setup() function".format(self._base_type.__name__, kind),
+                "{} plugin '{}' did not provide a setup() function".format(self._plugin_type, kind),
                 reason="missing-setup-function",
             ) from e
         except TypeError as e:
             raise PluginError(
-                "setup symbol in {} plugin '{}' is not a function".format(self._base_type.__name__, kind),
+                "setup symbol in {} plugin '{}' is not a function".format(self._plugin_type, kind),
                 reason="setup-is-not-function",
             ) from e
 
         self._assert_plugin(kind, plugin_type)
         self._assert_min_version(kind, plugin_type)
 
-        return (plugin_type, defaults)
+        return plugin_type
 
-    def _assert_plugin(self, kind, plugin_type):
+    # _assert_plugin():
+    #
+    # Performs assertions on the loaded plugin
+    #
+    # Args:
+    #    kind (str): The plugin kind to load
+    #    plugin_type (type): The loaded plugin type
+    #
+    # Raises:
+    #    (PluginError): In case something went wrong loading the plugin
+    #
+    def _assert_plugin(self, kind: str, plugin_type: Type[Plugin]):
         if kind in self._types:
             raise PluginError(
                 "Tried to register {} plugin for existing kind '{}' "
-                "(already registered {})".format(self._base_type.__name__, kind, self._types[kind].__name__)
+                "(already registered {})".format(self._plugin_type, kind, self._types[kind].__name__)
             )
+
+        base_type: Type[Plugin]
+        if self._plugin_type == PluginType.SOURCE:
+            base_type = Source
+        elif self._plugin_type == PluginType.ELEMENT:
+            base_type = Element
+
         try:
-            if not issubclass(plugin_type, self._base_type):
+            if not issubclass(plugin_type, base_type):
                 raise PluginError(
                     "{} plugin '{}' returned type '{}', which is not a subclass of {}".format(
-                        self._base_type.__name__, kind, plugin_type.__name__, self._base_type.__name__
+                        self._plugin_type, kind, plugin_type.__name__, base_type.__name__
                     ),
                     reason="setup-returns-bad-type",
                 )
         except TypeError as e:
             raise PluginError(
                 "{} plugin '{}' returned something that is not a type (expected subclass of {})".format(
-                    self._base_type.__name__, kind, self._base_type.__name__
+                    self._plugin_type, kind, self._plugin_type
                 ),
                 reason="setup-returns-not-type",
             ) from e
 
+    # _assert_min_version():
+    #
+    # Performs the version checks on the loaded plugin type,
+    # ensuring that the loaded plugin is intended to work
+    # with this version of BuildStream.
+    #
+    # Args:
+    #    kind (str): The plugin kind to load
+    #    plugin_type (type): The loaded plugin type
+    #
+    # Raises:
+    #    (PluginError): In case something went wrong loading the plugin
+    #
     def _assert_min_version(self, kind, plugin_type):
 
         if plugin_type.BST_MIN_VERSION is None:
             raise PluginError(
-                "{} plugin '{}' did not specify BST_MIN_VERSION".format(self._base_type.__name__, kind),
+                "{} plugin '{}' did not specify BST_MIN_VERSION".format(self._plugin_type, kind),
                 reason="missing-min-version",
                 detail="Are you trying to use a BuildStream 1 plugin with a BuildStream 2 project ?",
             )
@@ -356,7 +390,7 @@ class PluginFactory:
         except UtilError as e:
             raise PluginError(
                 "{} plugin '{}' specified malformed BST_MIN_VERSION: {}".format(
-                    self._base_type.__name__, kind, plugin_type.BST_MIN_VERSION
+                    self._plugin_type, kind, plugin_type.BST_MIN_VERSION
                 ),
                 reason="malformed-min-version",
                 detail="BST_MIN_VERSION must be specified as 'MAJOR.MINOR' with "
@@ -368,7 +402,7 @@ class PluginFactory:
         if min_version_major != bst_major:
             raise PluginError(
                 "{} plugin '{}' requires BuildStream {}, but is being loaded with BuildStream {}".format(
-                    self._base_type.__name__, kind, min_version_major, bst_major
+                    self._plugin_type, kind, min_version_major, bst_major
                 ),
                 reason="incompatible-major-version",
                 detail="You will need to find the correct version of this plugin for your project.",
@@ -377,7 +411,7 @@ class PluginFactory:
         if min_version_minor > bst_minor:
             raise PluginError(
                 "{} plugin '{}' requires BuildStream {}, but is being loaded with BuildStream {}.{}".format(
-                    self._base_type.__name__, kind, plugin_type.BST_MIN_VERSION, bst_major, bst_minor
+                    self._plugin_type, kind, plugin_type.BST_MIN_VERSION, bst_major, bst_minor
                 ),
                 reason="incompatible-minor-version",
                 detail="Please upgrade to BuildStream {}".format(plugin_type.BST_MIN_VERSION),
diff --git a/src/buildstream/_pluginfactory/pluginorigin.py b/src/buildstream/_pluginfactory/pluginorigin.py
index e865006ac..43080c662 100644
--- a/src/buildstream/_pluginfactory/pluginorigin.py
+++ b/src/buildstream/_pluginfactory/pluginorigin.py
@@ -15,20 +15,38 @@
 #  License along with this library. If not, see <http://www.gnu.org/licenses/>.
 #
 
-import os
-
 from ..types import FastEnum
 from ..node import ScalarNode, MappingNode
 from .._exceptions import LoadError
 from ..exceptions import LoadErrorReason
 
 
+# PluginType()
+#
+# A type of plugin
+#
+class PluginType(FastEnum):
+
+    # A Source plugin
+    SOURCE = "source"
+
+    # An Element plugin
+    ELEMENT = "element"
+
+    def __str__(self):
+        return str(self.value)
+
+
 # PluginOriginType:
 #
 # An enumeration depicting the type of plugin origin
 #
 class PluginOriginType(FastEnum):
+
+    # A local plugin
     LOCAL = "local"
+
+    # A pip plugin
     PIP = "pip"
 
 
@@ -59,49 +77,64 @@ class PluginOrigin:
         self.elements = {}  # A dictionary of PluginConfiguration
         self.sources = {}  # A dictionary of PluginConfiguration objects
         self.provenance = None
+        self.project = None
 
         # Private
-        self._project = None
         self._kinds = {}
         self._allow_deprecated = False
 
-    # new_from_node()
+    # initialize()
     #
-    # Load a PluginOrigin from the YAML in project.conf
+    # Initializes the origin, resulting in loading the origin
+    # node.
+    #
+    # This is the bottom half of the initialization, it is done
+    # separately because load_plugin_origin() needs to stay in
+    # __init__.py in order to avoid cyclic dependencies between
+    # PluginOrigin and it's subclasses.
     #
     # Args:
-    #    project (Project): The project from whence this origin is loaded
+    #    project (Project): The project this PluginOrigin was loaded for
     #    origin_node (MappingNode): The node defining this origin
     #
-    # Returns:
-    #    (PluginOrigin): The newly created PluginOrigin
-    #
-    @classmethod
-    def new_from_node(cls, project, origin_node):
-
-        origin_type = origin_node.get_enum("origin", PluginOriginType)
-
-        if origin_type == PluginOriginType.LOCAL:
-            origin = PluginOriginLocal()
-        elif origin_type == PluginOriginType.PIP:
-            origin = PluginOriginPip()
+    def initialize(self, project, origin_node):
 
-        origin.provenance = origin_node.get_provenance()
-        origin._project = project
-        origin._load(origin_node)
+        self.provenance = origin_node.get_provenance()
+        self.project = project
+        self.load_config(origin_node)
 
         # Parse commonly defined aspects of PluginOrigins
-        origin._allow_deprecated = origin_node.get_bool("allow-deprecated", False)
+        self._allow_deprecated = origin_node.get_bool("allow-deprecated", False)
 
         element_sequence = origin_node.get_sequence("elements", [])
-        origin._load_plugin_configurations(element_sequence, origin.elements)
+        self._load_plugin_configurations(element_sequence, self.elements)
 
         source_sequence = origin_node.get_sequence("sources", [])
-        origin._load_plugin_configurations(source_sequence, origin.sources)
+        self._load_plugin_configurations(source_sequence, self.sources)
 
-        return origin
+    ##############################################
+    #              Abstract methods              #
+    ##############################################
+
+    # get_plugin_paths():
+    #
+    # Abstract method for loading the details about a specific plugin,
+    # the PluginFactory uses this to get the assets needed to actually
+    # load the plugins.
+    #
+    # Args:
+    #    kind (str): The plugin
+    #    plugin_type (PluginType): The kind of plugin to load
+    #
+    # Returns:
+    #    (str): The full path to the directory containing the plugin
+    #    (str): The full path to the accompanying .yaml file containing
+    #           the plugin's preferred defaults.
+    #
+    def get_plugin_paths(self, kind, plugin_type):
+        pass
 
-    # _load()
+    # load_config()
     #
     # Abstract method for loading data from the origin node, this
     # method should not load the source and element lists.
@@ -109,9 +142,13 @@ class PluginOrigin:
     # Args:
     #    origin_node (MappingNode): The node defining this origin
     #
-    def _load(self, origin_node):
+    def load_config(self, origin_node):
         pass
 
+    ##############################################
+    #               Private methods              #
+    ##############################################
+
     # _load_plugin_configurations()
     #
     # Helper function to load the list of source or element
@@ -143,43 +180,3 @@ class PluginOrigin:
                 )
 
             dictionary[kind] = conf
-
-
-# PluginOriginLocal
-#
-# PluginOrigin for local plugins
-#
-class PluginOriginLocal(PluginOrigin):
-    def __init__(self):
-        super().__init__(PluginOriginType.LOCAL)
-
-        # An absolute path to where the plugin can be found
-        #
-        self.path = None
-
-    def _load(self, origin_node):
-
-        origin_node.validate_keys(["path", *PluginOrigin._COMMON_CONFIG_KEYS])
-
-        path_node = origin_node.get_scalar("path")
-        path = self._project.get_path_from_node(path_node, check_is_dir=True)
-
-        self.path = os.path.join(self._project.directory, path)
-
-
-# PluginOriginPip
-#
-# PluginOrigin for pip plugins
-#
-class PluginOriginPip(PluginOrigin):
-    def __init__(self):
-        super().__init__(PluginOriginType.PIP)
-
-        # The pip package name to extract plugins from
-        #
-        self.package_name = None
-
-    def _load(self, origin_node):
-
-        origin_node.validate_keys(["package-name", *PluginOrigin._COMMON_CONFIG_KEYS])
-        self.package_name = origin_node.get_str("package-name")
diff --git a/src/buildstream/_pluginfactory/pluginoriginlocal.py b/src/buildstream/_pluginfactory/pluginoriginlocal.py
new file mode 100644
index 000000000..5cfe2fd3a
--- /dev/null
+++ b/src/buildstream/_pluginfactory/pluginoriginlocal.py
@@ -0,0 +1,47 @@
+#
+#  Copyright (C) 2020 Codethink Limited
+#
+#  This program is free software; you can redistribute it and/or
+#  modify it under the terms of the GNU Lesser General Public
+#  License as published by the Free Software Foundation; either
+#  version 2 of the License, or (at your option) any later version.
+#
+#  This library is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the GNU
+#  Lesser General Public License for more details.
+#
+#  You should have received a copy of the GNU Lesser General Public
+#  License along with this library. If not, see <http://www.gnu.org/licenses/>.
+#
+import os
+
+from .pluginorigin import PluginOrigin, PluginOriginType
+
+
+# PluginOriginLocal
+#
+# PluginOrigin for local plugins
+#
+class PluginOriginLocal(PluginOrigin):
+    def __init__(self):
+        super().__init__(PluginOriginType.LOCAL)
+
+        # An absolute path to where plugins from this origin are found
+        self._path = None
+
+    def get_plugin_paths(self, kind, plugin_type):
+        defaults = os.path.join(self._path, "{}.yaml".format(kind))
+        if not os.path.exists(defaults):
+            defaults = None
+
+        return self._path, defaults
+
+    def load_config(self, origin_node):
+
+        origin_node.validate_keys(["path", *PluginOrigin._COMMON_CONFIG_KEYS])
+
+        path_node = origin_node.get_scalar("path")
+        path = self.project.get_path_from_node(path_node, check_is_dir=True)
+
+        self._path = os.path.join(self.project.directory, path)
diff --git a/src/buildstream/_pluginfactory/pluginoriginpip.py b/src/buildstream/_pluginfactory/pluginoriginpip.py
new file mode 100644
index 000000000..3a9c63f7e
--- /dev/null
+++ b/src/buildstream/_pluginfactory/pluginoriginpip.py
@@ -0,0 +1,97 @@
+#
+#  Copyright (C) 2020 Codethink Limited
+#
+#  This program is free software; you can redistribute it and/or
+#  modify it under the terms of the GNU Lesser General Public
+#  License as published by the Free Software Foundation; either
+#  version 2 of the License, or (at your option) any later version.
+#
+#  This library is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the GNU
+#  Lesser General Public License for more details.
+#
+#  You should have received a copy of the GNU Lesser General Public
+#  License along with this library. If not, see <http://www.gnu.org/licenses/>.
+#
+import os
+
+from .._exceptions import PluginError
+
+from .pluginorigin import PluginType, PluginOrigin, PluginOriginType
+
+
+# PluginOriginPip
+#
+# PluginOrigin for pip plugins
+#
+class PluginOriginPip(PluginOrigin):
+    def __init__(self):
+        super().__init__(PluginOriginType.PIP)
+
+        # The pip package name to extract plugins from
+        #
+        self._package_name = None
+
+    def get_plugin_paths(self, kind, plugin_type):
+
+        import pkg_resources
+
+        # Sources and elements are looked up in separate
+        # entrypoint groups from the same package.
+        #
+        if plugin_type == PluginType.SOURCE:
+            entrypoint_group = "buildstream.plugins.sources"
+        elif plugin_type == PluginType.ELEMENT:
+            entrypoint_group = "buildstream.plugins.elements"
+
+        # key by a tuple to avoid collision
+        try:
+            package = pkg_resources.get_entry_info(self._package_name, entrypoint_group, kind)
+        except pkg_resources.DistributionNotFound as e:
+            raise PluginError(
+                "{}: Failed to load {} plugin '{}': {}".format(self.provenance, plugin_type, kind, e),
+                reason="package-not-found",
+            ) from e
+        except pkg_resources.VersionConflict as e:
+            raise PluginError(
+                "{}: Version conflict encountered while loading {} plugin '{}'".format(
+                    self.provenance, plugin_type, kind
+                ),
+                detail=e.report(),
+                reason="package-version-conflict",
+            ) from e
+        except pkg_resources.RequirementParseError as e:
+            raise PluginError(
+                "{}: Malformed package-name '{}' encountered: {}".format(self.provenance, self._package_name, e),
+                reason="package-malformed-requirement",
+            ) from e
+
+        if package is None:
+            raise PluginError(
+                "{}: Pip package {} does not contain a plugin named '{}'".format(
+                    self.provenance, self._package_name, kind
+                ),
+                reason="plugin-not-found",
+            )
+
+        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.
+        try:
+            defaults = package.dist.get_resource_filename(
+                pkg_resources._manager, package.module_name.replace(".", os.sep) + ".yaml"
+            )
+        except KeyError:
+            # The plugin didn't have an accompanying YAML file
+            defaults = None
+
+        return os.path.dirname(location), defaults
+
+    def load_config(self, origin_node):
+
+        origin_node.validate_keys(["package-name", *PluginOrigin._COMMON_CONFIG_KEYS])
+        self._package_name = origin_node.get_str("package-name")
diff --git a/src/buildstream/_pluginfactory/sourcefactory.py b/src/buildstream/_pluginfactory/sourcefactory.py
index d616702ef..2ed78f838 100644
--- a/src/buildstream/_pluginfactory/sourcefactory.py
+++ b/src/buildstream/_pluginfactory/sourcefactory.py
@@ -17,11 +17,8 @@
 #  Authors:
 #        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
 
-from .. import _site
-from ..source import Source
-
 from .pluginfactory import PluginFactory
-
+from .pluginorigin import PluginType
 
 # A SourceFactory creates Source instances
 # in the context of a given factory
@@ -31,10 +28,7 @@ from .pluginfactory import PluginFactory
 #
 class SourceFactory(PluginFactory):
     def __init__(self, plugin_base):
-
-        super().__init__(
-            plugin_base, Source, [_site.source_plugins], "buildstream.plugins.sources",
-        )
+        super().__init__(plugin_base, PluginType.SOURCE)
 
     # create():
     #
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 1c6e7e950..508afa68b 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -36,7 +36,7 @@ from ._artifactcache import ArtifactCache
 from ._sourcecache import SourceCache
 from .node import ScalarNode, SequenceNode, _assert_symbol_name
 from .sandbox import SandboxRemote
-from ._pluginfactory import ElementFactory, SourceFactory, PluginOrigin
+from ._pluginfactory import ElementFactory, SourceFactory, load_plugin_origin
 from .types import CoreWarnings
 from ._projectrefs import ProjectRefs, ProjectRefStorage
 from ._loader import Loader
@@ -951,7 +951,7 @@ class Project:
         # Load the plugin origins and register them to their factories
         origins = config.get_sequence("plugins", default=[])
         for origin_node in origins:
-            origin = PluginOrigin.new_from_node(self, origin_node)
+            origin = load_plugin_origin(self, origin_node)
             for kind, conf in origin.elements.items():
                 output.element_factory.register_plugin_origin(kind, origin, conf.allow_deprecated)
             for kind, conf in origin.sources.items():
diff --git a/src/buildstream/_scheduler/jobs/jobpickler.py b/src/buildstream/_scheduler/jobs/jobpickler.py
index 066e518c8..1ebad7d49 100644
--- a/src/buildstream/_scheduler/jobs/jobpickler.py
+++ b/src/buildstream/_scheduler/jobs/jobpickler.py
@@ -141,7 +141,7 @@ def _pickle_child_job_data(child_job_data, projects):
     ]
 
     plugin_class_to_factory = {
-        cls: factory for factory in factory_list if factory is not None for cls, _ in factory.all_loaded_plugins()
+        cls: factory for factory in factory_list if factory is not None for _, cls, _ in factory.list_plugins()
     }
 
     pickled_data = io.BytesIO()