NEWS

@@ -2,6 +2,10 @@
 buildstream 1.3.1
 =================
 
+  o BREAKING CHANGE: Attempting to use an element that does not have the `.bst`
+    extension, will result in an error message.
+    All elements must now be suffixed with `.bst`
+
   o BREAKING CHANGE: The 'manual' element lost its default 'MAKEFLAGS' and 'V'
     environment variables. There is already a 'make' element with the same
     variables. Note that this is a breaking change, it will require users to

buildstream/_context.py

@@ -27,7 +27,7 @@ from . import _cachekey
 from . import _signals
 from . import _site
 from . import _yaml
-from ._exceptions import LoadError, LoadErrorReason, BstError
+from ._exceptions import LoadError, LoadErrorReason, BstError, PluginError
 from ._message import Message, MessageType
 from ._profile import Topics, profile_start, profile_end
 from ._artifactcache import ArtifactCache
@@ -142,6 +142,37 @@ class Context():
         self._log_handle = None
         self._log_filename = None
 
+    # Print a warning message, checks warning_token against project configuration
+    #
+    # Args:
+    #     project (Project): The related project
+    #     brief (str): The brief message
+    #     detail (str): An optional detailed message, can be multiline output
+    #     warning_token (str): An optional configurable warning assosciated with this warning,
+    #                          this will cause PluginError to be raised if this warning is configured as fatal.
+    #                          (*Since 1.4*)
+    #
+    # Raises:
+    #     (:class:`.PluginError`): When warning_token is considered fatal by the project configuration
+    #
+    def warn(self, project, plugin, brief, *, detail=None, warning_token=None):
+        # TODO _prefix_warning should probably move somewhere else
+        from .plugin import _prefix_warning
+        if warning_token:
+            warning_token = _prefix_warning(plugin, warning_token)
+            brief = "[{}]: {}".format(warning_token, brief)
+
+            if project._warning_is_fatal(warning_token):
+                detail = detail if detail else ""
+                raise PluginError(message="{}\n{}".format(brief, detail), reason=warning_token)
+
+        unique_id = None
+        if plugin:
+            unique_id = plugin._get_unique_id()
+
+        message = Message(unique_id, MessageType.WARN, brief, detail=detail)
+        self.message(message)
+
     # load()
     #
     # Loads the configuration files

buildstream/_frontend/cli.py

@@ -109,7 +109,11 @@ def complete_target(args, incomplete):
     if element_directory:
         base_directory = os.path.join(base_directory, element_directory)
 
-    return complete_path("File", incomplete, base_directory=base_directory)
+    complete_list = []
+    for p in complete_path("File", incomplete, base_directory=base_directory):
+        if p.endswith(".bst ") or p.endswith("/") or p.endswith(".conf "):
+            complete_list.append(p)
+    return complete_list
 
 
 def override_completions(cmd, cmd_param, args, incomplete):

buildstream/_loader/loadelement.py

@@ -145,11 +145,14 @@ def _extract_depends_from_node(node, *, key=None):
 
     depends = _yaml.node_get(node, list, key, default_value=[])
     output_deps = []
+    invalid_elements = []
 
     for index, dep in enumerate(depends):
         dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[index])
 
         if isinstance(dep, str):
+            if not dep.endswith(".bst"):
+                invalid_elements.append(dep)
             dependency = Dependency(dep, provenance=dep_provenance, dep_type=default_dep_type)
 
         elif isinstance(dep, Mapping):

buildstream/_loader/loader.py

@@ -97,6 +97,7 @@ class Loader():
     # Returns: The toplevel LoadElement
     def load(self, targets, rewritable=False, ticker=None, fetch_subprojects=False):
 
+        invalid_elements = []
         for filename in targets:
             if os.path.isabs(filename):
                 # XXX Should this just be an assertion ?
@@ -106,6 +107,18 @@ class Loader():
                                 "path to the base project directory: {}"
                                 .format(filename, self._basedir))
 
+            if not filename.endswith(".bst") and not filename.endswith("/") and not filename.endswith(".conf"):
+                invalid_elements.append(filename)
+
+        if invalid_elements:
+            # TODO CoreWarnings should probably move somewhere else
+            from ..plugin import CoreWarnings
+
+            self._context.warn(self.project, None,
+                               "Target elements '{}' do not have expected file extension `.bst` "
+                               "Improperly named elements will not be discoverable by commands"
+                               .format(invalid_elements),
+                               warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX)
         # First pass, recursively load files and populate our table of LoadElements
         #
         deps = []
@@ -124,6 +137,15 @@ class Loader():
                 deps.append(Dependency(name, junction=junction))
                 profile_end(Topics.LOAD_PROJECT, target)
 
+        if invalid_elements:
+            # TODO CoreWarnings should probably move somewhere else
+            from ..plugin import CoreWarnings
+
+            self._context.warn(self.project, None,
+                               "Target elements '{}' do not have expected file extension `.bst` "
+                               "Improperly named elements will not be discoverable by commands"
+                               .format(invalid_elements),
+                               warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX)
         #
         # Now that we've resolve the dependencies, scan them for circular dependencies
         #
@@ -214,7 +236,7 @@ class Loader():
     # Returns:
     #    (LoadElement): A loaded LoadElement
     #
-    def _load_file(self, filename, rewritable, ticker, fetch_subprojects, yaml_cache=None):
+    def _load_file(self, filename, rewritable, ticker, fetch_subprojects, yaml_cache=None, suffix_check=False):
 
         # Silently ignore already loaded files
         if filename in self._elements:
@@ -270,6 +292,12 @@ class Loader():
 
         # Load all dependency files for the new LoadElement
         for dep in element.deps:
+            print("DEP: {}".format(dep.name))
+            invalid_elements = []
+            if not dep.name.endswith(".bst") and not dep.name.endswith("/") and not dep.name.endswith(".conf"):
+                invalid_elements.append(dep.name)
+                continue
+
             if dep.junction:
                 self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache)
                 loader = self._get_loader(dep.junction, rewritable=rewritable, ticker=ticker,
@@ -278,12 +306,19 @@ class Loader():
                 loader = self
 
             dep_element = loader._load_file(dep.name, rewritable, ticker, fetch_subprojects, yaml_cache)
-
             if _yaml.node_get(dep_element.node, str, Symbol.KIND) == 'junction':
                 raise LoadError(LoadErrorReason.INVALID_DATA,
                                 "{}: Cannot depend on junction"
                                 .format(dep.provenance))
-
+        if invalid_elements:
+            # TODO CoreWarnings should probably move somewhere else
+            from ..plugin import CoreWarnings
+
+            self._context.warn(self.project, None,
+                               "Target elements '{}' do not have expected file extension `.bst` "
+                               "Improperly named elements will not be discoverable by commands"
+                               .format(invalid_elements),
+                               warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX)
         return element
 
     # _check_circular_deps():
@@ -516,7 +551,7 @@ class Loader():
                 return loader
 
         try:
-            self._load_file(filename, rewritable, ticker, fetch_subprojects)
+            self._load_file(filename, rewritable, ticker, fetch_subprojects, suffix_check=True)
         except LoadError as e:
             if e.reason != LoadErrorReason.MISSING_FILE:
                 # other load error
@@ -621,15 +656,24 @@ class Loader():
     #            - (str): name of the element
     #            - (Loader): loader for sub-project
     #
-    def _parse_name(self, name, rewritable, ticker, fetch_subprojects=False):
+    def _parse_name(self, name, rewritable, ticker, fetch_subprojects=False, suffix_check=False):
         # We allow to split only once since deep junctions names are forbidden.
         # Users who want to refer to elements in sub-sub-projects are required
         # to create junctions on the top level project.
         junction_path = name.rsplit(':', 1)
         if len(junction_path) == 1:
+            if suffix_check:
+                return None, junction_path[-1], self, "Pass"
             return None, junction_path[-1], self
         else:
             self._load_file(junction_path[-2], rewritable, ticker, fetch_subprojects)
             loader = self._get_loader(junction_path[-2], rewritable=rewritable, ticker=ticker,
                                       fetch_subprojects=fetch_subprojects)
+            if suffix_check:
+                if not junction_path[-2].endswith(".bst") and not junction_path[-2].endswith("/") and not junction_path[-2].endswith(".conf"):
+                    result = "Fail"
+                else:
+                    result = "Pass"
+                return junction_path[-2], junction_path[-1], loader, result 
+
             return junction_path[-2], junction_path[-1], loader

buildstream/_project.py

@@ -445,6 +445,9 @@ class Project():
         self.config.options = OptionPool(self.element_path)
         self.first_pass_config.options = OptionPool(self.element_path)
 
+        # Early initialise fatal warnings
+        self._fatal_warnings = _yaml.node_get(pre_config_node, list, 'fatal-warnings', default_value=[])
+
         self.loader = Loader(self._context, self,
                              parent=parent_loader,
                              tempdir=tempdir)

buildstream/plugin.py

@@ -492,28 +492,8 @@ class Plugin():
         self.__message(MessageType.INFO, brief, detail=detail)
 
     def warn(self, brief, *, detail=None, warning_token=None):
-        """Print a warning message, checks warning_token against project configuration
-
-        Args:
-           brief (str): The brief message
-           detail (str): An optional detailed message, can be multiline output
-           warning_token (str): An optional configurable warning assosciated with this warning,
-                                this will cause PluginError to be raised if this warning is configured as fatal.
-                                (*Since 1.4*)
-
-        Raises:
-           (:class:`.PluginError`): When warning_token is considered fatal by the project configuration
-        """
-        if warning_token:
-            warning_token = _prefix_warning(self, warning_token)
-            brief = "[{}]: {}".format(warning_token, brief)
-            project = self._get_project()
-
-            if project._warning_is_fatal(warning_token):
-                detail = detail if detail else ""
-                raise PluginError(message="{}\n{}".format(brief, detail), reason=warning_token)
-
-        self.__message(MessageType.WARN, brief=brief, detail=detail)
+        context = self._get_context()
+        context.warn(self._get_project(), self, brief, detail=detail, warning_token=warning_token)
 
     def log(self, brief, *, detail=None):
         """Log a message into the plugin's log file
@@ -784,6 +764,12 @@ class CoreWarnings():
     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 doesn not end in .bst
+    is referenced either on the command line or by another element
+    """
+
 
 __CORE_WARNINGS = [
     value

tests/completions/completions.py

@@ -66,6 +66,13 @@ PROJECT_ELEMENTS = [
     "target.bst"
 ]
 
+INVALID_ELEMENTS = [
+    "target.foo"
+    "target.bst.bar"
+]
+
+MIXED_ELEMENTS = PROJECT_ELEMENTS + INVALID_ELEMENTS
+
 
 def assert_completion(cli, cmd, word_idx, expected, cwd=None):
     result = cli.run(cwd=cwd, env={
@@ -85,6 +92,24 @@ def assert_completion(cli, cmd, word_idx, expected, cwd=None):
     assert words == expected
 
 
+def assert_completion_failed(cli, cmd, word_idx, expected, cwd=None):
+    result = cli.run(cwd=cwd, env={
+        '_BST_COMPLETION': 'complete',
+        'COMP_WORDS': cmd,
+        'COMP_CWORD': str(word_idx)
+    })
+    words = []
+    if result.output:
+        words = result.output.splitlines()
+
+    # The order is meaningless, bash will
+    # take the results and order it by its
+    # own little heuristics
+    words = sorted(words)
+    expected = sorted(expected)
+    assert words != expected
+
+
 @pytest.mark.parametrize("cmd,word_idx,expected", [
     ('bst', 0, []),
     ('bst ', 1, MAIN_COMMANDS),
@@ -226,6 +251,19 @@ def test_argument_element(datafiles, cli, project, cmd, word_idx, expected, subd
     assert_completion(cli, cmd, word_idx, expected, cwd=cwd)
 
 
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("project,cmd,word_idx,expected,subdir", [
+
+    # When element has invalid suffix
+    ('project', 'bst --directory ../ show ', 4, [e + ' ' for e in MIXED_ELEMENTS], 'files')
+])
+def test_argument_element_invalid(datafiles, cli, project, cmd, word_idx, expected, subdir):
+    cwd = os.path.join(str(datafiles), project)
+    if subdir:
+        cwd = os.path.join(cwd, subdir)
+    assert_completion_failed(cli, cmd, word_idx, expected, cwd=cwd)
+
+
 @pytest.mark.parametrize("cmd,word_idx,expected", [
     ('bst he', 1, ['help ']),
     ('bst help ', 2, MAIN_COMMANDS),

tests/frontend/buildcheckout.py

@@ -60,9 +60,34 @@ def test_build_checkout(datafiles, cli, strict, hardlinks):
     assert os.path.exists(filename)
 
 
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("strict,hardlinks", [
+    ("non-strict", "hardlinks"),
+])
+def test_build_invalid_suffix(datafiles, cli, strict, hardlinks):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    checkout = os.path.join(cli.directory, 'checkout')
+
+    result = cli.run(project=project, args=strict_args(['build', 'target.foo'], strict))
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("strict,hardlinks", [
+    ("non-strict", "hardlinks"),
+])
+def test_build_invalid_suffix_dep(datafiles, cli, strict, hardlinks):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    checkout = os.path.join(cli.directory, 'checkout')
+
+    # target2.bst depends on an element called target.foo
+    result = cli.run(project=project, args=strict_args(['build', 'target2.bst'], strict))
+    result.assert_main_error(ErrorDomain.PLUGIN, "bad-element-suffix")
+
+
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.parametrize("deps", [("run"), ("none")])
-def test_build_checkout_deps(datafiles, cli, deps):
+def test_build_checkoue_deps(datafiles, cli, deps):
     project = os.path.join(datafiles.dirname, datafiles.basename)
     checkout = os.path.join(cli.directory, 'checkout')
 

tests/frontend/project/elements/target.foo

+kind: stack
+description: |
+
+  Main stack target for the bst build test

tests/frontend/project/elements/target2.bst

+kind: stack
+description: |
+
+  Main stack target for the bst build test
+
+depends:
+- target.foo

tests/frontend/project/project.conf

@@ -2,3 +2,6 @@
 name: test
 
 element-path: elements
+
+fatal-warnings:
+- bad-element-suffix

tests/integration/source-determinism.py

@@ -33,7 +33,7 @@ def create_test_directory(*path, mode=0o644):
 @pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
 def test_deterministic_source_umask(cli, tmpdir, datafiles, kind, integration_cache):
     project = str(datafiles)
-    element_name = 'list'
+    element_name = 'list.bst'
     element_path = os.path.join(project, 'elements', element_name)
     repodir = os.path.join(str(tmpdir), 'repo')
     sourcedir = os.path.join(project, 'source')
@@ -108,7 +108,7 @@ def test_deterministic_source_local(cli, tmpdir, datafiles, integration_cache):
     """Only user rights should be considered for local source.
     """
     project = str(datafiles)
-    element_name = 'test'
+    element_name = 'test.bst'
     element_path = os.path.join(project, 'elements', element_name)
     sourcedir = os.path.join(project, 'source')