Merge branch 'tristan/backport-junction-includes' into 'bst-1'
Process options in includes files with the options of their junction
See merge request BuildStream/buildstream!1912
diff --git a/NEWS b/NEWS
index 7da79eb..cb13e48 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,17 @@
=================
+ (unreleased)
+=================
+
+ o Process options in included files in the context of the project they
+ were included from.
+
+ This is technically a breaking change, however it is highly unlikely
+ that this will break projects. In some cases projects were working around
+ the broken behavior by ensuring matching project option names in junctioned
+ projects, and in other cases simply avoiding including files which have
+ project option conditional statements.
+
+=================
buildstream 1.4.2
=================
diff --git a/buildstream/_includes.py b/buildstream/_includes.py
index e300036..e6f32ca 100644
--- a/buildstream/_includes.py
+++ b/buildstream/_includes.py
@@ -26,16 +26,56 @@
#
# 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
+ ):
+
if isinstance(node.get('(@)'), str):
includes = [_yaml.node_get(node, str, '(@)')]
else:
@@ -61,9 +101,12 @@
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)
@@ -75,10 +118,13 @@
del node[key]
for _, value in _yaml.node_items(node):
- 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()
#
@@ -94,6 +140,7 @@
junction, include = include.split(':', 1)
junction_loader = loader._get_loader(junction, fetch_subprojects=True)
current_loader = junction_loader
+ current_loader.project.ensure_fully_loaded()
else:
current_loader = loader
project = current_loader.project
@@ -116,18 +163,25 @@
# 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
+ ):
if isinstance(value, Mapping):
- 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 isinstance(value, list):
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/buildstream/_loader/loader.py b/buildstream/_loader/loader.py
index ec929ea..24f2b59 100644
--- a/buildstream/_loader/loader.py
+++ b/buildstream/_loader/loader.py
@@ -267,8 +267,6 @@
self._includes.process(node)
- self._options.process_node(node)
-
element = LoadElement(node, filename, self)
self._elements[filename] = element
diff --git a/buildstream/_project.py b/buildstream/_project.py
index c5172e2..5530aa2 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -436,7 +436,7 @@
self._project_includes = Includes(self.loader, copy_tree=False)
project_conf_first_pass = _yaml.node_copy(self._project_conf)
- 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 = _yaml.node_copy(self._default_config_node)
_yaml.composite(config_no_include, project_conf_first_pass)
@@ -460,7 +460,7 @@
#
def _load_second_pass(self):
project_conf_second_pass = _yaml.node_copy(self._project_conf)
- self._project_includes.process(project_conf_second_pass)
+ self._project_includes.process(project_conf_second_pass, process_project_options=False)
config = _yaml.node_copy(self._default_config_node)
_yaml.composite(config, project_conf_second_pass)
diff --git a/tests/format/include.py b/tests/format/include.py
index 36e723e..cfbfb66 100644
--- a/tests/format/include.py
+++ b/tests/format/include.py
@@ -261,3 +261,61 @@
result.assert_success()
loaded = _yaml.load_data(result.output)
assert loaded['included'] == 'True'
+
+
+@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 loaded["is-default"] == 'False'
+
+
+@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 loaded["is-default"] == 'False'
+
+
+@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 loaded["is-default"] == 'False'
diff --git a/tests/format/include/junction_options/element.bst b/tests/format/include/junction_options/element.bst
new file mode 100644
index 0000000..4d7f702
--- /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 0000000..4836c5f
--- /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 0000000..1edbeee
--- /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 0000000..33ab0c8
--- /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 0000000..4d7f702
--- /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 0000000..2525081
--- /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 0000000..faa1a40
--- /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 0000000..f0cd282
--- /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 0000000..1edbeee
--- /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 0000000..d44ccd8
--- /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 0000000..c815951
--- /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 0000000..b327536
--- /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 0000000..1edbeee
--- /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 0000000..33ab0c8
--- /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 efc429e..9ab63eb 100644
--- a/tests/testutils/junction.py
+++ b/tests/testutils/junction.py
@@ -16,7 +16,7 @@
# 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
#
@@ -31,6 +31,10 @@
repo.source_config(ref=source_ref)
]
}
+
+ if options:
+ element["config"] = {"options": options}
+
_yaml.dump(element, junction_path)
return ref
