Merge branch 'bugfix/fix_tree_processing_env_vars' into 'main'

Fix expansion of env vars

Closes PACMAN-357

See merge request espressif/idf-component-manager!128

.pre-commit-config.yaml

@@ -2,7 +2,7 @@
 # See https://pre-commit.com/hooks.html for more hooks
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.1.0
+    rev: v4.2.0
     hooks:
       - id: check-yaml
         exclude: tests
@@ -23,7 +23,7 @@ repos:
         exclude: ^tests|integration_tests.*$
         args: ["-s", "B101"] # disable B101: test for use of assert
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v0.931
+    rev: v0.960
     hooks:
       - id: mypy
         additional_dependencies: ['types-six', 'types-requests', 'types-PyYAML<5.3']
@@ -40,6 +40,6 @@ repos:
     hooks:
       - id: isort
   - repo: https://github.com/myint/eradicate/
-    rev: v2.0.0
+    rev: v2.1.0
     hooks:
       - id: eradicate

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix expansion of environment variables in manifest for `rules`
+
 ## [1.1.0] 2022-05-19
 
 ### Added

idf_component_tools/manifest/env_expander.py

@@ -0,0 +1,40 @@
+import os
+from string import Template
+
+from ..errors import ManifestError
+
+try:
+    from typing import Any
+except ImportError:
+    pass
+
+
+def _expand_env_vars_in_str(s, env):  # type: (str, dict[str, Any]) -> str
+    try:
+        return Template(s).substitute(env)
+    except KeyError as e:
+        raise ManifestError(
+            'Using environment variable "{}" in the manifest file but not specifying it'.format(e.args[0]))
+
+
+def expand_env_vars(
+        obj,  # type: dict[str, Any] | list | str | Any
+        env=None  # type: dict | None
+):
+    # type: (...) -> dict[str, Any] | list | str | Any
+    '''
+    Expand variables in the results of YAML/JSON file parsing
+    '''
+    if env is None:
+        env = dict(os.environ)
+
+    if isinstance(obj, dict):
+        return {k: expand_env_vars(v, env) for k, v in obj.items()}
+    elif isinstance(obj, str):
+        return _expand_env_vars_in_str(obj, env)
+    elif isinstance(obj, list):
+        # yaml dict won't have other iterable data types like set or tuple
+        return [expand_env_vars(i, env) for i in obj]
+
+    # we don't process other data types, like numbers
+    return obj

idf_component_tools/manifest/manager.py

@@ -1,11 +1,11 @@
 import os
 from io import open
-from string import Template
 
 import yaml
 
 from ..errors import ManifestError
 from .constants import MANIFEST_FILENAME
+from .env_expander import expand_env_vars
 from .manifest import Manifest
 from .validator import ManifestValidator
 
@@ -17,31 +17,6 @@
 EMPTY_MANIFEST = dict()  # type: Dict[str, Any]
 
 
-def _convert_env_vars_in_str(s):  # type: (str) -> str
-    try:
-        return Template(s).substitute(os.environ)
-    except KeyError as e:
-        raise ManifestError(
-            'Using environment variable "{}" in the manifest file but not specifying it'.format(e.args[0]))
-
-
-def _convert_env_vars_in_yaml_dict(d):  # type: (dict[str, Any]) -> dict[str, Any]
-    if not isinstance(d, dict):
-        return d
-
-    for k, v in d.items():
-        # we only support env var in values.
-        if isinstance(v, dict):
-            d[k] = _convert_env_vars_in_yaml_dict(v)
-        elif isinstance(v, str):
-            d[k] = _convert_env_vars_in_str(v)
-        elif isinstance(v, list):  # yaml dict won't have other iterable data types like set or tuple
-            d[k] = [_convert_env_vars_in_str(i) for i in v]
-        # we don't care other data types, like numbers
-
-    return d
-
-
 class ManifestManager(object):
     """Parser for manifest files in the project"""
     def __init__(
@@ -116,10 +91,12 @@ def parse_manifest_file(self):  # type: () -> Dict
                 if manifest_data is None:
                     manifest_data = EMPTY_MANIFEST
 
-                if not isinstance(manifest_data, dict):
+                expanded_manifest_data = expand_env_vars(manifest_data)
+
+                if not isinstance(expanded_manifest_data, dict):
                     raise ManifestError('Unknown format of the manifest file: {}'.format(self._path))
 
-                return _convert_env_vars_in_yaml_dict(manifest_data)
+                return expanded_manifest_data
 
             except yaml.YAMLError:
                 raise ManifestError(

tests/manifest/test_env_expander.py

@@ -0,0 +1,48 @@
+import pytest
+
+from idf_component_tools.manifest.env_expander import expand_env_vars
+
+TEST_ENVIRON = {
+    'A': '',
+    'B': 'b',
+}
+
+
+@pytest.mark.parametrize(
+    'inp,env,exp', [
+        (
+            {
+                'a': 1,
+                'b': None,
+                'c': '$A${B}C'
+            },
+            TEST_ENVIRON,
+            {
+                'a': 1,
+                'b': None,
+                'c': 'bC'
+            },
+        ), (
+            {
+                'a': ['1', '2', '3'],
+            },
+            {},
+            {
+                'a': ['1', '2', '3'],
+            },
+        ), (
+            {
+                'a': [{
+                    'b': '$B'
+                }],
+            },
+            TEST_ENVIRON,
+            {
+                'a': [{
+                    'b': 'b'
+                }],
+            },
+        )
+    ])
+def test_env_expander(inp, env, exp):
+    assert expand_env_vars(inp, env) == exp

tests/manifest/test_manifest.py

@@ -322,6 +322,7 @@ def test_known_targets_env(self, monkeypatch):
         assert 'test' in result
 
     def test_known_targets_idf(self, monkeypatch):
+        monkeypatch.delenv('IDF_COMPONENT_MANAGER_KNOWN_TARGETS', raising=False)
         monkeypatch.setenv(
             'IDF_PATH', os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', 'fixtures', 'fake_idf'))
         result = known_targets()
@@ -330,6 +331,8 @@ def test_known_targets_idf(self, monkeypatch):
         assert 'test' in result
 
     def test_known_targets_default(self, monkeypatch):
+        monkeypatch.delenv('IDF_COMPONENT_MANAGER_KNOWN_TARGETS', raising=False)
+        monkeypatch.delenv('IDF_PATH', raising=False)
         result = known_targets()
 
         assert result == DEFAULT_KNOWN_TARGETS