From e3a97089945e0fc8e67055fd64599dd09810bd1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Tue, 14 Jan 2025 19:13:55 +0100 Subject: [PATCH 1/8] Fix hint_noarch_python_use_python_min to correctly assert no hints Fix the outdated message in assertion for no hints in `hint_noarch_python_use_python_min` tests. The tests no longer correctly failed if the check produced hints, since the message was changed. Instead, assert on plain `python_min` substring, as that is unlikely to ever disappear. --- tests/test_lint_recipe.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index dc0fa104e..521505c06 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -3374,11 +3374,7 @@ def test_hint_noarch_python_use_python_min( for expected_hint in expected_hints: assert any(expected_hint in hint for hint in hints), hints else: - assert all( - "noarch: python recipes should almost always follow the syntax in" - not in hint - for hint in hints - ) + assert all("python_min" not in hint for hint in hints) @pytest.mark.parametrize( @@ -3551,11 +3547,7 @@ def test_hint_noarch_python_use_python_min_v1( for expected_hint in expected_hints: assert any(expected_hint in hint for hint in hints), hints else: - assert all( - "noarch: python recipes should almost always follow the syntax in" - not in hint - for hint in hints - ) + assert all("python_min" not in hint for hint in hints) def test_hint_noarch_python_from_main_v1(): From f262a2dd5d8b3e1f9f9abdbc7f405bd56aaef698 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Tue, 14 Jan 2025 20:22:52 +0100 Subject: [PATCH 2/8] Support multi-output recipes in hint_noarch_python_use_python_min Check all `noarch: python` outputs for missing `{{ python_min }}` usage. Move the code for grabbing test dependencies from v0 and v1 recipes to a common function in `conda_smithy.linter.utils`. Fixes #2188 --- conda_smithy/lint_recipe.py | 21 +--- conda_smithy/linter/hints.py | 82 +++++++++---- conda_smithy/linter/utils.py | 22 ++++ tests/test_lint_recipe.py | 231 +++++++++++++++++++++++++++++++++++ 4 files changed, 316 insertions(+), 40 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index f3e25bce9..6d3d2798b 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -73,6 +73,7 @@ RATTLER_BUILD_TOOL, find_local_config_file, flatten_v1_if_else, + get_all_test_requirements, get_section, load_linter_toml_metdata, ) @@ -525,25 +526,7 @@ def run_conda_forge_specific( build_section = get_section(meta, "build", lints, recipe_version) noarch_value = build_section.get("noarch") - - if recipe_version == 1: - test_section = get_section(meta, "tests", lints, recipe_version) - test_reqs = [] - for test_element in test_section: - test_reqs += (test_element.get("requirements") or {}).get( - "run" - ) or [] - - if ( - "python" in test_element - and test_element["python"].get("python_version") is not None - ): - test_reqs.append( - f"python {test_element['python']['python_version']}" - ) - else: - test_section = get_section(meta, "test", lints, recipe_version) - test_reqs = test_section.get("requires") or [] + test_reqs = get_all_test_requirements(meta, lints, recipe_version) # Fetch list of recipe maintainers maintainers = extra_section.get("recipe-maintainers", []) diff --git a/conda_smithy/linter/hints.py b/conda_smithy/linter/hints.py index 1397ca1f9..0c17d95c1 100644 --- a/conda_smithy/linter/hints.py +++ b/conda_smithy/linter/hints.py @@ -3,6 +3,7 @@ import shutil import subprocess import sys +from collections.abc import Mapping from glob import glob from typing import Any @@ -12,6 +13,7 @@ VALID_PYTHON_BUILD_BACKENDS, find_local_config_file, flatten_v1_if_else, + get_all_test_requirements, is_selector_line, ) from conda_smithy.utils import get_yaml @@ -236,22 +238,21 @@ def hint_pip_no_build_backend(host_or_build_section, package_name, hints): ) -def hint_noarch_python_use_python_min( +def hint_noarch_python_use_python_min_inner( host_reqs, run_reqs, test_reqs, - outputs_section, noarch_value, recipe_version, - hints, ): - if noarch_value == "python" and not outputs_section: + hint = set() + + if noarch_value == "python": if recipe_version == 1: host_reqs = flatten_v1_if_else(host_reqs) run_reqs = flatten_v1_if_else(run_reqs) test_reqs = flatten_v1_if_else(test_reqs) - hint = "" for section_name, syntax, report_syntax, reqs in [ ( "host", @@ -291,24 +292,63 @@ def hint_noarch_python_use_python_min( ): break else: - hint += ( + hint.add( f"\n - For the `{section_name}` section of the recipe, you should usually use `{report_syntax}` " f"for the `python` entry." ) + return hint - if hint: - hint = ( - ( - "`noarch: python` recipes should usually follow the syntax in " - "our [documentation](https://conda-forge.org/docs/maintainer/knowledge_base/#noarch-python) " - "for specifying the Python version." - ) - + hint - + ( - "\n - If the package requires a newer Python version than the currently supported minimum " - "version on `conda-forge`, you can override the `python_min` variable by adding a " - "Jinja2 `set` statement at the top of your recipe (or using an equivalent `context` " - "variable for v1 recipes)." - ) + +def hint_noarch_python_use_python_min( + host_reqs, + run_reqs, + test_reqs, + outputs_section, + noarch_value, + recipe_version, + hints, +): + hint = set() + + if outputs_section: + for output in outputs_section: + requirements = output.get("requirements", {}) + if isinstance(requirements, Mapping): + output_host_reqs = requirements.get("host") + output_run_reqs = requirements.get("run") + else: + output_host_reqs = requirements + output_run_reqs = requirements + + hint.update(hint_noarch_python_use_python_min_inner( + output_host_reqs or [], + output_run_reqs or [], + get_all_test_requirements(output, [], recipe_version), + output.get("build", {}).get("noarch"), + recipe_version, + )) + else: + hint.update(hint_noarch_python_use_python_min_inner( + host_reqs, + run_reqs, + test_reqs, + noarch_value, + recipe_version, + )) + + if hint: + hint = ( + ( + "`noarch: python` recipes should usually follow the syntax in " + "our [documentation](https://conda-forge.org/docs/maintainer/knowledge_base/#noarch-python) " + "for specifying the Python version." ) - hints.append(hint) + + "".join(sorted(hint)) + + ( + "\n - If the package requires a newer Python version than the currently supported minimum " + "version on `conda-forge`, you can override the `python_min` variable by adding a " + "Jinja2 `set` statement at the top of your recipe (or using an equivalent `context` " + "variable for v1 recipes)." + ) + ) + hints.append(hint) diff --git a/conda_smithy/linter/utils.py b/conda_smithy/linter/utils.py index 825d6d046..db1c8d6e1 100644 --- a/conda_smithy/linter/utils.py +++ b/conda_smithy/linter/utils.py @@ -248,3 +248,25 @@ def flatten_v1_if_else(requirements: list[str | dict]) -> list[str]: else: flattened_requirements.append(req) return flattened_requirements + + +def get_all_test_requirements(meta: dict, lints: list[str], recipe_version: int) -> list[str]: + if recipe_version == 1: + test_section = get_section(meta, "tests", lints, recipe_version) + test_reqs = [] + for test_element in test_section: + test_reqs += (test_element.get("requirements") or {}).get( + "run" + ) or [] + + if ( + "python" in test_element + and test_element["python"].get("python_version") is not None + ): + test_reqs.append( + f"python {test_element['python']['python_version']}" + ) + else: + test_section = get_section(meta, "test", lints, recipe_version) + test_reqs = test_section.get("requires") or [] + return test_reqs diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index 521505c06..82caeffed 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -3339,6 +3339,123 @@ def test_hint_pip_no_build_backend( ), [], ), + ( + textwrap.dedent( + """ + package: + name: python + + outputs: + - name: python-foo + build: + noarch: python + + requirements: + host: + - python {{ python_min }} + run: + - python >={{ python_min }} + + test: + requires: + - python {{ python_min }} + """ + ), + [], + ), + ( + textwrap.dedent( + """ + package: + name: python + + outputs: + - name: python-foo + build: + noarch: python + + requirements: + host: + - python {{ python_min }} + run: + - python + + test: + requires: + - python {{ python_min }} + """ + ), + ["python >={{ python_min }}"], + ), + ( + textwrap.dedent( + """ + package: + name: python + + outputs: + - name: python-foo + build: + noarch: python + + requirements: + host: + - python {{ python_min }} + run: + - python >={{ python_min }} + + test: + requires: + - python {{ python_min }} + - name: python-bar + build: + noarch: python + + requirements: + - python + + test: + requires: + - python + """ + ), + ["python {{ python_min }}"], + ), + ( + textwrap.dedent( + """ + package: + name: python + + outputs: + - name: python-foo + build: + noarch: python + + requirements: + host: + - python {{ python_min }} + run: + - python >={{ python_min }} + + test: + requires: + - python {{ python_min }} + - name: python-bar + + requirements: + host: + - python + run: + - python + + test: + requires: + - python + """ + ), + [], + ), ], ) @pytest.mark.parametrize("skip", [False, True]) @@ -3525,6 +3642,120 @@ def test_hint_noarch_python_use_python_min( """, [], ), + ( + textwrap.dedent( + """ + recipe: + name: python + + outputs: + - package: + name: python-foo + build: + noarch: python + requirements: + host: + - if: blah + then: blahblah + else: python ${{ python_min }} + run: + - python >=${{ python_min }} + + tests: + - requirements: + run: + - python ${{ python_min }} + """ + ), + [], + ), + ( + textwrap.dedent( + """ + recipe: + name: python + + outputs: + - package: + name: python-foo + build: + noarch: python + requirements: + host: + - if: blah + then: blahblah + else: python + run: + - python >=${{ python_min }} + + tests: + - requirements: + run: + - python ${{ python_min }} + - package: + name: python-bar + build: + noarch: python + requirements: + host: + - if: blah + then: blahblah + else: python ${{ python_min }} + run: + - python + + tests: + - requirements: + run: + - python ${{ python_min }} + """ + ), + [ + "python ${{ python_min }}", + "python >=${{ python_min }}", + ], + ), + ( + textwrap.dedent( + """ + recipe: + name: python + + outputs: + - package: + name: python-foo + build: + noarch: python + requirements: + host: + - if: blah + then: blahblah + else: python ${{ python_min }} + run: + - python >=${{ python_min }} + + tests: + - requirements: + run: + - python ${{ python_min }} + - package: + name: python-bar + requirements: + host: + - if: blah + then: blahblah + else: python + run: + - python + + tests: + - requirements: + run: + - python + """ + ), + [], + ), ], ) def test_hint_noarch_python_use_python_min_v1( From 539cc56bb0e99a69025b4bafd411b2c8c65f19d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Tue, 14 Jan 2025 20:46:01 +0100 Subject: [PATCH 3/8] Fix flattening v1 if/then/else blocks with non-list clauses Fix flattening v1 `if/then/else` blocks where the `then` or `else` clauses are immediate strings rather than sublists, e.g.: ``` - if: foo then: bar else: baz ``` --- conda_smithy/linter/utils.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/conda_smithy/linter/utils.py b/conda_smithy/linter/utils.py index db1c8d6e1..6774d95f0 100644 --- a/conda_smithy/linter/utils.py +++ b/conda_smithy/linter/utils.py @@ -237,13 +237,19 @@ def load_linter_toml_metdata_internal(time_salt): return tomllib.loads(hints_toml_str) -def flatten_v1_if_else(requirements: list[str | dict]) -> list[str]: +def flatten_v1_if_else(requirements: list[str | dict] | str) -> list[str]: flattened_requirements = [] for req in requirements: if isinstance(req, dict): - flattened_requirements.extend(flatten_v1_if_else(req["then"])) flattened_requirements.extend( - flatten_v1_if_else(req.get("else") or []) + flatten_v1_if_else(req["then"]) + if isinstance(req["then"], list) + else [req["then"]] + ) + flattened_requirements.extend( + flatten_v1_if_else(req.get("else", [])) + if isinstance(req.get("else", []), list) + else [req["else"]] ) else: flattened_requirements.append(req) From 63f22801dc4f2518df00893be59c87dace0a8923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Tue, 14 Jan 2025 20:57:22 +0100 Subject: [PATCH 4/8] Add a news entry --- news/python-min-multi-output.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/python-min-multi-output.rst diff --git a/news/python-min-multi-output.rst b/news/python-min-multi-output.rst new file mode 100644 index 000000000..32c98d66c --- /dev/null +++ b/news/python-min-multi-output.rst @@ -0,0 +1,23 @@ +**Added:** + +* Added support for multi-output recipes in ``hint_noarch_python_use_python_min`` check. (#2218) + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* Fixed flattening ``if / then / else`` clauses in v1 recipes with string values of ``then`` and ``else`` keys. (#2218) + +**Security:** + +* From 2ef29f6e98e01a58ff9411898eea8f5a0f2fd1dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Tue, 14 Jan 2025 20:57:50 +0100 Subject: [PATCH 5/8] Reformat --- conda_smithy/linter/hints.py | 32 ++++++++++++++++++-------------- conda_smithy/linter/utils.py | 4 +++- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/conda_smithy/linter/hints.py b/conda_smithy/linter/hints.py index 0c17d95c1..11eeaa50c 100644 --- a/conda_smithy/linter/hints.py +++ b/conda_smithy/linter/hints.py @@ -320,21 +320,25 @@ def hint_noarch_python_use_python_min( output_host_reqs = requirements output_run_reqs = requirements - hint.update(hint_noarch_python_use_python_min_inner( - output_host_reqs or [], - output_run_reqs or [], - get_all_test_requirements(output, [], recipe_version), - output.get("build", {}).get("noarch"), - recipe_version, - )) + hint.update( + hint_noarch_python_use_python_min_inner( + output_host_reqs or [], + output_run_reqs or [], + get_all_test_requirements(output, [], recipe_version), + output.get("build", {}).get("noarch"), + recipe_version, + ) + ) else: - hint.update(hint_noarch_python_use_python_min_inner( - host_reqs, - run_reqs, - test_reqs, - noarch_value, - recipe_version, - )) + hint.update( + hint_noarch_python_use_python_min_inner( + host_reqs, + run_reqs, + test_reqs, + noarch_value, + recipe_version, + ) + ) if hint: hint = ( diff --git a/conda_smithy/linter/utils.py b/conda_smithy/linter/utils.py index 6774d95f0..ed094b3a1 100644 --- a/conda_smithy/linter/utils.py +++ b/conda_smithy/linter/utils.py @@ -256,7 +256,9 @@ def flatten_v1_if_else(requirements: list[str | dict] | str) -> list[str]: return flattened_requirements -def get_all_test_requirements(meta: dict, lints: list[str], recipe_version: int) -> list[str]: +def get_all_test_requirements( + meta: dict, lints: list[str], recipe_version: int +) -> list[str]: if recipe_version == 1: test_section = get_section(meta, "tests", lints, recipe_version) test_reqs = [] From 432cfe1911922374f32c4e82da4f7dd16b0463f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Wed, 15 Jan 2025 15:50:54 +0100 Subject: [PATCH 6/8] Include the output name in hints --- conda_smithy/linter/hints.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/conda_smithy/linter/hints.py b/conda_smithy/linter/hints.py index 11eeaa50c..bfa6f7a12 100644 --- a/conda_smithy/linter/hints.py +++ b/conda_smithy/linter/hints.py @@ -244,8 +244,9 @@ def hint_noarch_python_use_python_min_inner( test_reqs, noarch_value, recipe_version, + output_name, ): - hint = set() + hint = [] if noarch_value == "python": if recipe_version == 1: @@ -292,8 +293,11 @@ def hint_noarch_python_use_python_min_inner( ): break else: - hint.add( - f"\n - For the `{section_name}` section of the recipe, you should usually use `{report_syntax}` " + section_desc = ( + f"`{output_name}` output" if output_name else "recipe" + ) + hint.append( + f"\n - For the `{section_name}` section of {section_desc}, you should usually use `{report_syntax}` " f"for the `python` entry." ) return hint @@ -308,10 +312,10 @@ def hint_noarch_python_use_python_min( recipe_version, hints, ): - hint = set() + hint = [] if outputs_section: - for output in outputs_section: + for output_num, output in enumerate(outputs_section): requirements = output.get("requirements", {}) if isinstance(requirements, Mapping): output_host_reqs = requirements.get("host") @@ -320,23 +324,27 @@ def hint_noarch_python_use_python_min( output_host_reqs = requirements output_run_reqs = requirements - hint.update( + hint.extend( hint_noarch_python_use_python_min_inner( output_host_reqs or [], output_run_reqs or [], get_all_test_requirements(output, [], recipe_version), output.get("build", {}).get("noarch"), recipe_version, + output.get("package", {}).get( + "name", f" Date: Wed, 15 Jan 2025 09:11:33 -0600 Subject: [PATCH 7/8] Apply suggestions from code review --- conda_smithy/linter/hints.py | 8 ++++---- conda_smithy/linter/utils.py | 10 ++++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/conda_smithy/linter/hints.py b/conda_smithy/linter/hints.py index bfa6f7a12..55cf282b7 100644 --- a/conda_smithy/linter/hints.py +++ b/conda_smithy/linter/hints.py @@ -238,7 +238,7 @@ def hint_pip_no_build_backend(host_or_build_section, package_name, hints): ) -def hint_noarch_python_use_python_min_inner( +def _hint_noarch_python_use_python_min_inner( host_reqs, run_reqs, test_reqs, @@ -321,11 +321,11 @@ def hint_noarch_python_use_python_min( output_host_reqs = requirements.get("host") output_run_reqs = requirements.get("run") else: - output_host_reqs = requirements + output_host_reqs = None output_run_reqs = requirements hint.extend( - hint_noarch_python_use_python_min_inner( + _hint_noarch_python_use_python_min_inner( output_host_reqs or [], output_run_reqs or [], get_all_test_requirements(output, [], recipe_version), @@ -338,7 +338,7 @@ def hint_noarch_python_use_python_min( ) else: hint.extend( - hint_noarch_python_use_python_min_inner( + _hint_noarch_python_use_python_min_inner( host_reqs, run_reqs, test_reqs, diff --git a/conda_smithy/linter/utils.py b/conda_smithy/linter/utils.py index ed094b3a1..5d25b6be1 100644 --- a/conda_smithy/linter/utils.py +++ b/conda_smithy/linter/utils.py @@ -269,11 +269,13 @@ def get_all_test_requirements( if ( "python" in test_element - and test_element["python"].get("python_version") is not None ): - test_reqs.append( - f"python {test_element['python']['python_version']}" - ) + if test_element["python"].get("python_version") is not None: + test_reqs.append( + f"python {test_element['python']['python_version']}" + ) + else: + test_reqs.append("python") else: test_section = get_section(meta, "test", lints, recipe_version) test_reqs = test_section.get("requires") or [] From c5a8e7f2ab0595c2a50399c87da37e8552244d9a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 15 Jan 2025 15:12:16 +0000 Subject: [PATCH 8/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- conda_smithy/linter/utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/conda_smithy/linter/utils.py b/conda_smithy/linter/utils.py index 5d25b6be1..5153bc6b9 100644 --- a/conda_smithy/linter/utils.py +++ b/conda_smithy/linter/utils.py @@ -267,9 +267,7 @@ def get_all_test_requirements( "run" ) or [] - if ( - "python" in test_element - ): + if "python" in test_element: if test_element["python"].get("python_version") is not None: test_reqs.append( f"python {test_element['python']['python_version']}"