Merge pull request #585 from vshawrh/vshawfromager

fix: handle undefined variable in substitute_template

Author: mergify[bot]

src/fromager/packagesettings.py

@@ -457,11 +457,20 @@ def substitute_template(value: str, template_env: dict[str, str]) -> str:
     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)
+        default = modict["default"]
+        # Only set the default if one is explicitly provided.
+        # This ensures that undefined variables without defaults
+        # will raise KeyError later
+        if default is not None:
+            localdefault.setdefault(name, default)
+            # Replace ${var:-default} with ${var}
+            value = value.replace(mo.group(0), f"${{{name}}}")
+    try:
+        return string.Template(value).substitute(localdefault)
+    except KeyError as e:
+        raise ValueError(
+            f"Undefined environment variable {e!r} referenced in expression {value!r}"
+        ) from e
 
 
 def get_cpu_count() -> int:

tests/test_packagesettings.py

@@ -468,3 +468,14 @@ def test_parallel_jobs(
 )
 def test_substitute_template(value: str, template_env: dict[str, str], expected: str):
     assert substitute_template(value, template_env) == expected
+
+
+def test_substitute_template_key_error():
+    # This test expects a ValueError to be raised by substitute_template
+    with pytest.raises(ValueError) as excinfo:
+        substitute_template("${DEFAULT:-default} ${UNKNOWN}", {})
+    # Verify that the error message matches the expected message
+    assert (
+        str(excinfo.value)
+        == "Undefined environment variable KeyError('UNKNOWN') referenced in expression '${DEFAULT} ${UNKNOWN}'"
+    )