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

Piggyback migrator for stdlib-migration #2135

Merged
merged 28 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b605588
ENH: piggyback migrator for adding `{{ stdlib("c") }}`
h-vetinari Feb 5, 2024
edb1793
add test setup for testing stdlib-migration
h-vetinari Feb 5, 2024
257fdb6
add test for stdlib-migration: arrow
h-vetinari Feb 5, 2024
38ecd45
add test for stdlib-migration: polars
h-vetinari Feb 7, 2024
4f2c04c
ensure we can parse recipes containing stdlib-jinja
h-vetinari Feb 7, 2024
4daba02
add test for stdlib-migration: skip already migrated feedstocks
h-vetinari Feb 7, 2024
d2c804e
expand pattern for valid selectors
h-vetinari Feb 7, 2024
f472dd6
insert stdlib after compiler, i.e. in build, not host
h-vetinari Feb 8, 2024
8dc7307
prefer C compilers for indent/line/selector
h-vetinari Feb 8, 2024
6d8ba9b
be a bit stricter in line-matching patterns
h-vetinari Feb 8, 2024
9086907
prefer requirements.build over build.{number,...} for line_build
h-vetinari Feb 8, 2024
3259924
adjust test expectations to shifting stdlib from host: to build:
h-vetinari Feb 8, 2024
840a09e
expand keys used to detect non-requirement `build:` section
h-vetinari Feb 8, 2024
96b1fcf
ensure we don't skip migrator where selectors may lead to ignored deps
h-vetinari Feb 10, 2024
01fa795
deal with split of compilers between c and m2w64_c
h-vetinari Feb 10, 2024
84cd793
ensure selectors are maintained and aligned
h-vetinari Feb 10, 2024
d1899d2
add test for stdlib-migration: go
h-vetinari Feb 10, 2024
aad08a5
Merge remote-tracking branch 'up/master' into stdlib
h-vetinari Feb 10, 2024
9e8f172
add comments to stdlib-tests
h-vetinari Feb 10, 2024
3294429
add test for stdlib-migration: daal4py
h-vetinari Feb 10, 2024
09054c1
replace sysroot_linux-64 from recipes with stdlib-spec to CBC
h-vetinari Feb 10, 2024
c2e45c3
add test for stdlib-migration: sinabs
h-vetinari Feb 10, 2024
ec5d5a4
add missed condition for writing to cbc
h-vetinari Feb 10, 2024
2b5a7f3
rewrite MACOSX_DEPLOYMENT_TARGET in cbc with c_stdlib_version
h-vetinari Feb 10, 2024
013a2fc
improve handling of last_line_was_build
h-vetinari Feb 12, 2024
ec6f2ab
ensure stdlib run-exports get taken care of in make_graph.py
h-vetinari Feb 13, 2024
65afee3
Merge branch 'master' into stdlib
beckermr Mar 25, 2024
47c2ea1
Merge branch 'master' into stdlib
beckermr Mar 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions conda_forge_tick/auto_tick.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
PipWheelMigrator,
QtQtMainMigrator,
Replacement,
StdlibMigrator,
UpdateCMakeArgsMigrator,
UpdateConfigSubGuessMigrator,
Version,
Expand Down Expand Up @@ -688,6 +689,10 @@
piggy_back_migrations.append(JpegTurboMigrator())
if migration_name == "boost_cpp_to_libboost":
piggy_back_migrations.append(LibboostMigrator())
if migration_name == "boost1840":

Check warning on line 692 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L692

Added line #L692 was not covered by tests
# testing phase: only a single migration
# TODO: piggyback for all migrations
beckermr marked this conversation as resolved.
Show resolved Hide resolved
piggy_back_migrations.append(StdlibMigrator())

Check warning on line 695 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L695

Added line #L695 was not covered by tests
cycles = list(nx.simple_cycles(total_graph))
migrator = MigrationYaml(
migration_yaml,
Expand Down
1 change: 1 addition & 0 deletions conda_forge_tick/make_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
# appear here
COMPILER_STUBS_WITH_STRONG_EXPORTS = [
"c_compiler_stub",
"c_stdlib_stub",
"cxx_compiler_stub",
"fortran_compiler_stub",
"cuda_compiler_stub",
Expand Down
1 change: 1 addition & 0 deletions conda_forge_tick/migrators/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
UpdateCMakeArgsMigrator,
UpdateConfigSubGuessMigrator,
)
from .cstdlib import StdlibMigrator
from .dep_updates import DependencyUpdateMigrator
from .duplicate_lines import DuplicateLinesCleanup
from .extra_jinj2a_keys_cleanup import ExtraJinja2KeysCleanup
Expand Down
236 changes: 236 additions & 0 deletions conda_forge_tick/migrators/cstdlib.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
import os
import re

from conda_forge_tick.migrators.core import MiniMigrator
from conda_forge_tick.migrators.libboost import _replacer, _slice_into_output_sections

pat_stub = re.compile(r"(c|cxx|fortran)_compiler_stub")
rgx_idt = r"(?P<indent>\s*)-\s*"
rgx_pre = r"(?P<compiler>\{\{\s*compiler\([\"\']"
rgx_post = r"[\"\']\)\s*\}\})"
rgx_sel = r"(?P<selector>\s*\#\s+\[[\w\s()<>!=.,\-\'\"]+\])?"

pat_compiler_c = re.compile("".join([rgx_idt, rgx_pre, "c", rgx_post, rgx_sel]))
pat_compiler_m2c = re.compile("".join([rgx_idt, rgx_pre, "m2w64_c", rgx_post, rgx_sel]))
pat_compiler_other = re.compile(
"".join([rgx_idt, rgx_pre, "(m2w64_)?(cxx|fortran)", rgx_post, rgx_sel])
)
pat_compiler = re.compile(
"".join([rgx_idt, rgx_pre, "(m2w64_)?(c|cxx|fortran)", rgx_post, rgx_sel])
)
pat_stdlib = re.compile(r".*\{\{\s*stdlib\([\"\']c[\"\']\)\s*\}\}.*")
# no version other than 2.17 currently available (except 2.12 as default on linux-64)
pat_sysroot_217 = re.compile(r"- sysroot_linux-64\s*=?=?2\.17")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pat_sysroot_217 = re.compile(r"- sysroot_linux-64\s*=?=?2\.17")
pat_sysroot = re.compile(r"- sysroot_linux-64\s*=?=?2\.(17|28)")

We have 2.28 in the wild right now and formally people can build with it if they do not use cdts. Maybe we should account for it now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had checked at the time and there were no users of 2.28. Now that I've expanded the search a bit, there's only cudnn (which seems misspecified actually because it pins both 2.17 & 2.28 on aarch).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. As I meant to say totally optional.



def _process_section(name, attrs, lines):
"""
Migrate requirements per section.

We want to migrate as follows:
- if there's _any_ `{{ stdlib("c") }}` in the recipe, abort (consider it migrated)
- if there's `{{ compiler("c") }}` in build, add `{{ stdlib("c") }}` in host
- where there's no host-section, add it

If we find `sysroot_linux-64 2.17`, remove those lines and write the spec to CBC.
"""
write_stdlib_to_cbc = False
# remove occurrences of __osx due to MACOSX_DEPLOYMENT_TARGET (see migrate() below)
lines = _replacer(lines, "- __osx", "")

outputs = attrs["meta_yaml"].get("outputs", [])
global_reqs = attrs["meta_yaml"].get("requirements", {})
if name == "global":
reqs = global_reqs
else:
filtered = [o for o in outputs if o["name"] == name]
if len(filtered) == 0:
raise RuntimeError(f"Could not find output {name}!")

Check warning on line 48 in conda_forge_tick/migrators/cstdlib.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/migrators/cstdlib.py#L48

Added line #L48 was not covered by tests
reqs = filtered[0].get("requirements", {})

build_reqs = reqs.get("build", set()) or set()
global_build_reqs = global_reqs.get("build", set()) or set()

# either there's a compiler in the output we're processing, or the
# current output has no build-section but relies on the global one
needs_stdlib = any(pat_stub.search(x or "") for x in build_reqs)
needs_stdlib |= not bool(build_reqs) and any(
pat_stub.search(x or "") for x in global_build_reqs
)
# see more computation further down depending on dependencies
# ignored due to selectors, where we need the line numbers below.

line_build = line_compiler_c = line_compiler_m2c = line_compiler_other = 0
line_host = line_run = line_constrain = line_test = 0
indent_c = indent_m2c = indent_other = ""
selector_c = selector_m2c = selector_other = ""
last_line_was_build = False
for i, line in enumerate(lines):
if last_line_was_build:
# process this separately from the if-else-chain below
keys_after_nonreq_build = [
"binary_relocation",
"force_ignore_keys",
"ignore_run_exports(_from)?",
"missing_dso_whitelist",
"noarch",
"number",
"run_exports",
"script",
"skip",
]
if re.match(rf"^\s*({'|'.join(keys_after_nonreq_build)}):.*", line):
# last match was spurious, reset line_build
line_build = 0
last_line_was_build = False

if re.match(r"^\s*build:.*", line):
# we need to avoid build.{number,...}, but cannot use multiline
# regexes here. So leave a marker that we can skip on
last_line_was_build = True
line_build = i
elif pat_compiler_c.search(line):
line_compiler_c = i
indent_c = pat_compiler_c.match(line).group("indent")
selector_c = pat_compiler_c.match(line).group("selector") or ""
elif pat_compiler_m2c.search(line):
line_compiler_m2c = i
indent_m2c = pat_compiler_m2c.match(line).group("indent")
selector_m2c = pat_compiler_m2c.match(line).group("selector") or ""
elif pat_compiler_other.search(line):
line_compiler_other = i
indent_other = pat_compiler_other.match(line).group("indent")
selector_other = pat_compiler_other.match(line).group("selector") or ""
elif re.match(r"^\s*host:.*", line):
line_host = i
elif re.match(r"^\s*run:.*", line):
line_run = i
elif re.match(r"^\s*run_constrained:.*", line):
line_constrain = i
elif re.match(r"^\s*test:.*", line):
line_test = i
# ensure we don't read past test section (may contain unrelated deps)
break

if line_build:
# double-check whether there are compilers in the build section
# that may have gotten ignored by selectors; we explicitly only
# want to match with compilers in build, not host or run
build_reqs = lines[
line_build : (line_host or line_run or line_constrain or line_test or -1)
]
needs_stdlib |= any(pat_compiler.search(line) for line in build_reqs)

if not needs_stdlib:
if any(pat_sysroot_217.search(line) for line in lines):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if any(pat_sysroot_217.search(line) for line in lines):
if any(pat_sysroot.search(line) for line in lines):

# if there are no compilers, but we still find sysroot_linux-64,
# replace it; remove potential selectors, as only that package is
# linux-only, not the requirement for a c-stdlib
from_this, to_that = "sysroot_linux-64.*", '{{ stdlib("c") }}'
lines = _replacer(lines, from_this, to_that, max_times=1)
lines = _replacer(lines, "sysroot_linux-64.*", "")
write_stdlib_to_cbc = True
# otherwise, no change
return lines, write_stdlib_to_cbc

# in case of several compilers, prefer line, indent & selector of c compiler
line_compiler = line_compiler_c or line_compiler_m2c or line_compiler_other
indent = indent_c or indent_m2c or indent_other
selector = selector_c or selector_m2c or selector_other
if indent == "":
# no compiler in current output; take first line of section as reference (without last \n);
# ensure it works for both global build section as well as for `- name: <output>`.
indent = (
re.sub(r"^([\s\-]*).*", r"\1", lines[0][:-1]).replace("-", " ") + " " * 4
)

# align selectors between {{ compiler(...) }} with {{ stdlib(...) }}
selector = " " + selector if selector else ""
to_insert = indent + '- {{ stdlib("c") }}' + selector + "\n"
if line_build == 0:
# no build section, need to add it
to_insert = indent[:-2] + "build:\n" + to_insert

# if there's no build section, try to insert (in order of preference)
# before the sections for host, run, run_constrained, test
line_insert = line_host or line_run or line_constrain or line_test
if not line_insert:
raise RuntimeError("Don't know where to insert build section!")

Check warning on line 158 in conda_forge_tick/migrators/cstdlib.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/migrators/cstdlib.py#L158

Added line #L158 was not covered by tests
if line_compiler:
# by default, we insert directly after the compiler
line_insert = line_compiler + 1

lines = lines[:line_insert] + [to_insert] + lines[line_insert:]
if line_compiler_c and line_compiler_m2c:
# we have both compiler("c") and compiler("m2w64_c"), likely with complementary
# selectors; add a second stdlib line after m2w64_c with respective selector
selector_m2c = " " * 8 + selector_m2c if selector_m2c else ""
to_insert = indent + '- {{ stdlib("c") }}' + selector_m2c + "\n"
line_insert = line_compiler_m2c + 1 + (line_compiler_c < line_compiler_m2c)
lines = lines[:line_insert] + [to_insert] + lines[line_insert:]

# check if someone specified a newer sysroot in recipe already,
# leave indicator to migrate() function that we need to write spec to CBC
if any(pat_sysroot_217.search(line) for line in lines):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if any(pat_sysroot_217.search(line) for line in lines):
if any(pat_sysroot.search(line) for line in lines):

write_stdlib_to_cbc = True
# as we've already inserted a stdlib-jinja next to the compiler,
# simply remove any remaining occurrences of sysroot_linux-64
lines = _replacer(lines, "sysroot_linux-64.*", "")
return lines, write_stdlib_to_cbc


class StdlibMigrator(MiniMigrator):
def filter(self, attrs, not_bad_str_start=""):
lines = attrs["raw_meta_yaml"].splitlines()
already_migrated = any(pat_stdlib.search(line) for line in lines)
has_compiler = any(pat_compiler.search(line) for line in lines)
has_sysroot = any(pat_sysroot_217.search(line) for line in lines)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
has_sysroot = any(pat_sysroot_217.search(line) for line in lines)
has_sysroot = any(pat_sysroot.search(line) for line in lines)

# filter() returns True if we _don't_ want to migrate
return already_migrated or not (has_compiler or has_sysroot)

def migrate(self, recipe_dir, attrs, **kwargs):
outputs = attrs["meta_yaml"].get("outputs", [])

new_lines = []
write_stdlib_to_cbc = False
fname = os.path.join(recipe_dir, "meta.yaml")
if os.path.exists(fname):
with open(fname) as fp:
lines = fp.readlines()

sections = _slice_into_output_sections(lines, attrs)
for name, section in sections.items():
# _process_section returns list of lines already
chunk, cbc = _process_section(name, attrs, section)
new_lines += chunk
write_stdlib_to_cbc |= cbc

with open(fname, "w") as fp:
fp.write("".join(new_lines))

fname = os.path.join(recipe_dir, "conda_build_config.yaml")
if write_stdlib_to_cbc:
with open(fname, "a") as fp:
# append ("a") to existing CBC (or create it if it exista already),
# no need to differentiate as no-one is using c_stdlib_version yet;
# selector can just be linux as that matches default on aarch/ppc
fp.write(
'\nc_stdlib_version: # [linux]\n - "2.17" # [linux]\n'
)

if os.path.exists(fname):
with open(fname) as fp:
cbc_lines = fp.readlines()
# in a well-formed recipe, all deviations from the baseline
# MACOSX_DEPLOYMENT_TARGET come with a constraint on `__osx` in meta.yaml.
# Since the c_stdlib_version (together with the macosx_deployment_target
# metapackage) satisfies exactly that role, we can unconditionally replace
# that in the conda_build_config, and remove all `__osx` constraints in
# the meta.yaml (see further up).
# this line almost always has a selector, keep the alignment
cbc_lines = _replacer(
cbc_lines, r"^MACOSX_DEPLOYMENT_TARGET:", "c_stdlib_version: "
)

with open(fname, "w") as fp:
fp.write("".join(cbc_lines))
3 changes: 3 additions & 0 deletions conda_forge_tick/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

PACKAGE_STUBS = [
"_compiler_stub",
"_stdlib_stub",
"subpackage_stub",
"compatible_pin_stub",
"cdt_stub",
Expand All @@ -43,6 +44,7 @@
os=os,
environ=defaultdict(str),
compiler=lambda x: x + "_compiler_stub",
stdlib=lambda x: x + "_stdlib_stub",
pin_subpackage=lambda *args, **kwargs: args[0],
pin_compatible=lambda *args, **kwargs: args[0],
cdt=lambda *args, **kwargs: "cdt_stub",
Expand All @@ -61,6 +63,7 @@ def _munge_dict_repr(dct: Dict[Any, Any]) -> str:
os=os,
environ=defaultdict(str),
compiler=lambda x: x + "_compiler_stub",
stdlib=lambda x: x + "_stdlib_stub",
# The `max_pin, ` stub is so we know when people used the functions
# to create the pins
pin_subpackage=lambda *args, **kwargs: _munge_dict_repr(
Expand Down
69 changes: 69 additions & 0 deletions tests/test_stdlib.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import os
import re

import pytest
from flaky import flaky
from test_migrators import run_test_migration

from conda_forge_tick.migrators import StdlibMigrator, Version

TEST_YAML_PATH = os.path.join(os.path.dirname(__file__), "test_yaml")


STDLIB = StdlibMigrator()
VERSION_WITH_STDLIB = Version(
set(),
piggy_back_migrations=[STDLIB],
)


@pytest.mark.parametrize(
"feedstock,new_ver,expect_cbc",
[
# package with many outputs, includes inheritance from global build env
("arrow", "1.10.0", False),
# package without c compiler, but with selectors
("daal4py", "1.10.0", False),
# package involving selectors and m2w64_c compilers, and compilers in
# unusual places (e.g. in host & run sections)
("go", "1.10.0", True),
# package with rust compilers
("polars", "1.10.0", False),
# package without compilers, but with sysroot_linux-64
("sinabs", "1.10.0", True),
# test that we skip recipes that already contain a {{ stdlib("c") }}
("skip_migration", "1.10.0", False),
],
)
def test_stdlib(feedstock, new_ver, expect_cbc, tmpdir):
before = f"stdlib_{feedstock}_before_meta.yaml"
with open(os.path.join(TEST_YAML_PATH, before)) as fp:
in_yaml = fp.read()

after = f"stdlib_{feedstock}_after_meta.yaml"
with open(os.path.join(TEST_YAML_PATH, after)) as fp:
out_yaml = fp.read()

recipe_dir = os.path.join(tmpdir, f"{feedstock}-feedstock")
os.makedirs(recipe_dir, exist_ok=True)

run_test_migration(
m=VERSION_WITH_STDLIB,
inp=in_yaml,
output=out_yaml,
kwargs={"new_version": new_ver},
prb="Dependencies have been updated if changed",
mr_out={
"migrator_name": "Version",
"migrator_version": VERSION_WITH_STDLIB.migrator_version,
"version": new_ver,
},
tmpdir=recipe_dir,
should_filter=False,
)

cbc_pth = os.path.join(recipe_dir, "conda_build_config.yaml")
if expect_cbc:
with open(cbc_pth) as fp:
lines = fp.readlines()
assert any(re.match(r"c_stdlib_version:\s+\#\s\[linux\]", x) for x in lines)
Loading