Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge c_stdlib_version & MACOSX_DEPLOYMENT_TARGET on osx #1889

Merged
merged 9 commits into from
Mar 25, 2024
87 changes: 84 additions & 3 deletions conda_smithy/configure_feedstock.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import warnings
from collections import Counter, OrderedDict, namedtuple
from copy import deepcopy
from functools import lru_cache
from itertools import chain, product
from os import fspath
from pathlib import Path, PurePath
Expand All @@ -34,6 +35,7 @@

from conda.models.match_spec import MatchSpec
from conda.models.version import VersionOrder
from conda.exceptions import InvalidVersionSpec

import conda_build.api
import conda_build.render
Expand Down Expand Up @@ -85,6 +87,13 @@
)


# use lru_cache to avoid repeating warnings endlessly;
# this keeps track of 10 different messages and then warns again
@lru_cache(10)
def warn_once(msg: str):
logger.warning(msg)


def package_key(config, used_loop_vars, subdir):
# get the build string from whatever conda-build makes of the configuration
key = "".join(
Expand Down Expand Up @@ -421,6 +430,64 @@ def _get_used_key_values_by_input_order(
return used_key_values


def _merge_deployment_target(container_of_dicts, has_macdt):
"""
For a collection of variant dictionaries, merge deployment target specs.

- The "old" way is MACOSX_DEPLOYMENT_TARGET, the new way is c_stdlib_version;
For now, take the maximum to populate both.
- In any case, populate MACOSX_DEPLOYMENT_TARGET, as that is the key picked
up by https://github.com/conda-forge/conda-forge-ci-setup-feedstock
"""
result = []
for var_dict in container_of_dicts:
# cases where no updates are necessary
if not var_dict.get("target_platform", "dummy").startswith("osx"):
result.append(var_dict)
continue
if "c_stdlib_version" not in var_dict:
result.append(var_dict)
continue
# case where we need to do processing
v_stdlib = var_dict["c_stdlib_version"]
macdt = var_dict.get("MACOSX_DEPLOYMENT_TARGET", v_stdlib)
# error out if someone puts in a range of versions; we need a single version
try:
cond_update = VersionOrder(v_stdlib) < VersionOrder(macdt)
except InvalidVersionSpec:
raise ValueError(
"both and c_stdlib_version/MACOSX_DEPLOYMENT_TARGET need to be a "
"single version, not a version range!"
)
if v_stdlib != macdt:
# determine maximum version and use it to populate both
v_stdlib = macdt if cond_update else v_stdlib
msg = (
"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"Using {v_stdlib}=max(c_stdlib_version, MACOSX_DEPLOYMENT_TARGET)."
)
# we don't want to warn for recipes that do not use MACOSX_DEPLOYMENT_TARGET
# in the local CBC, but only inherit it from the global pinning
if has_macdt:
warn_once(msg)

# we set MACOSX_DEPLOYMENT_TARGET to match c_stdlib_version,
# for ease of use in conda-forge-ci-setup;
# use new dictionary to avoid mutating existing var_dict in place
new_dict = conda_build.utils.HashableDict(
{
**var_dict,
"c_stdlib_version": v_stdlib,
"MACOSX_DEPLOYMENT_TARGET": v_stdlib,
}
)
result.append(new_dict)
# ensure we keep type of wrapper container (set stays set, etc.)
return type(container_of_dicts)(result)


def _collapse_subpackage_variants(
list_of_metas, root_path, platform, arch, forge_config
):
Expand Down Expand Up @@ -459,6 +526,19 @@ def _collapse_subpackage_variants(
if not meta.noarch:
is_noarch = False

# determine if MACOSX_DEPLOYMENT_TARGET appears in recipe-local CBC;
# all metas in list_of_metas come from same recipe, so path is identical
cbc_path = os.path.join(list_of_metas[0].path, "conda_build_config.yaml")
has_macdt = False
if os.path.exists(cbc_path):
with open(cbc_path, "r") as f:
lines = f.readlines()
if any(re.match(r"^\s*MACOSX_DEPLOYMENT_TARGET:", x) for x in lines):
has_macdt = True

# on osx, merge MACOSX_DEPLOYMENT_TARGET & c_stdlib_version to max of either; see #1884
all_variants = _merge_deployment_target(all_variants, has_macdt)

top_level_loop_vars = list_of_metas[0].get_used_loop_vars(
force_top_level=True
)
Expand All @@ -473,9 +553,10 @@ def _collapse_subpackage_variants(
# this is the initial collection of all variants before we discard any. "Squishing"
# them is necessary because the input form is already broken out into one matrix
# configuration per item, and we want a single dict, with each key representing many values
squished_input_variants = (
conda_build.variants.list_of_dicts_to_dict_of_lists(
list_of_metas[0].config.input_variants
squished_input_variants = conda_build.variants.list_of_dicts_to_dict_of_lists(
# ensure we update the input_variants in the same way as all_variants
_merge_deployment_target(
list_of_metas[0].config.input_variants, has_macdt
)
)
squished_used_variants = (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* Ensure we populate MACOSX_DEPLOYMENT_TARGET for use in conda-forge-ci-setup also when using `c_stdlib_version` (#1884 via #1889)

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
24 changes: 24 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,30 @@ def stdlib_recipe(config_yaml, request):
)


@pytest.fixture(scope="function")
def stdlib_deployment_target_recipe(config_yaml, stdlib_recipe):
# append to existing stdlib_config.yaml from stdlib_recipe
with open(
os.path.join(config_yaml, "recipe", "stdlib_config.yaml"), "a"
) as f:
f.write(
"""\
MACOSX_DEPLOYMENT_TARGET: # [osx]
- 10.14 # [osx and x86_64]
- 12.0 # [osx and arm64]
"""
)
return RecipeConfigPair(
str(config_yaml),
_load_forge_config(
config_yaml,
exclusive_config_file=os.path.join(
config_yaml, "recipe", "stdlib_config.yaml"
),
),
)


@pytest.fixture(scope="function")
def upload_on_branch_recipe(config_yaml, request):
with open(os.path.join(config_yaml, "recipe", "meta.yaml"), "w") as fh:
Expand Down
44 changes: 35 additions & 9 deletions tests/test_configure_feedstock.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,26 +226,52 @@ def test_stdlib_on_azure(stdlib_recipe, jinja_env):
linux_lines = f.readlines()
linux_content = "".join(linux_lines)
# multiline pattern to ensure we don't match other stuff accidentally
assert bool(re.match(r"(?s).*c_stdlib:\s*- sysroot", linux_content))
assert bool(
re.match(r"(?s).*c_stdlib_version:\s*- ['\"]?2\.\d+", linux_content)
)
assert re.match(r"(?s).*c_stdlib:\s*- sysroot", linux_content)
assert re.match(r"(?s).*c_stdlib_version:\s*- ['\"]?2\.\d+", linux_content)
with open(os.path.join(matrix_dir, "osx_64_.yaml")) as f:
osx_lines = f.readlines()
osx_content = "".join(osx_lines)
assert bool(
re.match(r"(?s).*c_stdlib:\s*- macosx_deployment_target", osx_content)
assert re.match(
r"(?s).*c_stdlib:\s*- macosx_deployment_target", osx_content
)
assert bool(
re.match(r"(?s).*c_stdlib_version:\s*- ['\"]?1\d\.\d+", osx_content)
assert re.match(r"(?s).*c_stdlib_version:\s*- ['\"]?10\.9", osx_content)
# ensure MACOSX_DEPLOYMENT_TARGET _also_ gets set to the same value
assert re.match(
r"(?s).*MACOSX_DEPLOYMENT_TARGET:\s*- ['\"]?10\.9", osx_content
)
with open(os.path.join(matrix_dir, "win_64_.yaml")) as f:
win_lines = f.readlines()
win_content = "".join(win_lines)
assert bool(re.match(r"(?s).*c_stdlib:\s*- vs", win_content))
assert re.match(r"(?s).*c_stdlib:\s*- vs", win_content)
# no stdlib-version expected on windows


def test_stdlib_deployment_target(
stdlib_deployment_target_recipe, jinja_env, caplog
):
with caplog.at_level(logging.WARNING):
configure_feedstock.render_azure(
jinja_env=jinja_env,
forge_config=stdlib_deployment_target_recipe.config,
forge_dir=stdlib_deployment_target_recipe.recipe,
)
# this configuration should be run
assert stdlib_deployment_target_recipe.config["azure"]["enabled"]
matrix_dir = os.path.join(
stdlib_deployment_target_recipe.recipe, ".ci_support"
)
assert os.path.isdir(matrix_dir)
with open(os.path.join(matrix_dir, "osx_64_.yaml")) as f:
lines = f.readlines()
content = "".join(lines)
# ensure both MACOSX_DEPLOYMENT_TARGET and c_stdlib_version match
# the maximum of either, c.f. stdlib_deployment_target_recipe fixture
assert re.match(r"(?s).*c_stdlib_version:\s*- ['\"]?10\.14", content)
assert re.match(
r"(?s).*MACOSX_DEPLOYMENT_TARGET:\s*- ['\"]?10\.14", content
)


def test_upload_on_branch_azure(upload_on_branch_recipe, jinja_env):
configure_feedstock.render_azure(
jinja_env=jinja_env,
Expand Down
Loading