From 861811372c189549e8faeaf695506526ef93f25b Mon Sep 17 00:00:00 2001 From: beckermr Date: Tue, 6 Aug 2024 10:36:41 -0500 Subject: [PATCH 01/10] fix: make sure to catch jinja2 vars used in for statements and set statements --- conda_build/variants.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/conda_build/variants.py b/conda_build/variants.py index b185a7eb34..86a1897a43 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -748,12 +748,18 @@ def find_used_variables_in_text(variant, recipe_text, selectors_only=False): conditional_regex = ( r"(?:^|[^\{])\{%\s*(?:el)?if\s*.*" + v_regex + r"\s*(?:[^%]*?)?%\}" ) + 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): From ffa42129a5c81bc88b94e1f66aea4230c97a1615 Mon Sep 17 00:00:00 2001 From: beckermr Date: Tue, 6 Aug 2024 10:40:14 -0500 Subject: [PATCH 02/10] doc: add news --- news/5447-jinja2-for-set-vars | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 news/5447-jinja2-for-set-vars diff --git a/news/5447-jinja2-for-set-vars b/news/5447-jinja2-for-set-vars new file mode 100644 index 0000000000..8865fe397e --- /dev/null +++ b/news/5447-jinja2-for-set-vars @@ -0,0 +1,20 @@ +### Enhancements + +* + +### Bug fixes + +* Variables used in jinja2 ``for`` and ``set`` statements are now properly included in the variant + matrix for some edge cases. (#5447) + +### Deprecations + +* + +### Docs + +* + +### Other + +* From f24b4090bbe8c1c3f7e1cc3e08c65a13181c1374 Mon Sep 17 00:00:00 2001 From: beckermr Date: Tue, 6 Aug 2024 10:41:12 -0500 Subject: [PATCH 03/10] style: ruffen --- conda_build/variants.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/conda_build/variants.py b/conda_build/variants.py index 86a1897a43..705c859c25 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -748,18 +748,22 @@ def find_used_variables_in_text(variant, recipe_text, selectors_only=False): conditional_regex = ( r"(?:^|[^\{])\{%\s*(?:el)?if\s*.*" + v_regex + r"\s*(?:[^%]*?)?%\}" ) - for_regex = ( - r"(?:^|[^\{])\{%\s*for\s*.*\s*in\s*" + v_regex + r"(?:[^%]*?)?%\}" - ) - set_regex = ( - r"(?:^|[^\{])\{%\s*set\s*.*\s*=\s*.*" + v_regex + r"(?:[^%]*?)?%\}" - ) + 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, for_regex, set_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): From cb51abb8137ab1d929139b7131c5c138231ab5b9 Mon Sep 17 00:00:00 2001 From: beckermr Date: Tue, 6 Aug 2024 14:17:45 -0500 Subject: [PATCH 04/10] test: added test for jinja2 w/ variants --- .../conda_build_config.yaml | 27 ++++++++++++ .../variants/jinja2_used_variables/meta.yaml | 42 +++++++++++++++++++ tests/test_variants.py | 30 +++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 tests/test-recipes/variants/jinja2_used_variables/conda_build_config.yaml create mode 100644 tests/test-recipes/variants/jinja2_used_variables/meta.yaml 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 new file mode 100644 index 0000000000..c5920d67c4 --- /dev/null +++ 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 new file mode 100644 index 0000000000..cb1280b9a0 --- /dev/null +++ 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 }}.* + + tests: + commands: + {% for var in FOO.split() %} + - echo {{ var }} + {% endfor %} + +tests: + commands: + {% for var in FOOBAR.split() %} + - echo {{ var }} + {% endfor %} diff --git a/tests/test_variants.py b/tests/test_variants.py index 3c79e36e16..bcaab919cd 100644 --- a/tests/test_variants.py +++ b/tests/test_variants.py @@ -426,6 +426,36 @@ 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", + "target_platform", + } + + def test_reprovisioning_source(): api.render(os.path.join(variants_dir, "20_reprovision_source")) From b5bb1bae68eeb2ef54157559b00a7e3388f6f0f3 Mon Sep 17 00:00:00 2001 From: beckermr Date: Tue, 6 Aug 2024 14:21:03 -0500 Subject: [PATCH 05/10] fix: wrong section name, test makes more sense now --- tests/test-recipes/variants/jinja2_used_variables/meta.yaml | 4 ++-- tests/test_variants.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test-recipes/variants/jinja2_used_variables/meta.yaml b/tests/test-recipes/variants/jinja2_used_variables/meta.yaml index cb1280b9a0..88c591b8b7 100644 --- a/tests/test-recipes/variants/jinja2_used_variables/meta.yaml +++ b/tests/test-recipes/variants/jinja2_used_variables/meta.yaml @@ -29,13 +29,13 @@ outputs: run: - clang {{ CLANG_VERSION }}.* - tests: + test: commands: {% for var in FOO.split() %} - echo {{ var }} {% endfor %} -tests: +test: commands: {% for var in FOOBAR.split() %} - echo {{ var }} diff --git a/tests/test_variants.py b/tests/test_variants.py index bcaab919cd..84c1d96404 100644 --- a/tests/test_variants.py +++ b/tests/test_variants.py @@ -452,6 +452,7 @@ def test_get_used_loop_vars_jinja2(): "CL_VERSION", "VCVER", "FOO", + "FOOBAR", "target_platform", } From 6ce0ab08c59acca37dc2d180ae41e942591418d0 Mon Sep 17 00:00:00 2001 From: "Matthew R. Becker" Date: Tue, 6 Aug 2024 14:54:36 -0500 Subject: [PATCH 06/10] doc: clarify that this only covers single line statements --- news/5447-jinja2-for-set-vars | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/5447-jinja2-for-set-vars b/news/5447-jinja2-for-set-vars index 8865fe397e..421e0aa975 100644 --- a/news/5447-jinja2-for-set-vars +++ b/news/5447-jinja2-for-set-vars @@ -4,7 +4,7 @@ ### Bug fixes -* Variables used in jinja2 ``for`` and ``set`` statements are now properly included in the variant +* Variables used in single-line jinja2 ``for`` and ``set`` statements are now properly included in the variant matrix for some edge cases. (#5447) ### Deprecations From cbdb5bea4bb0902ed24b69f90b9aec1dd2184e7a Mon Sep 17 00:00:00 2001 From: beckermr Date: Tue, 6 Aug 2024 16:09:49 -0500 Subject: [PATCH 07/10] doc: add comment on jinja2 regex --- conda_build/variants.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/conda_build/variants.py b/conda_build/variants.py index 705c859c25..819d108675 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -745,9 +745,15 @@ 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\[[^\]]*?(?!\]]" + # NOTE: why use a regex instead of the jinja2 parser/AST? + # Once can ask the jinj2 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 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 pof vars or multiline + # jinja2 blocks. 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 From fdcffef3202d90406ed7f35b43f2a84ea4df5a7e Mon Sep 17 00:00:00 2001 From: beckermr Date: Tue, 6 Aug 2024 16:11:38 -0500 Subject: [PATCH 08/10] fix: typos --- conda_build/variants.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/conda_build/variants.py b/conda_build/variants.py index 819d108675..15517f0244 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -746,14 +746,14 @@ def find_used_variables_in_text(variant, recipe_text, selectors_only=False): variant_regex = rf"\{{\s*(?:pin_[a-z]+\(\s*?['\"])?{v_regex}[^'\"]*?\}}\}}" selector_regex = rf"^[^#\[]*?\#?\s\[[^\]]*?(?!\]]" # NOTE: why use a regex instead of the jinja2 parser/AST? - # Once can ask the jinj2 parser for undefined variables, but conda-build moves whole + # One can ask the jinj2 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 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 pof vars or multiline - # jinja2 blocks. + # 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 From ef31c92d136a576bde6d4fe22e4af397b0f52888 Mon Sep 17 00:00:00 2001 From: "Matthew R. Becker" Date: Tue, 6 Aug 2024 16:12:31 -0500 Subject: [PATCH 09/10] doc: fix typo again --- conda_build/variants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda_build/variants.py b/conda_build/variants.py index 15517f0244..dac31382d8 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -748,7 +748,7 @@ def find_used_variables_in_text(variant, recipe_text, selectors_only=False): # NOTE: why use a regex instead of the jinja2 parser/AST? # One can ask the jinj2 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 reaches this function is not necessarily valid jinja2 syntax. :/ + # So the text that reaches this function is not necessarily valid jinja2 syntax. :/ conditional_regex = ( r"(?:^|[^\{])\{%\s*(?:el)?if\s*.*" + v_regex + r"\s*(?:[^%]*?)?%\}" ) From ac0ab32a33a2c516a3771c7f550c17d2e208b1ac Mon Sep 17 00:00:00 2001 From: jaimergp Date: Wed, 7 Aug 2024 09:56:44 +0200 Subject: [PATCH 10/10] Apply suggestions from code review --- conda_build/variants.py | 2 +- news/5447-jinja2-for-set-vars | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conda_build/variants.py b/conda_build/variants.py index dac31382d8..3eef82266d 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -746,7 +746,7 @@ def find_used_variables_in_text(variant, recipe_text, selectors_only=False): variant_regex = rf"\{{\s*(?:pin_[a-z]+\(\s*?['\"])?{v_regex}[^'\"]*?\}}\}}" selector_regex = rf"^[^#\[]*?\#?\s\[[^\]]*?(?!\]]" # NOTE: why use a regex instead of the jinja2 parser/AST? - # One can ask the jinj2 parser for undefined variables, but conda-build moves whole + # 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 = ( diff --git a/news/5447-jinja2-for-set-vars b/news/5447-jinja2-for-set-vars index 421e0aa975..fbca651f89 100644 --- a/news/5447-jinja2-for-set-vars +++ b/news/5447-jinja2-for-set-vars @@ -4,7 +4,7 @@ ### Bug fixes -* Variables used in single-line jinja2 ``for`` and ``set`` statements are now properly included in the variant +* Variables used in single-line jinja2 `for` and `set` statements are now properly included in the variant matrix for some edge cases. (#5447) ### Deprecations