From ab669fe2719f037fe661807c2c6c64919e0083b6 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 24 Jul 2024 16:39:41 +0200 Subject: [PATCH 1/8] add noarch hint for rattler --- conda_smithy/lint_recipe.py | 1 + conda_smithy/linter/hints.py | 58 +++++++++++------- tests/test_lint_recipe.py | 110 +++++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 22 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index 322972c72..a401fd815 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -298,6 +298,7 @@ def lintify_meta_yaml( conda_forge, recipe_fname, hints, + is_rattler_build, ) # 3: suggest fixing all recipe/*.sh shellcheck findings diff --git a/conda_smithy/linter/hints.py b/conda_smithy/linter/hints.py index 54be017e0..0338d74c8 100644 --- a/conda_smithy/linter/hints.py +++ b/conda_smithy/linter/hints.py @@ -9,6 +9,8 @@ from conda_smithy.linter.utils import find_local_config_file, is_selector_line from conda_smithy.utils import get_yaml +from conda_smithy import rattler_linter + def hint_pip_usage(build_section, hints): if "script" in build_section: @@ -24,7 +26,14 @@ def hint_pip_usage(build_section, hints): def hint_suggest_noarch( - noarch_value, build_reqs, is_staged_recipes, conda_forge, meta_fname, hints + noarch_value, + build_reqs, + raw_requirements_section, + is_staged_recipes, + conda_forge, + recipe_fname, + hints, + is_rattler_build: bool = False, ): if ( noarch_value is None @@ -33,30 +42,35 @@ def hint_suggest_noarch( and ("pip" in build_reqs) and (is_staged_recipes or not conda_forge) ): - with io.open(meta_fname, "rt") as fh: - in_runreqs = False - no_arch_possible = True - for line in fh: - line_s = line.strip() - if line_s == "host:" or line_s == "run:": - in_runreqs = True - runreqs_spacing = line[: -len(line.lstrip())] - continue - if line_s.startswith("skip:") and is_selector_line(line): - no_arch_possible = False - break - if in_runreqs: - if runreqs_spacing == line[: -len(line.lstrip())]: - in_runreqs = False + if is_rattler_build: + rattler_linter.hint_noarch_usage( + build_reqs, raw_requirements_section, hints + ) + else: + with io.open(recipe_fname, "rt") as fh: + in_runreqs = False + no_arch_possible = True + for line in fh: + line_s = line.strip() + if line_s == "host:" or line_s == "run:": + in_runreqs = True + runreqs_spacing = line[: -len(line.lstrip())] continue - if is_selector_line(line): + if line_s.startswith("skip:") and is_selector_line(line): no_arch_possible = False break - if no_arch_possible: - hints.append( - "Whenever possible python packages should use noarch. " - "See https://conda-forge.org/docs/maintainer/knowledge_base.html#noarch-builds" - ) + if in_runreqs: + if runreqs_spacing == line[: -len(line.lstrip())]: + in_runreqs = False + continue + if is_selector_line(line): + no_arch_possible = False + break + if no_arch_possible: + hints.append( + "Whenever possible python packages should use noarch. " + "See https://conda-forge.org/docs/maintainer/knowledge_base.html#noarch-builds" + ) def hint_shellcheck_usage(recipe_dir, hints): diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index a3f025c70..99a64da04 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -162,6 +162,33 @@ def test_osx_noarch_hint(where): assert not any(h.startswith(avoid_message) for h in hints) +def test_rattler_osx_noarch_hint(): + # don't warn on packages that are using __osx as a noarch-marker, see + # https://conda-forge.org/docs/maintainer/knowledge_base/#noarch-packages-with-os-specific-dependencies + avoid_message = "You're setting a constraint on the `__osx` virtual" + + with tmp_directory() as recipe_dir: + with io.open(os.path.join(recipe_dir, "recipe.yaml"), "w") as fh: + fh.write( + """ + package: + name: foo + requirements: + run: + - if: osx + then: __osx + """ + ) + + with io.open(os.path.join(recipe_dir, "conda-forge.yml"), "w") as fh: + fh.write("conda_build_tool: rattler-build") + + _, hints = linter.main( + recipe_dir, return_hints=True, feedstock_dir=recipe_dir + ) + assert not any(h.startswith(avoid_message) for h in hints) + + @pytest.mark.parametrize( "std_selector", ["unix", "linux or (osx and x86_64)"], @@ -889,6 +916,89 @@ def assert_noarch_hint(meta_string, is_good=False): is_good=True, ) + def test_suggest_rattler_noarch(self): + expected_start = "Whenever possible python packages should use noarch." + with tmp_directory() as recipe_dir: + + def assert_noarch_hint(meta_string, is_good=False): + with io.open( + os.path.join(recipe_dir, "recipe.yaml"), "w" + ) as fh: + fh.write(meta_string) + + with io.open( + os.path.join(recipe_dir, "conda-forge.yml"), "w" + ) as fh: + fh.write("conda_build_tool: rattler-build") + + lints, hints = linter.main( + recipe_dir, return_hints=True, feedstock_dir=recipe_dir + ) + if is_good: + message = ( + "Found hints when there shouldn't have " + "been a hint for '{}'." + ).format(meta_string) + else: + message = ( + "Expected hints for '{}', but didn't " "get any." + ).format(meta_string) + self.assertEqual( + not is_good, + any(lint.startswith(expected_start) for lint in hints), + message, + ) + + assert_noarch_hint( + """ + build: + noarch: python + script: + - echo "hello" + requirements: + build: + - python + - pip + """, + is_good=True, + ) + assert_noarch_hint( + """ + build: + script: + - echo "hello" + requirements: + build: + - python + - pip + """ + ) + assert_noarch_hint( + """ + build: + script: + - echo "hello" + requirements: + build: + - python + """, + is_good=True, + ) + assert_noarch_hint( + """ + build: + script: + - echo "hello" + requirements: + build: + - python + - ${{ compiler('c') }} + - pip + """, + is_good=True, + ) + + def test_jinja_os_environ(self): # Test that we can use os.environ in a recipe. We don't care about # the results here. From 296ce144fcbd8bd9c2a1161231e2757007c14019 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 24 Jul 2024 16:46:33 +0200 Subject: [PATCH 2/8] add noarch hint tests and first hint --- conda_smithy/lint_recipe.py | 39 +++++++++++++++++++++------ conda_smithy/linter/hints.py | 2 +- conda_smithy/linter/rattler_linter.py | 37 +++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index a401fd815..16250de18 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -57,6 +57,7 @@ from glob import glob from inspect import cleandoc from textwrap import indent +from pathlib import Path import github @@ -69,6 +70,7 @@ ensure_valid_license_family, ) from conda_smithy.validate_schema import validate_json_schema +from conda_smithy.configure_feedstock import _read_forge_config from conda_smithy.utils import render_meta_yaml, get_yaml @@ -537,17 +539,38 @@ def _format_validation_msg(error: "jsonschema.ValidationError"): ) -def main(recipe_dir, conda_forge=False, return_hints=False): +def main(recipe_dir, conda_forge=False, return_hints=False, feedstock_dir=None): recipe_dir = os.path.abspath(recipe_dir) - recipe_meta = os.path.join(recipe_dir, "meta.yaml") - if not os.path.exists(recipe_dir): - raise IOError("Feedstock has no recipe/meta.yaml.") + build_tool = CONDA_BUILD_TOOL + if feedstock_dir: + feedstock_dir = os.path.abspath(feedstock_dir) + forge_config = _read_forge_config(feedstock_dir) + if forge_config.get("conda_build_tool", "") == RATTLER_BUILD_TOOL: + build_tool = RATTLER_BUILD_TOOL + + if build_tool == RATTLER_BUILD_TOOL: + recipe_file = os.path.join(recipe_dir, "recipe.yaml") + else: + recipe_file = os.path.join(recipe_dir, "meta.yaml") + + if not os.path.exists(recipe_file): + raise IOError( + f"Feedstock has no recipe/{os.path.basename(recipe_file)}" + ) - with io.open(recipe_meta, "rt") as fh: - content = render_meta_yaml("".join(fh)) - meta = get_yaml().load(content) + if build_tool == CONDA_BUILD_TOOL: + with io.open(recipe_file, "rt") as fh: + content = render_meta_yaml("".join(fh)) + meta = get_yaml().load(content) + else: + meta = get_yaml().load(Path(recipe_file)) - results, hints = lintify_meta_yaml(meta, recipe_dir, conda_forge) + results, hints = lintify_meta_yaml( + meta, + recipe_dir, + conda_forge, + is_rattler_build=build_tool == RATTLER_BUILD_TOOL, + ) validation_errors, validation_hints = lintify_forge_yaml( recipe_dir=recipe_dir ) diff --git a/conda_smithy/linter/hints.py b/conda_smithy/linter/hints.py index 0338d74c8..62f23c273 100644 --- a/conda_smithy/linter/hints.py +++ b/conda_smithy/linter/hints.py @@ -9,7 +9,7 @@ from conda_smithy.linter.utils import find_local_config_file, is_selector_line from conda_smithy.utils import get_yaml -from conda_smithy import rattler_linter +from conda_smithy.linter import rattler_linter def hint_pip_usage(build_section, hints): diff --git a/conda_smithy/linter/rattler_linter.py b/conda_smithy/linter/rattler_linter.py index 9b05538e5..60463ffec 100644 --- a/conda_smithy/linter/rattler_linter.py +++ b/conda_smithy/linter/rattler_linter.py @@ -1,3 +1,5 @@ +from typing import Any + REQUIREMENTS_ORDER = ["build", "host", "run"] EXPECTED_SINGLE_OUTPUT_SECTION_ORDER = [ @@ -20,3 +22,38 @@ "about", "extra", ] + +def hint_noarch_usage( + build_section: dict[str, Any], + requirement_section: dict[str, Any], + hints: list[str], +): + build_reqs = requirement_section.get("build", None) + if ( + build_reqs + and not any( + [ + b.startswith("${{") + and ("compiler('c')" in b or 'compiler("c")' in b) + for b in build_reqs + ] + ) + and ("pip" in build_reqs) + ): + no_arch_possible = True + if "skip" in build_section: + no_arch_possible = False + + for _, section_requirements in requirement_section.items(): + if any( + isinstance(requirement, dict) + for requirement in section_requirements + ): + no_arch_possible = False + break + + if no_arch_possible: + hints.append( + "Whenever possible python packages should use noarch. " + "See https://conda-forge.org/docs/maintainer/knowledge_base.html#noarch-builds" + ) From 002e5471445490b318493e7271da353dea17a663 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 24 Jul 2024 16:46:59 +0200 Subject: [PATCH 3/8] lint --- conda_smithy/lint_recipe.py | 4 +++- conda_smithy/linter/rattler_linter.py | 1 + tests/test_lint_recipe.py | 1 - 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index 16250de18..d4c0a422d 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -539,7 +539,9 @@ def _format_validation_msg(error: "jsonschema.ValidationError"): ) -def main(recipe_dir, conda_forge=False, return_hints=False, feedstock_dir=None): +def main( + recipe_dir, conda_forge=False, return_hints=False, feedstock_dir=None +): recipe_dir = os.path.abspath(recipe_dir) build_tool = CONDA_BUILD_TOOL if feedstock_dir: diff --git a/conda_smithy/linter/rattler_linter.py b/conda_smithy/linter/rattler_linter.py index 60463ffec..90e0e524f 100644 --- a/conda_smithy/linter/rattler_linter.py +++ b/conda_smithy/linter/rattler_linter.py @@ -23,6 +23,7 @@ "extra", ] + def hint_noarch_usage( build_section: dict[str, Any], requirement_section: dict[str, Any], diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index 99a64da04..c07664bb0 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -998,7 +998,6 @@ def assert_noarch_hint(meta_string, is_good=False): is_good=True, ) - def test_jinja_os_environ(self): # Test that we can use os.environ in a recipe. We don't care about # the results here. From c84337aea3a229394eb6a9c0a80fadb346be606a Mon Sep 17 00:00:00 2001 From: nichmor Date: Thu, 25 Jul 2024 17:54:23 +0300 Subject: [PATCH 4/8] misc: fix pinning function --- conda_smithy/lint_recipe.py | 4 ++- conda_smithy/linter/lints.py | 48 +++++++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index dd87a78d0..da8486387 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -277,7 +277,9 @@ def lintify_meta_yaml( # 26: pin_subpackage is for subpackages and pin_compatible is for # non-subpackages of the recipe. Contact @carterbox for troubleshooting # this lint. - lint_pin_subpackages(meta, outputs_section, package_section, lints) + lint_pin_subpackages( + meta, outputs_section, package_section, lints, is_rattler_build + ) # 27: Check usage of whl files as a source lint_check_usage_of_whls(recipe_fname, noarch_value, lints, hints) diff --git a/conda_smithy/linter/lints.py b/conda_smithy/linter/lints.py index e5c413739..b8adaf267 100644 --- a/conda_smithy/linter/lints.py +++ b/conda_smithy/linter/lints.py @@ -501,7 +501,13 @@ def lint_require_lower_bound_on_python_version( ) -def lint_pin_subpackages(meta, outputs_section, package_section, lints): +def lint_pin_subpackages( + meta, + outputs_section, + package_section, + lints, + is_rattler_build: bool = False, +): subpackage_names = [] for out in outputs_section: if "name" in out: @@ -512,7 +518,10 @@ def lint_pin_subpackages(meta, outputs_section, package_section, lints): def check_pins(pinning_section): if pinning_section is None: return - for pin in fnmatch.filter(pinning_section, "compatible_pin*"): + filter_pin = ( + "pin_compatible*" if is_rattler_build else "compatible_pin*" + ) + for pin in fnmatch.filter(pinning_section, filter_pin): if pin.split()[1] in subpackage_names: lints.append( "pin_subpackage should be used instead of" @@ -520,7 +529,10 @@ def check_pins(pinning_section): " because it is one of the known outputs of this recipe:" f" {subpackage_names}." ) - for pin in fnmatch.filter(pinning_section, "subpackage_pin*"): + filter_pin = ( + "pin_subpackage*" if is_rattler_build else "subpackage_pin*" + ) + for pin in fnmatch.filter(pinning_section, filter_pin): if pin.split()[1] not in subpackage_names: lints.append( "pin_compatible should be used instead of" @@ -530,12 +542,30 @@ def check_pins(pinning_section): ) def check_pins_build_and_requirements(top_level): - if "build" in top_level and "run_exports" in top_level["build"]: - check_pins(top_level["build"]["run_exports"]) - if "requirements" in top_level and "run" in top_level["requirements"]: - check_pins(top_level["requirements"]["run"]) - if "requirements" in top_level and "host" in top_level["requirements"]: - check_pins(top_level["requirements"]["host"]) + if not is_rattler_build: + if "build" in top_level and "run_exports" in top_level["build"]: + check_pins(top_level["build"]["run_exports"]) + if ( + "requirements" in top_level + and "run" in top_level["requirements"] + ): + check_pins(top_level["requirements"]["run"]) + if ( + "requirements" in top_level + and "host" in top_level["requirements"] + ): + check_pins(top_level["requirements"]["host"]) + else: + if ( + "requirements" in top_level + and "run_exports" in top_level["requirements"] + ): + if "strong" in top_level["requirements"]["run_exports"]: + check_pins( + top_level["requirements"]["run_exports"]["strong"] + ) + else: + check_pins(top_level["requirements"]["run_exports"]) check_pins_build_and_requirements(meta) for out in outputs_section: From 35c64d3422cd4a7fcc37a6a3afdbc4b1f351e944 Mon Sep 17 00:00:00 2001 From: nichmor Date: Thu, 25 Jul 2024 17:58:31 +0300 Subject: [PATCH 5/8] misc: move hints in one place --- conda_smithy/linter/hints.py | 6 ++---- conda_smithy/linter/rattler_linter.py | 7 +++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/conda_smithy/linter/hints.py b/conda_smithy/linter/hints.py index 62f23c273..4cd5c5bef 100644 --- a/conda_smithy/linter/hints.py +++ b/conda_smithy/linter/hints.py @@ -8,6 +8,7 @@ from conda_smithy.linter.utils import find_local_config_file, is_selector_line from conda_smithy.utils import get_yaml +from conda_smithy.linter.errors import HINT_NO_ARCH from conda_smithy.linter import rattler_linter @@ -67,10 +68,7 @@ def hint_suggest_noarch( no_arch_possible = False break if no_arch_possible: - hints.append( - "Whenever possible python packages should use noarch. " - "See https://conda-forge.org/docs/maintainer/knowledge_base.html#noarch-builds" - ) + hints.append(HINT_NO_ARCH) def hint_shellcheck_usage(recipe_dir, hints): diff --git a/conda_smithy/linter/rattler_linter.py b/conda_smithy/linter/rattler_linter.py index 90e0e524f..2be3e6dd4 100644 --- a/conda_smithy/linter/rattler_linter.py +++ b/conda_smithy/linter/rattler_linter.py @@ -1,5 +1,7 @@ from typing import Any +from conda_smithy.linter.errors import HINT_NO_ARCH + REQUIREMENTS_ORDER = ["build", "host", "run"] EXPECTED_SINGLE_OUTPUT_SECTION_ORDER = [ @@ -54,7 +56,4 @@ def hint_noarch_usage( break if no_arch_possible: - hints.append( - "Whenever possible python packages should use noarch. " - "See https://conda-forge.org/docs/maintainer/knowledge_base.html#noarch-builds" - ) + hints.append(HINT_NO_ARCH) From f621f7abf4724afc92b0415dd3018f26b07e0e4f Mon Sep 17 00:00:00 2001 From: nichmor Date: Thu, 25 Jul 2024 18:17:58 +0300 Subject: [PATCH 6/8] misc: pass feedstock dir --- conda_smithy/cli.py | 1 + conda_smithy/linter/errors.py | 6 ++++++ 2 files changed, 7 insertions(+) create mode 100644 conda_smithy/linter/errors.py diff --git a/conda_smithy/cli.py b/conda_smithy/cli.py index 5957ac1c1..4f16b8a44 100644 --- a/conda_smithy/cli.py +++ b/conda_smithy/cli.py @@ -646,6 +646,7 @@ def __call__(self, args): os.path.join(recipe), conda_forge=args.conda_forge, return_hints=True, + feedstock_dir=args.feedstock_dir, ) if lints: all_good = False diff --git a/conda_smithy/linter/errors.py b/conda_smithy/linter/errors.py new file mode 100644 index 000000000..be6cfc700 --- /dev/null +++ b/conda_smithy/linter/errors.py @@ -0,0 +1,6 @@ +""" +Collection of errors that can be raised by the linter. +""" + +HINT_NO_ARCH = """Whenever possible python packages should use noarch. + See https://conda-forge.org/docs/maintainer/knowledge_base.html#noarch-builds""" From 73f5ed698b61c5e49d87c02bb9a2121512a8114c Mon Sep 17 00:00:00 2001 From: nichmor Date: Thu, 25 Jul 2024 18:33:42 +0300 Subject: [PATCH 7/8] misc: fix typing --- conda_smithy/linter/rattler_linter.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/conda_smithy/linter/rattler_linter.py b/conda_smithy/linter/rattler_linter.py index 2be3e6dd4..3d6010c71 100644 --- a/conda_smithy/linter/rattler_linter.py +++ b/conda_smithy/linter/rattler_linter.py @@ -1,4 +1,4 @@ -from typing import Any +from typing import Any, Dict, List from conda_smithy.linter.errors import HINT_NO_ARCH @@ -27,9 +27,9 @@ def hint_noarch_usage( - build_section: dict[str, Any], - requirement_section: dict[str, Any], - hints: list[str], + build_section: Dict[str, Any], + requirement_section: Dict[str, Any], + hints: List[str], ): build_reqs = requirement_section.get("build", None) if ( From 7eb04ae763d4e60b2c0f9ae4828d2a1ac24c57d1 Mon Sep 17 00:00:00 2001 From: nichmor Date: Thu, 25 Jul 2024 18:53:04 +0300 Subject: [PATCH 8/8] misc: fix hint branching --- conda_smithy/lint_recipe.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index da8486387..15418739d 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -295,9 +295,11 @@ def lintify_meta_yaml( hint_pip_usage(build_section, hints) # 2: suggest python noarch (skip on feedstocks) + raw_requirements_section = meta.get("requirements", {}) hint_suggest_noarch( noarch_value, build_requirements, + raw_requirements_section, is_staged_recipes, conda_forge, recipe_fname,