Merge pull request #1929 from h-vetinari/lint_osx_cbc

conda_smithy/lint_recipe.py

@@ -436,6 +436,7 @@ def lintify_meta_yaml(
                 )
             )
 
+    conda_build_config_filename = None
     if recipe_dir:
         conda_build_config_filename = find_local_config_file(
             recipe_dir, "conda_build_config.yaml"
@@ -886,6 +887,133 @@ def check_pins_build_and_requirements(top_level):
         if osx_hint not in hints:
             hints.append(osx_hint)
 
+    # stdlib issues in CBC
+    cbc_lines = []
+    if conda_build_config_filename:
+        with open(conda_build_config_filename, "r") as fh:
+            cbc_lines = fh.readlines()
+
+    # filter on osx-relevant lines
+    pat = re.compile(
+        r"^([^\#]*?)\s+\#\s\[.*(not\s(osx|unix)|(?<!not\s)(linux|win)).*\]\s*$"
+    )
+    # remove lines with selectors that don't apply to osx, i.e. if they contain
+    # "not osx", "not unix", "linux" or "win"; this also removes trailing newlines
+    cbc_lines_osx = [pat.sub("", x) for x in cbc_lines]
+    cbc_content_osx = "\n".join(cbc_lines_osx)
+    cbc_osx = get_yaml().load(cbc_content_osx) or {}
+    # filter None values out of cbc_osx dict, can appear for example with
+    # ```
+    # c_stdlib_version:  # [unix]
+    #   - 2.17           # [linux]
+    #   # note lack of osx
+    # ```
+    cbc_osx = dict(filter(lambda item: item[1] is not None, cbc_osx.items()))
+
+    def sort_osx(versions):
+        # we need to have a known order for [x64, arm64]; in the absence of more
+        # complicated regex processing, we assume that if there are two versions
+        # being specified, the higher one is osx-arm64.
+        if len(versions) == 2:
+            if VersionOrder(str(versions[0])) > VersionOrder(str(versions[1])):
+                versions = versions[::-1]
+        return versions
+
+    baseline_version = ["10.13", "11.0"]
+    v_stdlib = sort_osx(cbc_osx.get("c_stdlib_version", baseline_version))
+    macdt = sort_osx(cbc_osx.get("MACOSX_DEPLOYMENT_TARGET", baseline_version))
+    sdk = sort_osx(cbc_osx.get("MACOSX_SDK_VERSION", baseline_version))
+
+    if {"MACOSX_DEPLOYMENT_TARGET", "c_stdlib_version"} <= set(cbc_osx.keys()):
+        # both specified, check that they match
+        if len(v_stdlib) != len(macdt):
+            # if lengths aren't matching, assume it's a legal combination
+            # where one key is specified for less arches than the other and
+            # let the rerender deal with the details
+            pass
+        else:
+            mismatch_hint = (
+                "Conflicting specification for minimum macOS deployment target!\n"
+                "If your conda_build_config.yaml sets `MACOSX_DEPLOYMENT_TARGET`, "
+                "please change the name of that key to `c_stdlib_version`!\n"
+                f"Continuing with `max(c_stdlib_version, MACOSX_DEPLOYMENT_TARGET)`."
+            )
+            merged_dt = []
+            for v_std, v_mdt in zip(v_stdlib, macdt):
+                # versions with a single dot may have been read as floats
+                v_std, v_mdt = str(v_std), str(v_mdt)
+                if VersionOrder(v_std) != VersionOrder(v_mdt):
+                    if mismatch_hint not in hints:
+                        hints.append(mismatch_hint)
+                merged_dt.append(
+                    v_mdt
+                    if VersionOrder(v_std) < VersionOrder(v_mdt)
+                    else v_std
+                )
+            cbc_osx["merged"] = merged_dt
+    elif "MACOSX_DEPLOYMENT_TARGET" in cbc_osx.keys():
+        cbc_osx["merged"] = macdt
+        # only MACOSX_DEPLOYMENT_TARGET, should be renamed
+        deprecated_dt = (
+            "In your conda_build_config.yaml, please change the name of "
+            "`MACOSX_DEPLOYMENT_TARGET`, to `c_stdlib_version`!"
+        )
+        if deprecated_dt not in hints:
+            hints.append(deprecated_dt)
+    elif "c_stdlib_version" in cbc_osx.keys():
+        cbc_osx["merged"] = v_stdlib
+        # only warn if version is below baseline
+        outdated_hint = (
+            "You are setting `c_stdlib_version` below the current global baseline "
+            "in conda-forge. If this is your intention, you also need to override "
+            "`MACOSX_DEPLOYMENT_TARGET` (with the same value) locally."
+        )
+        if len(v_stdlib) == len(macdt):
+            # if length matches, compare individually
+            for v_std, v_mdt in zip(v_stdlib, macdt):
+                if VersionOrder(str(v_std)) < VersionOrder(str(v_mdt)):
+                    if outdated_hint not in hints:
+                        hints.append(outdated_hint)
+        elif len(v_stdlib) == 1:
+            # if length doesn't match, only warn if a single stdlib version
+            # is lower than _all_ baseline deployment targets
+            if all(
+                VersionOrder(str(v_stdlib[0])) < VersionOrder(str(v_mdt))
+                for v_mdt in macdt
+            ):
+                if outdated_hint not in hints:
+                    hints.append(outdated_hint)
+
+    # warn if SDK is lower than merged v_stdlib/macdt
+    merged_dt = cbc_osx.get("merged", baseline_version)
+    sdk_hint = (
+        "You are setting `MACOSX_SDK_VERSION` below `c_stdlib_version`, "
+        "in conda_build_config.yaml which is not possible! Please ensure "
+        "`MACOSX_SDK_VERSION` is at least `c_stdlib_version` "
+        "(you can leave it out if it is equal).\n"
+        "If you are not setting `c_stdlib_version` yourself, this means "
+        "you are requesting a version below the current global baseline in "
+        "conda-forge. In this case, you also need to override "
+        "`c_stdlib_version` and `MACOSX_DEPLOYMENT_TARGET` locally."
+    )
+    if len(sdk) == len(merged_dt):
+        # if length matches, compare individually
+        for v_sdk, v_mdt in zip(sdk, merged_dt):
+            # versions with a single dot may have been read as floats
+            v_sdk, v_mdt = str(v_sdk), str(v_mdt)
+            if VersionOrder(v_sdk) < VersionOrder(v_mdt):
+                if sdk_hint not in hints:
+                    hints.append(sdk_hint)
+    elif len(sdk) == 1:
+        # if length doesn't match, only warn if a single SDK version
+        # is lower than _all_ merged deployment targets
+        if all(
+            VersionOrder(str(sdk[0])) < VersionOrder(str(v_mdt))
+            for v_mdt in merged_dt
+        ):
+            if sdk_hint not in hints:
+                hints.append(sdk_hint)
+
     return lints, hints
 
 

news/1929-lint-osx-misconfigurations-in-CBC.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* Provide linter hints if macOS quantities are misconfigured in `conda_build_config.yaml` (#1929)
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

tests/test_lint_recipe.py

@@ -116,6 +116,102 @@ def test_osx_noarch_hint(where):
         assert not any(h.startswith(avoid_message) for h in hints)
 
 
+@pytest.mark.parametrize("with_linux", [True, False])
+@pytest.mark.parametrize(
+    "reverse_arch",
+    # we reverse x64/arm64 separately per deployment target, stdlib & sdk
+    [(False, False, False), (True, True, True), (False, True, False)],
+)
+@pytest.mark.parametrize(
+    "macdt,v_std,sdk,exp_hint",
+    [
+        # matching -> no warning
+        (["10.9", "11.0"], ["10.9", "11.0"], None, None),
+        # mismatched length -> no warning (leave it to rerender)
+        (["10.9", "11.0"], ["10.9"], None, None),
+        # mismatch between stdlib and deployment target -> warn
+        (["10.9", "11.0"], ["10.13", "11.0"], None, "Conflicting spec"),
+        (["10.13", "11.0"], ["10.13", "12.3"], None, "Conflicting spec"),
+        # only deployment target -> warn
+        (["10.13", "11.0"], None, None, "In your conda_build_config.yaml"),
+        # only stdlib -> no warning
+        (None, ["10.13", "11.0"], None, None),
+        (None, ["10.15"], None, None),
+        # only stdlib, but outdated -> warn
+        (None, ["10.9", "11.0"], None, "You are"),
+        (None, ["10.9"], None, "You are"),
+        # sdk below stdlib / deployment target -> warn
+        (["10.13", "11.0"], ["10.13", "11.0"], ["10.12"], "You are"),
+        (["10.13", "11.0"], ["10.13", "11.0"], ["10.12", "12.0"], "You are"),
+        # sdk above stdlib / deployment target -> no warning
+        (["10.13", "11.0"], ["10.13", "11.0"], ["12.0", "12.0"], None),
+        # only one sdk version, not universally below deployment target
+        # -> no warning (because we don't know enough to diagnose)
+        (["10.13", "11.0"], ["10.13", "11.0"], ["10.15"], None),
+        # mismatched version + wrong sdk; requires merge logic to work before
+        # checking sdk version; to avoid unnecessary complexity in the exp_hint
+        # handling below, repeat same test twice with different expected hints
+        (["10.9", "11.0"], ["10.13", "11.0"], ["10.12"], "Conflicting spec"),
+        (["10.9", "11.0"], ["10.13", "11.0"], ["10.12"], "You are"),
+        # only sdk -> no warning
+        (None, None, ["10.13"], None),
+        (None, None, ["10.14", "12.0"], None),
+        # only sdk, but below global baseline -> warning
+        (None, None, ["10.12"], "You are"),
+        (None, None, ["10.12", "11.0"], "You are"),
+    ],
+)
+def test_cbc_osx_hints(with_linux, reverse_arch, macdt, v_std, sdk, exp_hint):
+    with tmp_directory() as rdir:
+        with open(os.path.join(rdir, "meta.yaml"), "w") as fh:
+            fh.write("package:\n   name: foo")
+        with open(os.path.join(rdir, "conda_build_config.yaml"), "a") as fh:
+            if macdt is not None:
+                fh.write(
+                    f"""\
+MACOSX_DEPLOYMENT_TARGET:   # [osx]
+  - {macdt[0]}              # [osx and {"arm64" if reverse_arch[0] else "x86_64"}]
+  - {macdt[1]}              # [osx and {"x86_64" if reverse_arch[0] else "arm64"}]
+"""
+                )
+            if v_std is not None or with_linux:
+                arch1 = "arm64" if reverse_arch[1] else "x86_64"
+                arch2 = "x86_64" if reverse_arch[1] else "arm64"
+                fh.write("c_stdlib_version:           # [unix]")
+                if v_std is not None:
+                    fh.write(f"\n  - {v_std[0]}       # [osx and {arch1}]")
+                if v_std is not None and len(v_std) > 1:
+                    fh.write(f"\n  - {v_std[1]}       # [osx and {arch2}]")
+                if with_linux:
+                    # to check that other stdlib specifications don't mess us up
+                    fh.write("\n  - 2.17              # [linux]")
+            if sdk is not None:
+                # often SDK is set uniformly for osx; test this as well
+                fh.write(
+                    f"""
+MACOSX_SDK_VERSION:         # [osx]
+  - {sdk[0]}                # [osx and {"arm64" if reverse_arch[2] else "x86_64"}]
+  - {sdk[1]}                # [osx and {"x86_64" if reverse_arch[2] else "arm64"}]
+"""
+                    if len(sdk) == 2
+                    else f"""
+MACOSX_SDK_VERSION:         # [osx]
+  - {sdk[0]}                # [osx]
+"""
+                )
+        # run the linter
+        _, hints = linter.main(rdir, return_hints=True)
+        # show CBC/hints for debugging
+        with open(os.path.join(rdir, "conda_build_config.yaml"), "r") as fh:
+            print("".join(fh.readlines()))
+            print(hints)
+        # validate against expectations
+        if exp_hint is None:
+            assert not hints
+        else:
+            assert any(h.startswith(exp_hint) for h in hints)
+
+
 class Test_linter(unittest.TestCase):
     def test_pin_compatible_in_run_exports(self):
         meta = {