From e2bf2cbf67492fc1b766a2e2360488f4a2dc533b Mon Sep 17 00:00:00 2001 From: Aaron Opfer Date: Mon, 16 Dec 2024 11:17:20 -0600 Subject: [PATCH] properly convert env interactions into context (#271) * 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 --- conda_recipe_manager/parser/_types.py | 7 +++++++ .../parser/recipe_parser_convert.py | 19 +++++++++++++++++-- conda_recipe_manager/parser/recipe_reader.py | 4 ++-- tests/parser/test_recipe_parser_convert.py | 8 ++++++++ tests/test_aux_files/environ_get.yaml | 6 ++++++ .../pre_processor/pp_environ_get.yaml | 6 ++++++ .../unprocessed_environ_get.yaml | 6 ++++++ .../v1_format/v1_environ_get.yaml | 9 +++++++++ 8 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 tests/test_aux_files/environ_get.yaml create mode 100644 tests/test_aux_files/pre_processor/pp_environ_get.yaml create mode 100644 tests/test_aux_files/unprocessed_environ_get.yaml create mode 100644 tests/test_aux_files/v1_format/v1_environ_get.yaml diff --git a/conda_recipe_manager/parser/_types.py b/conda_recipe_manager/parser/_types.py index 83790762..94a21964 100644 --- a/conda_recipe_manager/parser/_types.py +++ b/conda_recipe_manager/parser/_types.py @@ -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}:" diff --git a/conda_recipe_manager/parser/recipe_parser_convert.py b/conda_recipe_manager/parser/recipe_parser_convert.py index 14cfc508..9bf835d1 100644 --- a/conda_recipe_manager/parser/recipe_parser_convert.py +++ b/conda_recipe_manager/parser/recipe_parser_convert.py @@ -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). @@ -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 = groups[0] key = groups[1] @@ -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) diff --git a/conda_recipe_manager/parser/recipe_reader.py b/conda_recipe_manager/parser/recipe_reader.py index 782de49b..800eb0ba 100644 --- a/conda_recipe_manager/parser/recipe_reader.py +++ b/conda_recipe_manager/parser/recipe_reader.py @@ -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 diff --git a/tests/parser/test_recipe_parser_convert.py b/tests/parser/test_recipe_parser_convert.py index 6278d67d..bf4e767d 100644 --- a/tests/parser/test_recipe_parser_convert.py +++ b/tests/parser/test_recipe_parser_convert.py @@ -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"), ], @@ -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 # ( diff --git a/tests/test_aux_files/environ_get.yaml b/tests/test_aux_files/environ_get.yaml new file mode 100644 index 00000000..e28eb412 --- /dev/null +++ b/tests/test_aux_files/environ_get.yaml @@ -0,0 +1,6 @@ +{% set a = env.get("a") %} +{% set b = env.get("b", default="0") %} + +package: + name: abc + version: 0 diff --git a/tests/test_aux_files/pre_processor/pp_environ_get.yaml b/tests/test_aux_files/pre_processor/pp_environ_get.yaml new file mode 100644 index 00000000..e28eb412 --- /dev/null +++ b/tests/test_aux_files/pre_processor/pp_environ_get.yaml @@ -0,0 +1,6 @@ +{% set a = env.get("a") %} +{% set b = env.get("b", default="0") %} + +package: + name: abc + version: 0 diff --git a/tests/test_aux_files/unprocessed_environ_get.yaml b/tests/test_aux_files/unprocessed_environ_get.yaml new file mode 100644 index 00000000..8625f451 --- /dev/null +++ b/tests/test_aux_files/unprocessed_environ_get.yaml @@ -0,0 +1,6 @@ +{% set a = environ["a"] %} +{% set b = environ.get("b", "0") %} + +package: + name: abc + version: 0 diff --git a/tests/test_aux_files/v1_format/v1_environ_get.yaml b/tests/test_aux_files/v1_format/v1_environ_get.yaml new file mode 100644 index 00000000..cc5fe8de --- /dev/null +++ b/tests/test_aux_files/v1_format/v1_environ_get.yaml @@ -0,0 +1,9 @@ +schema_version: 1 + +context: + a: ${{env.get("a")}} + b: ${{env.get("b", default="0")}} + +package: + name: abc + version: 0