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

properly convert env interactions into context #271

Merged
merged 5 commits into from
Dec 16, 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
7 changes: 7 additions & 0 deletions conda_recipe_manager/parser/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,13 @@ class Regex:
# Finds `environ[]` used by a some recipe files. Requires a whitespace character to prevent matches with
# `os.environ[]`, which is very rare.
PRE_PROCESS_ENVIRON: Final[re.Pattern[str]] = re.compile(r"\s+environ\[(\"|')(.*)(\"|')\]")

# Finds `environ.get` which is used in a closed source community of which the author of this comment
# participates in. See Issue #271.
PRE_PROCESS_ENVIRON_GET: Final[re.Pattern[str]] = re.compile(
r"\s+environ \| get\((\"|')(.*)(\"|'), *(\"|')(.*)(\"|')\)"
)

# Finds commonly used variants of `{{ hash_type }}:` which is a substitution for the `sha256` field
PRE_PROCESS_JINJA_HASH_TYPE_KEY: Final[re.Pattern[str]] = re.compile(
r"'{0,1}\{\{ (hash_type|hash|hashtype) \}\}'{0,1}:"
Expand Down
19 changes: 17 additions & 2 deletions conda_recipe_manager/parser/recipe_parser_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ def _upgrade_jinja_to_context_obj(self) -> None:
self._msg_tbl.add_message(MessageCategory.WARNING, f"The variable `{name}` is an unsupported type.")
continue
# Function calls need to preserve JINJA escaping or else they turn into unevaluated strings.
if isinstance(value, str) and search_any_regex(Regex.JINJA_FUNCTIONS_SET, value):
# See issue #271 for details about env.get( string here.
if isinstance(value, str) and (
search_any_regex(Regex.JINJA_FUNCTIONS_SET, value) or value.startswith("env.get(")
):
value = "{{" + value + "}}"
context_obj[name] = value
# Ensure that we do not include an empty context object (which is forbidden by the schema).
Expand Down Expand Up @@ -728,7 +731,7 @@ def pre_process_recipe_text(content: str) -> str:
# - This is mostly used by Bioconda recipes and R-based-packages in the `license_file` field.
# - From our search, it looks like we never deal with more than one set of outer quotes within the brackets
replacements: list[tuple[str, str]] = []
for groups in cast(list[str], Regex.PRE_PROCESS_ENVIRON.findall(content)):
for groups in cast(list[tuple[str, ...]], Regex.PRE_PROCESS_ENVIRON.findall(content)):
# Each match should return ["<quote char>", "<key>", "<quote_char>"]
quote_char = groups[0]
key = groups[1]
Expand All @@ -738,6 +741,18 @@ def pre_process_recipe_text(content: str) -> str:
f"env.get({quote_char}{key}{quote_char})",
)
)

for groups in cast(list[tuple[str, ...]], Regex.PRE_PROCESS_ENVIRON_GET.findall(content)):
environ_key = f"{groups[0]}{groups[1]}{groups[2]}"
environ_default = f"{groups[3]}{groups[4]}{groups[5]}"

replacements.append(
(
f"environ | get({environ_key}, {environ_default})",
f"env.get({environ_key}, default={environ_default})",
)
)

for old, new in replacements:
content = content.replace(old, new, 1)

Expand Down
4 changes: 2 additions & 2 deletions conda_recipe_manager/parser/recipe_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -703,8 +703,8 @@ def render(self) -> str:
# `/context`.
if self._schema_version == SchemaVersion.V0:
for key, val in self._vars_tbl.items():
# Double quote strings
if isinstance(val, str):
# Double quote strings, except for when we detect a env.get() expression. See issue #271.
if isinstance(val, str) and not val.startswith("env.get("):
AaronOpfer marked this conversation as resolved.
Show resolved Hide resolved
val = f'"{val}"'
lines.append(f"{{% set {key} = {val} %}}")
# Add spacing if variables have been set
Expand Down
8 changes: 8 additions & 0 deletions tests/parser/test_recipe_parser_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
("dot_function_replacement.yaml", "pre_processor/pp_dot_function_replacement.yaml"),
# Upgrading multiline quoted strings
("quoted_multiline_str.yaml", "pre_processor/pp_quoted_multiline_str.yaml"),
# Issue #271 environ.get() conversions
("unprocessed_environ_get.yaml", "pre_processor/pp_environ_get.yaml"),
# Unchanged file
("simple-recipe.yaml", "simple-recipe.yaml"),
],
Expand Down Expand Up @@ -173,6 +175,12 @@ def test_pre_process_recipe_text(input_file: str, expected_file: str) -> None:
"Field at `/about/license_family` is no longer supported.",
],
),
# Issue #271 properly elevate environ.get() into context
(
"environ_get.yaml",
[],
[],
),
# TODO complete: The `rust.yaml` test contains many edge cases and selectors that aren't directly supported in
# the V1 recipe format
# (
Expand Down
6 changes: 6 additions & 0 deletions tests/test_aux_files/environ_get.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{% set a = env.get("a") %}
{% set b = env.get("b", default="0") %}

package:
name: abc
version: 0
6 changes: 6 additions & 0 deletions tests/test_aux_files/pre_processor/pp_environ_get.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{% set a = env.get("a") %}
{% set b = env.get("b", default="0") %}

package:
name: abc
version: 0
6 changes: 6 additions & 0 deletions tests/test_aux_files/unprocessed_environ_get.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{% set a = environ["a"] %}
{% set b = environ.get("b", "0") %}

package:
name: abc
version: 0
9 changes: 9 additions & 0 deletions tests/test_aux_files/v1_format/v1_environ_get.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
schema_version: 1

context:
a: ${{env.get("a")}}
b: ${{env.get("b", default="0")}}

package:
name: abc
version: 0
Loading