Merge pull request #626 from jkloetzke/conditional-tool-env

jkloetzke · web-flow · commit fb86eba7d57c · 2025-03-23T20:38:50.000+01:00
Conditional environment defiitions
diff --git a/doc/manual/configuration.rst b/doc/manual/configuration.rst
@@ -1315,7 +1315,9 @@ The following settings are supported:
 |             |                 |    environment:                                     |
 |             |                 |        FOO: value                                   |
 |             |                 |        BAR: baz                                     |
-|             |                 |        BAZ: "${VAR}"                                |
+|             |                 |        BAZ:                                         |
+|             |                 |            value: "${VAR}"                          |
+|             |                 |            if: "${CONDITION}"                       |
 |             |                 |                                                     |
 |             |                 | Value strings in this clause are subject to         |
 |             |                 | :ref:`configuration-principle-subst`.               |
@@ -1388,16 +1390,33 @@ name requirement.  Both packages are based on the identical recipe
 environment
 ~~~~~~~~~~~
 
-Type: Dictionary (String -> String)
+Type::
+
+    {
+        str : str | {
+            "value" : str,
+            Optional("if") : str | IfExpression
+        }
+    }
 
 Defines environment variables in the scope of the current recipe. Any inherited
 variables of the downstream recipe with the same name are overwritten. All
 variables are passed to upstream recipes.
 
-Example::
+The definition of a variable can optionally be guarded by an ``if`` condition.
+Only if the ``if`` property evaluates to true, the variable is actually
+defined. Might be a string or an IfExpression. See
+:ref:`configuration-principle-booleans` for details about the evaluation.
 
-   environment:
-      PKG_VERSION: "1.2.3"
+Examples::
+
+    environment:
+        PKG_VERSION: "1.2.3"
+
+    environment:
+        PKG_VERSION:
+            value: "1.2.3"
+            if: "$(eq,$FOO,bar)"
 
 All environment keys are eligible to variable substitution. The environment of
 the recipe and inherited classes are merged together. Suppose the project has
@@ -1609,7 +1628,14 @@ Not available on Windows.
 metaEnvironment
 ~~~~~~~~~~~~~~~
 
-Type: Dictionary (String -> String)
+Type::
+
+    {
+        str : str | {
+            "value" : str,
+            Optional("if") : str | IfExpression
+        }
+    }
 
 metaEnvironment variables behave like :ref:`configuration-recipes-privateenv` variables.
 They overrule other environment variables and can be used in all steps. In addition all
@@ -1622,6 +1648,11 @@ All metaEnvironment variables are subject to :ref:`string substitution
 <configuration-principle-subst>`, unless the :ref:`policies-substituteMetaEnv`
 policy is configured for the old behaviour.
 
+The definition of a metaEnvironment variable can optionally be guarded by an
+``if`` condition.  Only if the ``if`` property evaluates to true, the variable
+is actually defined. Might be a string or an IfExpression. See
+:ref:`configuration-principle-booleans` for details about the evaluation.
+
 .. _configuration-recipes-multipackage:
 
 multiPackage
@@ -1664,7 +1695,14 @@ header files and other needed files to link with this library.
 privateEnvironment
 ~~~~~~~~~~~~~~~~~~
 
-Type: Dictionary (String -> String)
+Type::
+
+    {
+        str : str | {
+            "value" : str,
+            Optional("if") : str | IfExpression
+        }
+    }
 
 Defines environment variables just for the current recipe. Any inherited
 variables with the same name of the downstream recipe or others that were
@@ -1680,6 +1718,11 @@ The ``privateEnvironment`` of the recipe and inherited classes are merged
 together.  See :ref:`configuration-recipes-env` for the merge and string
 substitution behaviour.
 
+The definition of a variable can optionally be guarded by an ``if`` condition.
+Only if the ``if`` property evaluates to true, the variable is actually
+defined. Might be a string or an IfExpression. See
+:ref:`configuration-principle-booleans` for details about the evaluation.
+
 .. _configuration-recipes-providedeps:
 
 provideDeps
@@ -1735,6 +1778,9 @@ consuming recipes. Example::
          netAccess: True
          environment:
             CC: gcc
+            CXX:
+               value: g++
+               if: "$ENABLE_CPP"
             LD: ld
          fingerprintIf: True
          fingerprintScript: |
@@ -1761,6 +1807,11 @@ recipe they must define distinct variables because no particular order between
 tools is defined. The values defined in this attribute are subject to variable
 substitution.
 
+The definition of an ``environment`` variable can optionally be guarded by an
+``if`` condition.  Only if the ``if`` property evaluates to true, the variable
+is actually defined. Might be a string or an IfExpression. See
+:ref:`configuration-principle-booleans` for details about the evaluation.
+
 The ``fingerprintScript`` attribute defines a fingerprint script like in a
 normal recipe by :ref:`configuration-recipes-fingerprintScript`. A fingerprint
 script defined by a tool is implicitly added to the fingerprint scripts of all
@@ -1780,7 +1831,14 @@ by giving the relative path directly::
 provideVars
 ~~~~~~~~~~~
 
-Type: Dictionary (String -> String)
+Type::
+
+    {
+        str : str | {
+            "value" : str,
+            Optional("if") : str | IfExpression
+        }
+    }
 
 Declares arbitrary environment variables with values that should be passed to
 the downstream recipe. The values of the declared variables are subject to
@@ -1792,6 +1850,11 @@ package environment. Example::
         CROSS_COMPILE: "arm-linux-${ABI}-"
 
 
+The definition of a variable can optionally be guarded by an ``if`` condition.
+Only if the ``if`` property evaluates to true, the variable is actually
+defined. Might be a string or an IfExpression. See
+:ref:`configuration-principle-booleans` for details about the evaluation.
+
 By default these provided variables are not picked up by downstream recipes. This
 must be declared explicitly by a ``use: [environment]`` attribute in the
 dependency section of the downstream recipe. Only then are the provided variables
@@ -1842,6 +1905,11 @@ actually used (i.e. the parent recipe defined ``sandbox`` in the ``use``
 section and the user builds with ``--sandbox``). In this case the variables
 defined here have a higher precedence that the ones defined in ``provideVars``.
 
+The definition of an ``environment`` variable can optionally be guarded by an
+``if`` condition.  Only if the ``if`` property evaluates to true, the variable
+is actually defined. Might be a string or an IfExpression. See
+:ref:`configuration-principle-booleans` for details about the evaluation.
+
 Variable substitution is possible for the mount paths and environment
 variables. See :ref:`configuration-principle-subst` for the available
 substations. The mount paths are also subject to an additional variable
@@ -1865,6 +1933,9 @@ Example::
             - ["\\$SSH_AUTH_SOCK", "\\SSH_AUTH_SOCK", [nofail, nojenkins]]
         environment:
             AUTOCONF_BUILD: "x86_64-linux-gnu"
+            ORIGINAL_ARCH:
+                value: "$AUTOCONF_BUILD"
+                if: "$(ne,$AUTOCONF_BUILD,x86_64-linux-gnu)"
         user: nobody
 
 The example assumes that the variable ``MYREPO`` was set somewhere in the
diff --git a/pym/bob/input.py b/pym/bob/input.py
@@ -566,8 +566,8 @@ def prepare(self, coreStepRef, env):
         """Create concrete tool for given step."""
         path = env.substitute(self.path, "provideTools::path")
         libs = [ env.substitute(l, "provideTools::libs") for l in self.libs ]
-        environment = { k : env.substitute(v, "provideTools::environment::"+k)
-            for k, v in self.environment.items() }
+        environment = env.substituteCondDict(self.environment,
+                                             "provideTools::environment")
         return CoreTool(coreStepRef, path, libs, self.netAccess, environment,
                         self.fingerprintScript, self.fingerprintIf,
                         self.fingerprintVars)
@@ -698,10 +698,8 @@ def __init__(self, coreStep, env, enabled, spec):
             if (m[0] != "") and (m[1] != ""):
                 self.mounts.append(m)
         self.mounts.extend(recipeSet.getSandboxMounts())
-        self.environment = {
-            k : env.substitute(v, "providedSandbox::environment")
-            for (k, v) in spec.get('environment', {}).items()
-        }
+        self.environment = env.substituteCondDict(spec.get('environment', {}),
+                                                  "providedSandbox::environment")
         self.user = recipeSet.getSandboxUser() or spec.get('user', "nobody")
 
         # Calculate a "resultId" so that only identical sandboxes match
@@ -1793,14 +1791,22 @@ def validate(self, data):
         return LayerSpec(name, RecipeSet.LAYERS_SCM_SCHEMA.validate(_data)[0])
 
 class VarDefineValidator:
-    def __init__(self, keyword):
-        self.__varName = re.compile(r'^[A-Za-z_][A-Za-z0-9_]*$')
+    VAR_NAME = re.compile(r'^[A-Za-z_][A-Za-z0-9_]*$')
+    VAR_DEF = schema.Schema({
+            'value' : str,
+            schema.Optional("if"): schema.Or(str, IfExpression),
+        })
+
+    def __init__(self, keyword, conditional=True):
         self.__keyword = keyword
+        self.__conditional = conditional
 
     def validate(self, data):
         if not isinstance(data, dict):
             raise schema.SchemaUnexpectedTypeError(
                 "{}: must be a dictionary".format(self.__keyword), None)
+
+        data = data.copy()
         for key,value in sorted(data.items()):
             if not isinstance(key, str):
                 raise schema.SchemaUnexpectedTypeError(
@@ -1812,13 +1818,19 @@ def validate(self, data):
                     "{}: bad variable '{}'. Environment variables starting with 'BOB_' are reserved!"
                         .format(self.__keyword, key),
                     None)
-            if self.__varName.match(key) is None:
+            if self.VAR_NAME.match(key) is None:
                 raise schema.SchemaWrongKeyError(
                     "{}: bad variable name '{}'.".format(self.__keyword, key),
                     None)
-            if not isinstance(value, str):
+            if isinstance(value, dict) and self.__conditional:
+                self.VAR_DEF.validate(value, error=f"{self.__keyword}: {key}: invalid definition!")
+                data[key] = (value['value'], value.get('if'))
+            elif isinstance(value, str):
+                if self.__conditional:
+                    data[key] = (value, None)
+            else:
                 raise schema.SchemaUnexpectedTypeError(
-                    "{}: bad variable '{}'. Environment variable values must be strings!"
+                    "{}: {}: bad variable definition type."
                         .format(self.__keyword, key),
                     None)
         return data
@@ -2378,8 +2390,7 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None,
             "__tools" : tools })
         env = inputEnv.derive()
         for i in self.__varSelf:
-            env.update(( (key, env.substitute(value, "environment::"+key))
-                         for key, value in i.items() ))
+            env.update(env.substituteCondDict(i, "environment"))
         states = { n : s.copy() for (n,s) in inputStates.items() }
 
         # update plugin states
@@ -2455,8 +2466,7 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None,
 
             if dep.envOverride:
                 thisDepEnv = thisDepEnv.derive(
-                    { key : env.substitute(value, "depends["+realName+"].environment["+key+"]")
-                      for key, value in dep.envOverride.items() })
+                    env.substituteCondDict(dep.envOverride, "depends["+realName+"].environment"))
 
             r = self.__recipeSet.getRecipe(recipeName)
             try:
@@ -2597,15 +2607,13 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None,
 
         # apply private environment
         for i in self.__varPrivate:
-            env = env.derive({ key : env.substitute(value, "privateEnvironment::"+key)
-                               for key, value in i.items() })
+            env.update(env.substituteCondDict(i, "privateEnvironment"))
 
         # meta variables override existing variables
         if self.__recipeSet.getPolicy('substituteMetaEnv'):
-            metaEnv = { key : env.substitute(value, "metaEnvironment::"+key)
-                for key, value in self.__metaEnv.items() }
+            metaEnv = env.substituteCondDict(self.__metaEnv, "metaEnvironment")
         else:
-            metaEnv = self.__metaEnv
+            metaEnv = { k : v[0] for k, v in self.__metaEnv.items() if env.evaluate(v[1], k) }
         env.update(metaEnv)
 
         # set fixed built-in variables
@@ -2711,10 +2719,7 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None,
             [CoreRef(buildCoreStep)], doFingerprint, toolDepPackage, toolDepPackageWeak)
 
         # provide environment
-        provideEnv = {}
-        for (key, value) in self.__provideVars.items():
-            provideEnv[key] = env.substitute(value, "provideVars::"+key)
-        packageCoreStep.providedEnv = provideEnv
+        packageCoreStep.providedEnv = env.substituteCondDict(self.__provideVars, "provideVars")
 
         # provide tools
         packageCoreStep.providedTools = { name : tool.prepare(packageCoreStep, env)
@@ -3305,7 +3310,7 @@ def removeWhiteList(x):
                 lambda x: updateDicRecursive(self.__commandConfig, x) if not self._ignoreCmdConfig else None
             ),
             "environment" : BuiltinSetting(
-                VarDefineValidator("environment"),
+                VarDefineValidator("environment", conditional=False),
                 lambda x: self.__defaultEnv.update(x)
             ),
             "fallbackMirror"        : BuiltinSetting(self.MIRRORS_SCHEMA, updateFallbackMirror, True),
diff --git a/pym/bob/stringparser.py b/pym/bob/stringparser.py
@@ -556,6 +556,14 @@ def evaluate(self, condition, prop):
         s = self.substitute(condition, "condition on "+prop)
         return not isFalse(s)
 
+    def substituteCondDict(self, values, prop, nounset=True):
+        try:
+            return { key : self.substitute(value, key, nounset)
+                     for key, (value, condition) in values.items()
+                     if self.evaluate(condition, key) }
+        except ParseError as e:
+            raise ParseError(f"{prop}: {e.slogan}")
+
     def touchReset(self):
         self.touched = self.touched + [ set() ]
 
diff --git a/test/unit/test_input_env.py b/test/unit/test_input_env.py
@@ -7,6 +7,7 @@
 import schema
 
 from bob.input import Env, VarDefineValidator
+from bob.errors import ParseError
 
 class TestEnv(TestCase):
 
@@ -66,18 +67,47 @@ def testTouch(self):
         e1.touch(['foo'])
         self.assertEqual(e1.touchedKeys(), set(['foo']))
 
+    def testSubstituteCondDictErrors(self):
+        e = Env({"A" : "a"})
+
+        self.assertEqual(e.substituteCondDict({ "X" : ("$A", None)}, "prop"),
+                         {"X" : "a"})
+
+        with self.assertRaises(ParseError) as exc:
+            e.substituteCondDict({ "X" : ("$B", None)}, "|prop|")
+        self.assertIn("|prop|", exc.exception.slogan)
+
+        self.assertEqual(e.substituteCondDict({ "X" : ("$B", None)}, "prop", nounset=False),
+                         {"X" : ""})
+
 
 class TestVarDefineValidator(TestCase):
     def setUp(self):
         self.v = VarDefineValidator("foo")
 
     def testValid(self):
-        self.assertEqual(self.v.validate({"FOO": "bar"}), {"FOO": "bar"})
+        self.assertEqual(self.v.validate({"FOO": "bar"}), {"FOO": ("bar", None)})
+        self.assertEqual(
+            self.v.validate({"FOO": {"value" : "bar"}}),
+            {"FOO": ("bar", None)})
+        self.assertEqual(
+            self.v.validate({"FOO": {"value" : "bar", "if" : "condition"}}),
+            {"FOO": ("bar", "condition")})
 
     def testWrongTypes(self):
         self.assertRaises(schema.SchemaError, self.v.validate, "boom")
         self.assertRaises(schema.SchemaError, self.v.validate, {1 : "bar"})
         self.assertRaises(schema.SchemaError, self.v.validate, {"foo" : True})
+        self.assertRaises(schema.SchemaError, self.v.validate,
+                          {"foo" : {}})
+        self.assertRaises(schema.SchemaError, self.v.validate,
+                          {"foo" : []})
+        self.assertRaises(schema.SchemaError, self.v.validate,
+                          {"foo" : {"value" : 1}})
+        self.assertRaises(schema.SchemaError, self.v.validate,
+                          {"foo" : {"value" : "bar", "if" : 1}})
+        self.assertRaises(schema.SchemaError, self.v.validate,
+                          {"foo" : {"value" : "bar", "wrong-key" : "baz"}})
 
     def testWrongNames(self):
         self.assertRaises(schema.SchemaError, self.v.validate, {"0abc" : "bar"})
diff --git a/test/unit/test_input_recipe.py b/test/unit/test_input_recipe.py
diff --git a/test/unit/test_input_recipeset.py b/test/unit/test_input_recipeset.py
