fix: make sure to catch jinja2 vars used in single-line for and `se…

…t` statements (#5447) * fix: make sure to catch jinja2 vars used in for statements and set statements * doc: add news * style: ruffen * test: added test for jinja2 w/ variants * fix: wrong section name, test makes more sense now * doc: clarify that this only covers single line statements * doc: add comment on jinja2 regex * fix: typos * doc: fix typo again * Apply suggestions from code review --------- Co-authored-by: jaimergp <[email protected]>
conda · Aug 7, 2024 · 242bbce · 242bbce
1 parent 977e609
commit 242bbce
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 1 deletion.
diff --git a/conda_build/variants.py b/conda_build/variants.py
@@ -745,15 +745,31 @@ def find_used_variables_in_text(variant, recipe_text, selectors_only=False):
         v_req_regex = "[-_]".join(map(re.escape, v.split("_")))
         variant_regex = rf"\{{\s*(?:pin_[a-z]+\(\s*?['\"])?{v_regex}[^'\"]*?\}}\}}"
         selector_regex = rf"^[^#\[]*?\#?\s\[[^\]]*?(?<![_\w\d]){v_regex}[=\s<>!\]]"
+        # NOTE: why use a regex instead of the jinja2 parser/AST?
+        # One can ask the jinja2 parser for undefined variables, but conda-build moves whole
+        # blocks of text around when searching for variables and applies selectors to the text.
+        # So the text that reaches this function is not necessarily valid jinja2 syntax. :/
         conditional_regex = (
             r"(?:^|[^\{])\{%\s*(?:el)?if\s*.*" + v_regex + r"\s*(?:[^%]*?)?%\}"
         )
+        # TODO: this `for` regex won't catch some common cases like lists of vars, multiline
+        # jinja2 blocks, if filters on the for loop, etc.
+        for_regex = r"(?:^|[^\{])\{%\s*for\s*.*\s*in\s*" + v_regex + r"(?:[^%]*?)?%\}"
+        set_regex = r"(?:^|[^\{])\{%\s*set\s*.*\s*=\s*.*" + v_regex + r"(?:[^%]*?)?%\}"
         # plain req name, no version spec.  Look for end of line after name, or comment or selector
         requirement_regex = rf"^\s+\-\s+{v_req_regex}\s*(?:\s[\[#]|$)"
         if selectors_only:
             all_res.insert(0, selector_regex)
         else:
-            all_res.extend([variant_regex, requirement_regex, conditional_regex])
+            all_res.extend(
+                [
+                    variant_regex,
+                    requirement_regex,
+                    conditional_regex,
+                    for_regex,
+                    set_regex,
+                ]
+            )
         # consolidate all re's into one big one for speedup
         all_res = r"|".join(all_res)
         if any(re.search(all_res, line) for line in variant_lines):

diff --git a/news/5447-jinja2-for-set-vars b/news/5447-jinja2-for-set-vars
@@ -0,0 +1,20 @@
+### Enhancements
+
+* <news item>
+
+### Bug fixes
+
+* Variables used in single-line jinja2 `for` and `set` statements are now properly included in the variant
+  matrix for some edge cases. (#5447)
+
+### Deprecations
+
+* <news item>
+
+### Docs
+
+* <news item>
+
+### Other
+
+* <news item>
diff --git a/tests/test-recipes/variants/jinja2_used_variables/conda_build_config.yaml b/tests/test-recipes/variants/jinja2_used_variables/conda_build_config.yaml
@@ -0,0 +1,27 @@
+CLANG_VERSION:
+  - 16.0.6
+  - 17.0.6
+  - 18.1.8
+  - 19.1.0.rc1
+
+VCVER:
+  - 14.3
+  - 14.2
+CL_VERSION:
+  - 19.40.33808
+  - 19.29.30139
+
+BLAH:
+  - a
+  - b
+
+FOO:
+  - cdf
+
+FOOBAR:
+  - hgf
+
+zip_keys:
+  -
+    - VCVER
+    - CL_VERSION
diff --git a/tests/test-recipes/variants/jinja2_used_variables/meta.yaml b/tests/test-recipes/variants/jinja2_used_variables/meta.yaml
@@ -0,0 +1,42 @@
+{% if CLANG_VERSION is not defined %}
+{% set CLANG_VERSION = "16.0.6" %}
+{% set CL_VERSION = "19.29" %}
+{% set VCVER = "" %}
+{% set FOO = "" %}
+{% set FOOBAR = "" %}
+{% endif %}
+{% set clang_major = CLANG_VERSION.split(".")[0] %}
+{% set cl_minor = CL_VERSION.split(".")[1] %}
+{% set vc_major = VCVER.split(".")[0] %}
+
+package:
+  name: clang-win-activation
+  version: {{ CLANG_VERSION }}
+
+build:
+  number: 0
+  {% if clang_major|int == 16 and cl_minor|int >= 40 %}
+  skip: true
+  {% endif %}
+
+outputs:
+  - name: clang_win-64
+    build:
+      run_exports:
+        strong:
+          - vc >={{ VCVER }}
+    requirements:
+      run:
+        - clang {{ CLANG_VERSION }}.*
+
+    test:
+      commands:
+        {% for var in FOO.split() %}
+        - echo {{ var }}
+        {% endfor %}
+
+test:
+  commands:
+    {% for var in FOOBAR.split() %}
+    - echo {{ var }}
+    {% endfor %}
diff --git a/tests/test_variants.py b/tests/test_variants.py
@@ -426,6 +426,37 @@ def test_get_used_loop_vars():
     }
 
 
+def test_get_used_loop_vars_jinja2():
+    metadata = api.render(
+        os.path.join(variants_dir, "jinja2_used_variables"),
+        finalize=False,
+        bypass_env_check=True,
+    )
+    # 4 CLANG_VERSION values x 2 VCVER values - one skipped because of jinja2 conditionals
+    assert len(metadata) == 7
+    for m, _, _ in metadata:
+        assert m.get_used_loop_vars(force_top_level=False) == {"CLANG_VERSION", "VCVER"}
+        assert m.get_used_loop_vars(force_top_level=True) == {
+            "CL_VERSION",
+            "CLANG_VERSION",
+            "VCVER",
+        }
+        assert m.get_used_vars(force_top_level=False) == {
+            "CLANG_VERSION",
+            "VCVER",
+            "FOO",
+            "target_platform",
+        }
+        assert m.get_used_vars(force_top_level=True) == {
+            "CLANG_VERSION",
+            "CL_VERSION",
+            "VCVER",
+            "FOO",
+            "FOOBAR",
+            "target_platform",
+        }
+
+
 def test_reprovisioning_source():
     api.render(os.path.join(variants_dir, "20_reprovision_source"))