Skip to content

Commit

Permalink
fix(whl_library): fix the dependency generation for multi-python depe…
Browse files Browse the repository at this point in the history 
…nency closures

We start using the recently introduced is_python_config_setting to make
it possible to have a working select statement when multiple python
version selection needs to happen in a whl_library

Work towards #735.
  • Loading branch information
@aignas
aignas committed May 4, 2024
1 parent d3cec48 commit 151d40f
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 93 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ A brief description of the categories of changes:
See [#1371](https://github.com/bazelbuild/rules_python/issues/1371).
* (refactor) The pre-commit developer workflow should now pass `isort` and `black`
checks (see [#1674](https://github.com/bazelbuild/rules_python/issues/1674)).
* (whl_library): Use `is_python_config_setting` to correctly handle multi-python
version dependency select statements when the `experimental_target_platforms`
includes the Python ABI.

### Added

Expand Down
72 changes: 26 additions & 46 deletions python/pip_install/private/generate_whl_library_build_bazel.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ def _render_list_and_select(deps, deps_by_platform, tmpl):
return "{} + {}".format(deps, deps_by_platform)

def _render_config_settings(dependencies_by_platform):
py_version_by_os_arch = {}
loads = []
additional_content = []
for p in dependencies_by_platform:
# p can be one of the following formats:
# * @platforms//os:{value}
Expand All @@ -146,66 +147,45 @@ def _render_config_settings(dependencies_by_platform):
if not abi.startswith("cp"):
tail = p
abi = ""

os, _, arch = tail.partition("_")
os = "" if os == "anyos" else os
arch = "" if arch == "anyarch" else arch

py_version_by_os_arch.setdefault((os, arch), []).append(abi)

if not py_version_by_os_arch:
return None, None

loads = []
additional_content = []
for (os, arch), abis in py_version_by_os_arch.items():
constraint_values = []
if os:
constraint_values.append("@platforms//os:{}".format(os))
if arch:
constraint_values.append("@platforms//cpu:{}".format(arch))
if os:
constraint_values.append("@platforms//os:{}".format(os))

os_arch = (os or "anyos") + "_" + (arch or "anyarch")
additional_content.append(
"""\
config_setting(
constraint_values_str = render.indent(render.list(constraint_values)).lstrip()

if abi and not loads:
loads.append("""load("@rules_python//python/config_settings:config_settings.bzl", "is_python_config_setting")""")
elif abi:
additional_content.append(
"""\
is_python_config_setting(
name = "is_{name}",
constraint_values = {values},
python_version = "3.{minor_version}",
constraint_values = {constraint_values},
visibility = ["//visibility:private"],
)""".format(
name = os_arch,
values = render.indent(render.list(sorted([str(Label(c)) for c in constraint_values]))).strip(),
),
)

if abis == [""]:
if not os or not arch:
fail("BUG: both os and arch should be set in this case")
continue

for abi in abis:
if not loads:
loads.append("""load("@bazel_skylib//lib:selects.bzl", "selects")""")
minor_version = int(abi[len("cp3"):])
setting = "@@{rules_python}//python/config_settings:is_python_3.{version}".format(
rules_python = str(Label("//:BUILD.bazel")).partition("//")[0].strip("@"),
version = minor_version,
name = p.replace("cp3", "python_3."),
minor_version = abi[len("cp3"):],
constraint_values = constraint_values_str,
),
)
settings = [
":is_" + os_arch,
setting,
]

plat = "{}_{}".format(abi, os_arch)

else:
additional_content.append(
"""\
selects.config_setting_group(
name = "{name}",
match_all = {values},
config_setting(
name = "is_{name}",
constraint_values = {constraint_values},
visibility = ["//visibility:private"],
)""".format(
name = _plat_label(plat).lstrip(":"),
values = render.indent(render.list(sorted(settings))).strip(),
name = p.replace("cp3", "python_3."),
constraint_values = constraint_values_str,
),
)

Expand Down Expand Up @@ -379,7 +359,7 @@ def generate_whl_library_build_bazel(
contents = "\n".join(
[
_BUILD_TEMPLATE.format(
loads = "\n".join(loads),
loads = "\n".join(sorted(loads)),
py_library_label = py_library_label,
dependencies = render.indent(lib_dependencies, " " * 4).lstrip(),
whl_file_deps = render.indent(whl_file_deps, " " * 4).lstrip(),
Expand Down
60 changes: 13 additions & 47 deletions tests/pip_install/whl_library/generate_build_bazel_tests.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ _tests = []

def _test_simple(env):
want = """\
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
package(default_visibility = ["//visibility:public"])
Expand Down Expand Up @@ -86,9 +86,9 @@ _tests.append(_test_simple)

def _test_dep_selects(env):
want = """\
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@bazel_skylib//lib:selects.bzl", "selects")
load("@rules_python//python/config_settings:config_settings.bzl", "is_python_config_setting")
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
package(default_visibility = ["//visibility:public"])
Expand Down Expand Up @@ -158,54 +158,20 @@ py_library(
visibility = ["//visibility:public"],
)
config_setting(
name = "is_linux_ppc",
constraint_values = [
"@platforms//cpu:ppc",
"@platforms//os:linux",
],
visibility = ["//visibility:private"],
)
selects.config_setting_group(
name = "is_cp310_linux_ppc",
match_all = [
":is_linux_ppc",
"@//python/config_settings:is_python_3.10",
],
visibility = ["//visibility:private"],
)
config_setting(
name = "is_anyos_aarch64",
is_python_config_setting(
name = "is_python_3.9_anyos_aarch64",
python_version = "3.9",
constraint_values = ["@platforms//cpu:aarch64"],
visibility = ["//visibility:private"],
)
selects.config_setting_group(
name = "is_cp39_anyos_aarch64",
match_all = [
":is_anyos_aarch64",
"@//python/config_settings:is_python_3.9",
],
visibility = ["//visibility:private"],
)
config_setting(
name = "is_linux_anyarch",
is_python_config_setting(
name = "is_python_3.9_linux_anyarch",
python_version = "3.9",
constraint_values = ["@platforms//os:linux"],
visibility = ["//visibility:private"],
)
selects.config_setting_group(
name = "is_cp39_linux_anyarch",
match_all = [
":is_linux_anyarch",
"@//python/config_settings:is_python_3.9",
],
visibility = ["//visibility:private"],
)
config_setting(
name = "is_linux_x86_64",
constraint_values = [
Expand Down Expand Up @@ -239,8 +205,8 @@ _tests.append(_test_dep_selects)

def _test_with_annotation(env):
want = """\
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
package(default_visibility = ["//visibility:public"])
Expand Down Expand Up @@ -327,8 +293,8 @@ _tests.append(_test_with_annotation)

def _test_with_entry_points(env):
want = """\
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
package(default_visibility = ["//visibility:public"])
Expand Down Expand Up @@ -401,8 +367,8 @@ _tests.append(_test_with_entry_points)

def _test_group_member(env):
want = """\
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
package(default_visibility = ["//visibility:public"])
Expand Down Expand Up @@ -503,8 +469,8 @@ _tests.append(_test_group_member)

def _test_group_member_deps_to_hub(env):
want = """\
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
package(default_visibility = ["//visibility:public"])
Expand Down

0 comments on commit 151d40f

Please sign in to comment.