diff options
author | Jürg Billeter <j@bitron.ch> | 2018-11-30 17:21:37 +0000 |
---|---|---|
committer | Jürg Billeter <j@bitron.ch> | 2018-11-30 17:21:37 +0000 |
commit | 7cea464a0a1f9286b768c037332e3c8f38920aa8 (patch) | |
tree | e1837589a1edfb2dc96058f9860a72706b8e514b | |
parent | ad293ed38b5d9d72432ac882901a3c28b720069b (diff) | |
parent | d4bc662a99750a71f40e9e92094c9d5d626f43b2 (diff) | |
download | buildstream-7cea464a0a1f9286b768c037332e3c8f38920aa8.tar.gz |
Merge branch 'mandatory_suffix' into 'master'
Mandatory .bst suffix
See merge request BuildStream/buildstream!967
-rw-r--r-- | NEWS | 4 | ||||
-rw-r--r-- | buildstream/_frontend/cli.py | 6 | ||||
-rw-r--r-- | buildstream/_loader/loader.py | 40 | ||||
-rw-r--r-- | buildstream/_project.py | 6 | ||||
-rw-r--r-- | buildstream/plugin.py | 6 | ||||
-rw-r--r-- | tests/completions/completions.py | 44 | ||||
-rw-r--r-- | tests/frontend/buildcheckout.py | 25 | ||||
-rw-r--r-- | tests/frontend/project/elements/target.foo | 4 | ||||
-rw-r--r-- | tests/frontend/project/elements/target2.bst | 7 | ||||
-rw-r--r-- | tests/frontend/project/project.conf | 3 | ||||
-rw-r--r-- | tests/integration/source-determinism.py | 4 | ||||
-rw-r--r-- | tests/loader/__init__.py | 10 |
12 files changed, 149 insertions, 10 deletions
@@ -2,6 +2,10 @@ buildstream 1.3.1 ================= + o All elements must now be suffixed with `.bst` + Attempting to use an element that does not have the `.bst` extension, + will result in a warning. + 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 diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py index c7d4c1eed..b845d00eb 100644 --- a/buildstream/_frontend/cli.py +++ b/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("/"): + complete_list.append(p) + return complete_list def override_completions(cmd, cmd_param, args, incomplete): diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py index a3c4ea8f0..a172a10c8 100644 --- a/buildstream/_loader/loader.py +++ b/buildstream/_loader/loader.py @@ -36,6 +36,8 @@ from .types import Symbol, Dependency from .loadelement import LoadElement from . import MetaElement from . import MetaSource +from ..plugin import CoreWarnings +from .._message import Message, MessageType # Loader(): @@ -97,6 +99,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 +109,14 @@ class Loader(): "path to the base project directory: {}" .format(filename, self._basedir)) + if not filename.endswith(".bst"): + invalid_elements.append(filename) + + if invalid_elements: + self._warn("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 = [] @@ -269,7 +280,12 @@ class Loader(): self._elements[filename] = element # Load all dependency files for the new LoadElement + invalid_elements = [] for dep in element.deps: + if not dep.name.endswith(".bst"): + 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, @@ -284,6 +300,11 @@ class Loader(): "{}: Cannot depend on junction" .format(dep.provenance)) + if invalid_elements: + self._warn("The following dependencies 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(): @@ -639,3 +660,22 @@ class Loader(): loader = self._get_loader(junction_path[-2], rewritable=rewritable, ticker=ticker, fetch_subprojects=fetch_subprojects) return junction_path[-2], junction_path[-1], loader + + # Print a warning message, checks warning_token against project configuration + # + # Args: + # brief (str): The brief message + # 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:`.LoadError`): When warning_token is considered fatal by the project configuration + # + def _warn(self, brief, *, warning_token=None): + if warning_token: + if self.project._warning_is_fatal(warning_token): + raise LoadError(warning_token, brief) + + message = Message(None, MessageType.WARN, brief) + self._context.message(message) diff --git a/buildstream/_project.py b/buildstream/_project.py index e91114361..52408b7e5 100644 --- a/buildstream/_project.py +++ b/buildstream/_project.py @@ -446,6 +446,9 @@ class Project(): self.config.options = OptionPool(self.element_path) self.first_pass_config.options = OptionPool(self.element_path) + # 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) @@ -506,9 +509,6 @@ class Project(): # Load project split rules self._splits = _yaml.node_get(config, Mapping, 'split-rules') - # Fatal warnings - self._fatal_warnings = _yaml.node_get(config, list, 'fatal-warnings', default_value=[]) - # Support backwards compatibility for fail-on-overlap fail_on_overlap = _yaml.node_get(config, bool, 'fail-on-overlap', default_value=None) diff --git a/buildstream/plugin.py b/buildstream/plugin.py index 00e0ed795..37dd78cc8 100644 --- a/buildstream/plugin.py +++ b/buildstream/plugin.py @@ -784,6 +784,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 does not end in .bst + is referenced either on the command line or by another element + """ + __CORE_WARNINGS = [ value diff --git a/tests/completions/completions.py b/tests/completions/completions.py index 7b63e67fe..af35fb23a 100644 --- a/tests/completions/completions.py +++ b/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), @@ -193,19 +218,19 @@ def test_option_directory(datafiles, cli, cmd, word_idx, expected, subdir): # When running in the project directory ('no-element-path', 'bst show ', 2, - [e + ' ' for e in (PROJECT_ELEMENTS + ['project.conf'])] + ['files/'], None), + [e + ' ' for e in PROJECT_ELEMENTS] + ['files/'], None), ('no-element-path', 'bst build com', 2, ['compose-all.bst ', 'compose-include-bin.bst ', 'compose-exclude-dev.bst '], None), # When running from the files subdir ('no-element-path', 'bst show ', 2, - [e + ' ' for e in (PROJECT_ELEMENTS + ['project.conf'])] + ['files/'], 'files'), + [e + ' ' for e in PROJECT_ELEMENTS] + ['files/'], 'files'), ('no-element-path', 'bst build com', 2, ['compose-all.bst ', 'compose-include-bin.bst ', 'compose-exclude-dev.bst '], 'files'), # When passing the project directory ('no-element-path', 'bst --directory ../ show ', 4, - [e + ' ' for e in (PROJECT_ELEMENTS + ['project.conf'])] + ['files/'], 'files'), + [e + ' ' for e in PROJECT_ELEMENTS] + ['files/'], 'files'), ('no-element-path', 'bst --directory ../ show f', 4, ['files/'], 'files'), ('no-element-path', 'bst --directory ../ show files/', 4, ['files/bin-files/', 'files/dev-files/'], 'files'), ('no-element-path', 'bst --directory ../ build com', 4, @@ -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), diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py index 159af2d74..1299fa190 100644 --- a/tests/frontend/buildcheckout.py +++ b/tests/frontend/buildcheckout.py @@ -61,6 +61,31 @@ def test_build_checkout(datafiles, cli, strict, hardlinks): @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, "bad-element-suffix") + + +@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.LOAD, "bad-element-suffix") + + +@pytest.mark.datafiles(DATA_DIR) @pytest.mark.parametrize("deps", [("run"), ("none")]) def test_build_checkout_deps(datafiles, cli, deps): project = os.path.join(datafiles.dirname, datafiles.basename) diff --git a/tests/frontend/project/elements/target.foo b/tests/frontend/project/elements/target.foo new file mode 100644 index 000000000..d644c89ba --- /dev/null +++ b/tests/frontend/project/elements/target.foo @@ -0,0 +1,4 @@ +kind: stack +description: | + + Main stack target for the bst build test diff --git a/tests/frontend/project/elements/target2.bst b/tests/frontend/project/elements/target2.bst new file mode 100644 index 000000000..259819f59 --- /dev/null +++ b/tests/frontend/project/elements/target2.bst @@ -0,0 +1,7 @@ +kind: stack +description: | + + Main stack target for the bst build test + +depends: +- target.foo diff --git a/tests/frontend/project/project.conf b/tests/frontend/project/project.conf index 854e38693..a7e4a023c 100644 --- a/tests/frontend/project/project.conf +++ b/tests/frontend/project/project.conf @@ -2,3 +2,6 @@ name: test element-path: elements + +fatal-warnings: +- bad-element-suffix diff --git a/tests/integration/source-determinism.py b/tests/integration/source-determinism.py index a970c7dc9..fa12593df 100644 --- a/tests/integration/source-determinism.py +++ b/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') diff --git a/tests/loader/__init__.py b/tests/loader/__init__.py index fcefdacf5..812888181 100644 --- a/tests/loader/__init__.py +++ b/tests/loader/__init__.py @@ -1,14 +1,22 @@ +import os from buildstream._context import Context from buildstream._project import Project from buildstream._loader import Loader - # # This is used by the loader test modules, these should # be removed in favor of testing the functionality via # the CLI like in the frontend tests anyway. # + + +def dummy_handler(message, context): + pass + + def make_loader(basedir): context = Context() + context.load(config=os.devnull) + context.set_message_handler(dummy_handler) project = Project(basedir, context) return project.loader |