From 151d40f6532e156152b1e56d65ab591eb157cbf5 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 4 May 2024 12:47:16 +0900 Subject: [PATCH] fix(whl_library): fix the dependency generation for multi-python depenency 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. --- CHANGELOG.md | 3 + .../generate_whl_library_build_bazel.bzl | 72 +++++++------------ .../generate_build_bazel_tests.bzl | 60 ++++------------ 3 files changed, 42 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 415b936e8..045f41ba2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/python/pip_install/private/generate_whl_library_build_bazel.bzl b/python/pip_install/private/generate_whl_library_build_bazel.bzl index 8010ccbad..5ab3144ad 100644 --- a/python/pip_install/private/generate_whl_library_build_bazel.bzl +++ b/python/pip_install/private/generate_whl_library_build_bazel.bzl @@ -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} @@ -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, ), ) @@ -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(), diff --git a/tests/pip_install/whl_library/generate_build_bazel_tests.bzl b/tests/pip_install/whl_library/generate_build_bazel_tests.bzl index 66126cf6f..05ee51eb1 100644 --- a/tests/pip_install/whl_library/generate_build_bazel_tests.bzl +++ b/tests/pip_install/whl_library/generate_build_bazel_tests.bzl @@ -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"]) @@ -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"]) @@ -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 = [ @@ -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"]) @@ -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"]) @@ -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"]) @@ -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"])