Merge branch 'bschubert/resolve-variables' into 'master'
Always resolve variables in elements
See merge request BuildStream/buildstream!1919
diff --git a/NEWS b/NEWS
index f7e79b1..b69e3c9 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,14 @@
o The `pip` element has been removed. Please use the one from bst-plugins-experimental
+API
+---
+
+ o `Element.node_subst_vars` and `Element.node_subst_sequence_vars` are now deprecated
+ and will get removed in the next version. All config entries are now resolved so there
+ is no need to use them anymore.
+
+
==================
buildstream 1.93.3
==================
diff --git a/src/buildstream/_options/option.py b/src/buildstream/_options/option.py
index 71d2f12..5c0eb50 100644
--- a/src/buildstream/_options/option.py
+++ b/src/buildstream/_options/option.py
@@ -76,9 +76,8 @@
# Args:
# node (Mapping): The YAML loaded key/value dictionary
# to load the value from
- # transform (callbable): Transform function for variable substitution
#
- def load_value(self, node, *, transform=None):
+ def load_value(self, node):
pass # pragma: nocover
# set_value()
diff --git a/src/buildstream/_options/optionbool.py b/src/buildstream/_options/optionbool.py
index f615982..57dbd16 100644
--- a/src/buildstream/_options/optionbool.py
+++ b/src/buildstream/_options/optionbool.py
@@ -36,11 +36,8 @@
node.validate_keys(OPTION_SYMBOLS + ["default"])
self.value = node.get_bool("default")
- def load_value(self, node, *, transform=None):
- if transform:
- self.set_value(transform(node.get_scalar(self.name)))
- else:
- self.value = node.get_bool(self.name)
+ def load_value(self, node):
+ self.value = node.get_bool(self.name)
def set_value(self, value):
if value in ("True", "true"):
diff --git a/src/buildstream/_options/optionenum.py b/src/buildstream/_options/optionenum.py
index 12ec7cb..da3a7dc 100644
--- a/src/buildstream/_options/optionenum.py
+++ b/src/buildstream/_options/optionenum.py
@@ -58,12 +58,9 @@
# Allow subclass to define the default value
self.value = self.load_default_value(node)
- def load_value(self, node, *, transform=None):
+ def load_value(self, node):
value_node = node.get_scalar(self.name)
- if transform:
- self.value = transform(value_node)
- else:
- self.value = value_node.as_str()
+ self.value = value_node.as_str()
self.validate(self.value, value_node)
diff --git a/src/buildstream/_options/optionflags.py b/src/buildstream/_options/optionflags.py
index b19a227..042ef64 100644
--- a/src/buildstream/_options/optionflags.py
+++ b/src/buildstream/_options/optionflags.py
@@ -60,13 +60,9 @@
self.value = value_node.as_str_list()
self.validate(self.value, value_node)
- def load_value(self, node, *, transform=None):
+ def load_value(self, node):
value_node = node.get_sequence(self.name)
- if transform:
- self.value = [transform(x) for x in value_node]
- else:
- self.value = value_node.as_str_list()
- self.value = sorted(self.value)
+ self.value = sorted(value_node.as_str_list())
self.validate(self.value, value_node)
def set_value(self, value):
diff --git a/src/buildstream/_options/optionpool.py b/src/buildstream/_options/optionpool.py
index dee038f..c05c559 100644
--- a/src/buildstream/_options/optionpool.py
+++ b/src/buildstream/_options/optionpool.py
@@ -104,7 +104,7 @@
# Args:
# node (dict): The loaded YAML options
#
- def load_yaml_values(self, node, *, transform=None):
+ def load_yaml_values(self, node):
for option_name, option_value in node.items():
try:
option = self._options[option_name]
@@ -113,7 +113,7 @@
raise LoadError(
"{}: Unknown option '{}' specified".format(p, option_name), LoadErrorReason.INVALID_DATA
) from e
- option.load_value(node, transform=transform)
+ option.load_value(node)
# load_cli_values()
#
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 45f3b4d..06c6770 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -836,7 +836,7 @@
output.options.load(options_node)
if self.junction:
# load before user configuration
- output.options.load_yaml_values(self.junction.options, transform=self.junction.node_subst_vars)
+ output.options.load_yaml_values(self.junction.options)
# Collect option values specified in the user configuration
overrides = self._context.get_overrides(self.name)
diff --git a/src/buildstream/_variables.pyx b/src/buildstream/_variables.pyx
index f9de2a6..9dc6cf6 100644
--- a/src/buildstream/_variables.pyx
+++ b/src/buildstream/_variables.pyx
@@ -25,7 +25,7 @@
from ._exceptions import LoadError
from .exceptions import LoadErrorReason
-from .node cimport MappingNode
+from .node cimport MappingNode, Node, ScalarNode, SequenceNode
# Variables are allowed to have dashes here
#
@@ -75,6 +75,27 @@
self._expstr_map = self._resolve(node)
self.flat = self._flatten()
+ # expand()
+ #
+ # Expand all the variables found in the given Node, recursively.
+ # This does the change in place, modifying the node. If you want to keep
+ # the node untouched, you should use `node.clone()` beforehand
+ #
+ # Args:
+ # (Node): A node for which to substitute the values
+ #
+ cpdef expand(self, Node node):
+ if isinstance(node, ScalarNode):
+ (<ScalarNode> node).value = self.subst((<ScalarNode> node).value)
+ elif isinstance(node, SequenceNode):
+ for entry in (<SequenceNode> node).value:
+ self.expand(entry)
+ elif isinstance(node, MappingNode):
+ for entry in (<MappingNode> node).value.values():
+ self.expand(entry)
+ else:
+ assert False, "Unknown 'Node' type"
+
# subst():
#
# Substitutes any variables in 'string' and returns the result.
@@ -88,7 +109,7 @@
# Raises:
# LoadError, if the string contains unresolved variable references.
#
- def subst(self, str string):
+ cpdef subst(self, str string):
expstr = _parse_expstr(string)
try:
diff --git a/src/buildstream/buildelement.py b/src/buildstream/buildelement.py
index 32b64a8..ce6bb33 100644
--- a/src/buildstream/buildelement.py
+++ b/src/buildstream/buildelement.py
@@ -172,10 +172,7 @@
node.validate_keys(_command_steps)
for command_name in _legacy_command_steps:
- if command_name in _command_steps:
- self.__commands[command_name] = self.__get_commands(node, command_name)
- else:
- self.__commands[command_name] = []
+ self.__commands[command_name] = node.get_str_list(command_name, [])
def preflight(self):
pass
@@ -305,10 +302,6 @@
#############################################################
# Private Local Methods #
#############################################################
- def __get_commands(self, node, name):
- raw_commands = node.get_sequence(name, [])
- return [self.node_subst_vars(command) for command in raw_commands]
-
def __run_command(self, sandbox, cmd):
# Note the -e switch to 'sh' means to exit with an error
# if any untested command fails.
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index d8c0f6f..67a915e 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -76,6 +76,7 @@
import re
import stat
import copy
+import warnings
from collections import OrderedDict
import contextlib
from contextlib import contextmanager
@@ -286,7 +287,8 @@
# Collect the composited environment now that we have variables
unexpanded_env = self.__extract_environment(project, meta)
- self.__environment = self.__expand_environment(unexpanded_env)
+ self.__variables.expand(unexpanded_env)
+ self.__environment = unexpanded_env.strip_node_info()
# Collect the environment nocache blacklist list
nocache = self.__extract_env_nocache(project, meta)
@@ -300,6 +302,8 @@
# Collect the composited element configuration and
# ask the element to configure itself.
self.__config = self.__extract_config(meta)
+ self.__variables.expand(self.__config)
+
self._configure(self.__config)
# Extract remote execution URL
@@ -502,6 +506,8 @@
def node_subst_vars(self, node: "ScalarNode") -> str:
"""Replace any variables in the string contained in the node and returns it.
+ **Warning**: The method is deprecated and will get removed in the next version
+
Args:
node: A ScalarNode loaded from YAML
@@ -519,15 +525,17 @@
# variables in the returned string
name = self.node_subst_vars(node.get_scalar('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
+ # FIXME: remove this
+ warnings.warn(
+ "configuration is now automatically expanded, this is a no-op and will be removed.", DeprecationWarning
+ )
+ return node.as_str()
def node_subst_sequence_vars(self, node: "SequenceNode[ScalarNode]") -> List[str]:
"""Substitute any variables in the given sequence
+ **Warning**: The method is deprecated and will get removed in the next version
+
Args:
node: A SequenceNode loaded from YAML
@@ -538,14 +546,11 @@
:class:`.LoadError`
"""
- ret = []
- for value in node:
- try:
- ret.append(self.__variables.subst(value.as_str()))
- except LoadError as e:
- provenance = value.get_provenance()
- raise LoadError("{}: {}".format(provenance, e), e.reason, detail=e.detail) from e
- return ret
+ # FIXME: remove this
+ warnings.warn(
+ "configuration is now automatically expanded, this is a no-op and will be removed.", DeprecationWarning
+ )
+ return node.as_str_list()
def compute_manifest(
self, *, include: Optional[List[str]] = None, exclude: Optional[List[str]] = None, orphans: bool = True
@@ -760,11 +765,8 @@
if bstdata is not None:
with sandbox.batch(SandboxFlags.NONE):
- commands = bstdata.get_sequence("integration-commands", [])
- for command in commands:
- cmd = self.node_subst_vars(command)
-
- sandbox.run(["sh", "-e", "-c", cmd], 0, env=environment, cwd="/", label=cmd)
+ for command in bstdata.get_str_list("integration-commands", []):
+ sandbox.run(["sh", "-e", "-c", command], 0, env=environment, cwd="/", label=command)
def stage_sources(self, sandbox: "Sandbox", directory: str) -> None:
"""Stage this element's sources to a directory in the sandbox
@@ -2566,17 +2568,6 @@
return environment
- # This will resolve the final environment to be used when
- # creating sandboxes for this element
- #
- def __expand_environment(self, environment):
- # Resolve variables in environment value strings
- final_env = {}
- for key, value in environment.items():
- final_env[key] = self.node_subst_vars(value)
-
- return final_env
-
@classmethod
def __extract_env_nocache(cls, project, meta):
if meta.is_junction:
diff --git a/src/buildstream/plugin.py b/src/buildstream/plugin.py
index dfb5433..6795043 100644
--- a/src/buildstream/plugin.py
+++ b/src/buildstream/plugin.py
@@ -301,11 +301,6 @@
should be used to ensure that the user has not specified keys in `node` which are unsupported
by the plugin.
- .. note::
-
- For Elements, when variable substitution is desirable, the
- :func:`Element.node_subst_vars() <buildstream.element.Element.node_subst_vars>`
- method can be used.
"""
raise ImplError(
"{tag} plugin '{kind}' does not implement configure()".format(tag=self.__type_tag, kind=self.get_kind())
diff --git a/src/buildstream/plugins/elements/import.py b/src/buildstream/plugins/elements/import.py
index d9961aa..de7ee8a 100644
--- a/src/buildstream/plugins/elements/import.py
+++ b/src/buildstream/plugins/elements/import.py
@@ -46,8 +46,8 @@
def configure(self, node):
node.validate_keys(["source", "target"])
- self.source = self.node_subst_vars(node.get_scalar("source"))
- self.target = self.node_subst_vars(node.get_scalar("target"))
+ self.source = node.get_str("source")
+ self.target = node.get_str("target")
def preflight(self):
# Assert that we have at least one source to fetch.
diff --git a/src/buildstream/plugins/elements/script.py b/src/buildstream/plugins/elements/script.py
index 9d780eb..502212e 100644
--- a/src/buildstream/plugins/elements/script.py
+++ b/src/buildstream/plugins/elements/script.py
@@ -46,14 +46,13 @@
def configure(self, node):
for n in node.get_sequence("layout", []):
- dst = self.node_subst_vars(n.get_scalar("destination"))
- elm = self.node_subst_vars(n.get_scalar("element", None))
+ dst = n.get_str("destination")
+ elm = n.get_str("element", None)
self.layout_add(elm, dst)
node.validate_keys(["commands", "root-read-only", "layout"])
- cmds = self.node_subst_sequence_vars(node.get_sequence("commands"))
- self.add_commands("commands", cmds)
+ self.add_commands("commands", node.get_str_list("commands"))
self.set_work_dir()
self.set_install_root()
