Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure variable finding works properly and add unit tests for variable finding #5535

Merged
merged 7 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion conda_build/variants.py
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,9 @@ def find_used_variables_in_text(variant, recipe_text, selectors_only=False):
continue
v_regex = re.escape(v)
v_req_regex = "[-_]".join(map(re.escape, v.split("_")))
variant_regex = rf"\{{\s*(?:pin_[a-z]+\(\s*?['\"])?{v_regex}[^'\"]*?\}}\}}"
variant_regex = (
rf"\{{\s*(?:pin_[a-z]+\(\s*?['\"])?{v_regex}[^_0-9a-zA-Z].*?\}}\}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of the change:

  • The previous regex was non-greedy-matching any characters after the variant name that were NOT a single or double quote. That meant that partial matches were possible (e.g. it found pari in pin_compatible('napari').
  • The new regex tries to non-greedy-match any character after the variant name that is NOT a valid variant name character (underscores, numbers and letters).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. We assert the variable name is terminated by a non-allowed character for variables. Then we get the rest of the characters up until the jinja2 statement is closed, asserting it is closed.

)
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
Expand Down
21 changes: 21 additions & 0 deletions news/5535-fix-variant-finding.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
### Enhancements

* <news item>

### Bug fixes

* Fixed a bug where variants were incorrectly found as being used when they matched a leading substring of
another variant. (#5535)
* Fixed a bug where variants were not found when variables were used in ``pin_*`` statements. (#5535)

### Deprecations

* <news item>

### Docs

* <news item>

### Other

* <news item>
63 changes: 63 additions & 0 deletions tests/test_variants.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
filter_combined_spec_to_used_keys,
find_used_variables_in_batch_script,
find_used_variables_in_shell_script,
find_used_variables_in_text,
get_package_variants,
get_vars,
validate_spec,
Expand Down Expand Up @@ -816,6 +817,68 @@ def test_get_vars():
assert get_vars(variants) == {"nodejs"}


@pytest.mark.parametrize(
"vars,text,found_vars",
[
# basic tests
(
("python", "python_min"),
"{{ python }}",
{"python"},
),
(
("python", "python_min"),
"{{ python_min }}",
{"python_min"},
),
# filters and other text
(
("python", "python_min"),
"python {{ python_min }}",
{"python_min"},
),
(
("python", "python_min"),
"python {{ python }}",
{"python"},
),
(
("python", "python_min"),
"python {{ python|lower }}",
{"python"},
),
(
("python", "python_min"),
"{{ python_min|lower }}",
{"python_min"},
),
# pin_* statements
(
("python", "python_min"),
"{{ pin_compatible('python') }}",
{"python"},
),
(
("python", "python_min"),
"{{ pin_compatible('python', max_pin='x.x') }}",
{"python"},
),
(
("python", "python_min"),
"{{ pin_compatible('python_min') }}",
{"python_min"},
),
(
("python", "python_min"),
"{{ pin_compatible('python_min', max_pin='x.x') }}",
{"python_min"},
),
],
)
def test_find_used_variables_in_text(vars, text, found_vars):
assert find_used_variables_in_text(vars, text) == found_vars


def test_find_used_variables_in_shell_script(tmp_path: Path) -> None:
variants = ("FOO", "BAR", "BAZ", "QUX")
(script := tmp_path / "script.sh").write_text(
Expand Down
Loading