From 43a8c40d19adfb9651b85ca3c8d66c502e24c417 Mon Sep 17 00:00:00 2001 From: Charles Bousseau Date: Thu, 10 Nov 2022 15:54:14 -0500 Subject: [PATCH 1/6] update jinja and selectors handling --- .flake8 | 2 +- anaconda_linter/recipe.py | 136 ++++++++++---------------------------- 2 files changed, 36 insertions(+), 102 deletions(-) diff --git a/.flake8 b/.flake8 index a70982cf..8166c398 100644 --- a/.flake8 +++ b/.flake8 @@ -1,3 +1,3 @@ [flake8] -max-line-length = 100 +max-line-length = 120 ignore = E203, W503 diff --git a/anaconda_linter/recipe.py b/anaconda_linter/recipe.py index 5c2dc4ff..426a4f6a 100644 --- a/anaconda_linter/recipe.py +++ b/anaconda_linter/recipe.py @@ -14,7 +14,6 @@ import sys import tempfile import types -from collections import defaultdict from contextlib import redirect_stderr, redirect_stdout from copy import deepcopy from io import StringIO @@ -151,7 +150,8 @@ class Recipe: JINJA_VARS = { "cran_mirror": "https://cloud.r-project.org", "compiler": lambda x: f"compiler_{x}", - "pin_compatible": lambda x, max_pin=None, min_pin=None: f"{x}", + "pin_compatible": lambda x, max_pin=None, min_pin=None, lower_bound=None, upper_bound=None: f"{x}", + "pin_subpackage": lambda x, max_pin=None, min_pin=None, exact=False: f"{x}", "cdt": lambda x: x, } @@ -171,6 +171,8 @@ def __init__(self, recipe_dir): # These will be filled in by load_from_string() #: Lines of the raw recipe file self.meta_yaml: List[str] = [] + #: Selectors configuration + self.selector_dict: Dict[str, Any] = {} # Filled in by update filter self.version_data: Dict[str, Any] = {} #: Original recipe before modifications (updated by load_from_string) @@ -200,27 +202,40 @@ def __str__(self) -> str: def __repr__(self) -> str: return f'{self.__class__.__name__} "{self.recipe_dir}"' - def load_from_string(self, data, selector_dict) -> "Recipe": + def load_from_string(self, data) -> "Recipe": """Load and `render` recipe contents from disk""" - self.meta_yaml = self.apply_selector(data, selector_dict) + self.meta_yaml = data if not self.meta_yaml: raise EmptyRecipe(self) self.render() return self def read_conda_build_config( - self, variant_config_files: List[str] = [], exclusive_config_files: List[str] = [] + self, + selector_dict, + variant_config_files: List[str] = [], + exclusive_config_files: List[str] = [], ): + self.selector_dict = selector_dict + # List conda_build_config files for linter render. self.conda_build_config_files = utils.find_config_files( self.recipe_dir, variant_config_files, exclusive_config_files ) - # Cache contents of conda_build_config.yaml for conda_render. - path = Path(self.recipe_dir, "conda_build_config.yaml") - if path.is_file(): - self.conda_build_config = path.read_text() - else: - self.conda_build_config = "" + # Update selector dict + for cbc in self.conda_build_config_files: + with open(cbc) as f_cbc: + try: + cbc_selectors_str = self.apply_selector(f_cbc.read(), self.selector_dict) + cbc_selectors_yml = yaml.load("\n".join(cbc_selectors_str)) + if cbc_selectors_yml: + for k, v in cbc_selectors_yml.items(): + if type(v) is list: + self.selector_dict[k] = v[-1] + except DuplicateKeyError as err: + line = err.problem_mark.line + 1 + column = err.problem_mark.column + 1 + raise DuplicateKey(self, line=line, column=column) def read_build_scripts(self): # Cache contents of build scripts for conda_render since conda-build @@ -271,19 +286,10 @@ def from_string( """Create new `Recipe` object from string""" try: recipe = cls("") - recipe.load_from_string(recipe_text, selector_dict) - except Exception as exc: - if return_exceptions: - return exc - raise exc - try: - recipe.read_conda_build_config(variant_config_files, exclusive_config_files) - except Exception as exc: - if return_exceptions: - return exc - raise exc - try: - recipe.read_build_scripts() + recipe.read_conda_build_config( + selector_dict, variant_config_files, exclusive_config_files + ) + recipe.load_from_string(recipe_text) except Exception as exc: if return_exceptions: return exc @@ -308,9 +314,10 @@ def from_file( if recipe_fname.endswith("meta.yaml"): recipe_fname = os.path.dirname(recipe_fname) recipe = cls(recipe_fname) + recipe.read_conda_build_config(selector_dict, variant_config_files, exclusive_config_files) try: with open(os.path.join(recipe_fname, "meta.yaml")) as text: - recipe.load_from_string(text.read(), selector_dict) + recipe.load_from_string(text.read()) except FileNotFoundError: exc = MissingMetaYaml(recipe_fname) if return_exceptions: @@ -320,18 +327,6 @@ def from_file( if return_exceptions: return exc raise exc - try: - recipe.read_conda_build_config(variant_config_files, exclusive_config_files) - except Exception as exc: - if return_exceptions: - return exc - raise exc - try: - recipe.read_build_scripts() - except Exception as exc: - if return_exceptions: - return exc - raise exc recipe.set_original() return recipe @@ -350,57 +345,6 @@ def dump(self): """Dump recipe content""" return "\n".join(self.meta_yaml) + "\n" - @staticmethod - def _rewrite_selector_block(text, block_top, block_left): - if not block_left: - return None # never the whole yaml - lines = text.splitlines() - block_height = 0 - variants: Dict[str, List[str]] = defaultdict(list) - - for block_height, line in enumerate(lines[block_top:]): - if line.strip() and not line.startswith(" " * block_left): - break - _, _, selector = line.partition("#") - if selector: - variants[selector.strip("[] ")].append(line) - else: - for variant in variants: - variants[variant].append(line) - else: - # end of file, need to add one to block height - block_height += 1 - - if not block_height: # empty lines? - return None - if not variants: - return None - if any(" " in v for v in variants): - # can't handle "[py2k or osx]" style things - return None - - new_lines = [] - for variant in variants.values(): - first = True - for line in variant: - if first: - new_lines.append("".join((" " * block_left, "- ", line))) - first = False - else: - new_lines.append("".join((" " * (block_left + 2), line))) - - logger.debug( - "Replacing: lines %i - %i with %i lines:\n%s\n---\n%s", - block_top, - block_top + block_height, - len(new_lines), - "\n".join(lines[block_top : block_top + block_height]), - "\n".join(new_lines), - ) - - lines[block_top : block_top + block_height] = new_lines - return "\n".join(lines) - def apply_selector(self, data, selector_dict): """Apply selectors # [...]""" updated_data = [] @@ -422,7 +366,8 @@ def get_template(self): # Storing it means the recipe cannot be pickled, which in turn # means we cannot pass it to ProcessExecutors. try: - return utils.jinja_silent_undef.from_string("\n".join(self.meta_yaml)) + meta_yaml_selectors_applied = self.apply_selector(self.meta_yaml, self.selector_dict) + return utils.jinja_silent_undef.from_string("\n".join(meta_yaml_selectors_applied)) except jinja2.exceptions.TemplateSyntaxError as exc: raise RenderFailure(self, message=exc.message, line=exc.lineno) except jinja2.exceptions.TemplateError as exc: @@ -458,18 +403,7 @@ def render(self) -> None: except DuplicateKeyError as err: line = err.problem_mark.line + 1 column = err.problem_mark.column + 1 - logger.debug("fixing duplicate key at %i:%i", line, column) - # We may have encountered a recipe with linux/osx variants using line selectors - yaml_text = self._rewrite_selector_block( - yaml_text, err.context_mark.line, err.context_mark.column - ) - if yaml_text: - try: - self.meta = yaml.load(yaml_text) - except DuplicateKeyError: - raise DuplicateKey(self, line=line, column=column) - else: - raise DuplicateKey(self, line=line, column=column) + raise DuplicateKey(self, line=line, column=column) if ( "package" not in self.meta From f6c9c56225da4f9728154743f1a00bf31875c954 Mon Sep 17 00:00:00 2001 From: Charles Bousseau Date: Thu, 10 Nov 2022 17:43:31 -0500 Subject: [PATCH 2/6] add tests and changelog --- CHANGELOG.md | 9 +++++ anaconda_linter/recipe.py | 9 ++++- tests/test_lint.py | 69 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb4e2b24..f92de1aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ # Changelog Note: version releases in the 0.x.y range may introduce breaking changes. +## 0.0.5 + +Bug fixes: +- Correct mocks of conda-build jinja functions. PR: #131, Issues: #118, #126 +- Correct error line reporting. PR: #131, Issues: #123 + +Enhancements: +- Render recipe using cbc files defined variables. PR: #131 + ## 0.0.4 - Add test suite diff --git a/anaconda_linter/recipe.py b/anaconda_linter/recipe.py index 426a4f6a..db0c6ac5 100644 --- a/anaconda_linter/recipe.py +++ b/anaconda_linter/recipe.py @@ -397,7 +397,14 @@ def render(self) -> None: - parse yaml - normalize """ - yaml_text = self.get_template().render(self.JINJA_VARS) + try: + yaml_text = self.get_template().render(self.JINJA_VARS) + except jinja2.exceptions.TemplateSyntaxError as exc: + raise RenderFailure(self, message=exc.message, line=exc.lineno) + except jinja2.exceptions.TemplateError as exc: + raise RenderFailure(self, message=exc.message) + except TypeError as exc: + raise RenderFailure(self, message=str(exc)) try: self.meta = yaml.load(yaml_text) except DuplicateKeyError as err: diff --git a/tests/test_lint.py b/tests/test_lint.py index bfbdab9f..0e217530 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -6,7 +6,7 @@ from anaconda_linter import lint, utils from anaconda_linter.lint import ERROR, INFO, WARNING -from anaconda_linter.recipe import Recipe +from anaconda_linter.recipe import Recipe, RecipeError class dummy_info(lint.LintCheck): @@ -150,3 +150,70 @@ def test_lint_list(): lint_checks_file = [line.strip() for line in f.readlines() if line.strip()] lint_checks_lint = [str(chk) for chk in lint.get_checks() if not str(chk).startswith("dummy_")] assert sorted(lint_checks_file) == sorted(lint_checks_lint) + +@pytest.mark.parametrize( + "jinja_func,expected", + [ + ("cran_mirror", False), + ("compiler('c')", False), + ("cdt('cdt-cos6-plop')", False), + ("pin_compatible('dotnet-runtime', min_pin='x.x.x.x.x.x', max_pin='x', lower_bound=None, upper_bound=None)", False), + ("pin_subpackage('dotnet-runtime', min_pin='x.x.x.x.x.x', max_pin='x', exact=True)", False), + ("pin_subpackage('dotnet-runtime', min_pin='x.x.x.x.x.x', max_pin='x')", False), + ("pin_subpackage('dotnet-runtime', exact=True)", False), + ("pin_subpackage('dotnet-runtime', min_pin='x.x.x.x.x.x')", False), + ("pin_subpackage('dotnet-runtime', exact=True, badParam=False)", True), + ], +) +def test_jinja_functions(base_yaml, jinja_func, expected): + + def run_lint(yaml_str): + config_file = os.path.abspath(os.path.dirname(__file__) + "/../anaconda_linter/config.yaml") + config = utils.load_config(config_file) + linter = lint.Linter(config=config) + + try: + _recipe = Recipe.from_string(yaml_str) + linter.lint([_recipe]) + messages = linter.get_messages() + except RecipeError as exc: + _recipe = Recipe("") + check_cls = lint.recipe_error_to_lint_check.get(exc.__class__, lint.linter_failure) + messages = [check_cls.make_message(recipe=_recipe, line=getattr(exc, "line"))] + + return messages + + yaml_str = ( + base_yaml + + f""" + build: + number: 0 + + outputs: + - name: dotnet + version: 1 + requirements: + run: + - {{{{ {jinja_func} }}}} + + """ + ) + + lint_check = "jinja_render_failure" + messages = run_lint(yaml_str) + assert any([str(msg.check) == lint_check for msg in messages]) == expected + +def test_error_report_line(base_yaml): + yaml_str = ( + base_yaml + + """ + plop # [aaa] + plip # [aaa] + plep # [aaa] + about: + license: BSE-3-Clause + """ + ) + lint_check = "incorrect_license" + messages = check(lint_check, yaml_str) + assert len(messages) == 1 and messages[0].start_line == 5 \ No newline at end of file From ea979a5b590560ae4ed356496323057de3c9a4f1 Mon Sep 17 00:00:00 2001 From: Charles Bousseau Date: Thu, 10 Nov 2022 17:44:47 -0500 Subject: [PATCH 3/6] pre-commit --- tests/test_lint.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/test_lint.py b/tests/test_lint.py index 0e217530..77e7bd59 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -151,13 +151,17 @@ def test_lint_list(): lint_checks_lint = [str(chk) for chk in lint.get_checks() if not str(chk).startswith("dummy_")] assert sorted(lint_checks_file) == sorted(lint_checks_lint) + @pytest.mark.parametrize( "jinja_func,expected", [ ("cran_mirror", False), ("compiler('c')", False), ("cdt('cdt-cos6-plop')", False), - ("pin_compatible('dotnet-runtime', min_pin='x.x.x.x.x.x', max_pin='x', lower_bound=None, upper_bound=None)", False), + ( + "pin_compatible('dotnet-runtime', min_pin='x.x.x.x.x.x', max_pin='x', lower_bound=None, upper_bound=None)", + False, + ), ("pin_subpackage('dotnet-runtime', min_pin='x.x.x.x.x.x', max_pin='x', exact=True)", False), ("pin_subpackage('dotnet-runtime', min_pin='x.x.x.x.x.x', max_pin='x')", False), ("pin_subpackage('dotnet-runtime', exact=True)", False), @@ -166,7 +170,6 @@ def test_lint_list(): ], ) def test_jinja_functions(base_yaml, jinja_func, expected): - def run_lint(yaml_str): config_file = os.path.abspath(os.path.dirname(__file__) + "/../anaconda_linter/config.yaml") config = utils.load_config(config_file) @@ -180,7 +183,7 @@ def run_lint(yaml_str): _recipe = Recipe("") check_cls = lint.recipe_error_to_lint_check.get(exc.__class__, lint.linter_failure) messages = [check_cls.make_message(recipe=_recipe, line=getattr(exc, "line"))] - + return messages yaml_str = ( @@ -188,7 +191,7 @@ def run_lint(yaml_str): + f""" build: number: 0 - + outputs: - name: dotnet version: 1 @@ -203,6 +206,7 @@ def run_lint(yaml_str): messages = run_lint(yaml_str) assert any([str(msg.check) == lint_check for msg in messages]) == expected + def test_error_report_line(base_yaml): yaml_str = ( base_yaml @@ -216,4 +220,4 @@ def test_error_report_line(base_yaml): ) lint_check = "incorrect_license" messages = check(lint_check, yaml_str) - assert len(messages) == 1 and messages[0].start_line == 5 \ No newline at end of file + assert len(messages) == 1 and messages[0].start_line == 5 From 0f7ddfd53bc0db87f4674814d90b45b2e2a3fc22 Mon Sep 17 00:00:00 2001 From: Charles Bousseau Date: Thu, 10 Nov 2022 20:31:28 -0500 Subject: [PATCH 4/6] correct changelog --- CHANGELOG.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f92de1aa..ea609a7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,15 +1,6 @@ # Changelog Note: version releases in the 0.x.y range may introduce breaking changes. -## 0.0.5 - -Bug fixes: -- Correct mocks of conda-build jinja functions. PR: #131, Issues: #118, #126 -- Correct error line reporting. PR: #131, Issues: #123 - -Enhancements: -- Render recipe using cbc files defined variables. PR: #131 - ## 0.0.4 - Add test suite @@ -18,6 +9,9 @@ Enhancements: - Add new check: `patch_unnecessary` - Add `--severity` option to control the severity level of linter messages - Remove `load_skips` function +- Bug fix: Correct mocks of conda-build jinja functions. PR: #131, Issues: #118, #126 +- Bug fix: Correct error line reporting. PR: #131, Issues: #123 +- Enhancement: Render recipe using cbc files defined variables. PR: #131 ## 0.0.3 From de0ba4f95f4859db4ac5520e634ffa4da204b0cc Mon Sep 17 00:00:00 2001 From: Charles Bousseau Date: Fri, 11 Nov 2022 12:00:30 -0500 Subject: [PATCH 5/6] Handle YAML parsing errors --- CHANGELOG.md | 3 ++- anaconda_linter/lint/__init__.py | 10 +++++++++- anaconda_linter/recipe.py | 26 +++++++++++++++++++------- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea609a7e..13e1f0db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,9 @@ Note: version releases in the 0.x.y range may introduce breaking changes. - Add new check: `patch_unnecessary` - Add `--severity` option to control the severity level of linter messages - Remove `load_skips` function -- Bug fix: Correct mocks of conda-build jinja functions. PR: #131, Issues: #118, #126 +- Bug fix: Correct mocks of conda-build jinja functions. PR: #131, Issues: #118 - Bug fix: Correct error line reporting. PR: #131, Issues: #123 +- Bug fix: Handle YAML parsing errors. PR: #131, Issues: #126 - Enhancement: Render recipe using cbc files defined variables. PR: #131 ## 0.0.3 diff --git a/anaconda_linter/lint/__init__.py b/anaconda_linter/lint/__init__.py index f32b38ff..c4527dd0 100644 --- a/anaconda_linter/lint/__init__.py +++ b/anaconda_linter/lint/__init__.py @@ -455,6 +455,13 @@ class jinja_render_failure(LintCheck): """ +class yaml_load_failure(LintCheck): + """The recipe could not be loaded by yaml + + Check your selectors and overall yaml validity. + """ + + class unknown_check(LintCheck): """Something went wrong inside the linter @@ -471,7 +478,8 @@ class unknown_check(LintCheck): _recipe.HasSelector: unknown_selector, _recipe.MissingMetaYaml: missing_meta_yaml, _recipe.CondaRenderFailure: conda_render_failure, - _recipe.RenderFailure: jinja_render_failure, + _recipe.JinjaRenderFailure: jinja_render_failure, + _recipe.YAMLRenderFailure: yaml_load_failure, } diff --git a/anaconda_linter/recipe.py b/anaconda_linter/recipe.py index db0c6ac5..aeaf0e15 100644 --- a/anaconda_linter/recipe.py +++ b/anaconda_linter/recipe.py @@ -30,11 +30,12 @@ try: from ruamel.yaml import YAML from ruamel.yaml.constructor import DuplicateKeyError + from ruamel.yaml.parser import ParserError # from ruamel.yaml.error import YAMLError except ModuleNotFoundError: from ruamel_yaml import YAML - from ruamel_yaml.constructor import DuplicateKeyError + from ruamel_yaml.parser import ParserError # from ruamel_yaml.error import YAMLError @@ -120,7 +121,7 @@ class CondaRenderFailure(RecipeError): template = "could not be rendered by conda-build: %s" -class RenderFailure(RecipeError): +class JinjaRenderFailure(RecipeError): """Raised on Jinja rendering problems May have self.line @@ -129,6 +130,15 @@ class RenderFailure(RecipeError): template = "failed to render in Jinja2. Error was: %s" +class YAMLRenderFailure(RecipeError): + """Raised on YAML parsing problems + + May have self.line + """ + + template = "failed to load YAML. Error was: %s" + + class Recipe: """Represents a recipe (meta.yaml) in editable form @@ -369,9 +379,9 @@ def get_template(self): meta_yaml_selectors_applied = self.apply_selector(self.meta_yaml, self.selector_dict) return utils.jinja_silent_undef.from_string("\n".join(meta_yaml_selectors_applied)) except jinja2.exceptions.TemplateSyntaxError as exc: - raise RenderFailure(self, message=exc.message, line=exc.lineno) + raise JinjaRenderFailure(self, message=exc.message, line=exc.lineno) except jinja2.exceptions.TemplateError as exc: - raise RenderFailure(self, message=exc.message) + raise JinjaRenderFailure(self, message=exc.message) def get_simple_modules(self): """Yield simple replacement values from template @@ -400,13 +410,15 @@ def render(self) -> None: try: yaml_text = self.get_template().render(self.JINJA_VARS) except jinja2.exceptions.TemplateSyntaxError as exc: - raise RenderFailure(self, message=exc.message, line=exc.lineno) + raise JinjaRenderFailure(self, message=exc.message, line=exc.lineno) except jinja2.exceptions.TemplateError as exc: - raise RenderFailure(self, message=exc.message) + raise JinjaRenderFailure(self, message=exc.message) except TypeError as exc: - raise RenderFailure(self, message=str(exc)) + raise JinjaRenderFailure(self, message=str(exc)) try: self.meta = yaml.load(yaml_text) + except ParserError as exc: + raise YAMLRenderFailure(self, line=exc.problem_mark.line) except DuplicateKeyError as err: line = err.problem_mark.line + 1 column = err.problem_mark.column + 1 From 9aaa1589f6ba0d5e89db3c9c84f72a90183fa382 Mon Sep 17 00:00:00 2001 From: Charles Bousseau Date: Fri, 11 Nov 2022 12:17:11 -0500 Subject: [PATCH 6/6] add yaml_load_failure to lint names --- anaconda_linter/lint_names.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/anaconda_linter/lint_names.md b/anaconda_linter/lint_names.md index 03b0cd69..c7fe07ac 100644 --- a/anaconda_linter/lint_names.md +++ b/anaconda_linter/lint_names.md @@ -87,3 +87,5 @@ unknown_selector uses_setuptools version_constraints_missing_whitespace + +yaml_load_failure