Skip to content

Commit

Permalink
properly convert env interactions into context (#271)
Browse files Browse the repository at this point in the history
* properly convert env interactions into context

This input:

```yaml
{% set a = environ["a"] %}
{% set b = environ.get("b", "0") %}

package:
  name: abc
  version: 0
```

on `main` emits this recipe that needs manual correction:

```yaml
schema_version: 1

context:
  a: "\"env.get(\"a\")\""
  b: "\"environ | get(\"b\", \"0\")\""

package:
  name: abc
  version: 0
```

but after this PR, it will instead emit:
```yaml
schema_version: 1

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

package:
  name: abc
  version: 0
```
which does not need any further editing.

* fix black formatting issues

* fix incorrect mypy typing

* add test case

* add tests, comments, consistency pass on env.get
  • Loading branch information
AaronOpfer authored Dec 16, 2024
1 parent b02a878 commit e2bf2cb
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 4 deletions.
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("):
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

0 comments on commit e2bf2cb

Please sign in to comment.