Evaluate options before includes
Since option directives `(?)` expect lists, they cannot easily be
merged. So multiple includes with conflicting option directives
would only get one list if option processing is done after
includes.
diff --git a/src/buildstream/_frontend/widget.py b/src/buildstream/_frontend/widget.py
index 81ca2f7..eab701b 100644
--- a/src/buildstream/_frontend/widget.py
+++ b/src/buildstream/_frontend/widget.py
@@ -473,11 +473,12 @@
# Project Options
values = OrderedDict()
- project.options.printable_variables(values)
- if values:
- text += self.content_profile.fmt("Project Options\n", bold=True)
- text += self._format_values(values)
- text += "\n"
+ if project.options:
+ project.options.printable_variables(values)
+ if values:
+ text += self.content_profile.fmt("Project Options\n", bold=True)
+ text += self._format_values(values)
+ text += "\n"
# Plugins
text += self._format_plugins(
diff --git a/src/buildstream/_includes.py b/src/buildstream/_includes.py
index 7f4863e..f623423 100644
--- a/src/buildstream/_includes.py
+++ b/src/buildstream/_includes.py
@@ -27,9 +27,10 @@
# 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)
+ # first_pass (bool): Whether to use first pass options.
+ # process_options (bool): Whether to process options.
+ def process(self, node, *, only_local=False, first_pass=False, process_options=True):
+ self._process(node, only_local=only_local, first_pass=first_pass, process_options=process_options)
# _process()
#
@@ -41,12 +42,20 @@
# 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(self, node, *, included=None, current_loader=None, only_local=False, process_project_options=True):
+ # first_pass (bool): Whether to use first pass options.
+ # process_options (bool): Whether to process options.
+ def _process(
+ self, node, *, included=None, current_loader=None, only_local=False, first_pass=False, process_options=True,
+ ):
if current_loader is None:
current_loader = self._loader
- if process_project_options:
+ if not process_options:
+ pass
+ elif first_pass:
+ current_loader.project.first_pass_config.options.process_node(node)
+ else:
+ current_loader.project.ensure_fully_loaded()
current_loader.project.options.process_node(node)
self._process_node(
@@ -54,7 +63,8 @@
included=included,
only_local=only_local,
current_loader=current_loader,
- process_project_options=process_project_options,
+ first_pass=first_pass,
+ process_options=process_options,
)
# _process_node()
@@ -67,9 +77,10 @@
# 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
+ # first_pass (bool): Whether to use first pass options.
+ # process_options (bool): Whether to process options.
def _process_node(
- self, node, *, included=None, current_loader=None, only_local=False, process_project_options=True
+ self, node, *, included=None, current_loader=None, only_local=False, first_pass=False, process_options=True,
):
if included is None:
included = set()
@@ -123,7 +134,8 @@
included=included,
current_loader=sub_loader,
only_local=only_local,
- process_project_options=process_project_options or current_loader != sub_loader,
+ first_pass=first_pass,
+ process_options=process_options,
)
finally:
included.remove(file_path)
@@ -136,7 +148,8 @@
included=included,
current_loader=current_loader,
only_local=only_local,
- process_project_options=process_project_options,
+ first_pass=first_pass,
+ process_options=process_options,
)
# _include_file()
@@ -172,9 +185,10 @@
# 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
+ # first_pass (bool): Whether to use first pass options.
+ # process_options (bool): Whether to process options.
def _process_value(
- self, value, *, included=None, current_loader=None, only_local=False, process_project_options=True
+ self, value, *, included=None, current_loader=None, only_local=False, first_pass=False, process_options=True,
):
value_type = type(value)
@@ -184,7 +198,8 @@
included=included,
current_loader=current_loader,
only_local=only_local,
- process_project_options=process_project_options,
+ first_pass=first_pass,
+ process_options=process_options,
)
elif value_type is SequenceNode:
for v in value:
@@ -193,5 +208,6 @@
included=included,
current_loader=current_loader,
only_local=only_local,
- process_project_options=process_project_options,
+ first_pass=first_pass,
+ process_options=process_options,
)
diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index b73f5b8..28a1cff 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -69,7 +69,6 @@
self._context = context
self._options = project.options # Project options (OptionPool)
self._basedir = basedir # Base project directory
- self._first_pass_options = project.first_pass_config.options # Project options (OptionPool)
self._parent = parent # The parent loader
self._fetch_subprojects = fetch_subprojects
@@ -518,7 +517,7 @@
kind = node.get_str(Symbol.KIND)
if kind == "junction":
- self._first_pass_options.process_node(node)
+ self.project.first_pass_config.options.process_node(node)
else:
self.project.ensure_fully_loaded()
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 508afa6..031a687 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -680,9 +680,6 @@
self.directory, self.get_path_from_node(pre_config_node.get_scalar("element-path"), check_is_dir=True)
)
- self.config.options = OptionPool(self.element_path)
- self.first_pass_config.options = OptionPool(self.element_path)
-
defaults = pre_config_node.get_mapping("defaults")
defaults.validate_keys(["targets"])
self._default_targets = defaults.get_str_list("targets")
@@ -694,12 +691,17 @@
self._project_includes = Includes(self.loader, copy_tree=False)
+ # We need to process includes without options to load options included in separate files.
+ project_conf_for_options = self._project_conf.clone()
+ self._project_includes.process(project_conf_for_options, only_local=True, process_options=False)
+ self._load_options(project_conf_for_options, self.first_pass_config, ignore_unknown=True)
+
project_conf_first_pass = self._project_conf.clone()
- self._project_includes.process(project_conf_first_pass, only_local=True, process_project_options=False)
+ self._project_includes.process(project_conf_first_pass, only_local=True, first_pass=True)
config_no_include = self._default_config_node.clone()
project_conf_first_pass._composite(config_no_include)
- self._load_pass(config_no_include, self.first_pass_config, ignore_unknown=True)
+ self._load_pass(config_no_include, self.first_pass_config)
# Use separate file for storing source references
ref_storage_node = pre_config_node.get_scalar("ref-storage")
@@ -719,8 +721,13 @@
# Process the second pass of loading the project configuration.
#
def _load_second_pass(self):
+ # We need to process includes without options to load options included in separate files.
+ project_conf_for_options = self._project_conf.clone()
+ self._project_includes.process(project_conf_for_options, only_local=True, process_options=False)
+ self._load_options(project_conf_for_options, self.config)
+
project_conf_second_pass = self._project_conf.clone()
- self._project_includes.process(project_conf_second_pass, process_project_options=False)
+ self._project_includes.process(project_conf_second_pass)
config = self._default_config_node.clone()
project_conf_second_pass._composite(config)
@@ -818,19 +825,17 @@
self._shell_host_files.append(mount)
- # _load_pass():
+ # _load_options():
#
- # Loads parts of the project configuration that are different
- # for first and second pass configurations.
+ # Loads options for a given configuration pass
#
# Args:
# config (dict) - YaML node of the configuration file.
# output (ProjectConfig) - ProjectConfig to load configuration onto.
# ignore_unknown (bool) - Whether option loader shoud ignore unknown options.
#
- def _load_pass(self, config, output, *, ignore_unknown=False):
-
- self._load_plugin_factories(config, output)
+ def _load_options(self, config, output, *, ignore_unknown=False):
+ output.options = OptionPool(self.element_path)
# Load project options
options_node = config.get_mapping("options", default={})
@@ -855,6 +860,19 @@
# or conditionally specifying the project name; will be ignored.
output.options.process_node(config)
+ # _load_pass():
+ #
+ # Loads parts of the project configuration that are common
+ # for first and second pass configurations.
+ #
+ # Args:
+ # config (dict) - YaML node of the configuration file.
+ # output (ProjectConfig) - ProjectConfig to load configuration onto.
+ #
+ def _load_pass(self, config, output):
+
+ self._load_plugin_factories(config, output)
+
# Element and Source type configurations will be composited later onto
# element/source types, so we delete it from here and run our final
# assertion after.
@@ -886,6 +904,8 @@
# Export options into variables, if that was requested
output.options.export_variables(output.base_variables)
+ overrides = self._context.get_overrides(self.name)
+
# Override default_mirror if not set by command-line
output.default_mirror = self._default_mirror or overrides.get_str("default-mirror", default=None)
diff --git a/tests/format/include.py b/tests/format/include.py
index 12b043c..766a403 100644
--- a/tests/format/include.py
+++ b/tests/format/include.py
@@ -220,6 +220,20 @@
@pytest.mark.datafiles(DATA_DIR)
+def test_conditional_in_fragment_conflicts(cli, datafiles):
+ project = os.path.join(str(datafiles), "conditional-conflicts")
+
+ result = cli.run(
+ project=project,
+ args=["-o", "build_arch", "i586", "show", "--deps", "none", "--format", "%{vars}", "element.bst"],
+ )
+ result.assert_success()
+ loaded = _yaml.load_data(result.output)
+ assert loaded.get_str("enable-work-around") == "true"
+ assert loaded.get_str("size") == "4"
+
+
+@pytest.mark.datafiles(DATA_DIR)
def test_inner(cli, datafiles):
project = os.path.join(str(datafiles), "inner")
result = cli.run(
diff --git a/tests/format/include/conditional-conflicts/element.bst b/tests/format/include/conditional-conflicts/element.bst
new file mode 100644
index 0000000..4d7f702
--- /dev/null
+++ b/tests/format/include/conditional-conflicts/element.bst
@@ -0,0 +1 @@
+kind: manual
diff --git a/tests/format/include/conditional-conflicts/extra_conf.yml b/tests/format/include/conditional-conflicts/extra_conf.yml
new file mode 100644
index 0000000..dd58c98
--- /dev/null
+++ b/tests/format/include/conditional-conflicts/extra_conf.yml
@@ -0,0 +1,6 @@
+variables:
+ (?):
+ - build_arch == "i586":
+ size: "4"
+ - build_arch == "x86_64":
+ size: "8"
diff --git a/tests/format/include/conditional-conflicts/project.conf b/tests/format/include/conditional-conflicts/project.conf
new file mode 100644
index 0000000..6978fa8
--- /dev/null
+++ b/tests/format/include/conditional-conflicts/project.conf
@@ -0,0 +1,15 @@
+name: test
+min-version: 2.0
+
+options:
+ build_arch:
+ type: arch
+ description: Architecture
+ variable: build_arch
+ values:
+ - i586
+ - x86_64
+
+(@):
+ - extra_conf.yml
+ - work_around.yml
diff --git a/tests/format/include/conditional-conflicts/work_around.yml b/tests/format/include/conditional-conflicts/work_around.yml
new file mode 100644
index 0000000..a527fe1
--- /dev/null
+++ b/tests/format/include/conditional-conflicts/work_around.yml
@@ -0,0 +1,5 @@
+variables:
+ enable-work-around: "false"
+ (?):
+ - build_arch == "i586":
+ enable-work-around: "true"
