element.py: Always expand public data's variablesbschubert/resolve-public-variables

This fixes a bug introduced by d7d18c1a2e454c507afd9e1d3f1358639dd43871,
where public data integration commands and others would never get their
data expanded and would thus fail to run.

The previous however revelead a previous bug, where variables values in
public data of elements would not be part of the cache key of an element
or it's reverse dependencies, and thus, on variable change, rebuilds
would not happen when they would have been needed.

This ensures that all the public data is always resolved and part of the
element's cache key. This however will bring a rebuild of an element
whenever its integration commands variables change, which is simpler to
handle than to try to push that responsibility on reverse dependencies,
since public data can contain plugin-defined values.

5 files changed, 48 insertions, 14 deletions

diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 18e42c722..09b0e4c06 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -296,8 +296,8 @@ class Element(Plugin):
         self.__env_nocache = nocache
 
         # Grab public domain data declared for this instance
-        unexpanded_public = self.__extract_public(meta)
-        self.__public = self.__expand_splits(unexpanded_public)
+        self.__public = self.__extract_public(meta)
+        self.__variables.expand(self.__public)
         self.__dynamic_public = None
 
         # Collect the composited element configuration and
@@ -2745,18 +2745,6 @@ class Element(Plugin):
 
         return element_public
 
-    # Expand the splits in the public data using the Variables in the element
-    def __expand_splits(self, element_public):
-        element_bst = element_public.get_mapping("bst", default={})
-        element_splits = element_bst.get_mapping("split-rules", default={})
-
-        # Resolve any variables in the public split rules directly
-        for domain, splits in element_splits.items():
-            splits = [self.__variables.subst(split.strip()) for split in splits.as_str_list()]
-            element_splits[domain] = splits
-
-        return element_public
-
     def __init_splits(self):
         bstdata = self.get_public_data("bst")
         splits = bstdata.get_mapping("split-rules")
diff --git a/tests/format/variables.py b/tests/format/variables.py
index 5f07067f3..616dc20c1 100644
--- a/tests/format/variables.py
+++ b/tests/format/variables.py
@@ -138,3 +138,24 @@ def test_variables_are_resolved_in_elements_context(cli, datafiles):
         ["one.bst"],
         ["two.bst"],
     )
+
+
+@pytest.mark.datafiles(os.path.join(DATA_DIR, "public_data_variables"))
+def test_variables_are_resolved_in_public_section(cli, datafiles):
+    project = str(datafiles)
+
+    result = cli.run(project=project, args=["show", "--format", "%{public}", "public.bst"])
+    result.assert_success()
+
+    output = _yaml.load_data(result.output).strip_node_info()
+    expected = {"integration-commands": ["echo expanded"], "test": "expanded"}
+
+    assert {k: v for k, v in output.items() if k in expected} == expected
+
+
+@pytest.mark.datafiles(os.path.join(DATA_DIR, "public_data_variables"))
+def test_variables_resolving_errors_in_public_section(cli, datafiles):
+    project = str(datafiles)
+
+    result = cli.run(project=project, args=["show", "--format", "%{public}", "public_unresolved.bst"])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.UNRESOLVED_VARIABLE)
diff --git a/tests/format/variables/public_data_variables/project.conf b/tests/format/variables/public_data_variables/project.conf
new file mode 100644
index 000000000..ab47dc7c4
--- /dev/null
+++ b/tests/format/variables/public_data_variables/project.conf
@@ -0,0 +1,5 @@
+name: foo
+min-version: 2.0
+
+variables:
+    expand-me: "expanded"
diff --git a/tests/format/variables/public_data_variables/public.bst b/tests/format/variables/public_data_variables/public.bst
new file mode 100644
index 000000000..e5273e89a
--- /dev/null
+++ b/tests/format/variables/public_data_variables/public.bst
@@ -0,0 +1,11 @@
+kind: import
+
+sources:
+  - kind: local
+    path: public.bst
+
+public:
+  integration-commands:
+    - echo %{expand-me}
+
+  test: "%{expand-me}"
diff --git a/tests/format/variables/public_data_variables/public_unresolved.bst b/tests/format/variables/public_data_variables/public_unresolved.bst
new file mode 100644
index 000000000..024075134
--- /dev/null
+++ b/tests/format/variables/public_data_variables/public_unresolved.bst
@@ -0,0 +1,9 @@
+kind: import
+
+sources:
+  - kind: local
+    path: public_unresolved.bst
+
+public:
+  integration-commands:
+    - echo %{non-existent}