element.py: change 'substitute_variables' to take a 'ScalarNode' and rename

Previously 'substitute_variable' would take a str, which would prevent
us from doing nice error reporting. Using a 'ScalarNode' allows us
to get our errors nicely.

- rename it to 'node_subst_vars'.
- add a nicer try-except around it in order to get nicer error reporting to users.

2 files changed, 30 insertions, 7 deletions

diff --git a/src/buildstream/buildelement.py b/src/buildstream/buildelement.py
index b33acfe12..7fe97c168 100644
--- a/src/buildstream/buildelement.py
+++ b/src/buildstream/buildelement.py
@@ -281,9 +281,9 @@ class BuildElement(Element):
     #                   Private Local Methods                   #
     #############################################################
     def __get_commands(self, node, name):
-        raw_commands = node.get_str_list(name, [])
+        raw_commands = node.get_sequence(name, [])
         return [
-            self.substitute_variables(command)
+            self.node_subst_vars(command)
             for command in raw_commands
         ]
 
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 966f0f7cb..58fd2c33b 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -112,7 +112,7 @@ from .storage._casbaseddirectory import CasBasedDirectory
 from .storage.directory import VirtualDirectoryError
 
 if TYPE_CHECKING:
-    from .node import MappingNode
+    from .node import MappingNode, ScalarNode
     from .types import SourceRef
     from typing import Set, Tuple
 
@@ -536,8 +536,31 @@ class Element(Plugin):
 
         return None
 
-    def substitute_variables(self, value):
-        return self.__variables.subst(value)
+    def node_subst_vars(self, node: 'ScalarNode') -> str:
+        """Replace any variables in the string contained in the node and returns it.
+
+        Args:
+           node: A ScalarNode loaded from YAML
+
+        Returns:
+           The value with all variables replaced
+
+        Raises:
+           :class:`.LoadError`: When the node doesn't contain a string or a variable was not found.
+
+        **Example:**
+
+        .. code:: python
+
+          # Expect a string 'name' in 'node', substituting any
+          # variables in the returned string
+          name = self.node_subst_vars(node.get_str('name'))
+        """
+        try:
+            return self.__variables.subst(node.as_str())
+        except LoadError as e:
+            provenance = node.get_provenance()
+            raise LoadError('{}: {}'.format(provenance, e), e.reason, detail=e.detail) from e
 
     def node_subst_member(self, node: 'MappingNode[str, Any]', member_name: str, default: str = _node_sentinel) -> Any:
         """Fetch the value of a string node member, substituting any variables
@@ -877,9 +900,9 @@ class Element(Plugin):
 
         if bstdata is not None:
             with sandbox.batch(SandboxFlags.NONE):
-                commands = bstdata.get_str_list('integration-commands', [])
+                commands = bstdata.get_sequence('integration-commands', [])
                 for command in commands:
-                    cmd = self.substitute_variables(command)
+                    cmd = self.node_subst_vars(command)
 
                     sandbox.run(['sh', '-e', '-c', cmd], 0, env=environment, cwd='/',
                                 label=cmd)