From c93450146a63cdae40611024a31908cc4022b499 Mon Sep 17 00:00:00 2001 From: James Ennis Date: Wed, 30 Jan 2019 17:28:21 +0000 Subject: filter.py: Fail if declared domains do not exist in the parent element 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 --- buildstream/plugins/elements/filter.py | 27 ++++++++++++++++++++++ buildstream/plugins/elements/filter.yaml | 5 ++-- tests/elements/filter.py | 11 +++++++++ .../filter/basic/elements/deps-permitted.bst | 2 +- .../elements/output-include-nonexistent-domain.bst | 8 +++++++ 5 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 tests/elements/filter/basic/elements/output-include-nonexistent-domain.bst 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 + -- cgit v1.2.1