Merge branch 'tristan/misc-cleanup' into 'master'

Misc cleanups

See merge request BuildStream/buildstream!993

10 files changed, 77 insertions, 97 deletions

diff --git a/buildstream/__init__.py b/buildstream/__init__.py
index fc09f2812..baacc480b 100644
--- a/buildstream/__init__.py
+++ b/buildstream/__init__.py
@@ -28,7 +28,7 @@ if "_BST_COMPLETION" not in os.environ:
 
     from .utils import UtilError, ProgramNotFoundError
     from .sandbox import Sandbox, SandboxFlags, SandboxCommandError
-    from .types import Scope, Consistency
+    from .types import Scope, Consistency, CoreWarnings
     from .plugin import Plugin
     from .source import Source, SourceError, SourceFetcher
     from .element import Element, ElementError
diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py
index a172a10c8..0de0e2b9c 100644
--- a/buildstream/_loader/loader.py
+++ b/buildstream/_loader/loader.py
@@ -36,7 +36,7 @@ from .types import Symbol, Dependency
 from .loadelement import LoadElement
 from . import MetaElement
 from . import MetaSource
-from ..plugin import CoreWarnings
+from ..types import CoreWarnings
 from .._message import Message, MessageType
 
 
diff --git a/buildstream/_project.py b/buildstream/_project.py
index 52408b7e5..386036488 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -33,7 +33,7 @@ from ._artifactcache import ArtifactCache
 from .sandbox import SandboxRemote
 from ._elementfactory import ElementFactory
 from ._sourcefactory import SourceFactory
-from .plugin import CoreWarnings
+from .types import CoreWarnings
 from ._projectrefs import ProjectRefs, ProjectRefStorage
 from ._versions import BST_FORMAT_VERSION
 from ._loader import Loader
diff --git a/buildstream/element.py b/buildstream/element.py
index 27677b054..ed279458c 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -96,10 +96,9 @@ from . import _cachekey
 from . import _signals
 from . import _site
 from ._platform import Platform
-from .plugin import CoreWarnings
 from .sandbox._config import SandboxConfig
 from .sandbox._sandboxremote import SandboxRemote
-from .types import _KeyStrength
+from .types import _KeyStrength, CoreWarnings
 
 from .storage.directory import Directory
 from .storage._filebaseddirectory import FileBasedDirectory
diff --git a/buildstream/plugin.py b/buildstream/plugin.py
index 37dd78cc8..1e8dd4b9f 100644
--- a/buildstream/plugin.py
+++ b/buildstream/plugin.py
@@ -119,6 +119,7 @@ from . import _yaml
 from . import utils
 from ._exceptions import PluginError, ImplError
 from ._message import Message, MessageType
+from .types import CoreWarnings
 
 
 class Plugin():
@@ -766,38 +767,6 @@ class Plugin():
             return self.name
 
 
-class CoreWarnings():
-    """CoreWarnings()
-
-    Some common warnings which are raised by core functionalities within BuildStream are found in this class.
-    """
-
-    OVERLAPS = "overlaps"
-    """
-    This warning will be produced when buildstream detects an overlap on an element
-        which is not whitelisted. See :ref:`Overlap Whitelist <public_overlap_whitelist>`
-    """
-
-    REF_NOT_IN_TRACK = "ref-not-in-track"
-    """
-    This warning will be produced when a source is configured with a reference
-    which is found to be invalid based on the configured track
-    """
-
-    BAD_ELEMENT_SUFFIX = "bad-element-suffix"
-    """
-    This warning will be produced when an element whose name does not end in .bst
-    is referenced either on the command line or by another element
-    """
-
-
-__CORE_WARNINGS = [
-    value
-    for name, value in CoreWarnings.__dict__.items()
-    if not name.startswith("__")
-]
-
-
 # Hold on to a lookup table by counter of all instantiated plugins.
 # We use this to send the id back from child processes so we can lookup
 # corresponding element/source in the master process.
@@ -828,6 +797,24 @@ def _plugin_lookup(unique_id):
     return __PLUGINS_TABLE[unique_id]
 
 
+# No need for unregister, WeakValueDictionary() will remove entries
+# in itself when the referenced plugins are garbage collected.
+def _plugin_register(plugin):
+    global __PLUGINS_UNIQUE_ID                # pylint: disable=global-statement
+    __PLUGINS_UNIQUE_ID += 1
+    __PLUGINS_TABLE[__PLUGINS_UNIQUE_ID] = plugin
+    return __PLUGINS_UNIQUE_ID
+
+
+# A local table for _prefix_warning()
+#
+__CORE_WARNINGS = [
+    value
+    for name, value in CoreWarnings.__dict__.items()
+    if not name.startswith("__")
+]
+
+
 # _prefix_warning():
 #
 # Prefix a warning with the plugin kind. CoreWarnings are not prefixed.
@@ -843,12 +830,3 @@ def _prefix_warning(plugin, warning):
     if any((warning is core_warning for core_warning in __CORE_WARNINGS)):
         return warning
     return "{}:{}".format(plugin.get_kind(), warning)
-
-
-# No need for unregister, WeakValueDictionary() will remove entries
-# in itself when the referenced plugins are garbage collected.
-def _plugin_register(plugin):
-    global __PLUGINS_UNIQUE_ID                # pylint: disable=global-statement
-    __PLUGINS_UNIQUE_ID += 1
-    __PLUGINS_TABLE[__PLUGINS_UNIQUE_ID] = plugin
-    return __PLUGINS_UNIQUE_ID
diff --git a/buildstream/plugins/sources/git.py b/buildstream/plugins/sources/git.py
index e3dded1d4..7496f1f13 100644
--- a/buildstream/plugins/sources/git.py
+++ b/buildstream/plugins/sources/git.py
@@ -131,13 +131,14 @@ details on common configuration options for sources.
 
 **Configurable Warnings:**
 
-This plugin provides the following configurable warnings:
+This plugin provides the following :ref:`configurable warnings <configurable_warnings>`:
 
-- 'git:inconsistent-submodule' - A submodule was found to be missing from the underlying git repository.
+- ``git:inconsistent-submodule`` - A submodule was found to be missing from the underlying git repository.
 
-This plugin also utilises the following configurable core plugin warnings:
+This plugin also utilises the following configurable :class:`core warnings <buildstream.types.CoreWarnings>`:
 
-- 'ref-not-in-track' - The provided ref was not found in the provided track in the element's git repository.
+- :attr:`ref-not-in-track <buildstream.types.CoreWarnings.REF_NOT_IN_TRACK>` - The provided ref was not
+  found in the provided track in the element's git repository.
 """
 
 import os
@@ -149,15 +150,14 @@ from tempfile import TemporaryFile
 
 from configparser import RawConfigParser
 
-from buildstream import Source, SourceError, Consistency, SourceFetcher
+from buildstream import Source, SourceError, Consistency, SourceFetcher, CoreWarnings
 from buildstream import utils
-from buildstream.plugin import CoreWarnings
 from buildstream.utils import move_atomic, DirectoryExistsError
 
 GIT_MODULES = '.gitmodules'
 
 # Warnings
-INCONSISTENT_SUBMODULE = "inconsistent-submodules"
+WARN_INCONSISTENT_SUBMODULE = "inconsistent-submodule"
 
 
 # Because of handling of submodules, we maintain a GitMirror
@@ -408,7 +408,8 @@ class GitMirror(SourceFetcher):
                      "underlying git repository with `git submodule add`."
 
             self.source.warn("{}: Ignoring inconsistent submodule '{}'"
-                             .format(self.source, submodule), detail=detail, warning_token=INCONSISTENT_SUBMODULE)
+                             .format(self.source, submodule), detail=detail,
+                             warning_token=WARN_INCONSISTENT_SUBMODULE)
 
             return None
 
diff --git a/buildstream/source.py b/buildstream/source.py
index d6681b963..5dc5abb63 100644
--- a/buildstream/source.py
+++ b/buildstream/source.py
@@ -391,7 +391,8 @@ class Source(Plugin):
 
         If the backend in question supports resolving references from
         a symbolic tracking branch or tag, then this should be implemented
-        to perform this task on behalf of ``build-stream track`` commands.
+        to perform this task on behalf of :ref:`bst track <invoking_track>`
+        commands.
 
         This usually requires fetching new content from a remote origin
         to see if a new ref has appeared for your branch or tag. If the
diff --git a/buildstream/types.py b/buildstream/types.py
index 3eb4a9110..5e6265b4e 100644
--- a/buildstream/types.py
+++ b/buildstream/types.py
@@ -81,6 +81,31 @@ class Consistency():
     """
 
 
+class CoreWarnings():
+    """CoreWarnings()
+
+    Some common warnings which are raised by core functionalities within BuildStream are found in this class.
+    """
+
+    OVERLAPS = "overlaps"
+    """
+    This warning will be produced when buildstream detects an overlap on an element
+        which is not whitelisted. See :ref:`Overlap Whitelist <public_overlap_whitelist>`
+    """
+
+    REF_NOT_IN_TRACK = "ref-not-in-track"
+    """
+    This warning will be produced when a source is configured with a reference
+    which is found to be invalid based on the configured track
+    """
+
+    BAD_ELEMENT_SUFFIX = "bad-element-suffix"
+    """
+    This warning will be produced when an element whose name does not end in .bst
+    is referenced either on the command line or by another element
+    """
+
+
 # _KeyStrength():
 #
 # Strength of cache key
diff --git a/doc/source/format_project.rst b/doc/source/format_project.rst
index a85bf6470..4024cea19 100644
--- a/doc/source/format_project.rst
+++ b/doc/source/format_project.rst
@@ -143,7 +143,7 @@ Individual warnings can be configured as fatal by setting ``fatal-warnings`` to
   - ref-not-in-track
   - <plugin>:<warning>
 
-BuildStream provides a collection of :class:`Core Warnings <buildstream.plugin.CoreWarnings>` which may be raised
+BuildStream provides a collection of :class:`Core Warnings <buildstream.types.CoreWarnings>` which may be raised
 by a variety of plugins. Other configurable warnings are plugin specific and should be noted within their individual documentation.
 
 .. note::
diff --git a/tests/sources/git.py b/tests/sources/git.py
index 462200f97..124f036cf 100644
--- a/tests/sources/git.py
+++ b/tests/sources/git.py
@@ -414,45 +414,17 @@ def test_submodule_track_no_ref_or_track(cli, tmpdir, datafiles):
 
 @pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
 @pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
-def test_ref_not_in_track_warn(cli, tmpdir, datafiles):
+@pytest.mark.parametrize("fail", ['warn', 'error'])
+def test_ref_not_in_track(cli, tmpdir, datafiles, fail):
     project = os.path.join(datafiles.dirname, datafiles.basename)
 
-    # Create the repo from 'repofiles', create a branch without latest commit
-    repo = create_repo('git', str(tmpdir))
-    ref = repo.create(os.path.join(project, 'repofiles'))
-
-    gitsource = repo.source_config(ref=ref)
-
-    # Overwrite the track value to the added branch
-    gitsource['track'] = 'foo'
-
-    # Write out our test target
-    element = {
-        'kind': 'import',
-        'sources': [
-            gitsource
-        ]
-    }
-    _yaml.dump(element, os.path.join(project, 'target.bst'))
-
-    # Assert the warning is raised as ref is not in branch foo.
-    # Assert warning not error to the user, when not set as fatal.
-    result = cli.run(project=project, args=['build', 'target.bst'])
-    assert "The ref provided for the element does not exist locally" in result.stderr
-
-
-@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
-@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
-def test_ref_not_in_track_warn_error(cli, tmpdir, datafiles):
-    project = os.path.join(datafiles.dirname, datafiles.basename)
-
-    # Add fatal-warnings ref-not-in-track to project.conf
-    project_template = {
-        "name": "foo",
-        "fatal-warnings": [CoreWarnings.REF_NOT_IN_TRACK]
-    }
-
-    _yaml.dump(project_template, os.path.join(project, 'project.conf'))
+    # Make the warning an error if we're testing errors
+    if fail == 'error':
+        project_template = {
+            "name": "foo",
+            "fatal-warnings": [CoreWarnings.REF_NOT_IN_TRACK]
+        }
+        _yaml.dump(project_template, os.path.join(project, 'project.conf'))
 
     # Create the repo from 'repofiles', create a branch without latest commit
     repo = create_repo('git', str(tmpdir))
@@ -472,11 +444,15 @@ def test_ref_not_in_track_warn_error(cli, tmpdir, datafiles):
     }
     _yaml.dump(element, os.path.join(project, 'target.bst'))
 
-    # Assert that build raises a warning here that is captured
-    # as plugin error, due to the fatal warning being set
     result = cli.run(project=project, args=['build', 'target.bst'])
-    result.assert_main_error(ErrorDomain.STREAM, None)
-    result.assert_task_error(ErrorDomain.PLUGIN, CoreWarnings.REF_NOT_IN_TRACK)
+
+    # Assert a warning or an error depending on what we're checking
+    if fail == 'error':
+        result.assert_main_error(ErrorDomain.STREAM, None)
+        result.assert_task_error(ErrorDomain.PLUGIN, CoreWarnings.REF_NOT_IN_TRACK)
+    else:
+        result.assert_success()
+        assert "ref-not-in-track" in result.stderr
 
 
 @pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")