From 82ba06c8aeeafaf5fdc6badb693e488f6c520690 Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Mon, 8 Apr 2024 09:17:30 -0600 Subject: [PATCH 01/10] Refactors recipe conversion work - Recipe conversion tooling is now broken-out into a subclass of the original `RecipeParser` - This keeps the `RecipeParser file from getting any bigger and will allow some future flexibility in maintaining the conversion process - Test files are broken up accordingly - Adds `conftest.py` to share testing utilities across multiple unit test files --- conda_recipe_manager/commands/convert.py | 6 +- conda_recipe_manager/parser/recipe_parser.py | 343 +---------------- .../parser/recipe_parser_convert.py | 360 ++++++++++++++++++ tests/__init__.py | 0 tests/conftest.py | 44 +++ tests/parser/test_recipe_parser.py | 112 +----- tests/parser/test_recipe_parser_convert.py | 94 +++++ 7 files changed, 504 insertions(+), 455 deletions(-) create mode 100644 conda_recipe_manager/parser/recipe_parser_convert.py create mode 100644 tests/__init__.py create mode 100644 tests/conftest.py create mode 100644 tests/parser/test_recipe_parser_convert.py diff --git a/conda_recipe_manager/commands/convert.py b/conda_recipe_manager/commands/convert.py index ad806364d..62b62af8b 100644 --- a/conda_recipe_manager/commands/convert.py +++ b/conda_recipe_manager/commands/convert.py @@ -16,7 +16,7 @@ import click -from conda_recipe_manager.parser.recipe_parser import RecipeParser +from conda_recipe_manager.parser.recipe_parser_convert import RecipeParserConvert from conda_recipe_manager.parser.types import MessageCategory, MessageTable # Pre-CEP-13 name of the recipe file @@ -112,9 +112,9 @@ def convert_file(file_path: Path, output: Optional[Path], print_output: bool) -> conversion_result.msg_tbl.add_message(MessageCategory.EXCEPTION, e_msg) return conversion_result - parser: RecipeParser + parser: RecipeParserConvert try: - parser = RecipeParser(recipe_content) + parser = RecipeParserConvert(recipe_content) except Exception as e: # pylint: disable=broad-exception-caught e_msg = "EXCEPTION: An exception occurred while parsing the recipe file" if print_output: diff --git a/conda_recipe_manager/parser/recipe_parser.py b/conda_recipe_manager/parser/recipe_parser.py index c3c7d09a5..cae37cd44 100644 --- a/conda_recipe_manager/parser/recipe_parser.py +++ b/conda_recipe_manager/parser/recipe_parser.py @@ -36,8 +36,6 @@ from conda_recipe_manager.parser._types import ( RECIPE_MANAGER_SUB_MARKER, ROOT_NODE_VALUE, - TOP_LEVEL_KEY_SORT_ORDER, - V1_BUILD_SECTION_KEY_SORT_ORDER, ForceIndentDumper, Regex, StrStack, @@ -53,15 +51,7 @@ ) from conda_recipe_manager.parser.enums import SelectorConflictMode from conda_recipe_manager.parser.exceptions import JsonPatchValidationException -from conda_recipe_manager.parser.types import ( - CURRENT_RECIPE_SCHEMA_FORMAT, - JSON_PATCH_SCHEMA, - TAB_AS_SPACES, - TAB_SPACE_COUNT, - MessageCategory, - MessageTable, - MultilineVariant, -) +from conda_recipe_manager.parser.types import JSON_PATCH_SCHEMA, TAB_AS_SPACES, TAB_SPACE_COUNT, MultilineVariant from conda_recipe_manager.types import PRIMITIVES_TUPLE, JsonPatchType, JsonType, Primitives, SentinelType @@ -658,337 +648,6 @@ def render_to_object(self, replace_variables: bool = False) -> JsonType: return data - def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: - # pylint: disable=protected-access - """ - Takes the current recipe representation and renders it to the new format WITHOUT modifying the current recipe - state. - - The "new" format is defined in the following CEPs: - - https://github.com/conda-incubator/ceps/blob/main/cep-13.md - - https://github.com/conda-incubator/ceps/blob/main/cep-14.md - - (As of writing there is no official name other than "the new recipe format") - """ - # Approach: In the event that we want to expand support later, this function should be implemented in terms - # of a `RecipeParser` tree. This will make it easier to build an upgrade-path, if we so choose to pursue one. - - msg_tbl = MessageTable() - - # `copy.deepcopy()` produced some bizarre artifacts, namely single-line comments were being incorrectly rendered - # as list members. Although inefficient, we have tests that validate round-tripping the parser and there - # is no development cost in utilizing tools we already must maintain. - new_recipe: RecipeParser = RecipeParser(self.render()) - # Log the original - old_comments: Final[dict[str, str]] = new_recipe.get_comments_table() - - # Convenience wrapper that logs failed patches to the message table - def _patch_and_log(patch: JsonPatchType) -> None: - if not new_recipe.patch(patch): - msg_tbl.add_message(MessageCategory.ERROR, f"Failed to patch: {patch}") - - # Convenience function constructs missing paths. Useful when you have to construct more than 1 path level at - # once (the JSON patch standard only allows the creation of 1 new level at a time) - def _patch_add_missing_path(base_path: str, ext: str, value: JsonType = None) -> None: - temp_path: Final[str] = RecipeParser.append_to_path(base_path, ext) - if new_recipe.contains_value(temp_path): - return - _patch_and_log({"op": "add", "path": temp_path, "value": value}) - - # Convenience function that moves a value under an old path to a new one sharing a common base path BUT only if - # the old path exists. - def _patch_move_base_path(base_path: str, old_ext: str, new_ext: str) -> None: - old_path: Final[str] = RecipeParser.append_to_path(base_path, old_ext) - if not new_recipe.contains_value(old_path): - return - _patch_and_log({"op": "move", "from": old_path, "path": RecipeParser.append_to_path(base_path, new_ext)}) - - # Convenience function that sorts 1 level of keys, given a path. Optionally allows renaming of the target node. - def _sort_subtree_keys(sort_path: str, tbl: dict[str, int], rename: str = "") -> None: - def _comparison(n: Node) -> int: - return RecipeParser._canonical_sort_keys_comparison(n, tbl) - - node = traverse(new_recipe._root, str_to_stack_path(sort_path)) - if node is None: - msg_tbl.add_message(MessageCategory.WARNING, f"Failed to sort members of {sort_path}") - return - if rename: - node.value = rename - node.children.sort(key=_comparison) - - ## JINJA -> `context` object ## - - # Convert the JINJA variable table to a `context` section. Empty tables still add the `context` section for - # future developers' convenience. - _patch_and_log({"op": "add", "path": "/context", "value": None}) - # Filter-out any value not covered in the new format - for name, value in new_recipe._vars_tbl.items(): - if not isinstance(value, (str, int, float, bool)): - msg_tbl.add_message(MessageCategory.WARNING, f"The variable `{name}` is an unsupported type.") - continue - _patch_and_log({"op": "add", "path": f"/context/{name}", "value": value}) - - # Similarly, patch-in the new `schema_version` value to the top of the file - _patch_and_log({"op": "add", "path": "/schema_version", "value": CURRENT_RECIPE_SCHEMA_FORMAT}) - - # Swap all JINJA to use the new `${{ }}` format. - jinja_sub_locations: Final[list[str]] = new_recipe.search(Regex.JINJA_SUB) - for path in jinja_sub_locations: - value = new_recipe.get_value(path) - # Values that match the regex should only be strings. This prevents crashes that should not occur. - if not isinstance(value, str): - msg_tbl.add_message( - MessageCategory.WARNING, f"A non-string value was found as a JINJA substitution: {value}" - ) - continue - value = value.replace("{{", "${{") - _patch_and_log({"op": "replace", "path": path, "value": value}) - - ## Convert selectors into ternary statements or `if` blocks ## - for selector, instances in new_recipe._selector_tbl.items(): - for info in instances: - # Selectors can be applied to the parent node if they appear on the same line. We'll ignore these when - # building replacements. - if not info.node.is_leaf(): - continue - - # Strip the []'s around the selector - bool_expression = selector[1:-1] - # Convert to a public-facing path representation - selector_path = stack_path_to_str(info.path) - - # For now, if a selector lands on a boolean value, use a ternary statement. Otherwise use the - # conditional logic. - patch: JsonPatchType = { - "op": "replace", - "path": selector_path, - "value": "${{ true if " + bool_expression + " }}", - } - # `skip` is special and needs to be a list of boolean expressions. - if selector_path.endswith("/build/skip"): - patch["value"] = [bool_expression] - if not isinstance(info.node.value, bool): - # CEP-13 states that ONLY list members may use the `if/then/else` blocks - if not info.node.list_member_flag: - msg_tbl.add_message( - MessageCategory.WARNING, f"A non-list item had a selector at: {selector_path}" - ) - continue - bool_object = { - "if": bool_expression, - "then": None if isinstance(info.node.value, SentinelType) else info.node.value, - } - patch = { - "op": "replace", - "path": selector_path, - "value": cast(JsonType, bool_object), - } - # Apply the patch - _patch_and_log(patch) - new_recipe.remove_selector(selector_path) - - # Cached copy of all of the "outputs" in a recipe. This is useful for easily handling multi and single output - # recipes in 1 loop construct. - base_package_paths: Final[list[str]] = new_recipe.get_package_paths() - - # TODO Fix: comments are not preserved with patch operations (add a flag to `patch()`?) - - ## `build` section changes and validation ## - - for base_path in base_package_paths: - # Move `run_exports` and `ignore_run_exports` from `build` to `requirements` - - # `run_exports` - old_re_path = RecipeParser.append_to_path(base_path, "/build/run_exports") - if new_recipe.contains_value(old_re_path): - requirements_path = RecipeParser.append_to_path(base_path, "/requirements") - new_re_path = RecipeParser.append_to_path(base_path, "/requirements/run_exports") - if not new_recipe.contains_value(requirements_path): - _patch_and_log({"op": "add", "path": requirements_path, "value": None}) - _patch_and_log({"op": "move", "from": old_re_path, "path": new_re_path}) - # `ignore_run_exports` - old_ire_path = RecipeParser.append_to_path(base_path, "/build/ignore_run_exports") - if new_recipe.contains_value(old_re_path): - requirements_path = RecipeParser.append_to_path(base_path, "/requirements") - new_ire_path = RecipeParser.append_to_path(base_path, "/requirements/ignore_run_exports") - if not new_recipe.contains_value(requirements_path): - _patch_and_log({"op": "add", "path": requirements_path, "value": None}) - _patch_and_log({"op": "move", "from": old_ire_path, "path": new_ire_path}) - - # Perform internal section changes per `build/` section - build_path = RecipeParser.append_to_path(base_path, "/build") - if not new_recipe.contains_value(build_path): - continue - - # `build/entry_points` -> `build/python/entry_points` - if new_recipe.contains_value(RecipeParser.append_to_path(build_path, "/entry_points")): - _patch_add_missing_path(build_path, "/python") - _patch_move_base_path(build_path, "/entry_points", "/python/entry_points") - - # Canonically sort this section - _sort_subtree_keys(build_path, V1_BUILD_SECTION_KEY_SORT_ORDER) - - ## `about` section changes and validation ## - - # Warn if "required" fields are missing - about_required: Final[list[str]] = [ - "summary", - "description", - "license", - "license_file", - "license_url", - ] - for field in about_required: - path = f"/about/{field}" - if not new_recipe.contains_value(path): - msg_tbl.add_message(MessageCategory.WARNING, f"Required field missing: {path}") - - # Transform renamed fields - about_rename: Final[list[tuple[str, str]]] = [ - ("home", "homepage"), - ("dev_url", "repository"), - ("doc_url", "documentation"), - ] - for old, new in about_rename: - _patch_move_base_path("/about", old, new) - - # TODO validate: /about/license must be SPDX recognized. - - # Remove deprecated `about` fields - about_deprecated: Final[list[str]] = [ - "prelink_message", - "license_family", - "identifiers", - "tags", - "keywords", - "doc_source_url", - ] - for field in about_deprecated: - path = f"/about/{field}" - if new_recipe.contains_value(path): - _patch_and_log({"op": "remove", "path": path}) - - ## `test` section changes and upgrades ## - - # NOTE: For now, we assume that the existing test section comprises of a single test entity. Developers will - # have to use their best judgement to manually break-up the test into multiple tests as they see fit. - for base_path in base_package_paths: - test_path = RecipeParser.append_to_path(base_path, "/test") - if not new_recipe.contains_value(test_path): - continue - - # Moving `files` to `files/recipe` is not possible in a single `move` operation as a new path has to be - # created in the path being moved. - test_files_path = RecipeParser.append_to_path(test_path, "/files") - if new_recipe.contains_value(test_files_path): - test_files_value = new_recipe.get_value(test_files_path) - # TODO: Fix, replace does not work here, produces `- null`, Issue #20 - # _patch_and_log({"op": "replace", "path": test_files_path, "value": None}) - _patch_and_log({"op": "remove", "path": test_files_path}) - _patch_and_log({"op": "add", "path": test_files_path, "value": None}) - _patch_and_log( - { - "op": "add", - "path": RecipeParser.append_to_path(test_files_path, "/recipe"), - "value": test_files_value, - } - ) - # Edge case: `/source_files` exists but `/files` does not - elif new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/source_files")): - _patch_add_missing_path(test_path, "/files") - _patch_move_base_path(test_path, "/source_files", "/files/source") - - if new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/requires")): - _patch_add_missing_path(test_path, "/requirements") - _patch_move_base_path(test_path, "/requires", "/requirements/run") - - # Replace `- pip check` in `commands` with the new flag. If not found, set the flag to `False` (as the - # flag defaults to `True`). - commands = cast(list[str], new_recipe.get_value(RecipeParser.append_to_path(test_path, "/commands"), [])) - pip_check = False - for i, command in enumerate(commands): - if command != "pip check": - continue - # For now, we will only patch-out the first instance when no selector is attached - # TODO Future: handle selector logic/cases with `pip check || ` - _patch_and_log({"op": "remove", "path": RecipeParser.append_to_path(test_path, f"/commands/{i}")}) - pip_check = True - break - _patch_add_missing_path(test_path, "/python") - _patch_and_log( - {"op": "add", "path": RecipeParser.append_to_path(test_path, "/python/pip_check"), "value": pip_check} - ) - - _patch_move_base_path(test_path, "/commands", "/script") - _patch_move_base_path(test_path, "/imports", "/python/imports") - _patch_move_base_path(test_path, "/downstreams", "/downstream") - - # Move `test` to `tests` and encapsulate the pre-existing object into a list - new_test_path = f"{test_path}s" - test_element = cast(dict[str, JsonType], new_recipe.get_value(test_path)) - test_array: list[JsonType] = [] - # There are 3 types of test elements. We break them out of the original object, if they exist. - # `Python` Test Element - if "python" in test_element: - test_array.append({"python": test_element["python"]}) - del test_element["python"] - # `Downstream` Test Element - if "downstream" in test_element: - test_array.append({"downstream": test_element["downstream"]}) - del test_element["downstream"] - # What remains should be the `Command` Test Element type - if test_element: - test_array.append(test_element) - _patch_and_log({"op": "add", "path": new_test_path, "value": test_array}) - _patch_and_log({"op": "remove", "path": test_path}) - - ## Upgrade the multi-output section(s) ## - # TODO Complete - if new_recipe.contains_value("/outputs"): - # On the top-level, `package` -> `recipe` - _patch_move_base_path(ROOT_NODE_VALUE, "/package", "/recipe") - - for output_path in base_package_paths: - if output_path == ROOT_NODE_VALUE: - continue - - # Move `name` and `version` under `package` - if new_recipe.contains_value( - RecipeParser.append_to_path(output_path, "/name") - ) or new_recipe.contains_value(RecipeParser.append_to_path(output_path, "/version")): - _patch_add_missing_path(output_path, "/package") - _patch_move_base_path(output_path, "/name", "/package/name") - _patch_move_base_path(output_path, "/version", "/package/version") - - # Not all the top-level keys are found in each output section, but all the output section keys are - # found at the top-level. So for consistency, we sort on that ordering. - _sort_subtree_keys(output_path, TOP_LEVEL_KEY_SORT_ORDER) - - ## Final clean-up ## - - # TODO: Comment tracking may need improvement. The "correct way" of tracking comments with patch changes is a - # fairly big engineering effort and refactor. - # Alert the user which comments have been dropped. - new_comments: Final[dict[str, str]] = new_recipe.get_comments_table() - diff_comments: Final[dict[str, str]] = {k: v for k, v in old_comments.items() if k not in new_comments} - for path, comment in diff_comments.items(): - if not new_recipe.contains_value(path): - msg_tbl.add_message(MessageCategory.WARNING, f"Could not relocate comment: {comment}") - - # TODO Complete: move operations may result in empty fields we can eliminate. This may require changes to - # `contains_value()` - # TODO Complete: Attempt to combine consecutive If/Then blocks after other modifications. This should reduce the - # risk of screwing up critical list indices and ordering. - - # Hack: Wipe the existing table so the JINJA `set` statements don't render the final form - new_recipe._vars_tbl = {} - - # Sort the top-level keys to a "canonical" ordering. This should make previous patch operations look more - # "sensible" to a human reader. - _sort_subtree_keys("/", TOP_LEVEL_KEY_SORT_ORDER) - - return new_recipe.render(), msg_tbl - ## YAML Access Functions ## def list_value_paths(self) -> list[str]: diff --git a/conda_recipe_manager/parser/recipe_parser_convert.py b/conda_recipe_manager/parser/recipe_parser_convert.py new file mode 100644 index 000000000..ccb06f8e5 --- /dev/null +++ b/conda_recipe_manager/parser/recipe_parser_convert.py @@ -0,0 +1,360 @@ +""" +File: recipe_parser_convert.py +Description: Provides a subclass of RecipeParser that performs the conversion of a v0 recipe to the new v1 recipe + format. This tooling was originally part of the base class, but was broken-out for easier/cleaner code + maintenance. +""" +from __future__ import annotations + +from typing import Final, cast + +from conda_recipe_manager.parser._node import Node +from conda_recipe_manager.parser._traverse import traverse +from conda_recipe_manager.parser._types import ( + ROOT_NODE_VALUE, + TOP_LEVEL_KEY_SORT_ORDER, + V1_BUILD_SECTION_KEY_SORT_ORDER, + Regex, +) +from conda_recipe_manager.parser._utils import stack_path_to_str, str_to_stack_path +from conda_recipe_manager.parser.recipe_parser import RecipeParser +from conda_recipe_manager.parser.types import CURRENT_RECIPE_SCHEMA_FORMAT, MessageCategory, MessageTable +from conda_recipe_manager.types import JsonPatchType, JsonType, SentinelType + + +class RecipeParserConvert(RecipeParser): + """ + Extension of the base RecipeParser class to enables upgrading recipes from the old to new format. + This was originally part of the RecipeParser class but was broken-out for easier maintenance. + """ + + def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: + # pylint: disable=protected-access + """ + Takes the current recipe representation and renders it to the new format WITHOUT modifying the current recipe + state. + + The "new" format is defined in the following CEPs: + - https://github.com/conda-incubator/ceps/blob/main/cep-13.md + - https://github.com/conda-incubator/ceps/blob/main/cep-14.md + + (As of writing there is no official name other than "the new recipe format") + """ + # Approach: In the event that we want to expand support later, this function should be implemented in terms + # of a `RecipeParser` tree. This will make it easier to build an upgrade-path, if we so choose to pursue one. + + msg_tbl = MessageTable() + + # `copy.deepcopy()` produced some bizarre artifacts, namely single-line comments were being incorrectly rendered + # as list members. Although inefficient, we have tests that validate round-tripping the parser and there + # is no development cost in utilizing tools we already must maintain. + new_recipe: RecipeParser = RecipeParser(self.render()) + # Log the original + old_comments: Final[dict[str, str]] = new_recipe.get_comments_table() + + # Convenience wrapper that logs failed patches to the message table + def _patch_and_log(patch: JsonPatchType) -> None: + if not new_recipe.patch(patch): + msg_tbl.add_message(MessageCategory.ERROR, f"Failed to patch: {patch}") + + # Convenience function constructs missing paths. Useful when you have to construct more than 1 path level at + # once (the JSON patch standard only allows the creation of 1 new level at a time) + def _patch_add_missing_path(base_path: str, ext: str, value: JsonType = None) -> None: + temp_path: Final[str] = RecipeParser.append_to_path(base_path, ext) + if new_recipe.contains_value(temp_path): + return + _patch_and_log({"op": "add", "path": temp_path, "value": value}) + + # Convenience function that moves a value under an old path to a new one sharing a common base path BUT only if + # the old path exists. + def _patch_move_base_path(base_path: str, old_ext: str, new_ext: str) -> None: + old_path: Final[str] = RecipeParser.append_to_path(base_path, old_ext) + if not new_recipe.contains_value(old_path): + return + _patch_and_log({"op": "move", "from": old_path, "path": RecipeParser.append_to_path(base_path, new_ext)}) + + # Convenience function that sorts 1 level of keys, given a path. Optionally allows renaming of the target node. + def _sort_subtree_keys(sort_path: str, tbl: dict[str, int], rename: str = "") -> None: + def _comparison(n: Node) -> int: + return RecipeParser._canonical_sort_keys_comparison(n, tbl) + + node = traverse(new_recipe._root, str_to_stack_path(sort_path)) + if node is None: + msg_tbl.add_message(MessageCategory.WARNING, f"Failed to sort members of {sort_path}") + return + if rename: + node.value = rename + node.children.sort(key=_comparison) + + ## JINJA -> `context` object ## + + # Convert the JINJA variable table to a `context` section. Empty tables still add the `context` section for + # future developers' convenience. + _patch_and_log({"op": "add", "path": "/context", "value": None}) + # Filter-out any value not covered in the new format + for name, value in new_recipe._vars_tbl.items(): + if not isinstance(value, (str, int, float, bool)): + msg_tbl.add_message(MessageCategory.WARNING, f"The variable `{name}` is an unsupported type.") + continue + _patch_and_log({"op": "add", "path": f"/context/{name}", "value": value}) + + # Similarly, patch-in the new `schema_version` value to the top of the file + _patch_and_log({"op": "add", "path": "/schema_version", "value": CURRENT_RECIPE_SCHEMA_FORMAT}) + + # Swap all JINJA to use the new `${{ }}` format. + jinja_sub_locations: Final[list[str]] = new_recipe.search(Regex.JINJA_SUB) + for path in jinja_sub_locations: + value = new_recipe.get_value(path) + # Values that match the regex should only be strings. This prevents crashes that should not occur. + if not isinstance(value, str): + msg_tbl.add_message( + MessageCategory.WARNING, f"A non-string value was found as a JINJA substitution: {value}" + ) + continue + value = value.replace("{{", "${{") + _patch_and_log({"op": "replace", "path": path, "value": value}) + + ## Convert selectors into ternary statements or `if` blocks ## + for selector, instances in new_recipe._selector_tbl.items(): + for info in instances: + # Selectors can be applied to the parent node if they appear on the same line. We'll ignore these when + # building replacements. + if not info.node.is_leaf(): + continue + + # Strip the []'s around the selector + bool_expression = selector[1:-1] + # Convert to a public-facing path representation + selector_path = stack_path_to_str(info.path) + + # For now, if a selector lands on a boolean value, use a ternary statement. Otherwise use the + # conditional logic. + patch: JsonPatchType = { + "op": "replace", + "path": selector_path, + "value": "${{ true if " + bool_expression + " }}", + } + # `skip` is special and needs to be a list of boolean expressions. + if selector_path.endswith("/build/skip"): + patch["value"] = [bool_expression] + if not isinstance(info.node.value, bool): + # CEP-13 states that ONLY list members may use the `if/then/else` blocks + if not info.node.list_member_flag: + msg_tbl.add_message( + MessageCategory.WARNING, f"A non-list item had a selector at: {selector_path}" + ) + continue + bool_object = { + "if": bool_expression, + "then": None if isinstance(info.node.value, SentinelType) else info.node.value, + } + patch = { + "op": "replace", + "path": selector_path, + "value": cast(JsonType, bool_object), + } + # Apply the patch + _patch_and_log(patch) + new_recipe.remove_selector(selector_path) + + # Cached copy of all of the "outputs" in a recipe. This is useful for easily handling multi and single output + # recipes in 1 loop construct. + base_package_paths: Final[list[str]] = new_recipe.get_package_paths() + + # TODO Fix: comments are not preserved with patch operations (add a flag to `patch()`?) + + ## `build` section changes and validation ## + + for base_path in base_package_paths: + # Move `run_exports` and `ignore_run_exports` from `build` to `requirements` + + # `run_exports` + old_re_path = RecipeParser.append_to_path(base_path, "/build/run_exports") + if new_recipe.contains_value(old_re_path): + requirements_path = RecipeParser.append_to_path(base_path, "/requirements") + new_re_path = RecipeParser.append_to_path(base_path, "/requirements/run_exports") + if not new_recipe.contains_value(requirements_path): + _patch_and_log({"op": "add", "path": requirements_path, "value": None}) + _patch_and_log({"op": "move", "from": old_re_path, "path": new_re_path}) + # `ignore_run_exports` + old_ire_path = RecipeParser.append_to_path(base_path, "/build/ignore_run_exports") + if new_recipe.contains_value(old_re_path): + requirements_path = RecipeParser.append_to_path(base_path, "/requirements") + new_ire_path = RecipeParser.append_to_path(base_path, "/requirements/ignore_run_exports") + if not new_recipe.contains_value(requirements_path): + _patch_and_log({"op": "add", "path": requirements_path, "value": None}) + _patch_and_log({"op": "move", "from": old_ire_path, "path": new_ire_path}) + + # Perform internal section changes per `build/` section + build_path = RecipeParser.append_to_path(base_path, "/build") + if not new_recipe.contains_value(build_path): + continue + + # `build/entry_points` -> `build/python/entry_points` + if new_recipe.contains_value(RecipeParser.append_to_path(build_path, "/entry_points")): + _patch_add_missing_path(build_path, "/python") + _patch_move_base_path(build_path, "/entry_points", "/python/entry_points") + + # Canonically sort this section + _sort_subtree_keys(build_path, V1_BUILD_SECTION_KEY_SORT_ORDER) + + ## `about` section changes and validation ## + + # Warn if "required" fields are missing + about_required: Final[list[str]] = [ + "summary", + "description", + "license", + "license_file", + "license_url", + ] + for field in about_required: + path = f"/about/{field}" + if not new_recipe.contains_value(path): + msg_tbl.add_message(MessageCategory.WARNING, f"Required field missing: {path}") + + # Transform renamed fields + about_rename: Final[list[tuple[str, str]]] = [ + ("home", "homepage"), + ("dev_url", "repository"), + ("doc_url", "documentation"), + ] + for old, new in about_rename: + _patch_move_base_path("/about", old, new) + + # TODO validate: /about/license must be SPDX recognized. + + # Remove deprecated `about` fields + about_deprecated: Final[list[str]] = [ + "prelink_message", + "license_family", + "identifiers", + "tags", + "keywords", + "doc_source_url", + ] + for field in about_deprecated: + path = f"/about/{field}" + if new_recipe.contains_value(path): + _patch_and_log({"op": "remove", "path": path}) + + ## `test` section changes and upgrades ## + + # NOTE: For now, we assume that the existing test section comprises of a single test entity. Developers will + # have to use their best judgement to manually break-up the test into multiple tests as they see fit. + for base_path in base_package_paths: + test_path = RecipeParser.append_to_path(base_path, "/test") + if not new_recipe.contains_value(test_path): + continue + + # Moving `files` to `files/recipe` is not possible in a single `move` operation as a new path has to be + # created in the path being moved. + test_files_path = RecipeParser.append_to_path(test_path, "/files") + if new_recipe.contains_value(test_files_path): + test_files_value = new_recipe.get_value(test_files_path) + # TODO: Fix, replace does not work here, produces `- null`, Issue #20 + # _patch_and_log({"op": "replace", "path": test_files_path, "value": None}) + _patch_and_log({"op": "remove", "path": test_files_path}) + _patch_and_log({"op": "add", "path": test_files_path, "value": None}) + _patch_and_log( + { + "op": "add", + "path": RecipeParser.append_to_path(test_files_path, "/recipe"), + "value": test_files_value, + } + ) + # Edge case: `/source_files` exists but `/files` does not + elif new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/source_files")): + _patch_add_missing_path(test_path, "/files") + _patch_move_base_path(test_path, "/source_files", "/files/source") + + if new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/requires")): + _patch_add_missing_path(test_path, "/requirements") + _patch_move_base_path(test_path, "/requires", "/requirements/run") + + # Replace `- pip check` in `commands` with the new flag. If not found, set the flag to `False` (as the + # flag defaults to `True`). + commands = cast(list[str], new_recipe.get_value(RecipeParser.append_to_path(test_path, "/commands"), [])) + pip_check = False + for i, command in enumerate(commands): + if command != "pip check": + continue + # For now, we will only patch-out the first instance when no selector is attached + # TODO Future: handle selector logic/cases with `pip check || ` + _patch_and_log({"op": "remove", "path": RecipeParser.append_to_path(test_path, f"/commands/{i}")}) + pip_check = True + break + _patch_add_missing_path(test_path, "/python") + _patch_and_log( + {"op": "add", "path": RecipeParser.append_to_path(test_path, "/python/pip_check"), "value": pip_check} + ) + + _patch_move_base_path(test_path, "/commands", "/script") + _patch_move_base_path(test_path, "/imports", "/python/imports") + _patch_move_base_path(test_path, "/downstreams", "/downstream") + + # Move `test` to `tests` and encapsulate the pre-existing object into a list + new_test_path = f"{test_path}s" + test_element = cast(dict[str, JsonType], new_recipe.get_value(test_path)) + test_array: list[JsonType] = [] + # There are 3 types of test elements. We break them out of the original object, if they exist. + # `Python` Test Element + if "python" in test_element: + test_array.append({"python": test_element["python"]}) + del test_element["python"] + # `Downstream` Test Element + if "downstream" in test_element: + test_array.append({"downstream": test_element["downstream"]}) + del test_element["downstream"] + # What remains should be the `Command` Test Element type + if test_element: + test_array.append(test_element) + _patch_and_log({"op": "add", "path": new_test_path, "value": test_array}) + _patch_and_log({"op": "remove", "path": test_path}) + + ## Upgrade the multi-output section(s) ## + # TODO Complete + if new_recipe.contains_value("/outputs"): + # On the top-level, `package` -> `recipe` + _patch_move_base_path(ROOT_NODE_VALUE, "/package", "/recipe") + + for output_path in base_package_paths: + if output_path == ROOT_NODE_VALUE: + continue + + # Move `name` and `version` under `package` + if new_recipe.contains_value( + RecipeParser.append_to_path(output_path, "/name") + ) or new_recipe.contains_value(RecipeParser.append_to_path(output_path, "/version")): + _patch_add_missing_path(output_path, "/package") + _patch_move_base_path(output_path, "/name", "/package/name") + _patch_move_base_path(output_path, "/version", "/package/version") + + # Not all the top-level keys are found in each output section, but all the output section keys are + # found at the top-level. So for consistency, we sort on that ordering. + _sort_subtree_keys(output_path, TOP_LEVEL_KEY_SORT_ORDER) + + ## Final clean-up ## + + # TODO: Comment tracking may need improvement. The "correct way" of tracking comments with patch changes is a + # fairly big engineering effort and refactor. + # Alert the user which comments have been dropped. + new_comments: Final[dict[str, str]] = new_recipe.get_comments_table() + diff_comments: Final[dict[str, str]] = {k: v for k, v in old_comments.items() if k not in new_comments} + for path, comment in diff_comments.items(): + if not new_recipe.contains_value(path): + msg_tbl.add_message(MessageCategory.WARNING, f"Could not relocate comment: {comment}") + + # TODO Complete: move operations may result in empty fields we can eliminate. This may require changes to + # `contains_value()` + # TODO Complete: Attempt to combine consecutive If/Then blocks after other modifications. This should reduce the + # risk of screwing up critical list indices and ordering. + + # Hack: Wipe the existing table so the JINJA `set` statements don't render the final form + new_recipe._vars_tbl = {} + + # Sort the top-level keys to a "canonical" ordering. This should make previous patch operations look more + # "sensible" to a human reader. + _sort_subtree_keys("/", TOP_LEVEL_KEY_SORT_ORDER) + + return new_recipe.render(), msg_tbl diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 000000000..a736c9730 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,44 @@ +""" +File: conftest.py +Description: Constants and utilities used for pytest tests. +""" +from __future__ import annotations + +from pathlib import Path +from typing import Final + +from conda_recipe_manager.parser.recipe_parser import RecipeParser +from conda_recipe_manager.parser.recipe_parser_convert import RecipeParserConvert + +# Path to supplementary files used in test cases +TEST_FILES_PATH: Final[str] = "tests/test_aux_files" + + +def load_file(file: Path | str) -> str: + """ + Loads a file into a single string + :param file: Filename of the file to read + :returns: Text from the file + """ + with open(Path(file), "r", encoding="utf-8") as f: + return f.read() + + +def load_recipe(file_name: str) -> RecipeParser: + """ + Convenience function that simplifies initializing a recipe parser. + :param file_name: File name of the test recipe to load + :returns: RecipeParser instance, based on the file + """ + recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") + return RecipeParser(recipe) + + +def load_recipe_convert(file_name: str) -> RecipeParserConvert: + """ + Convenience function that simplifies initializing a recipe parser. + :param file_name: File name of the test recipe to load + :returns: RecipeParserConvert instance, based on the file + """ + recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") + return RecipeParserConvert(recipe) diff --git a/tests/parser/test_recipe_parser.py b/tests/parser/test_recipe_parser.py index 2bfdeb905..c72bed8ff 100644 --- a/tests/parser/test_recipe_parser.py +++ b/tests/parser/test_recipe_parser.py @@ -1,10 +1,9 @@ """ File: test_recipe_parser.py -Description: Unit tests for the recipe parser class and tools +Description: Unit tests for the RecipeParser class """ from __future__ import annotations -from pathlib import Path from typing import Final import pytest @@ -12,11 +11,8 @@ from conda_recipe_manager.parser.enums import SelectorConflictMode from conda_recipe_manager.parser.exceptions import JsonPatchValidationException from conda_recipe_manager.parser.recipe_parser import RecipeParser -from conda_recipe_manager.parser.types import MessageCategory from conda_recipe_manager.types import JsonType - -# Path to supplementary files used in test cases -TEST_FILES_PATH: Final[str] = "tests/test_aux_files" +from tests.conftest import TEST_FILES_PATH, load_file, load_recipe # Long multi-line description string found in the `simple-recipe.yaml` test file SIMPLE_DESCRIPTION: Final[ @@ -39,26 +35,6 @@ QUICK_FOX_SUB_CARROT_MINUS: Final[str] = "The quick brown tiger\njumped over the lazy dog" -def load_file(file: Path | str) -> str: - """ - Loads a file into a single string - :param file: Filename of the file to read - :returns: Text from the file - """ - with open(Path(file), "r", encoding="utf-8") as f: - return f.read() - - -def load_recipe(file_name: str) -> RecipeParser: - """ - Convenience function that simplifies initializing a recipe parser. - :param file_name: File name of the test recipe to load - :returns: RecipeParser instance, based on the file - """ - recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") - return RecipeParser(recipe) - - ## Construction and rendering sanity checks ## @@ -321,90 +297,6 @@ def test_render_to_object_multi_output() -> None: } -@pytest.mark.parametrize( - "file_base,errors,warnings", - [ - ( - "simple-recipe.yaml", - [], - [ - "A non-list item had a selector at: /requirements/empty_field2", - "Required field missing: /about/license_file", - "Required field missing: /about/license_url", - ], - ), - ( - "multi-output.yaml", - [], - [ - "Required field missing: /about/summary", - "Required field missing: /about/description", - "Required field missing: /about/license", - "Required field missing: /about/license_file", - "Required field missing: /about/license_url", - ], - ), - ( - "huggingface_hub.yaml", - [], - [ - "Required field missing: /about/license_url", - ], - ), - ( - "types-toml.yaml", - [], - [ - "Required field missing: /about/license_url", - ], - ), - # Regression test: Contains a `test` section that caused an empty dictionary to be inserted in the conversion - # process, causing an index-out-of-range exception. - ( - "pytest-pep8.yaml", - [], - [ - "Required field missing: /about/license_url", - ], - ), - # Regression test: TODO - ( - "google-cloud-cpp.yaml", - [], - [ - "A non-list item had a selector at: /outputs/0/script", - "A non-list item had a selector at: /outputs/1/script", - "A non-list item had a selector at: /outputs/0/script", - "A non-list item had a selector at: /outputs/1/script", - "Required field missing: /about/description", - "Required field missing: /about/license_url", - ], - ), - # TODO Complete: The `curl.yaml` test is far from perfect. It is very much a work in progress. - # ( - # "curl.yaml", - # [], - # [ - # "A non-list item had a selector at: /outputs/0/build/ignore_run_exports", - # ], - # ), - ], -) -def test_render_to_new_recipe_format(file_base: str, errors: list[str], warnings: list[str]) -> None: - """ - Validates rendering a recipe in the new format. - :param file_base: Base file name for both the input and the expected out - """ - parser = load_recipe(file_base) - result, tbl = parser.render_to_new_recipe_format() - assert result == load_file(f"{TEST_FILES_PATH}/new_format_{file_base}") - assert tbl.get_messages(MessageCategory.ERROR) == errors - assert tbl.get_messages(MessageCategory.WARNING) == warnings - # Ensure that the original file was untouched - assert not parser.is_modified() - assert parser.diff() == "" - - ## Values ## diff --git a/tests/parser/test_recipe_parser_convert.py b/tests/parser/test_recipe_parser_convert.py new file mode 100644 index 000000000..10288b0b1 --- /dev/null +++ b/tests/parser/test_recipe_parser_convert.py @@ -0,0 +1,94 @@ +""" +File: test_recipe_parser_convert.py +Description: Unit tests for the RecipeParserConvert class +""" +from __future__ import annotations + +import pytest + +from conda_recipe_manager.parser.types import MessageCategory +from tests.conftest import TEST_FILES_PATH, load_file, load_recipe_convert + + +@pytest.mark.parametrize( + "file_base,errors,warnings", + [ + ( + "simple-recipe.yaml", + [], + [ + "A non-list item had a selector at: /requirements/empty_field2", + "Required field missing: /about/license_file", + "Required field missing: /about/license_url", + ], + ), + ( + "multi-output.yaml", + [], + [ + "Required field missing: /about/summary", + "Required field missing: /about/description", + "Required field missing: /about/license", + "Required field missing: /about/license_file", + "Required field missing: /about/license_url", + ], + ), + ( + "huggingface_hub.yaml", + [], + [ + "Required field missing: /about/license_url", + ], + ), + ( + "types-toml.yaml", + [], + [ + "Required field missing: /about/license_url", + ], + ), + # Regression test: Contains a `test` section that caused an empty dictionary to be inserted in the conversion + # process, causing an index-out-of-range exception. + ( + "pytest-pep8.yaml", + [], + [ + "Required field missing: /about/license_url", + ], + ), + # Regression test: TODO + ( + "google-cloud-cpp.yaml", + [], + [ + "A non-list item had a selector at: /outputs/0/script", + "A non-list item had a selector at: /outputs/1/script", + "A non-list item had a selector at: /outputs/0/script", + "A non-list item had a selector at: /outputs/1/script", + "Required field missing: /about/description", + "Required field missing: /about/license_url", + ], + ), + # TODO Complete: The `curl.yaml` test is far from perfect. It is very much a work in progress. + # ( + # "curl.yaml", + # [], + # [ + # "A non-list item had a selector at: /outputs/0/build/ignore_run_exports", + # ], + # ), + ], +) +def test_render_to_new_recipe_format(file_base: str, errors: list[str], warnings: list[str]) -> None: + """ + Validates rendering a recipe in the new format. + :param file_base: Base file name for both the input and the expected out + """ + parser = load_recipe_convert(file_base) + result, tbl = parser.render_to_new_recipe_format() + assert result == load_file(f"{TEST_FILES_PATH}/new_format_{file_base}") + assert tbl.get_messages(MessageCategory.ERROR) == errors + assert tbl.get_messages(MessageCategory.WARNING) == warnings + # Ensure that the original file was untouched + assert not parser.is_modified() + assert parser.diff() == "" From 92f27558f65cd0e69a4192863aa0bb5bb1023ac4 Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Mon, 8 Apr 2024 09:30:48 -0600 Subject: [PATCH 02/10] Minor renaming --- tests/conftest.py | 44 ---------------------- tests/parser/test_recipe_parser.py | 2 +- tests/parser/test_recipe_parser_convert.py | 2 +- 3 files changed, 2 insertions(+), 46 deletions(-) delete mode 100644 tests/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py deleted file mode 100644 index a736c9730..000000000 --- a/tests/conftest.py +++ /dev/null @@ -1,44 +0,0 @@ -""" -File: conftest.py -Description: Constants and utilities used for pytest tests. -""" -from __future__ import annotations - -from pathlib import Path -from typing import Final - -from conda_recipe_manager.parser.recipe_parser import RecipeParser -from conda_recipe_manager.parser.recipe_parser_convert import RecipeParserConvert - -# Path to supplementary files used in test cases -TEST_FILES_PATH: Final[str] = "tests/test_aux_files" - - -def load_file(file: Path | str) -> str: - """ - Loads a file into a single string - :param file: Filename of the file to read - :returns: Text from the file - """ - with open(Path(file), "r", encoding="utf-8") as f: - return f.read() - - -def load_recipe(file_name: str) -> RecipeParser: - """ - Convenience function that simplifies initializing a recipe parser. - :param file_name: File name of the test recipe to load - :returns: RecipeParser instance, based on the file - """ - recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") - return RecipeParser(recipe) - - -def load_recipe_convert(file_name: str) -> RecipeParserConvert: - """ - Convenience function that simplifies initializing a recipe parser. - :param file_name: File name of the test recipe to load - :returns: RecipeParserConvert instance, based on the file - """ - recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") - return RecipeParserConvert(recipe) diff --git a/tests/parser/test_recipe_parser.py b/tests/parser/test_recipe_parser.py index c72bed8ff..21d9781af 100644 --- a/tests/parser/test_recipe_parser.py +++ b/tests/parser/test_recipe_parser.py @@ -12,7 +12,7 @@ from conda_recipe_manager.parser.exceptions import JsonPatchValidationException from conda_recipe_manager.parser.recipe_parser import RecipeParser from conda_recipe_manager.types import JsonType -from tests.conftest import TEST_FILES_PATH, load_file, load_recipe +from tests.file_loading import TEST_FILES_PATH, load_file, load_recipe # Long multi-line description string found in the `simple-recipe.yaml` test file SIMPLE_DESCRIPTION: Final[ diff --git a/tests/parser/test_recipe_parser_convert.py b/tests/parser/test_recipe_parser_convert.py index 10288b0b1..37071f753 100644 --- a/tests/parser/test_recipe_parser_convert.py +++ b/tests/parser/test_recipe_parser_convert.py @@ -7,7 +7,7 @@ import pytest from conda_recipe_manager.parser.types import MessageCategory -from tests.conftest import TEST_FILES_PATH, load_file, load_recipe_convert +from tests.file_loading import TEST_FILES_PATH, load_file, load_recipe_convert @pytest.mark.parametrize( From 249be19371e455a93d42d1cf4bcf80355e8e5192 Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Mon, 8 Apr 2024 10:40:45 -0600 Subject: [PATCH 03/10] Forgot to add test file --- tests/file_loading.py | 44 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/file_loading.py diff --git a/tests/file_loading.py b/tests/file_loading.py new file mode 100644 index 000000000..1ad688c67 --- /dev/null +++ b/tests/file_loading.py @@ -0,0 +1,44 @@ +""" +File: file_loading.py +Description: Constants and utilities used for loading files/recipes. +""" +from __future__ import annotations + +from pathlib import Path +from typing import Final + +from conda_recipe_manager.parser.recipe_parser import RecipeParser +from conda_recipe_manager.parser.recipe_parser_convert import RecipeParserConvert + +# Path to supplementary files used in test cases +TEST_FILES_PATH: Final[str] = "tests/test_aux_files" + + +def load_file(file: Path | str) -> str: + """ + Loads a file into a single string + :param file: Filename of the file to read + :returns: Text from the file + """ + with open(Path(file), "r", encoding="utf-8") as f: + return f.read() + + +def load_recipe(file_name: str) -> RecipeParser: + """ + Convenience function that simplifies initializing a recipe parser. + :param file_name: File name of the test recipe to load + :returns: RecipeParser instance, based on the file + """ + recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") + return RecipeParser(recipe) + + +def load_recipe_convert(file_name: str) -> RecipeParserConvert: + """ + Convenience function that simplifies initializing a recipe parser. + :param file_name: File name of the test recipe to load + :returns: RecipeParserConvert instance, based on the file + """ + recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") + return RecipeParserConvert(recipe) From c0fbe0b4f1554e6da01e1ca4074e6df8e64fa93f Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Tue, 9 Apr 2024 13:40:22 -0600 Subject: [PATCH 04/10] Test file path now uses a calculated absolute path --- tests/file_loading.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/file_loading.py b/tests/file_loading.py index 1ad688c67..05af5d461 100644 --- a/tests/file_loading.py +++ b/tests/file_loading.py @@ -11,7 +11,7 @@ from conda_recipe_manager.parser.recipe_parser_convert import RecipeParserConvert # Path to supplementary files used in test cases -TEST_FILES_PATH: Final[str] = "tests/test_aux_files" +TEST_FILES_PATH: Final[Path] = Path(__file__).parent / "test_aux_files" def load_file(file: Path | str) -> str: From 0c1f9abbbd0dfdd5ee43c6ac5f8623c0a7159ab5 Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Tue, 9 Apr 2024 13:41:55 -0600 Subject: [PATCH 05/10] Changing rattler-build output dir in CI to test theory --- .github/workflows/ci.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 92d0b9611..d19d016be 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -58,7 +58,8 @@ jobs: conda activate conda-recipe-manager conda install -y -c conda-forge rattler-build convert -o recipe/recipe.yaml recipe/meta.yaml - rattler-build build -r recipe/ + mkdir -p ../temp + rattler-build build -r recipe/ --output-dir=../temp ## Integration tests ## integration-rattler: runs-on: ubuntu-latest From 8ef66431946db4dfbd0a77922654d8accb6c5201 Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Thu, 11 Apr 2024 08:31:17 -0600 Subject: [PATCH 06/10] Update tests/file_loading.py Co-authored-by: Ken Odegard --- tests/file_loading.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/file_loading.py b/tests/file_loading.py index 05af5d461..6859714bb 100644 --- a/tests/file_loading.py +++ b/tests/file_loading.py @@ -30,7 +30,7 @@ def load_recipe(file_name: str) -> RecipeParser: :param file_name: File name of the test recipe to load :returns: RecipeParser instance, based on the file """ - recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") + recipe = load_file(TEST_FILES_PATH / file_name) return RecipeParser(recipe) From a3076015c14f7ba9eb0099ef9bd0d66048af6fae Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Thu, 11 Apr 2024 08:31:24 -0600 Subject: [PATCH 07/10] Update tests/file_loading.py Co-authored-by: Ken Odegard --- tests/file_loading.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/file_loading.py b/tests/file_loading.py index 6859714bb..567cc34fe 100644 --- a/tests/file_loading.py +++ b/tests/file_loading.py @@ -40,5 +40,5 @@ def load_recipe_convert(file_name: str) -> RecipeParserConvert: :param file_name: File name of the test recipe to load :returns: RecipeParserConvert instance, based on the file """ - recipe = load_file(f"{TEST_FILES_PATH}/{file_name}") + recipe = load_file(TEST_FILES_PATH / file_name) return RecipeParserConvert(recipe) From 22df115db8050c0287a09eda043fe10924d755eb Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Thu, 11 Apr 2024 09:09:49 -0600 Subject: [PATCH 08/10] Incorporates feedback on file reading --- tests/file_loading.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/file_loading.py b/tests/file_loading.py index 567cc34fe..17a9791c5 100644 --- a/tests/file_loading.py +++ b/tests/file_loading.py @@ -20,8 +20,7 @@ def load_file(file: Path | str) -> str: :param file: Filename of the file to read :returns: Text from the file """ - with open(Path(file), "r", encoding="utf-8") as f: - return f.read() + return Path(file).read_text(encoding="utf-8") def load_recipe(file_name: str) -> RecipeParser: From 88b7a104499e88319ca50f9a6f9a6b2b3e95cb4b Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Thu, 11 Apr 2024 09:36:22 -0600 Subject: [PATCH 09/10] State refactor for `render_to_new_recipe_format()` - Moves state stored in `render_to_new_recipe_format()` to simplify logic - Moves inner-functions of `render_to_new_recipe_format()` to private members of the `RecipeParserConvert` subclass --- conda_recipe_manager/commands/convert.py | 1 + .../commands/rattler_bulk_build.py | 1 + conda_recipe_manager/parser/_node.py | 1 + conda_recipe_manager/parser/_selector_info.py | 1 + conda_recipe_manager/parser/_traverse.py | 1 + conda_recipe_manager/parser/_types.py | 1 + conda_recipe_manager/parser/_utils.py | 1 + conda_recipe_manager/parser/enums.py | 1 + conda_recipe_manager/parser/exceptions.py | 1 + conda_recipe_manager/parser/recipe_parser.py | 1 + .../parser/recipe_parser_convert.py | 251 ++++++++++-------- conda_recipe_manager/parser/types.py | 1 + conda_recipe_manager/types.py | 1 + tests/commands/test_convert_cli.py | 1 + tests/file_loading.py | 1 + tests/parser/test_recipe_parser.py | 1 + tests/parser/test_recipe_parser_convert.py | 1 + 17 files changed, 156 insertions(+), 111 deletions(-) diff --git a/conda_recipe_manager/commands/convert.py b/conda_recipe_manager/commands/convert.py index 62b62af8b..40b7b34ca 100644 --- a/conda_recipe_manager/commands/convert.py +++ b/conda_recipe_manager/commands/convert.py @@ -2,6 +2,7 @@ File: convert.py Description: CLI for converting an old recipe file to the "new" format. """ + from __future__ import annotations import json diff --git a/conda_recipe_manager/commands/rattler_bulk_build.py b/conda_recipe_manager/commands/rattler_bulk_build.py index 52ceffe75..05c257eb6 100644 --- a/conda_recipe_manager/commands/rattler_bulk_build.py +++ b/conda_recipe_manager/commands/rattler_bulk_build.py @@ -2,6 +2,7 @@ File: rattler_bulk_build.py Description: CLI tool that performs a bulk build operation for rattler-build. """ + from __future__ import annotations import json diff --git a/conda_recipe_manager/parser/_node.py b/conda_recipe_manager/parser/_node.py index 5520d482c..cdc09925c 100644 --- a/conda_recipe_manager/parser/_node.py +++ b/conda_recipe_manager/parser/_node.py @@ -2,6 +2,7 @@ File: _node.py Description: Provides a private node class only used by the parser. This class is fundamental to tree formation. """ + from __future__ import annotations from typing import Optional diff --git a/conda_recipe_manager/parser/_selector_info.py b/conda_recipe_manager/parser/_selector_info.py index a3c8d5b5e..1e814f9e4 100644 --- a/conda_recipe_manager/parser/_selector_info.py +++ b/conda_recipe_manager/parser/_selector_info.py @@ -2,6 +2,7 @@ File: _selector_info.py Description: Provides the `SelectorInfo` class, used to store selector information. """ + from __future__ import annotations from typing import NamedTuple diff --git a/conda_recipe_manager/parser/_traverse.py b/conda_recipe_manager/parser/_traverse.py index 02b9b6c20..cc1db7248 100644 --- a/conda_recipe_manager/parser/_traverse.py +++ b/conda_recipe_manager/parser/_traverse.py @@ -2,6 +2,7 @@ File: py Description: Provides tree traversal functions only used by the parser. """ + from __future__ import annotations from typing import Callable, Final, Optional diff --git a/conda_recipe_manager/parser/_types.py b/conda_recipe_manager/parser/_types.py index 886ee0591..401eb5094 100644 --- a/conda_recipe_manager/parser/_types.py +++ b/conda_recipe_manager/parser/_types.py @@ -2,6 +2,7 @@ File: _types.py Description: Provides private types, type aliases, constants, and small classes used by the parser and related files. """ + from __future__ import annotations import re diff --git a/conda_recipe_manager/parser/_utils.py b/conda_recipe_manager/parser/_utils.py index f981821ce..2e5f5f85a 100644 --- a/conda_recipe_manager/parser/_utils.py +++ b/conda_recipe_manager/parser/_utils.py @@ -2,6 +2,7 @@ File: _utils.py Description: Provides private utility functions only used by the parser. """ + from __future__ import annotations import json diff --git a/conda_recipe_manager/parser/enums.py b/conda_recipe_manager/parser/enums.py index b55f38ba7..167cd6e60 100644 --- a/conda_recipe_manager/parser/enums.py +++ b/conda_recipe_manager/parser/enums.py @@ -2,6 +2,7 @@ File: enums.py Description: Provides enumerated types used by the parser. """ + from __future__ import annotations from enum import Enum diff --git a/conda_recipe_manager/parser/exceptions.py b/conda_recipe_manager/parser/exceptions.py index 03a74e1b7..dff9c44f8 100644 --- a/conda_recipe_manager/parser/exceptions.py +++ b/conda_recipe_manager/parser/exceptions.py @@ -2,6 +2,7 @@ File: exceptions.py Description: Provides exceptions thrown by the parser. """ + from __future__ import annotations import json diff --git a/conda_recipe_manager/parser/recipe_parser.py b/conda_recipe_manager/parser/recipe_parser.py index cae37cd44..b0066b553 100644 --- a/conda_recipe_manager/parser/recipe_parser.py +++ b/conda_recipe_manager/parser/recipe_parser.py @@ -11,6 +11,7 @@ - https://jsonpatch.com/ - https://datatracker.ietf.org/doc/html/rfc6902/ """ + # Allows older versions of python to use newer forms of type annotation. There are major features introduced in >=3.9 from __future__ import annotations diff --git a/conda_recipe_manager/parser/recipe_parser_convert.py b/conda_recipe_manager/parser/recipe_parser_convert.py index ccb06f8e5..6da2c3729 100644 --- a/conda_recipe_manager/parser/recipe_parser_convert.py +++ b/conda_recipe_manager/parser/recipe_parser_convert.py @@ -4,6 +4,7 @@ format. This tooling was originally part of the base class, but was broken-out for easier/cleaner code maintenance. """ + from __future__ import annotations from typing import Final, cast @@ -28,6 +29,72 @@ class RecipeParserConvert(RecipeParser): This was originally part of the RecipeParser class but was broken-out for easier maintenance. """ + def __init__(self, content: str): + """ + Constructs a convertible recipe object. This extension of the parser class keeps a modified copy of the original + recipe to work on and tracks some debugging state. + :param content: conda-build formatted recipe file, as a single text string. + """ + super().__init__(content) + # `copy.deepcopy()` produced some bizarre artifacts, namely single-line comments were being incorrectly rendered + # as list members. Although inefficient, we have tests that validate round-tripping the parser and there + # is no development cost in utilizing tools we already must maintain. + self.new_recipe: RecipeParser = RecipeParser(self.render()) + self.msg_tbl = MessageTable() + + def _patch_and_log(self, patch: JsonPatchType) -> None: + """ + Convenience function that logs failed patches to the message table. + :param patch: Patch operation to perform + """ + if not self.new_recipe.patch(patch): + self.msg_tbl.add_message(MessageCategory.ERROR, f"Failed to patch: {patch}") + + def _patch_add_missing_path(self, base_path: str, ext: str, value: JsonType = None) -> None: + """ + Convenience function that constructs missing paths. Useful when you have to construct more than 1 path level at + once (the JSON patch standard only allows the creation of 1 new level at a time). + :param base_path: Base path, to be extended + :param ext: Extension to create the full path to check for + :param value: `value` field for the patch-add operation + """ + temp_path: Final[str] = RecipeParser.append_to_path(base_path, ext) + if self.new_recipe.contains_value(temp_path): + return + self._patch_and_log({"op": "add", "path": temp_path, "value": value}) + + def _patch_move_base_path(self, base_path: str, old_ext: str, new_ext: str) -> None: + """ + Convenience function that moves a value under an old path to a new one sharing a common base path BUT only if + the old path exists. + :param base_path: Shared base path from old and new locations + :param old_ext: Old extension to the base path containing the data to move + :param new_ext: New extension to the base path of where the data should go + """ + old_path: Final[str] = RecipeParser.append_to_path(base_path, old_ext) + if not self.new_recipe.contains_value(old_path): + return + self._patch_and_log({"op": "move", "from": old_path, "path": RecipeParser.append_to_path(base_path, new_ext)}) + + def _sort_subtree_keys(self, sort_path: str, tbl: dict[str, int], rename: str = "") -> None: + """ + Convenience function that sorts 1 level of keys, given a path. Optionally allows renaming of the target node. + :param sort_path: Top-level path to target sorting of child keys + :param tbl: Table describing how keys should be sorted. Lower-value key names appear towards the top of the list + :param rename: (Optional) If specified, renames the top-level key + """ + + def _comparison(n: Node) -> int: + return RecipeParser._canonical_sort_keys_comparison(n, tbl) + + node = traverse(self.new_recipe._root, str_to_stack_path(sort_path)) # pylint: disable=protected-access + if node is None: + self.msg_tbl.add_message(MessageCategory.WARNING, f"Failed to sort members of {sort_path}") + return + if rename: + node.value = rename + node.children.sort(key=_comparison) + def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # pylint: disable=protected-access """ @@ -43,79 +110,39 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # Approach: In the event that we want to expand support later, this function should be implemented in terms # of a `RecipeParser` tree. This will make it easier to build an upgrade-path, if we so choose to pursue one. - msg_tbl = MessageTable() - - # `copy.deepcopy()` produced some bizarre artifacts, namely single-line comments were being incorrectly rendered - # as list members. Although inefficient, we have tests that validate round-tripping the parser and there - # is no development cost in utilizing tools we already must maintain. - new_recipe: RecipeParser = RecipeParser(self.render()) - # Log the original - old_comments: Final[dict[str, str]] = new_recipe.get_comments_table() - - # Convenience wrapper that logs failed patches to the message table - def _patch_and_log(patch: JsonPatchType) -> None: - if not new_recipe.patch(patch): - msg_tbl.add_message(MessageCategory.ERROR, f"Failed to patch: {patch}") - - # Convenience function constructs missing paths. Useful when you have to construct more than 1 path level at - # once (the JSON patch standard only allows the creation of 1 new level at a time) - def _patch_add_missing_path(base_path: str, ext: str, value: JsonType = None) -> None: - temp_path: Final[str] = RecipeParser.append_to_path(base_path, ext) - if new_recipe.contains_value(temp_path): - return - _patch_and_log({"op": "add", "path": temp_path, "value": value}) - - # Convenience function that moves a value under an old path to a new one sharing a common base path BUT only if - # the old path exists. - def _patch_move_base_path(base_path: str, old_ext: str, new_ext: str) -> None: - old_path: Final[str] = RecipeParser.append_to_path(base_path, old_ext) - if not new_recipe.contains_value(old_path): - return - _patch_and_log({"op": "move", "from": old_path, "path": RecipeParser.append_to_path(base_path, new_ext)}) - - # Convenience function that sorts 1 level of keys, given a path. Optionally allows renaming of the target node. - def _sort_subtree_keys(sort_path: str, tbl: dict[str, int], rename: str = "") -> None: - def _comparison(n: Node) -> int: - return RecipeParser._canonical_sort_keys_comparison(n, tbl) - - node = traverse(new_recipe._root, str_to_stack_path(sort_path)) - if node is None: - msg_tbl.add_message(MessageCategory.WARNING, f"Failed to sort members of {sort_path}") - return - if rename: - node.value = rename - node.children.sort(key=_comparison) + # Log the original comments + old_comments: Final[dict[str, str]] = self.new_recipe.get_comments_table() ## JINJA -> `context` object ## # Convert the JINJA variable table to a `context` section. Empty tables still add the `context` section for # future developers' convenience. - _patch_and_log({"op": "add", "path": "/context", "value": None}) + self._patch_and_log({"op": "add", "path": "/context", "value": None}) # Filter-out any value not covered in the new format - for name, value in new_recipe._vars_tbl.items(): + for name, value in self.new_recipe._vars_tbl.items(): if not isinstance(value, (str, int, float, bool)): - msg_tbl.add_message(MessageCategory.WARNING, f"The variable `{name}` is an unsupported type.") + self.msg_tbl.add_message(MessageCategory.WARNING, f"The variable `{name}` is an unsupported type.") continue - _patch_and_log({"op": "add", "path": f"/context/{name}", "value": value}) + self._patch_and_log({"op": "add", "path": f"/context/{name}", "value": value}) # Similarly, patch-in the new `schema_version` value to the top of the file - _patch_and_log({"op": "add", "path": "/schema_version", "value": CURRENT_RECIPE_SCHEMA_FORMAT}) + self._patch_and_log({"op": "add", "path": "/schema_version", "value": CURRENT_RECIPE_SCHEMA_FORMAT}) # Swap all JINJA to use the new `${{ }}` format. - jinja_sub_locations: Final[list[str]] = new_recipe.search(Regex.JINJA_SUB) + jinja_sub_locations: Final[list[str]] = self.new_recipe.search(Regex.JINJA_SUB) for path in jinja_sub_locations: - value = new_recipe.get_value(path) + value = self.new_recipe.get_value(path) # Values that match the regex should only be strings. This prevents crashes that should not occur. if not isinstance(value, str): - msg_tbl.add_message( + self.msg_tbl.add_message( MessageCategory.WARNING, f"A non-string value was found as a JINJA substitution: {value}" ) continue value = value.replace("{{", "${{") - _patch_and_log({"op": "replace", "path": path, "value": value}) + self._patch_and_log({"op": "replace", "path": path, "value": value}) ## Convert selectors into ternary statements or `if` blocks ## - for selector, instances in new_recipe._selector_tbl.items(): + for selector, instances in self.new_recipe._selector_tbl.items(): for info in instances: # Selectors can be applied to the parent node if they appear on the same line. We'll ignore these when # building replacements. @@ -140,7 +167,7 @@ def _comparison(n: Node) -> int: if not isinstance(info.node.value, bool): # CEP-13 states that ONLY list members may use the `if/then/else` blocks if not info.node.list_member_flag: - msg_tbl.add_message( + self.msg_tbl.add_message( MessageCategory.WARNING, f"A non-list item had a selector at: {selector_path}" ) continue @@ -154,12 +181,12 @@ def _comparison(n: Node) -> int: "value": cast(JsonType, bool_object), } # Apply the patch - _patch_and_log(patch) - new_recipe.remove_selector(selector_path) + self._patch_and_log(patch) + self.new_recipe.remove_selector(selector_path) # Cached copy of all of the "outputs" in a recipe. This is useful for easily handling multi and single output # recipes in 1 loop construct. - base_package_paths: Final[list[str]] = new_recipe.get_package_paths() + base_package_paths: Final[list[str]] = self.new_recipe.get_package_paths() # TODO Fix: comments are not preserved with patch operations (add a flag to `patch()`?) @@ -170,33 +197,33 @@ def _comparison(n: Node) -> int: # `run_exports` old_re_path = RecipeParser.append_to_path(base_path, "/build/run_exports") - if new_recipe.contains_value(old_re_path): + if self.new_recipe.contains_value(old_re_path): requirements_path = RecipeParser.append_to_path(base_path, "/requirements") new_re_path = RecipeParser.append_to_path(base_path, "/requirements/run_exports") - if not new_recipe.contains_value(requirements_path): - _patch_and_log({"op": "add", "path": requirements_path, "value": None}) - _patch_and_log({"op": "move", "from": old_re_path, "path": new_re_path}) + if not self.new_recipe.contains_value(requirements_path): + self._patch_and_log({"op": "add", "path": requirements_path, "value": None}) + self._patch_and_log({"op": "move", "from": old_re_path, "path": new_re_path}) # `ignore_run_exports` old_ire_path = RecipeParser.append_to_path(base_path, "/build/ignore_run_exports") - if new_recipe.contains_value(old_re_path): + if self.new_recipe.contains_value(old_re_path): requirements_path = RecipeParser.append_to_path(base_path, "/requirements") new_ire_path = RecipeParser.append_to_path(base_path, "/requirements/ignore_run_exports") - if not new_recipe.contains_value(requirements_path): - _patch_and_log({"op": "add", "path": requirements_path, "value": None}) - _patch_and_log({"op": "move", "from": old_ire_path, "path": new_ire_path}) + if not self.new_recipe.contains_value(requirements_path): + self._patch_and_log({"op": "add", "path": requirements_path, "value": None}) + self._patch_and_log({"op": "move", "from": old_ire_path, "path": new_ire_path}) # Perform internal section changes per `build/` section build_path = RecipeParser.append_to_path(base_path, "/build") - if not new_recipe.contains_value(build_path): + if not self.new_recipe.contains_value(build_path): continue # `build/entry_points` -> `build/python/entry_points` - if new_recipe.contains_value(RecipeParser.append_to_path(build_path, "/entry_points")): - _patch_add_missing_path(build_path, "/python") - _patch_move_base_path(build_path, "/entry_points", "/python/entry_points") + if self.new_recipe.contains_value(RecipeParser.append_to_path(build_path, "/entry_points")): + self._patch_add_missing_path(build_path, "/python") + self._patch_move_base_path(build_path, "/entry_points", "/python/entry_points") # Canonically sort this section - _sort_subtree_keys(build_path, V1_BUILD_SECTION_KEY_SORT_ORDER) + self._sort_subtree_keys(build_path, V1_BUILD_SECTION_KEY_SORT_ORDER) ## `about` section changes and validation ## @@ -210,8 +237,8 @@ def _comparison(n: Node) -> int: ] for field in about_required: path = f"/about/{field}" - if not new_recipe.contains_value(path): - msg_tbl.add_message(MessageCategory.WARNING, f"Required field missing: {path}") + if not self.new_recipe.contains_value(path): + self.msg_tbl.add_message(MessageCategory.WARNING, f"Required field missing: {path}") # Transform renamed fields about_rename: Final[list[tuple[str, str]]] = [ @@ -220,7 +247,7 @@ def _comparison(n: Node) -> int: ("doc_url", "documentation"), ] for old, new in about_rename: - _patch_move_base_path("/about", old, new) + self._patch_move_base_path("/about", old, new) # TODO validate: /about/license must be SPDX recognized. @@ -235,8 +262,8 @@ def _comparison(n: Node) -> int: ] for field in about_deprecated: path = f"/about/{field}" - if new_recipe.contains_value(path): - _patch_and_log({"op": "remove", "path": path}) + if self.new_recipe.contains_value(path): + self._patch_and_log({"op": "remove", "path": path}) ## `test` section changes and upgrades ## @@ -244,19 +271,19 @@ def _comparison(n: Node) -> int: # have to use their best judgement to manually break-up the test into multiple tests as they see fit. for base_path in base_package_paths: test_path = RecipeParser.append_to_path(base_path, "/test") - if not new_recipe.contains_value(test_path): + if not self.new_recipe.contains_value(test_path): continue # Moving `files` to `files/recipe` is not possible in a single `move` operation as a new path has to be # created in the path being moved. test_files_path = RecipeParser.append_to_path(test_path, "/files") - if new_recipe.contains_value(test_files_path): - test_files_value = new_recipe.get_value(test_files_path) + if self.new_recipe.contains_value(test_files_path): + test_files_value = self.new_recipe.get_value(test_files_path) # TODO: Fix, replace does not work here, produces `- null`, Issue #20 - # _patch_and_log({"op": "replace", "path": test_files_path, "value": None}) - _patch_and_log({"op": "remove", "path": test_files_path}) - _patch_and_log({"op": "add", "path": test_files_path, "value": None}) - _patch_and_log( + # self._patch_and_log({"op": "replace", "path": test_files_path, "value": None}) + self._patch_and_log({"op": "remove", "path": test_files_path}) + self._patch_and_log({"op": "add", "path": test_files_path, "value": None}) + self._patch_and_log( { "op": "add", "path": RecipeParser.append_to_path(test_files_path, "/recipe"), @@ -264,38 +291,40 @@ def _comparison(n: Node) -> int: } ) # Edge case: `/source_files` exists but `/files` does not - elif new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/source_files")): - _patch_add_missing_path(test_path, "/files") - _patch_move_base_path(test_path, "/source_files", "/files/source") + elif self.new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/source_files")): + self._patch_add_missing_path(test_path, "/files") + self._patch_move_base_path(test_path, "/source_files", "/files/source") - if new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/requires")): - _patch_add_missing_path(test_path, "/requirements") - _patch_move_base_path(test_path, "/requires", "/requirements/run") + if self.new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/requires")): + self._patch_add_missing_path(test_path, "/requirements") + self._patch_move_base_path(test_path, "/requires", "/requirements/run") # Replace `- pip check` in `commands` with the new flag. If not found, set the flag to `False` (as the # flag defaults to `True`). - commands = cast(list[str], new_recipe.get_value(RecipeParser.append_to_path(test_path, "/commands"), [])) + commands = cast( + list[str], self.new_recipe.get_value(RecipeParser.append_to_path(test_path, "/commands"), []) + ) pip_check = False for i, command in enumerate(commands): if command != "pip check": continue # For now, we will only patch-out the first instance when no selector is attached # TODO Future: handle selector logic/cases with `pip check || ` - _patch_and_log({"op": "remove", "path": RecipeParser.append_to_path(test_path, f"/commands/{i}")}) + self._patch_and_log({"op": "remove", "path": RecipeParser.append_to_path(test_path, f"/commands/{i}")}) pip_check = True break - _patch_add_missing_path(test_path, "/python") - _patch_and_log( + self._patch_add_missing_path(test_path, "/python") + self._patch_and_log( {"op": "add", "path": RecipeParser.append_to_path(test_path, "/python/pip_check"), "value": pip_check} ) - _patch_move_base_path(test_path, "/commands", "/script") - _patch_move_base_path(test_path, "/imports", "/python/imports") - _patch_move_base_path(test_path, "/downstreams", "/downstream") + self._patch_move_base_path(test_path, "/commands", "/script") + self._patch_move_base_path(test_path, "/imports", "/python/imports") + self._patch_move_base_path(test_path, "/downstreams", "/downstream") # Move `test` to `tests` and encapsulate the pre-existing object into a list new_test_path = f"{test_path}s" - test_element = cast(dict[str, JsonType], new_recipe.get_value(test_path)) + test_element = cast(dict[str, JsonType], self.new_recipe.get_value(test_path)) test_array: list[JsonType] = [] # There are 3 types of test elements. We break them out of the original object, if they exist. # `Python` Test Element @@ -309,41 +338,41 @@ def _comparison(n: Node) -> int: # What remains should be the `Command` Test Element type if test_element: test_array.append(test_element) - _patch_and_log({"op": "add", "path": new_test_path, "value": test_array}) - _patch_and_log({"op": "remove", "path": test_path}) + self._patch_and_log({"op": "add", "path": new_test_path, "value": test_array}) + self._patch_and_log({"op": "remove", "path": test_path}) ## Upgrade the multi-output section(s) ## # TODO Complete - if new_recipe.contains_value("/outputs"): + if self.new_recipe.contains_value("/outputs"): # On the top-level, `package` -> `recipe` - _patch_move_base_path(ROOT_NODE_VALUE, "/package", "/recipe") + self._patch_move_base_path(ROOT_NODE_VALUE, "/package", "/recipe") for output_path in base_package_paths: if output_path == ROOT_NODE_VALUE: continue # Move `name` and `version` under `package` - if new_recipe.contains_value( + if self.new_recipe.contains_value( RecipeParser.append_to_path(output_path, "/name") - ) or new_recipe.contains_value(RecipeParser.append_to_path(output_path, "/version")): - _patch_add_missing_path(output_path, "/package") - _patch_move_base_path(output_path, "/name", "/package/name") - _patch_move_base_path(output_path, "/version", "/package/version") + ) or self.new_recipe.contains_value(RecipeParser.append_to_path(output_path, "/version")): + self._patch_add_missing_path(output_path, "/package") + self._patch_move_base_path(output_path, "/name", "/package/name") + self._patch_move_base_path(output_path, "/version", "/package/version") # Not all the top-level keys are found in each output section, but all the output section keys are # found at the top-level. So for consistency, we sort on that ordering. - _sort_subtree_keys(output_path, TOP_LEVEL_KEY_SORT_ORDER) + self._sort_subtree_keys(output_path, TOP_LEVEL_KEY_SORT_ORDER) ## Final clean-up ## # TODO: Comment tracking may need improvement. The "correct way" of tracking comments with patch changes is a # fairly big engineering effort and refactor. # Alert the user which comments have been dropped. - new_comments: Final[dict[str, str]] = new_recipe.get_comments_table() + new_comments: Final[dict[str, str]] = self.new_recipe.get_comments_table() diff_comments: Final[dict[str, str]] = {k: v for k, v in old_comments.items() if k not in new_comments} for path, comment in diff_comments.items(): - if not new_recipe.contains_value(path): - msg_tbl.add_message(MessageCategory.WARNING, f"Could not relocate comment: {comment}") + if not self.new_recipe.contains_value(path): + self.msg_tbl.add_message(MessageCategory.WARNING, f"Could not relocate comment: {comment}") # TODO Complete: move operations may result in empty fields we can eliminate. This may require changes to # `contains_value()` @@ -351,10 +380,10 @@ def _comparison(n: Node) -> int: # risk of screwing up critical list indices and ordering. # Hack: Wipe the existing table so the JINJA `set` statements don't render the final form - new_recipe._vars_tbl = {} + self.new_recipe._vars_tbl = {} # Sort the top-level keys to a "canonical" ordering. This should make previous patch operations look more # "sensible" to a human reader. - _sort_subtree_keys("/", TOP_LEVEL_KEY_SORT_ORDER) + self._sort_subtree_keys("/", TOP_LEVEL_KEY_SORT_ORDER) - return new_recipe.render(), msg_tbl + return self.new_recipe.render(), self.msg_tbl diff --git a/conda_recipe_manager/parser/types.py b/conda_recipe_manager/parser/types.py index c5dd75432..03550771b 100644 --- a/conda_recipe_manager/parser/types.py +++ b/conda_recipe_manager/parser/types.py @@ -2,6 +2,7 @@ File: types.py Description: Provides public types, type aliases, constants, and small classes used by the parser. """ + from __future__ import annotations from enum import StrEnum, auto diff --git a/conda_recipe_manager/types.py b/conda_recipe_manager/types.py index 57ced2aad..fb6c16223 100644 --- a/conda_recipe_manager/types.py +++ b/conda_recipe_manager/types.py @@ -2,6 +2,7 @@ File: types.py Description: Provides public types, type aliases, constants, and small classes used by all modules. """ + from __future__ import annotations from typing import Final, Hashable, TypeVar, Union diff --git a/tests/commands/test_convert_cli.py b/tests/commands/test_convert_cli.py index ad99536dd..1dc267268 100644 --- a/tests/commands/test_convert_cli.py +++ b/tests/commands/test_convert_cli.py @@ -2,6 +2,7 @@ File: test_convert_cli.py Description: Tests the `convert` CLI """ + from click.testing import CliRunner from conda_recipe_manager.commands.convert import convert diff --git a/tests/file_loading.py b/tests/file_loading.py index 17a9791c5..aacf940d8 100644 --- a/tests/file_loading.py +++ b/tests/file_loading.py @@ -2,6 +2,7 @@ File: file_loading.py Description: Constants and utilities used for loading files/recipes. """ + from __future__ import annotations from pathlib import Path diff --git a/tests/parser/test_recipe_parser.py b/tests/parser/test_recipe_parser.py index 21d9781af..4771e5e68 100644 --- a/tests/parser/test_recipe_parser.py +++ b/tests/parser/test_recipe_parser.py @@ -2,6 +2,7 @@ File: test_recipe_parser.py Description: Unit tests for the RecipeParser class """ + from __future__ import annotations from typing import Final diff --git a/tests/parser/test_recipe_parser_convert.py b/tests/parser/test_recipe_parser_convert.py index 37071f753..8942732ee 100644 --- a/tests/parser/test_recipe_parser_convert.py +++ b/tests/parser/test_recipe_parser_convert.py @@ -2,6 +2,7 @@ File: test_recipe_parser_convert.py Description: Unit tests for the RecipeParserConvert class """ + from __future__ import annotations import pytest From 9d9f8b89da36b88f237cecc0c48ef4a34c0380af Mon Sep 17 00:00:00 2001 From: Schuyler Martin Date: Thu, 11 Apr 2024 09:39:41 -0600 Subject: [PATCH 10/10] Marks RecipeParserConvert state as private --- .../parser/recipe_parser_convert.py | 84 +++++++++---------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/conda_recipe_manager/parser/recipe_parser_convert.py b/conda_recipe_manager/parser/recipe_parser_convert.py index 6da2c3729..5ccfc09cf 100644 --- a/conda_recipe_manager/parser/recipe_parser_convert.py +++ b/conda_recipe_manager/parser/recipe_parser_convert.py @@ -39,16 +39,16 @@ def __init__(self, content: str): # `copy.deepcopy()` produced some bizarre artifacts, namely single-line comments were being incorrectly rendered # as list members. Although inefficient, we have tests that validate round-tripping the parser and there # is no development cost in utilizing tools we already must maintain. - self.new_recipe: RecipeParser = RecipeParser(self.render()) - self.msg_tbl = MessageTable() + self._new_recipe: RecipeParser = RecipeParser(self.render()) + self._msg_tbl = MessageTable() def _patch_and_log(self, patch: JsonPatchType) -> None: """ Convenience function that logs failed patches to the message table. :param patch: Patch operation to perform """ - if not self.new_recipe.patch(patch): - self.msg_tbl.add_message(MessageCategory.ERROR, f"Failed to patch: {patch}") + if not self._new_recipe.patch(patch): + self._msg_tbl.add_message(MessageCategory.ERROR, f"Failed to patch: {patch}") def _patch_add_missing_path(self, base_path: str, ext: str, value: JsonType = None) -> None: """ @@ -59,7 +59,7 @@ def _patch_add_missing_path(self, base_path: str, ext: str, value: JsonType = No :param value: `value` field for the patch-add operation """ temp_path: Final[str] = RecipeParser.append_to_path(base_path, ext) - if self.new_recipe.contains_value(temp_path): + if self._new_recipe.contains_value(temp_path): return self._patch_and_log({"op": "add", "path": temp_path, "value": value}) @@ -72,7 +72,7 @@ def _patch_move_base_path(self, base_path: str, old_ext: str, new_ext: str) -> N :param new_ext: New extension to the base path of where the data should go """ old_path: Final[str] = RecipeParser.append_to_path(base_path, old_ext) - if not self.new_recipe.contains_value(old_path): + if not self._new_recipe.contains_value(old_path): return self._patch_and_log({"op": "move", "from": old_path, "path": RecipeParser.append_to_path(base_path, new_ext)}) @@ -87,9 +87,9 @@ def _sort_subtree_keys(self, sort_path: str, tbl: dict[str, int], rename: str = def _comparison(n: Node) -> int: return RecipeParser._canonical_sort_keys_comparison(n, tbl) - node = traverse(self.new_recipe._root, str_to_stack_path(sort_path)) # pylint: disable=protected-access + node = traverse(self._new_recipe._root, str_to_stack_path(sort_path)) # pylint: disable=protected-access if node is None: - self.msg_tbl.add_message(MessageCategory.WARNING, f"Failed to sort members of {sort_path}") + self._msg_tbl.add_message(MessageCategory.WARNING, f"Failed to sort members of {sort_path}") return if rename: node.value = rename @@ -111,7 +111,7 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # of a `RecipeParser` tree. This will make it easier to build an upgrade-path, if we so choose to pursue one. # Log the original comments - old_comments: Final[dict[str, str]] = self.new_recipe.get_comments_table() + old_comments: Final[dict[str, str]] = self._new_recipe.get_comments_table() ## JINJA -> `context` object ## @@ -119,9 +119,9 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # future developers' convenience. self._patch_and_log({"op": "add", "path": "/context", "value": None}) # Filter-out any value not covered in the new format - for name, value in self.new_recipe._vars_tbl.items(): + for name, value in self._new_recipe._vars_tbl.items(): if not isinstance(value, (str, int, float, bool)): - self.msg_tbl.add_message(MessageCategory.WARNING, f"The variable `{name}` is an unsupported type.") + self._msg_tbl.add_message(MessageCategory.WARNING, f"The variable `{name}` is an unsupported type.") continue self._patch_and_log({"op": "add", "path": f"/context/{name}", "value": value}) @@ -129,12 +129,12 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: self._patch_and_log({"op": "add", "path": "/schema_version", "value": CURRENT_RECIPE_SCHEMA_FORMAT}) # Swap all JINJA to use the new `${{ }}` format. - jinja_sub_locations: Final[list[str]] = self.new_recipe.search(Regex.JINJA_SUB) + jinja_sub_locations: Final[list[str]] = self._new_recipe.search(Regex.JINJA_SUB) for path in jinja_sub_locations: - value = self.new_recipe.get_value(path) + value = self._new_recipe.get_value(path) # Values that match the regex should only be strings. This prevents crashes that should not occur. if not isinstance(value, str): - self.msg_tbl.add_message( + self._msg_tbl.add_message( MessageCategory.WARNING, f"A non-string value was found as a JINJA substitution: {value}" ) continue @@ -142,7 +142,7 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: self._patch_and_log({"op": "replace", "path": path, "value": value}) ## Convert selectors into ternary statements or `if` blocks ## - for selector, instances in self.new_recipe._selector_tbl.items(): + for selector, instances in self._new_recipe._selector_tbl.items(): for info in instances: # Selectors can be applied to the parent node if they appear on the same line. We'll ignore these when # building replacements. @@ -167,7 +167,7 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: if not isinstance(info.node.value, bool): # CEP-13 states that ONLY list members may use the `if/then/else` blocks if not info.node.list_member_flag: - self.msg_tbl.add_message( + self._msg_tbl.add_message( MessageCategory.WARNING, f"A non-list item had a selector at: {selector_path}" ) continue @@ -182,11 +182,11 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: } # Apply the patch self._patch_and_log(patch) - self.new_recipe.remove_selector(selector_path) + self._new_recipe.remove_selector(selector_path) # Cached copy of all of the "outputs" in a recipe. This is useful for easily handling multi and single output # recipes in 1 loop construct. - base_package_paths: Final[list[str]] = self.new_recipe.get_package_paths() + base_package_paths: Final[list[str]] = self._new_recipe.get_package_paths() # TODO Fix: comments are not preserved with patch operations (add a flag to `patch()`?) @@ -197,28 +197,28 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # `run_exports` old_re_path = RecipeParser.append_to_path(base_path, "/build/run_exports") - if self.new_recipe.contains_value(old_re_path): + if self._new_recipe.contains_value(old_re_path): requirements_path = RecipeParser.append_to_path(base_path, "/requirements") new_re_path = RecipeParser.append_to_path(base_path, "/requirements/run_exports") - if not self.new_recipe.contains_value(requirements_path): + if not self._new_recipe.contains_value(requirements_path): self._patch_and_log({"op": "add", "path": requirements_path, "value": None}) self._patch_and_log({"op": "move", "from": old_re_path, "path": new_re_path}) # `ignore_run_exports` old_ire_path = RecipeParser.append_to_path(base_path, "/build/ignore_run_exports") - if self.new_recipe.contains_value(old_re_path): + if self._new_recipe.contains_value(old_re_path): requirements_path = RecipeParser.append_to_path(base_path, "/requirements") new_ire_path = RecipeParser.append_to_path(base_path, "/requirements/ignore_run_exports") - if not self.new_recipe.contains_value(requirements_path): + if not self._new_recipe.contains_value(requirements_path): self._patch_and_log({"op": "add", "path": requirements_path, "value": None}) self._patch_and_log({"op": "move", "from": old_ire_path, "path": new_ire_path}) # Perform internal section changes per `build/` section build_path = RecipeParser.append_to_path(base_path, "/build") - if not self.new_recipe.contains_value(build_path): + if not self._new_recipe.contains_value(build_path): continue # `build/entry_points` -> `build/python/entry_points` - if self.new_recipe.contains_value(RecipeParser.append_to_path(build_path, "/entry_points")): + if self._new_recipe.contains_value(RecipeParser.append_to_path(build_path, "/entry_points")): self._patch_add_missing_path(build_path, "/python") self._patch_move_base_path(build_path, "/entry_points", "/python/entry_points") @@ -237,8 +237,8 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: ] for field in about_required: path = f"/about/{field}" - if not self.new_recipe.contains_value(path): - self.msg_tbl.add_message(MessageCategory.WARNING, f"Required field missing: {path}") + if not self._new_recipe.contains_value(path): + self._msg_tbl.add_message(MessageCategory.WARNING, f"Required field missing: {path}") # Transform renamed fields about_rename: Final[list[tuple[str, str]]] = [ @@ -262,7 +262,7 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: ] for field in about_deprecated: path = f"/about/{field}" - if self.new_recipe.contains_value(path): + if self._new_recipe.contains_value(path): self._patch_and_log({"op": "remove", "path": path}) ## `test` section changes and upgrades ## @@ -271,14 +271,14 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # have to use their best judgement to manually break-up the test into multiple tests as they see fit. for base_path in base_package_paths: test_path = RecipeParser.append_to_path(base_path, "/test") - if not self.new_recipe.contains_value(test_path): + if not self._new_recipe.contains_value(test_path): continue # Moving `files` to `files/recipe` is not possible in a single `move` operation as a new path has to be # created in the path being moved. test_files_path = RecipeParser.append_to_path(test_path, "/files") - if self.new_recipe.contains_value(test_files_path): - test_files_value = self.new_recipe.get_value(test_files_path) + if self._new_recipe.contains_value(test_files_path): + test_files_value = self._new_recipe.get_value(test_files_path) # TODO: Fix, replace does not work here, produces `- null`, Issue #20 # self._patch_and_log({"op": "replace", "path": test_files_path, "value": None}) self._patch_and_log({"op": "remove", "path": test_files_path}) @@ -291,18 +291,18 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: } ) # Edge case: `/source_files` exists but `/files` does not - elif self.new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/source_files")): + elif self._new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/source_files")): self._patch_add_missing_path(test_path, "/files") self._patch_move_base_path(test_path, "/source_files", "/files/source") - if self.new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/requires")): + if self._new_recipe.contains_value(RecipeParser.append_to_path(test_path, "/requires")): self._patch_add_missing_path(test_path, "/requirements") self._patch_move_base_path(test_path, "/requires", "/requirements/run") # Replace `- pip check` in `commands` with the new flag. If not found, set the flag to `False` (as the # flag defaults to `True`). commands = cast( - list[str], self.new_recipe.get_value(RecipeParser.append_to_path(test_path, "/commands"), []) + list[str], self._new_recipe.get_value(RecipeParser.append_to_path(test_path, "/commands"), []) ) pip_check = False for i, command in enumerate(commands): @@ -324,7 +324,7 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # Move `test` to `tests` and encapsulate the pre-existing object into a list new_test_path = f"{test_path}s" - test_element = cast(dict[str, JsonType], self.new_recipe.get_value(test_path)) + test_element = cast(dict[str, JsonType], self._new_recipe.get_value(test_path)) test_array: list[JsonType] = [] # There are 3 types of test elements. We break them out of the original object, if they exist. # `Python` Test Element @@ -343,7 +343,7 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: ## Upgrade the multi-output section(s) ## # TODO Complete - if self.new_recipe.contains_value("/outputs"): + if self._new_recipe.contains_value("/outputs"): # On the top-level, `package` -> `recipe` self._patch_move_base_path(ROOT_NODE_VALUE, "/package", "/recipe") @@ -352,9 +352,9 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: continue # Move `name` and `version` under `package` - if self.new_recipe.contains_value( + if self._new_recipe.contains_value( RecipeParser.append_to_path(output_path, "/name") - ) or self.new_recipe.contains_value(RecipeParser.append_to_path(output_path, "/version")): + ) or self._new_recipe.contains_value(RecipeParser.append_to_path(output_path, "/version")): self._patch_add_missing_path(output_path, "/package") self._patch_move_base_path(output_path, "/name", "/package/name") self._patch_move_base_path(output_path, "/version", "/package/version") @@ -368,11 +368,11 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # TODO: Comment tracking may need improvement. The "correct way" of tracking comments with patch changes is a # fairly big engineering effort and refactor. # Alert the user which comments have been dropped. - new_comments: Final[dict[str, str]] = self.new_recipe.get_comments_table() + new_comments: Final[dict[str, str]] = self._new_recipe.get_comments_table() diff_comments: Final[dict[str, str]] = {k: v for k, v in old_comments.items() if k not in new_comments} for path, comment in diff_comments.items(): - if not self.new_recipe.contains_value(path): - self.msg_tbl.add_message(MessageCategory.WARNING, f"Could not relocate comment: {comment}") + if not self._new_recipe.contains_value(path): + self._msg_tbl.add_message(MessageCategory.WARNING, f"Could not relocate comment: {comment}") # TODO Complete: move operations may result in empty fields we can eliminate. This may require changes to # `contains_value()` @@ -380,10 +380,10 @@ def render_to_new_recipe_format(self) -> tuple[str, MessageTable]: # risk of screwing up critical list indices and ordering. # Hack: Wipe the existing table so the JINJA `set` statements don't render the final form - self.new_recipe._vars_tbl = {} + self._new_recipe._vars_tbl = {} # Sort the top-level keys to a "canonical" ordering. This should make previous patch operations look more # "sensible" to a human reader. self._sort_subtree_keys("/", TOP_LEVEL_KEY_SORT_ORDER) - return self.new_recipe.render(), self.msg_tbl + return self._new_recipe.render(), self._msg_tbl