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.

This is a backport of Valentin David's work from 86f013dbffa87692250ce1aefd31cecd1559ddd5

19 files changed, 214 insertions, 28 deletions

diff --git a/buildstream/_includes.py b/buildstream/_includes.py
index e30003630..e6f32cae5 100644
--- a/buildstream/_includes.py
+++ b/buildstream/_includes.py
@@ -26,16 +26,56 @@ 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
+    ):
+
         if isinstance(node.get('(@)'), str):
             includes = [_yaml.node_get(node, str, '(@)')]
         else:
@@ -61,9 +101,12 @@ 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)
 
@@ -75,10 +118,13 @@ class Includes:
                     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 @@ class Includes:
             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 @@ 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
+    ):
         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 ec929eadb..24f2b595b 100644
--- a/buildstream/_loader/loader.py
+++ b/buildstream/_loader/loader.py
@@ -267,8 +267,6 @@ class Loader():
 
             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 c5172e210..5530aa23d 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -436,7 +436,7 @@ class Project():
         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 @@ class Project():
     #
     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 36e723ed0..cfbfb66e3 100644
--- a/tests/format/include.py
+++ b/tests/format/include.py
@@ -261,3 +261,61 @@ def test_include_project_file(cli, datafiles):
     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 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 efc429ef6..9ab63eb73 100644
--- a/tests/testutils/junction.py
+++ b/tests/testutils/junction.py
@@ -16,7 +16,7 @@ from buildstream import _yaml
 # 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 @@ def generate_junction(tmpdir, subproject_path, junction_path, *, store_ref=True)
             repo.source_config(ref=source_ref)
         ]
     }
+
+    if options:
+        element["config"] = {"options": options}
+
     _yaml.dump(element, junction_path)
 
     return ref