From 86f013dbffa87692250ce1aefd31cecd1559ddd5 Mon Sep 17 00:00:00 2001 From: Valentin David Date: Thu, 2 Apr 2020 17:54:29 +0200 Subject: Process options in includes files with the options of their junction Unfortunately the options from main project cannot always be processed in the include processing since project configuration might load option declarations from a separate file. For that reason the result of `Include.process` should still be passed through the option processor. But all options files included from junctioned are already evaluated. --- NEWS | 3 + src/buildstream/_includes.py | 78 ++++++++++++++++++++-- src/buildstream/_loader/loader.py | 2 - src/buildstream/_project.py | 4 +- src/buildstream/testing/_utils/junction.py | 6 +- tests/format/include.py | 58 ++++++++++++++++ tests/format/include/junction_options/element.bst | 1 + tests/format/include/junction_options/project.conf | 4 ++ .../junction_options/subproject/extra_conf.yml | 7 ++ .../junction_options/subproject/project.conf | 11 +++ .../include/junction_options_deep/element.bst | 1 + .../include/junction_options_deep/project.conf | 4 ++ .../subproject-1/extra_conf.yml | 2 + .../subproject-1/project.conf | 1 + .../subproject-2/extra_conf.yml | 7 ++ .../subproject-2/project.conf | 11 +++ .../include/junction_options_element/element.bst | 4 ++ .../include/junction_options_element/project.conf | 1 + .../subproject/extra_conf.yml | 7 ++ .../subproject/project.conf | 11 +++ tests/testutils/junction.py | 6 +- 21 files changed, 217 insertions(+), 12 deletions(-) create mode 100644 tests/format/include/junction_options/element.bst create mode 100644 tests/format/include/junction_options/project.conf create mode 100644 tests/format/include/junction_options/subproject/extra_conf.yml create mode 100644 tests/format/include/junction_options/subproject/project.conf create mode 100644 tests/format/include/junction_options_deep/element.bst create mode 100644 tests/format/include/junction_options_deep/project.conf create mode 100644 tests/format/include/junction_options_deep/subproject-1/extra_conf.yml create mode 100644 tests/format/include/junction_options_deep/subproject-1/project.conf create mode 100644 tests/format/include/junction_options_deep/subproject-2/extra_conf.yml create mode 100644 tests/format/include/junction_options_deep/subproject-2/project.conf create mode 100644 tests/format/include/junction_options_element/element.bst create mode 100644 tests/format/include/junction_options_element/project.conf create mode 100644 tests/format/include/junction_options_element/subproject/extra_conf.yml create mode 100644 tests/format/include/junction_options_element/subproject/project.conf diff --git a/NEWS b/NEWS index d1767f9ce..95bcb2e00 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,9 @@ Core o BuildStream now requires Python >= 3.6. + o BREAKING CHANGE: Conditional directives `(?)` from files included + through junctions are evaluated with the options defined in the + sub project the file comes from. ================== buildstream 1.93.1 diff --git a/src/buildstream/_includes.py b/src/buildstream/_includes.py index b49560947..b9a1c0d22 100644 --- a/src/buildstream/_includes.py +++ b/src/buildstream/_includes.py @@ -26,13 +26,51 @@ class Includes: # # Args: # node (dict): A YAML node + # only_local (bool): Whether to ignore junction files + # process_project_options (bool): Whether to process options from current project + def process(self, node, *, only_local=False, process_project_options=True): + self._process(node, only_local=only_local, process_project_options=process_project_options) + + # _process() + # + # Process recursively include directives in a YAML node. This + # method is a recursively called on loaded nodes from files. + # + # Args: + # node (dict): A YAML node # included (set): Fail for recursion if trying to load any files in this set # current_loader (Loader): Use alternative loader (for junction files) # only_local (bool): Whether to ignore junction files - def process(self, node, *, included=set(), current_loader=None, only_local=False): + # process_project_options (bool): Whether to process options from current project + def _process(self, node, *, included=set(), current_loader=None, only_local=False, process_project_options=True): if current_loader is None: current_loader = self._loader + if process_project_options: + current_loader.project.options.process_node(node) + + self._process_node( + node, + included=included, + only_local=only_local, + current_loader=current_loader, + process_project_options=process_project_options, + ) + + # _process_node() + # + # Process recursively include directives in a YAML node. This + # method is recursively called on all nodes. + # + # Args: + # node (dict): A YAML node + # included (set): Fail for recursion if trying to load any files in this set + # current_loader (Loader): Use alternative loader (for junction files) + # only_local (bool): Whether to ignore junction files + # process_project_options (bool): Whether to process options from current project + def _process_node( + self, node, *, included=set(), current_loader=None, only_local=False, process_project_options=True + ): includes_node = node.get_node("(@)", allowed_types=[ScalarNode, SequenceNode], allow_none=True) if includes_node: @@ -77,14 +115,26 @@ class Includes: try: included.add(file_path) - self.process(include_node, included=included, current_loader=sub_loader, only_local=only_local) + self._process( + include_node, + included=included, + current_loader=sub_loader, + only_local=only_local, + process_project_options=process_project_options or current_loader != sub_loader, + ) finally: included.remove(file_path) include_node._composite_under(node) for value in node.values(): - self._process_value(value, included=included, current_loader=current_loader, only_local=only_local) + self._process_value( + value, + included=included, + current_loader=current_loader, + only_local=only_local, + process_project_options=process_project_options, + ) # _include_file() # @@ -100,6 +150,7 @@ class Includes: junction, include = include.split(":", 1) junction_loader = loader._get_loader(junction) current_loader = junction_loader + current_loader.project.ensure_fully_loaded() else: current_loader = loader project = current_loader.project @@ -119,11 +170,26 @@ class Includes: # included (set): Fail for recursion if trying to load any files in this set # current_loader (Loader): Use alternative loader (for junction files) # only_local (bool): Whether to ignore junction files - def _process_value(self, value, *, included=set(), current_loader=None, only_local=False): + # process_project_options (bool): Whether to process options from current project + def _process_value( + self, value, *, included=set(), current_loader=None, only_local=False, process_project_options=True + ): value_type = type(value) if value_type is MappingNode: - self.process(value, included=included, current_loader=current_loader, only_local=only_local) + self._process_node( + value, + included=included, + current_loader=current_loader, + only_local=only_local, + process_project_options=process_project_options, + ) elif value_type is SequenceNode: for v in value: - self._process_value(v, included=included, current_loader=current_loader, only_local=only_local) + self._process_value( + v, + included=included, + current_loader=current_loader, + only_local=only_local, + process_project_options=process_project_options, + ) diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 35de1b97d..ea795a7d7 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -282,8 +282,6 @@ class Loader: self._includes.process(node) - self._options.process_node(node) - element = LoadElement(node, filename, self) self._elements[filename] = element diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py index 7b31f44b8..9e5d92b7c 100644 --- a/src/buildstream/_project.py +++ b/src/buildstream/_project.py @@ -654,7 +654,7 @@ class Project: self._project_includes = Includes(self.loader, copy_tree=False) project_conf_first_pass = self._project_conf.clone() - self._project_includes.process(project_conf_first_pass, only_local=True) + self._project_includes.process(project_conf_first_pass, only_local=True, process_project_options=False) config_no_include = self._default_config_node.clone() project_conf_first_pass._composite(config_no_include) @@ -679,7 +679,7 @@ class Project: # def _load_second_pass(self): project_conf_second_pass = self._project_conf.clone() - self._project_includes.process(project_conf_second_pass) + self._project_includes.process(project_conf_second_pass, process_project_options=False) config = self._default_config_node.clone() project_conf_second_pass._composite(config) diff --git a/src/buildstream/testing/_utils/junction.py b/src/buildstream/testing/_utils/junction.py index e4dbd5984..6f45ad590 100644 --- a/src/buildstream/testing/_utils/junction.py +++ b/src/buildstream/testing/_utils/junction.py @@ -19,7 +19,7 @@ from .site import HAVE_GIT, GIT, GIT_ENV # Returns: # (str): The ref # -def generate_junction(tmpdir, subproject_path, junction_path, *, store_ref=True): +def generate_junction(tmpdir, subproject_path, junction_path, *, store_ref=True, options={}): # Create a repo to hold the subproject and generate # a junction element for it # @@ -29,6 +29,10 @@ def generate_junction(tmpdir, subproject_path, junction_path, *, store_ref=True) source_ref = None element = {"kind": "junction", "sources": [repo.source_config(ref=source_ref)]} + + if options: + element["config"] = {"options": options} + _yaml.roundtrip_dump(element, junction_path) return ref diff --git a/tests/format/include.py b/tests/format/include.py index 3e7e0abf0..c64753be5 100644 --- a/tests/format/include.py +++ b/tests/format/include.py @@ -252,3 +252,61 @@ def test_local_to_junction(cli, tmpdir, datafiles): result.assert_success() loaded = _yaml.load_data(result.output) assert loaded.get_bool("included") + + +@pytest.mark.datafiles(DATA_DIR) +def test_option_from_junction(cli, tmpdir, datafiles): + project = os.path.join(str(datafiles), "junction_options") + + generate_junction( + tmpdir, + os.path.join(project, "subproject"), + os.path.join(project, "junction.bst"), + store_ref=True, + options={"local_option": "set"}, + ) + + result = cli.run(project=project, args=["show", "--deps", "none", "--format", "%{vars}", "element.bst"]) + result.assert_success() + loaded = _yaml.load_data(result.output) + assert not loaded.get_bool("is-default") + + +@pytest.mark.datafiles(DATA_DIR) +def test_option_from_junction_element(cli, tmpdir, datafiles): + project = os.path.join(str(datafiles), "junction_options_element") + + generate_junction( + tmpdir, + os.path.join(project, "subproject"), + os.path.join(project, "junction.bst"), + store_ref=True, + options={"local_option": "set"}, + ) + + result = cli.run(project=project, args=["show", "--deps", "none", "--format", "%{vars}", "element.bst"]) + result.assert_success() + loaded = _yaml.load_data(result.output) + assert not loaded.get_bool("is-default") + + +@pytest.mark.datafiles(DATA_DIR) +def test_option_from_deep_junction(cli, tmpdir, datafiles): + project = os.path.join(str(datafiles), "junction_options_deep") + + generate_junction( + tmpdir, + os.path.join(project, "subproject-2"), + os.path.join(project, "subproject-1", "junction-2.bst"), + store_ref=True, + options={"local_option": "set"}, + ) + + generate_junction( + tmpdir, os.path.join(project, "subproject-1"), os.path.join(project, "junction-1.bst"), store_ref=True, + ) + + result = cli.run(project=project, args=["show", "--deps", "none", "--format", "%{vars}", "element.bst"]) + result.assert_success() + loaded = _yaml.load_data(result.output) + assert not loaded.get_bool("is-default") diff --git a/tests/format/include/junction_options/element.bst b/tests/format/include/junction_options/element.bst new file mode 100644 index 000000000..4d7f70266 --- /dev/null +++ b/tests/format/include/junction_options/element.bst @@ -0,0 +1 @@ +kind: manual diff --git a/tests/format/include/junction_options/project.conf b/tests/format/include/junction_options/project.conf new file mode 100644 index 000000000..4836c5f8b --- /dev/null +++ b/tests/format/include/junction_options/project.conf @@ -0,0 +1,4 @@ +name: test + +(@): + - junction.bst:extra_conf.yml diff --git a/tests/format/include/junction_options/subproject/extra_conf.yml b/tests/format/include/junction_options/subproject/extra_conf.yml new file mode 100644 index 000000000..1edbeee36 --- /dev/null +++ b/tests/format/include/junction_options/subproject/extra_conf.yml @@ -0,0 +1,7 @@ +(?): +- local_option == 'default': + variables: + is-default: 'True' +- local_option == 'set': + variables: + is-default: 'False' diff --git a/tests/format/include/junction_options/subproject/project.conf b/tests/format/include/junction_options/subproject/project.conf new file mode 100644 index 000000000..33ab0c8af --- /dev/null +++ b/tests/format/include/junction_options/subproject/project.conf @@ -0,0 +1,11 @@ +name: test-sub + +options: + local_option: + type: enum + description: Testing + variable: local_option + default: default + values: + - default + - set diff --git a/tests/format/include/junction_options_deep/element.bst b/tests/format/include/junction_options_deep/element.bst new file mode 100644 index 000000000..4d7f70266 --- /dev/null +++ b/tests/format/include/junction_options_deep/element.bst @@ -0,0 +1 @@ +kind: manual diff --git a/tests/format/include/junction_options_deep/project.conf b/tests/format/include/junction_options_deep/project.conf new file mode 100644 index 000000000..2525081ce --- /dev/null +++ b/tests/format/include/junction_options_deep/project.conf @@ -0,0 +1,4 @@ +name: test + +(@): + - junction-1.bst:extra_conf.yml diff --git a/tests/format/include/junction_options_deep/subproject-1/extra_conf.yml b/tests/format/include/junction_options_deep/subproject-1/extra_conf.yml new file mode 100644 index 000000000..faa1a40f7 --- /dev/null +++ b/tests/format/include/junction_options_deep/subproject-1/extra_conf.yml @@ -0,0 +1,2 @@ +(@): + junction-2.bst:extra_conf.yml diff --git a/tests/format/include/junction_options_deep/subproject-1/project.conf b/tests/format/include/junction_options_deep/subproject-1/project.conf new file mode 100644 index 000000000..f0cd28202 --- /dev/null +++ b/tests/format/include/junction_options_deep/subproject-1/project.conf @@ -0,0 +1 @@ +name: test-sub-1 diff --git a/tests/format/include/junction_options_deep/subproject-2/extra_conf.yml b/tests/format/include/junction_options_deep/subproject-2/extra_conf.yml new file mode 100644 index 000000000..1edbeee36 --- /dev/null +++ b/tests/format/include/junction_options_deep/subproject-2/extra_conf.yml @@ -0,0 +1,7 @@ +(?): +- local_option == 'default': + variables: + is-default: 'True' +- local_option == 'set': + variables: + is-default: 'False' diff --git a/tests/format/include/junction_options_deep/subproject-2/project.conf b/tests/format/include/junction_options_deep/subproject-2/project.conf new file mode 100644 index 000000000..d44ccd809 --- /dev/null +++ b/tests/format/include/junction_options_deep/subproject-2/project.conf @@ -0,0 +1,11 @@ +name: test-sub-2 + +options: + local_option: + type: enum + description: Testing + variable: local_option + default: default + values: + - default + - set diff --git a/tests/format/include/junction_options_element/element.bst b/tests/format/include/junction_options_element/element.bst new file mode 100644 index 000000000..c815951b6 --- /dev/null +++ b/tests/format/include/junction_options_element/element.bst @@ -0,0 +1,4 @@ +kind: manual + +(@): + - junction.bst:extra_conf.yml diff --git a/tests/format/include/junction_options_element/project.conf b/tests/format/include/junction_options_element/project.conf new file mode 100644 index 000000000..b32753625 --- /dev/null +++ b/tests/format/include/junction_options_element/project.conf @@ -0,0 +1 @@ +name: test diff --git a/tests/format/include/junction_options_element/subproject/extra_conf.yml b/tests/format/include/junction_options_element/subproject/extra_conf.yml new file mode 100644 index 000000000..1edbeee36 --- /dev/null +++ b/tests/format/include/junction_options_element/subproject/extra_conf.yml @@ -0,0 +1,7 @@ +(?): +- local_option == 'default': + variables: + is-default: 'True' +- local_option == 'set': + variables: + is-default: 'False' diff --git a/tests/format/include/junction_options_element/subproject/project.conf b/tests/format/include/junction_options_element/subproject/project.conf new file mode 100644 index 000000000..33ab0c8af --- /dev/null +++ b/tests/format/include/junction_options_element/subproject/project.conf @@ -0,0 +1,11 @@ +name: test-sub + +options: + local_option: + type: enum + description: Testing + variable: local_option + default: default + values: + - default + - set diff --git a/tests/testutils/junction.py b/tests/testutils/junction.py index c086f6f17..99117b2cf 100644 --- a/tests/testutils/junction.py +++ b/tests/testutils/junction.py @@ -15,7 +15,7 @@ from buildstream.testing import create_repo # Returns: # (str): The ref # -def generate_junction(tmpdir, subproject_path, junction_path, *, store_ref=True): +def generate_junction(tmpdir, subproject_path, junction_path, *, store_ref=True, options={}): # Create a repo to hold the subproject and generate # a junction element for it # @@ -25,6 +25,10 @@ def generate_junction(tmpdir, subproject_path, junction_path, *, store_ref=True) source_ref = None element = {"kind": "junction", "sources": [repo.source_config(ref=source_ref)]} + + if options: + element["config"] = {"options": options} + _yaml.roundtrip_dump(element, junction_path) return ref -- cgit v1.2.1