Merge branch 'tristan/pip-plugin-versioning' into 'master'

pip plugin origin versioning

See merge request BuildStream/buildstream!1894

13 files changed, 366 insertions, 13 deletions

diff --git a/doc/source/format_project.rst b/doc/source/format_project.rst
index 211fc4dd0..aa2168d3f 100644
--- a/doc/source/format_project.rst
+++ b/doc/source/format_project.rst
@@ -371,8 +371,8 @@ A default mirror to consult first can be defined via
 
 .. _project_plugins:
 
-External plugins
-----------------
+Loading plugins
+---------------
 If your project makes use of any custom :mod:`Element <buildstream.element>` or
 :mod:`Source <buildstream.source>` plugins, then the project must inform BuildStream
 of the plugins it means to make use of and the origin from which they can be loaded.
@@ -385,14 +385,8 @@ Local plugins
 Local plugins are expected to be found in a subdirectory of the actual
 BuildStream project. :mod:`Element <buildstream.element>` and
 :mod:`Source <buildstream.source>` plugins should be stored in separate
-directories to avoid namespace collisions.
-
-The versions of local plugins are largely immaterial since they are
-revisioned along with the project by the user, usually in a VCS like git.
-However, for the sake of consistency with other plugin loading origins
-we require that you specify a version, this can always be ``0`` for a local
-plugin.
-
+directories to avoid namespace collisions, you can achieve this by
+specifying a separate *origin* for sources and elements.
 
 .. code:: yaml
 
@@ -406,6 +400,17 @@ plugin.
      sources:
      - mysource
 
+There is no strict versioning policy for plugins loaded from the local
+origin because the plugin is provided with the project data and as such,
+it is considered to be completely deterministic.
+
+Usually your project will be managed by a VCS like git, and any changes
+to your local plugins may have an impact on your project, such as changes
+to the artifact cache keys produced by elements which use these plugins.
+Changes to plugins might provide new YAML configuration options, changes
+in the semantics of existing configurations or even removal of existing
+YAML configurations.
+
 
 Pip plugins
 ~~~~~~~~~~~
@@ -431,6 +436,107 @@ system.
      elements:
      - starch
 
+Unlike local plugins, plugins loaded from the ``pip`` origin are
+loaded from the active *python environment*, and as such you do not
+usually have full control over the plugins your project uses unless
+one uses strict :ref:`version constraints <project_plugins_pip_version_constraints>`.
+
+The official plugin packages maintained by the BuildStream community are
+guaranteed to be fully API stable. If one chooses to load these plugins
+from the ``pip`` origin, then it is recommended to use *minimal bound dependency*
+:ref:`constraints <project_plugins_pip_version_constraints>` when using
+official plugin packages so as to be sure that you have access to all the
+features you intend to use in your project.
+
+
+.. _project_plugins_pip_version_constraints:
+
+Versioning constraints
+''''''''''''''''''''''
+When loading plugins from the ``pip`` plugin origin, it is possible to
+specify constraints on the versions of packages you want to load
+your plugins from.
+
+The syntax for specifying constraints are `explained here <https://python-poetry.org/docs/versions/>`_,
+and they are the same format supported by the ``pip`` package manager.
+
+.. note::
+
+   In order to be certain that versioning constraints work properly, plugin
+   packages should be careful to adhere to `PEP 440, Version Identification and Dependency
+   Specification <https://www.python.org/dev/peps/pep-0440/>`_.
+
+Here are a couple of examples:
+
+**Specifying minimal bound dependencies**:
+
+.. code:: yaml
+
+   plugins:
+
+   - origin: pip
+
+     # This project uses the API stable potato project and
+     # requires features from at least version 1.2
+     #
+     package-name: potato>=1.2
+
+**Specifying exact versions**:
+
+.. code:: yaml
+
+   plugins:
+
+   - origin: pip
+
+     # This project requires plugins from the potato
+     # project at exactly version 1.2.3
+     #
+     package-name: potato==1.2.3
+
+**Specifying version constraints**:
+
+.. code:: yaml
+
+   plugins:
+
+   - origin: pip
+
+     # This project requires plugins from the potato
+     # project from version 1.2.3 onward until 1.3.
+     #
+     package-name: potato>=1.2.3,<1.3
+
+.. important::
+
+   **Unstable plugin packages**
+
+   When using unstable plugins loaded from the ``pip`` origin, the installed
+   plugins can sometimes be incompatible with your project.
+
+   **Use virtual environments**
+
+   Your operating system's default python environment can only have one
+   version of a given package installed at a time, if you work on multiple
+   BuildStream projects on the same host, they may not agree on which versions
+   of plugins to use.
+
+   In order to guarantee that you can use a specific version of a plugin,
+   you may need to install BuildStream into a *virtual environment* in order
+   to control which python package versions are available when using your
+   project.
+
+   **Possible junction conflicts**
+
+   If you have multiple projects which are connected through
+   :mod:`junction <elements.junction>` elements, these projects can disagree
+   on which version of a plugin is needed from the ``pip`` origin.
+
+   Since only one version of a given plugin *package* can be installed
+   at a time in a given *python environment*, you must ensure that all
+   projects connected through :mod:`junction <elements.junction>` elements
+   agree on which versions of API unstable plugin packages to use.
+
 
 .. _project_plugins_deprecation:
 
diff --git a/setup.cfg b/setup.cfg
index 0853264f6..8f095fab1 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -9,7 +9,7 @@ parentdir_prefix = BuildStream-
 
 [tool:pytest]
 addopts = --verbose --basetemp ./tmp --durations=20 --timeout=1800
-norecursedirs = src tests/integration/project tests/plugins/loading integration-cache tmp __pycache__ .eggs
+norecursedirs = src tests/integration/project tests/plugins/loading tests/plugins/sample-plugins integration-cache tmp __pycache__ .eggs
 python_files = tests/*/*.py
 env =
     D:BST_TEST_SUITE=True
diff --git a/src/buildstream/_pluginfactory/pluginfactory.py b/src/buildstream/_pluginfactory/pluginfactory.py
index 2de110ed4..042d0f565 100644
--- a/src/buildstream/_pluginfactory/pluginfactory.py
+++ b/src/buildstream/_pluginfactory/pluginfactory.py
@@ -200,14 +200,39 @@ class PluginFactory:
         if ("pip", package_name) not in self._alternate_sources:
             import pkg_resources
 
+            origin = self._origins[kind]
+
             # 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(self._base_type.__name__, kind, e)) from 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(package_name, kind))
+                raise PluginError(
+                    "{}: Pip package {} does not contain a plugin named '{}'".format(
+                        origin.provenance, package_name, kind
+                    ),
+                    reason="plugin-not-found",
+                )
 
             location = package.dist.get_resource_filename(
                 pkg_resources._manager, package.module_name.replace(".", os.sep) + ".py"
diff --git a/src/buildstream/_pluginfactory/pluginorigin.py b/src/buildstream/_pluginfactory/pluginorigin.py
index 14d7a76bf..e865006ac 100644
--- a/src/buildstream/_pluginfactory/pluginorigin.py
+++ b/src/buildstream/_pluginfactory/pluginorigin.py
@@ -58,6 +58,7 @@ class PluginOrigin:
         self.origin_type = origin_type  # The PluginOriginType
         self.elements = {}  # A dictionary of PluginConfiguration
         self.sources = {}  # A dictionary of PluginConfiguration objects
+        self.provenance = None
 
         # Private
         self._project = None
@@ -85,6 +86,7 @@ class PluginOrigin:
         elif origin_type == PluginOriginType.PIP:
             origin = PluginOriginPip()
 
+        origin.provenance = origin_node.get_provenance()
         origin._project = project
         origin._load(origin_node)
 
diff --git a/tests/plugins/loading.py b/tests/plugins/loading.py
index 152f4080b..13d787b0e 100644
--- a/tests/plugins/loading.py
+++ b/tests/plugins/loading.py
@@ -42,6 +42,37 @@ def setup_element(project_path, plugin_type, plugin_name):
     _yaml.roundtrip_dump(element, element_path)
 
 
+# This function is used for pytest skipif() expressions.
+#
+# Tests which require our plugins in tests/plugins/pip-samples need
+# to check if these plugins are installed, they are only guaranteed
+# to be installed when running tox, but not when using pytest directly
+# to test that BuildStream works when integrated in your system.
+#
+def pip_sample_packages():
+    import pkg_resources
+
+    required = {"sample-plugins"}
+    installed = {pkg.key for pkg in pkg_resources.working_set}  # pylint: disable=not-an-iterable
+    missing = required - installed
+
+    if missing:
+        return False
+
+    return True
+
+
+SAMPLE_PACKAGES_SKIP_REASON = """
+The sample plugins package used to test pip plugin origins is not installed.
+
+This is usually tested automatically with `tox`, if you are running
+`pytest` directly then you can install these plugins directly using pip.
+
+The plugins are located in the tests/plugins/sample-plugins directory
+of your BuildStream checkout.
+"""
+
+
 ####################################################
 #                     Tests                        #
 ####################################################
@@ -303,3 +334,97 @@ def test_deprecation_warning_suppressed_specifically(cli, datafiles, plugin_type
     result = cli.run(project=project, args=["show", "element.bst"])
     result.assert_success()
     assert "Here is some detail." not in result.stderr
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("plugin_type", [("elements"), ("sources")])
+@pytest.mark.skipif("not pip_sample_packages()", reason=SAMPLE_PACKAGES_SKIP_REASON)
+def test_pip_origin_load_success(cli, datafiles, plugin_type):
+    project = str(datafiles)
+
+    update_project(
+        project, {"plugins": [{"origin": "pip", "package-name": "sample-plugins", plugin_type: ["sample"],}]},
+    )
+    setup_element(project, plugin_type, "sample")
+
+    result = cli.run(project=project, args=["show", "element.bst"])
+    result.assert_success()
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("plugin_type", [("elements"), ("sources")])
+@pytest.mark.skipif("not pip_sample_packages()", reason=SAMPLE_PACKAGES_SKIP_REASON)
+def test_pip_origin_with_constraints(cli, datafiles, plugin_type):
+    project = str(datafiles)
+
+    update_project(
+        project,
+        {
+            "plugins": [
+                {"origin": "pip", "package-name": "sample-plugins>=1.0,<1.2.5,!=1.1.3", plugin_type: ["sample"],}
+            ]
+        },
+    )
+    setup_element(project, plugin_type, "sample")
+
+    result = cli.run(project=project, args=["show", "element.bst"])
+    result.assert_success()
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("plugin_type", [("elements"), ("sources")])
+def test_pip_origin_package_not_found(cli, datafiles, plugin_type):
+    project = str(datafiles)
+
+    update_project(
+        project, {"plugins": [{"origin": "pip", "package-name": "not-a-package", plugin_type: ["sample"],}]},
+    )
+    setup_element(project, plugin_type, "sample")
+
+    result = cli.run(project=project, args=["show", "element.bst"])
+    result.assert_main_error(ErrorDomain.PLUGIN, "package-not-found")
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("plugin_type", [("elements"), ("sources")])
+@pytest.mark.skipif("not pip_sample_packages()", reason=SAMPLE_PACKAGES_SKIP_REASON)
+def test_pip_origin_plugin_not_found(cli, datafiles, plugin_type):
+    project = str(datafiles)
+
+    update_project(
+        project, {"plugins": [{"origin": "pip", "package-name": "sample-plugins", plugin_type: ["notfound"],}]},
+    )
+    setup_element(project, plugin_type, "notfound")
+
+    result = cli.run(project=project, args=["show", "element.bst"])
+    result.assert_main_error(ErrorDomain.PLUGIN, "plugin-not-found")
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("plugin_type", [("elements"), ("sources")])
+@pytest.mark.skipif("not pip_sample_packages()", reason=SAMPLE_PACKAGES_SKIP_REASON)
+def test_pip_origin_version_conflict(cli, datafiles, plugin_type):
+    project = str(datafiles)
+
+    update_project(
+        project, {"plugins": [{"origin": "pip", "package-name": "sample-plugins>=1.4", plugin_type: ["sample"],}]},
+    )
+    setup_element(project, plugin_type, "sample")
+
+    result = cli.run(project=project, args=["show", "element.bst"])
+    result.assert_main_error(ErrorDomain.PLUGIN, "package-version-conflict")
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("plugin_type", [("elements"), ("sources")])
+@pytest.mark.skipif("not pip_sample_packages()", reason=SAMPLE_PACKAGES_SKIP_REASON)
+def test_pip_origin_malformed_constraints(cli, datafiles, plugin_type):
+    project = str(datafiles)
+
+    update_project(
+        project, {"plugins": [{"origin": "pip", "package-name": "sample-plugins>1.4,A", plugin_type: ["sample"],}]},
+    )
+    setup_element(project, plugin_type, "sample")
+
+    result = cli.run(project=project, args=["show", "element.bst"])
+    result.assert_main_error(ErrorDomain.PLUGIN, "package-malformed-requirement")
diff --git a/tests/plugins/sample-plugins/setup.py b/tests/plugins/sample-plugins/setup.py
new file mode 100755
index 000000000..8429c7cd7
--- /dev/null
+++ b/tests/plugins/sample-plugins/setup.py
@@ -0,0 +1,36 @@
+#!/usr/bin/env python3
+#
+#  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/>.
+#
+
+from setuptools import setup, find_packages
+
+setup(
+    name="sample-plugins",
+    version="1.2.3",
+    description="A collection of sample plugins for testing.",
+    license="LGPL",
+    url="https://example.com/sample-plugins",
+    package_dir={"": "src"},
+    packages=find_packages(where="src"),
+    include_package_data=True,
+    entry_points={
+        "buildstream.plugins.elements": ["sample = sample_plugins.elements.sample",],
+        "buildstream.plugins.sources": ["sample = sample_plugins.sources.sample",],
+    },
+    zip_safe=False,
+)
+# eof setup()
diff --git a/tests/plugins/sample-plugins/src/sample_plugins/__init__.py b/tests/plugins/sample-plugins/src/sample_plugins/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/plugins/sample-plugins/src/sample_plugins/__init__.py
diff --git a/tests/plugins/sample-plugins/src/sample_plugins/elements/__init__.py b/tests/plugins/sample-plugins/src/sample_plugins/elements/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/plugins/sample-plugins/src/sample_plugins/elements/__init__.py
diff --git a/tests/plugins/sample-plugins/src/sample_plugins/elements/sample.py b/tests/plugins/sample-plugins/src/sample_plugins/elements/sample.py
new file mode 100644
index 000000000..590e99ee8
--- /dev/null
+++ b/tests/plugins/sample-plugins/src/sample_plugins/elements/sample.py
@@ -0,0 +1,19 @@
+from buildstream import Element
+
+
+class Sample(Element):
+    BST_MIN_VERSION = "2.0"
+
+    def configure(self, node):
+        pass
+
+    def preflight(self):
+        pass
+
+    def get_unique_key(self):
+        return {}
+
+
+# Plugin entry point
+def setup():
+    return Sample
diff --git a/tests/plugins/sample-plugins/src/sample_plugins/elements/sample.yaml b/tests/plugins/sample-plugins/src/sample_plugins/elements/sample.yaml
new file mode 100644
index 000000000..956ad14d4
--- /dev/null
+++ b/tests/plugins/sample-plugins/src/sample_plugins/elements/sample.yaml
@@ -0,0 +1,5 @@
+# Set a variable so that we can test that the yaml
+# was actually loaded using bst show.
+#
+variables:
+  sample-loaded: True
diff --git a/tests/plugins/sample-plugins/src/sample_plugins/sources/__init__.py b/tests/plugins/sample-plugins/src/sample_plugins/sources/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/plugins/sample-plugins/src/sample_plugins/sources/__init__.py
diff --git a/tests/plugins/sample-plugins/src/sample_plugins/sources/sample.py b/tests/plugins/sample-plugins/src/sample_plugins/sources/sample.py
new file mode 100644
index 000000000..968a0e342
--- /dev/null
+++ b/tests/plugins/sample-plugins/src/sample_plugins/sources/sample.py
@@ -0,0 +1,32 @@
+from buildstream import Source
+
+
+class Sample(Source):
+    BST_MIN_VERSION = "2.0"
+
+    def configure(self, node):
+        pass
+
+    def preflight(self):
+        pass
+
+    def get_unique_key(self):
+        return {}
+
+    def load_ref(self, node):
+        pass
+
+    def get_ref(self):
+        return {}
+
+    def set_ref(self, ref, node):
+        pass
+
+    def is_cached(self):
+        return False
+
+
+# Plugin entry point
+def setup():
+
+    return Sample
diff --git a/tox.ini b/tox.ini
index 94e4048ce..e481a18c1 100644
--- a/tox.ini
+++ b/tox.ini
@@ -36,6 +36,9 @@ deps =
     py{36,37,38}: -rrequirements/requirements.txt
     py{36,37,38}: -rrequirements/dev-requirements.txt
 
+    # Install local sample plugins for testing pip plugin origins
+    py{36,37,38}: {toxinidir}/tests/plugins/sample-plugins
+
     # Install external plugins for plugin tests
     py{36,37,38}-plugins: git+https://gitlab.com/buildstream/bst-plugins-experimental.git@{env:BST_PLUGINS_EXPERIMENTAL_VERSION:{[config]BST_PLUGINS_EXPERIMENTAL_VERSION}}#egg=bst_plugins_experimental[ostree,deb]