Merge pull request #479 from tiran/env-default

Add support for `${NAME:-somedefault}` syntax

docs/customization.md

@@ -137,16 +137,24 @@ Settings common to all variants of a given package can be placed in the the
 top-level `env` mapping. Variant env vars override global env vars.
 
 Environment files support simple parameter expansions `$NAME` and
-`${NAME}`. Values are taken from previous lines, then global env map, and
-finally process environment. Sub shell expression `$(cmd)` and extended
-parameter expansions like `${NAME:-default}` are not implemented. A literal
-`$` must be quoted as `$$`.
+`${NAME}` as well as default values `${NAME:-}` (empty string) and
+`${NAME:-somedefault}`. Values are taken from previous lines, then global env
+map, and finally process environment. Sub shell expression `$(cmd)` and
+extended parameter expansions like `${NAME:+alternative}` are not
+implemented. A literal `$` must be quoted as `$$`.
+
+```{versionchanged} 0.32.0
+
+Added support for default value syntax `${NAME:-}`.
+```
 
 ```yaml
 # example
 env:
     # pre-pend '/global/bin' to PATH
     PATH: "/global/bin:$PATH"
+    # default CFLAGS to empty string and append " -g"
+    CFLAGS: "${CFLAGS:-} -g"
 variants:
     cpu:
         env:

src/fromager/packagesettings.py

@@ -1,6 +1,7 @@
 import logging
 import os
 import pathlib
+import re
 import string
 import types
 import typing
@@ -420,6 +421,28 @@ def _resolve_template(
         raise
 
 
+_DEFAULT_PATTERN_RE = re.compile(
+    r"(?<!\$)"  # not preceeded by a second '$'
+    r"\$\{(?P<name>[a-z0-9_]+)"  # '${name'
+    r"(:-(?P<default>[^\}:]*))?"  # optional ':-default', capture value
+    r"\}",  # closing '}'
+    flags=re.ASCII | re.IGNORECASE,
+)
+
+
+def substitute_template(value: str, template_env: dict[str, str]) -> str:
+    """Substitute ${var} and ${var:-default} in value string"""
+    localdefault = template_env.copy()
+    for mo in _DEFAULT_PATTERN_RE.finditer(value):
+        modict = mo.groupdict()
+        name = modict["name"]
+        # add to local default, keep existing default
+        localdefault.setdefault(name, modict["default"])
+        # remove ":-default"
+        value = value.replace(mo.group(0), f"${{{name}}}")
+    return string.Template(value).substitute(localdefault)
+
+
 def get_cpu_count() -> int:
     """CPU count from scheduler affinity"""
     if hasattr(os, "sched_getaffinity"):
@@ -627,7 +650,7 @@ def get_extra_environ(
             entries.extend(vi.env.items())
 
         for key, value in entries:
-            value = string.Template(value).substitute(template_env)
+            value = substitute_template(value, template_env)
             extra_environ[key] = value
             # subsequent key-value pairs can depend on previously vars.
             template_env[key] = value

tests/test_packagesettings.py

@@ -15,6 +15,7 @@
     PackageSettings,
     SettingsFile,
     Variant,
+    substitute_template,
 )
 
 TEST_PKG = "test-pkg"
@@ -41,6 +42,7 @@
         "EGG_AGAIN": "$EGG",
         "SPAM": "alot $EXTRA",
         "QUOTES": "A\"BC'$$EGG",
+        "DEF": "${DEF:-default}",
     },
     "name": "test-pkg",
     "has_config": True,
@@ -149,6 +151,18 @@ def test_pbi_test_pkg_extra_environ(
             "EGG_AGAIN": "spam spam",
             "QUOTES": "A\"BC'$EGG",  # $$EGG is transformed into $EGG
             "SPAM": "alot extra",
+            "DEF": "default",
+        }
+        | parallel
+    )
+    assert (
+        pbi.get_extra_environ(template_env={"EXTRA": "extra", "DEF": "nondefault"})
+        == {
+            "EGG": "spam spam",
+            "EGG_AGAIN": "spam spam",
+            "QUOTES": "A\"BC'$EGG",  # $$EGG is transformed into $EGG
+            "SPAM": "alot extra",
+            "DEF": "nondefault",
         }
         | parallel
     )
@@ -162,6 +176,7 @@ def test_pbi_test_pkg_extra_environ(
             "EGG_AGAIN": "spam",
             "QUOTES": "A\"BC'$EGG",
             "SPAM": "",
+            "DEF": "default",
         }
         | parallel
     )
@@ -175,6 +190,7 @@ def test_pbi_test_pkg_extra_environ(
             "EGG_AGAIN": "spam",
             "QUOTES": "A\"BC'$EGG",
             "SPAM": "alot spam",
+            "DEF": "default",
         }
         | parallel
     )
@@ -192,6 +208,7 @@ def test_pbi_test_pkg_extra_environ(
             "EGG_AGAIN": "spam",
             "QUOTES": "A\"BC'$EGG",
             "SPAM": "alot spam",
+            "DEF": "default",
             "PATH": f"{build_env.path / 'bin'}:/sbin:/bin",
             "VIRTUAL_ENV": str(build_env.path),
         }
@@ -404,3 +421,19 @@ def test_parallel_jobs(
     testdata_context.settings.max_jobs = 4
     pbi = testdata_context.settings.package_build_info(TEST_PKG)
     assert pbi.parallel_jobs() == 4
+
+
+@pytest.mark.parametrize(
+    "value,template_env,expected",
+    [
+        ("", {}, ""),
+        ("${var}", {"var": "value"}, "value"),
+        ("$${var}", {"var": "value"}, "${var}"),
+        ("${var:-}", {}, ""),
+        ("${var:-default}", {}, "default"),
+        ("${var:-default}", {"var": "value"}, "value"),
+        ("$${var:-default}", {}, "${var:-default}"),
+    ],
+)
+def test_substitute_template(value: str, template_env: dict[str, str], expected: str):
+    assert substitute_template(value, template_env) == expected

tests/testdata/context/overrides/settings/test_pkg.yaml

@@ -14,6 +14,7 @@ env:
     EGG_AGAIN: "$EGG"
     SPAM: "alot $EXTRA"
     QUOTES: "A\"BC'$$EGG"
+    DEF: "${DEF:-default}"
 download_source:
     url: https://egg.test/${canonicalized_name}/v${version}.tar.gz
     destination_filename: ${canonicalized_name}-${version}.tar.gz