filter.py: Fail if declared domains do not exist in the parent elementjennis/warn_for_nonexistent_domains

This patch also uncovered the fact that our test_filter_deps_ok() test
has been inaccurate. Thus the element built in this test
(deps-permitted.bst) has been modified so that it build depends on the
input.bst element, as it should.

tests/filter.py: Ensure deps_ok test passes

5 files changed, 49 insertions, 4 deletions

diff --git a/buildstream/plugins/elements/filter.py b/buildstream/plugins/elements/filter.py
index b3632f265..5a2db2c44 100644
--- a/buildstream/plugins/elements/filter.py
+++ b/buildstream/plugins/elements/filter.py
@@ -168,6 +168,8 @@ class FilterElement(Element):
         self.include = self.node_get_member(node, list, 'include')
         self.exclude = self.node_get_member(node, list, 'exclude')
         self.include_orphans = self.node_get_member(node, bool, 'include-orphans')
+        self.include_provenance = self.node_provenance(node, member_name='include')
+        self.exclude_provenance = self.node_provenance(node, member_name='exclude')
 
     def preflight(self):
         # Exactly one build-depend is permitted
@@ -207,6 +209,31 @@ class FilterElement(Element):
     def assemble(self, sandbox):
         with self.timed_activity("Staging artifact", silent_nested=True):
             for dep in self.dependencies(Scope.BUILD, recurse=False):
+                # Check that all the included/excluded domains exist
+                pub_data = dep.get_public_data('bst')
+                split_rules = pub_data.get('split-rules', {})
+                unfound_includes = []
+                for domain in self.include:
+                    if domain not in split_rules:
+                        unfound_includes.append(domain)
+                unfound_excludes = []
+                for domain in self.exclude:
+                    if domain not in split_rules:
+                        unfound_excludes.append(domain)
+
+                detail = []
+                if unfound_includes:
+                    detail.append("Unknown domains were used in {}".format(self.include_provenance))
+                    detail.extend([' - {}'.format(domain) for domain in unfound_includes])
+
+                if unfound_excludes:
+                    detail.append("Unknown domains were used in {}".format(self.exclude_provenance))
+                    detail.extend([' - {}'.format(domain) for domain in unfound_excludes])
+
+                if detail:
+                    detail = '\n'.join(detail)
+                    raise ElementError("Unknown domains declared.", detail=detail)
+
                 dep.stage_artifact(sandbox, include=self.include,
                                    exclude=self.exclude, orphans=self.include_orphans)
         return ""
diff --git a/buildstream/plugins/elements/filter.yaml b/buildstream/plugins/elements/filter.yaml
index 78465a183..9c2bf69f4 100644
--- a/buildstream/plugins/elements/filter.yaml
+++ b/buildstream/plugins/elements/filter.yaml
@@ -6,9 +6,8 @@ config:
   # they were defined as public data in the parent
   # element's 'split-rules'.
   #
-  # Since domains can be added, it is not an error to
-  # specify domains which may not exist for all of the
-  # elements in this composition.
+  # If a domain is specified that does not exist, the
+  # filter element will fail to build.
   #
   # The default empty list indicates that all domains
   # of the parent's artifact should be included.
diff --git a/tests/elements/filter.py b/tests/elements/filter.py
index 8ff642c64..d40a8bdd1 100644
--- a/tests/elements/filter.py
+++ b/tests/elements/filter.py
@@ -484,3 +484,14 @@ def test_filter_include_with_indirect_deps(datafiles, cli, tmpdir):
     # indirect dependencies shouldn't be staged and filtered
     assert not os.path.exists(os.path.join(checkout, "foo"))
     assert not os.path.exists(os.path.join(checkout, "bar"))
+
+
+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'basic'))
+def test_filter_fails_for_nonexisting_domain(datafiles, cli, tmpdir):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    result = cli.run(project=project, args=['build', 'output-include-nonexistent-domain.bst'])
+    result.assert_main_error(ErrorDomain.STREAM, None)
+
+    error = "Unknown domains were used in output-include-nonexistent-domain.bst [line 7 column 2]"
+    assert error in result.stderr
+    assert '- unknown_file' in result.stderr
diff --git a/tests/elements/filter/basic/elements/deps-permitted.bst b/tests/elements/filter/basic/elements/deps-permitted.bst
index 00883b1f8..9fd41c543 100644
--- a/tests/elements/filter/basic/elements/deps-permitted.bst
+++ b/tests/elements/filter/basic/elements/deps-permitted.bst
@@ -1,6 +1,6 @@
 kind: filter
 depends:
-- filename: output-include.bst
+- filename: input.bst
   type: build
 - filename: output-exclude.bst
   type: runtime
diff --git a/tests/elements/filter/basic/elements/output-include-nonexistent-domain.bst b/tests/elements/filter/basic/elements/output-include-nonexistent-domain.bst
new file mode 100644
index 000000000..59706375d
--- /dev/null
+++ b/tests/elements/filter/basic/elements/output-include-nonexistent-domain.bst
@@ -0,0 +1,8 @@
+kind: filter
+depends:
+- filename: input.bst
+  type: build
+config:
+  include:
+  - unknown_file
+