From cbc1ab6752e678107dfc2f4a8443f36f3a064819 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 Feb 2024 00:07:11 +0900 Subject: [PATCH 01/12] feat: add config_settings for all platforms --- python/config_settings/BUILD.bazel | 9 ++++ python/config_settings/config_settings.bzl | 55 +++++++++++++++++++--- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 4f12ef4791..6516832b5b 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -12,5 +12,14 @@ filegroup( construct_config_settings( name = "construct_config_settings", + platforms = [ + ("linux", "aarch64"), + ("linux", "ppc"), + ("linux", "s390x"), + ("linux", "x86_64"), + ("osx", "aarch64"), + ("osx", "x86_64"), + ("windows", "x86_64"), + ], python_versions = TOOL_VERSIONS.keys(), ) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 9e6bbd6595..114bb88bcf 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -19,13 +19,15 @@ load("@bazel_skylib//lib:selects.bzl", "selects") load("@bazel_skylib//rules:common_settings.bzl", "string_flag") load("//python:versions.bzl", "MINOR_MAPPING") -def construct_config_settings(name, python_versions): +def construct_config_settings(name, python_versions, platforms = None): """Constructs a set of configs for all Python versions. Args: name: str, unused; only specified to satisfy buildifier lint checks and allow programatic modification of the target. python_versions: list of all (x.y.z) Python versions supported by rules_python. + platforms (list[tuple[os, cpu]], optional): a list of platforms to + generate settings for. """ # Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] @@ -50,26 +52,66 @@ def construct_config_settings(name, python_versions): visibility = ["//visibility:public"], ) + oses = [] + cpus = [] + for (os, cpu) in platforms or []: + if os not in oses: + oses.append(os) + if cpu not in cpus: + cpus.append(cpu) + + _construct_config_settings_for_os_cpu( + name = "_{}_{}".format(os, cpu), + minor_to_micro_versions = minor_to_micro_versions, + os = os, + cpu = cpu, + ) + + for os in oses: + _construct_config_settings_for_os_cpu(name = "_{}_any".format(os), minor_to_micro_versions = minor_to_micro_versions, os = os) + + for cpu in cpus: + _construct_config_settings_for_os_cpu(name = "_any_{}".format(cpu), minor_to_micro_versions = minor_to_micro_versions, cpu = cpu) + +def _construct_config_settings_for_os_cpu(*, name, minor_to_micro_versions, os = None, cpu = None): + """Constructs a set of configs for all Python versions. + + Args: + name: str, unused; only specified to satisfy buildifier lint checks + and allow programatic modification of the target. + minor_to_micro_versions: Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] + os: TODO + cpu: TODO + """ + + constraint_values = [] + if os: + constraint_values.append("@platforms//os:{}".format(os)) + if cpu: + constraint_values.append("@platforms//cpu:{}".format(cpu)) + for minor_version, micro_versions in minor_to_micro_versions.items(): # This matches the raw flag value, e.g. --//python/config_settings:python_version=3.8 # It's private because matching the concept of e.g. "3.8" value is done # using the `is_python_X.Y` config setting group, which is aware of the # minor versions that could match instead. - equals_minor_version_name = "_python_version_flag_equals_" + minor_version + equals_minor_version_name = "_python_version_flag_equals_" + minor_version + name native.config_setting( name = equals_minor_version_name, flag_values = {":python_version": minor_version}, + constraint_values = constraint_values, ) matches_minor_version_names = [equals_minor_version_name] default_micro_version = MINOR_MAPPING[minor_version] for micro_version in micro_versions: - is_micro_version_name = "is_python_" + micro_version + is_micro_version_name = "is_python_" + micro_version + name if default_micro_version != micro_version: native.config_setting( name = is_micro_version_name, flag_values = {":python_version": micro_version}, + constraint_values = constraint_values, visibility = ["//visibility:public"], ) matches_minor_version_names.append(is_micro_version_name) @@ -77,10 +119,11 @@ def construct_config_settings(name, python_versions): # Ensure that is_python_3.9.8 is matched if python_version is set # to 3.9 if MINOR_MAPPING points to 3.9.8 - equals_micro_name = "_python_version_flag_equals_" + micro_version + equals_micro_name = "_python_version_flag_equals_" + micro_version + name native.config_setting( name = equals_micro_name, flag_values = {":python_version": micro_version}, + constraint_values = constraint_values, ) # An alias pointing to an underscore-prefixed config_setting_group @@ -108,12 +151,12 @@ def construct_config_settings(name, python_versions): # Meanwhile, the micro-version tarets are named "is_python_3.10.1" -- # just a single dot vs underscore character difference. selects.config_setting_group( - name = "_is_python_" + minor_version, + name = "_is_python_" + minor_version + name, match_any = matches_minor_version_names, ) native.alias( - name = "is_python_" + minor_version, + name = "is_python_" + minor_version + name, actual = "_is_python_" + minor_version, visibility = ["//visibility:public"], ) From 8ef004938f364f7389ce1bd61a0c7803eef877e9 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 Feb 2024 13:05:39 +0900 Subject: [PATCH 02/12] create platform specific values only for existing platforms --- python/config_settings/BUILD.bazel | 14 +- python/config_settings/config_settings.bzl | 208 ++++++++++-------- .../generate_whl_library_build_bazel.bzl | 88 -------- .../tools/wheel_installer/wheel.py | 12 +- 4 files changed, 126 insertions(+), 196 deletions(-) diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 6516832b5b..37297666a7 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -12,14 +12,8 @@ filegroup( construct_config_settings( name = "construct_config_settings", - platforms = [ - ("linux", "aarch64"), - ("linux", "ppc"), - ("linux", "s390x"), - ("linux", "x86_64"), - ("osx", "aarch64"), - ("osx", "x86_64"), - ("windows", "x86_64"), - ], - python_versions = TOOL_VERSIONS.keys(), + python_versions = { + version: metadata["sha256"].keys() + for version, metadata in TOOL_VERSIONS.items() + }, ) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 114bb88bcf..cdbbc84615 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -19,26 +19,47 @@ load("@bazel_skylib//lib:selects.bzl", "selects") load("@bazel_skylib//rules:common_settings.bzl", "string_flag") load("//python:versions.bzl", "MINOR_MAPPING") -def construct_config_settings(name, python_versions, platforms = None): +def construct_config_settings(name, python_versions): """Constructs a set of configs for all Python versions. Args: name: str, unused; only specified to satisfy buildifier lint checks and allow programatic modification of the target. - python_versions: list of all (x.y.z) Python versions supported by rules_python. - platforms (list[tuple[os, cpu]], optional): a list of platforms to - generate settings for. + python_versions: a dict of all (x.y.z) Python versions with supported + platforms as a list for each version. This should be in the same + format as in //python:versions.bzl#TOOL_VERSIONS. """ # Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] minor_to_micro_versions = {} + minor_to_plats = {} + micro_to_plats = {} allowed_flag_values = [] - for micro_version in python_versions: + for micro_version, plats in python_versions.items(): minor, _, _ = micro_version.rpartition(".") minor_to_micro_versions.setdefault(minor, []).append(micro_version) allowed_flag_values.append(micro_version) + for plat in plats: + cpu, _, os = plat.partition("-") + if "linux" in os: + os = "linux" + elif "darwin" in os: + os = "osx" + elif "windows" in os: + os = "windows" + else: + fail("unknown os: {}".format(os)) + + p = (os, cpu) + + # TODO @aignas 2024-02-03: use bazel skylib sets + if minor not in minor_to_plats or p not in minor_to_plats[minor]: + minor_to_plats.setdefault(minor, []).append(p) + if micro_version not in micro_to_plats or (os, cpu) not in micro_to_plats[micro_version]: + micro_to_plats.setdefault(micro_version, []).append(p) + allowed_flag_values.extend(list(minor_to_micro_versions)) string_flag( @@ -52,111 +73,116 @@ def construct_config_settings(name, python_versions, platforms = None): visibility = ["//visibility:public"], ) - oses = [] - cpus = [] - for (os, cpu) in platforms or []: - if os not in oses: - oses.append(os) - if cpu not in cpus: - cpus.append(cpu) + _construct_config_settings_for_os_cpu( + minor_to_micro_versions = minor_to_micro_versions, + minor_to_plats = minor_to_plats, + micro_to_plats = micro_to_plats, + ) + +def _constraint_values(plats): + ret = { + "": [], + } + _plats = [] + for (os, cpu) in plats: + if (os, None) not in _plats: + _plats.append((os, None)) + if (None, cpu) not in _plats: + _plats.append((None, cpu)) - _construct_config_settings_for_os_cpu( - name = "_{}_{}".format(os, cpu), - minor_to_micro_versions = minor_to_micro_versions, - os = os, - cpu = cpu, - ) + _plats.append((os, cpu)) - for os in oses: - _construct_config_settings_for_os_cpu(name = "_{}_any".format(os), minor_to_micro_versions = minor_to_micro_versions, os = os) + for (os, cpu) in _plats: + constraint_values = [] + if os: + constraint_values.append("@platforms//os:{}".format(os)) + if cpu: + constraint_values.append("@platforms//cpu:{}".format(cpu)) - for cpu in cpus: - _construct_config_settings_for_os_cpu(name = "_any_{}".format(cpu), minor_to_micro_versions = minor_to_micro_versions, cpu = cpu) + os = os or "any" + cpu = cpu or "any" -def _construct_config_settings_for_os_cpu(*, name, minor_to_micro_versions, os = None, cpu = None): + ret["_".join(["", os, cpu])] = constraint_values + + return ret + +def _construct_config_settings_for_os_cpu(*, minor_to_micro_versions, minor_to_plats, micro_to_plats): """Constructs a set of configs for all Python versions. Args: - name: str, unused; only specified to satisfy buildifier lint checks - and allow programatic modification of the target. minor_to_micro_versions: Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] - os: TODO - cpu: TODO + minor_to_plats: TODO + micro_to_plats: TODO """ - - constraint_values = [] - if os: - constraint_values.append("@platforms//os:{}".format(os)) - if cpu: - constraint_values.append("@platforms//cpu:{}".format(cpu)) - for minor_version, micro_versions in minor_to_micro_versions.items(): - # This matches the raw flag value, e.g. --//python/config_settings:python_version=3.8 - # It's private because matching the concept of e.g. "3.8" value is done - # using the `is_python_X.Y` config setting group, which is aware of the - # minor versions that could match instead. - equals_minor_version_name = "_python_version_flag_equals_" + minor_version + name - native.config_setting( - name = equals_minor_version_name, - flag_values = {":python_version": minor_version}, - constraint_values = constraint_values, - ) - matches_minor_version_names = [equals_minor_version_name] - - default_micro_version = MINOR_MAPPING[minor_version] + matches_minor_version_names = {} + for name, constraint_values in _constraint_values(minor_to_plats[minor_version]).items(): + # This matches the raw flag value, e.g. --//python/config_settings:python_version=3.8 + # It's private because matching the concept of e.g. "3.8" value is done + # using the `is_python_X.Y` config setting group, which is aware of the + # minor versions that could match instead. + equals_minor_version_name = "_python_version_flag_equals_" + minor_version + name + native.config_setting( + name = equals_minor_version_name, + flag_values = {":python_version": minor_version}, + constraint_values = constraint_values, + ) + matches_minor_version_names[name] = [equals_minor_version_name] for micro_version in micro_versions: - is_micro_version_name = "is_python_" + micro_version + name - if default_micro_version != micro_version: + for name, constraint_values in _constraint_values(micro_to_plats[micro_version]).items(): + is_micro_version_name = "is_python_" + micro_version + name + if MINOR_MAPPING[minor_version] != micro_version: + native.config_setting( + name = is_micro_version_name, + flag_values = {":python_version": micro_version}, + constraint_values = constraint_values, + visibility = ["//visibility:public"], + ) + matches_minor_version_names[name].append(is_micro_version_name) + continue + + # Ensure that is_python_3.9.8 is matched if python_version is set + # to 3.9 if MINOR_MAPPING points to 3.9.8 + equals_micro_name = "_python_version_flag_equals_" + micro_version + name native.config_setting( - name = is_micro_version_name, + name = equals_micro_name, flag_values = {":python_version": micro_version}, constraint_values = constraint_values, - visibility = ["//visibility:public"], ) - matches_minor_version_names.append(is_micro_version_name) - continue - - # Ensure that is_python_3.9.8 is matched if python_version is set - # to 3.9 if MINOR_MAPPING points to 3.9.8 - equals_micro_name = "_python_version_flag_equals_" + micro_version + name - native.config_setting( - name = equals_micro_name, - flag_values = {":python_version": micro_version}, - constraint_values = constraint_values, - ) - # An alias pointing to an underscore-prefixed config_setting_group - # is used because config_setting_group creates - # `is_{minor}_N` targets, which are easily confused with the - # `is_{minor}.{micro}` (dot) targets. + # An alias pointing to an underscore-prefixed config_setting_group + # is used because config_setting_group creates + # `is_{minor}_N` targets, which are easily confused with the + # `is_{minor}.{micro}` (dot) targets. + selects.config_setting_group( + name = "_" + is_micro_version_name, + match_any = [ + equals_micro_name, + matches_minor_version_names[name][0], + ], + ) + native.alias( + name = is_micro_version_name, + actual = "_" + is_micro_version_name, + visibility = ["//visibility:public"], + ) + matches_minor_version_names[name].append(equals_micro_name) + + for name in _constraint_values(minor_to_plats[minor_version]).keys(): + # This is prefixed with an underscore to prevent confusion due to how + # config_setting_group is implemented and how our micro-version targets + # are named. config_setting_group will generate targets like + # "is_python_3.10_1" (where the `_N` suffix is len(match_any). + # Meanwhile, the micro-version tarets are named "is_python_3.10.1" -- + # just a single dot vs underscore character difference. selects.config_setting_group( - name = "_" + is_micro_version_name, - match_any = [ - equals_micro_name, - equals_minor_version_name, - ], + name = "_is_python_" + minor_version + name, + match_any = matches_minor_version_names[name], ) + native.alias( - name = is_micro_version_name, - actual = "_" + is_micro_version_name, + name = "is_python_" + minor_version + name, + actual = "_is_python_" + minor_version, visibility = ["//visibility:public"], ) - matches_minor_version_names.append(equals_micro_name) - - # This is prefixed with an underscore to prevent confusion due to how - # config_setting_group is implemented and how our micro-version targets - # are named. config_setting_group will generate targets like - # "is_python_3.10_1" (where the `_N` suffix is len(match_any). - # Meanwhile, the micro-version tarets are named "is_python_3.10.1" -- - # just a single dot vs underscore character difference. - selects.config_setting_group( - name = "_is_python_" + minor_version + name, - match_any = matches_minor_version_names, - ) - - native.alias( - name = "is_python_" + minor_version + name, - actual = "_is_python_" + minor_version, - visibility = ["//visibility:public"], - ) 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 19650d16d7..fc8ff2c298 100644 --- a/python/pip_install/private/generate_whl_library_build_bazel.bzl +++ b/python/pip_install/private/generate_whl_library_build_bazel.bzl @@ -140,87 +140,6 @@ def _render_list_and_select(deps, deps_by_platform, tmpl): else: return "{} + {}".format(deps, deps_by_platform) -def _render_config_settings(dependencies_by_platform): - py_version_by_os_arch = {} - for p in dependencies_by_platform: - # p can be one of the following formats: - # * @platforms//os:{value} - # * @platforms//cpu:{value} - # * @//python/config_settings:is_python_3.{minor_version} - # * {os}_{cpu} - # * cp3{minor_version}_{os}_{cpu} - if p.startswith("@"): - continue - - abi, _, tail = p.partition("_") - 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)) - - os_arch = (os or "anyos") + "_" + (arch or "anyarch") - additional_content.append( - """\ -config_setting( - name = "is_{name}", - constraint_values = {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, - ) - settings = [ - ":is_" + os_arch, - setting, - ] - - plat = "{}_{}".format(abi, os_arch) - - additional_content.append( - """\ -selects.config_setting_group( - name = "{name}", - match_all = {values}, - visibility = ["//visibility:private"], -)""".format( - name = _plat_label(plat).lstrip(":"), - values = render.indent(render.list(sorted(settings))).strip(), - ), - ) - - return loads, "\n\n".join(additional_content) - def generate_whl_library_build_bazel( *, repo_prefix, @@ -328,13 +247,6 @@ def generate_whl_library_build_bazel( """load("@bazel_skylib//rules:copy_file.bzl", "copy_file")""", ] - loads_, config_settings_content = _render_config_settings(dependencies_by_platform) - if config_settings_content: - for line in loads_: - if line not in loads: - loads.append(line) - additional_content.append(config_settings_content) - lib_dependencies = _render_list_and_select( deps = dependencies, deps_by_platform = dependencies_by_platform, diff --git a/python/pip_install/tools/wheel_installer/wheel.py b/python/pip_install/tools/wheel_installer/wheel.py index 750ebfcf7a..ac3ec7beac 100644 --- a/python/pip_install/tools/wheel_installer/wheel.py +++ b/python/pip_install/tools/wheel_installer/wheel.py @@ -168,16 +168,14 @@ def __str__(self) -> str: else: return f"{self.os}_{self.arch}" + prefix = "@//python/config_settings:is_python_3." if self.arch is None and self.os is None: - return f"@//python/config_settings:is_python_3.{self.minor_version}" + return f"{prefix}{self.minor_version}" - if self.arch is None: - return f"cp3{self.minor_version}_{self.os}_anyarch" - - if self.os is None: - return f"cp3{self.minor_version}_anyos_{self.arch}" + os = self.os or "any" + cpu = self.arch or "any" - return f"cp3{self.minor_version}_{self.os}_{self.arch}" + return f"{prefix}_{os}_{cpu}" @classmethod def from_string(cls, platform: Union[str, List[str]]) -> List["Platform"]: From 4d2d25699c7288c1ce78d7636af88bbbfd016cc4 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 Feb 2024 13:11:14 +0900 Subject: [PATCH 03/12] finish refactoring the wheel dep generation for multi-platform support --- .../generate_whl_library_build_bazel.bzl | 2 +- .../pip_install/tools/wheel_installer/wheel.py | 4 ++-- .../tools/wheel_installer/wheel_test.py | 17 ++++++++++++++--- 3 files changed, 17 insertions(+), 6 deletions(-) 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 fc8ff2c298..f36544fb3b 100644 --- a/python/pip_install/private/generate_whl_library_build_bazel.bzl +++ b/python/pip_install/private/generate_whl_library_build_bazel.bzl @@ -107,7 +107,7 @@ def _plat_label(plat): elif plat.startswith("@"): return str(Label(plat)) else: - return ":is_" + plat + fail("unsupported platform label") def _render_list_and_select(deps, deps_by_platform, tmpl): deps = render.list([tmpl.format(d) for d in sorted(deps)]) diff --git a/python/pip_install/tools/wheel_installer/wheel.py b/python/pip_install/tools/wheel_installer/wheel.py index ac3ec7beac..b093323d35 100644 --- a/python/pip_install/tools/wheel_installer/wheel.py +++ b/python/pip_install/tools/wheel_installer/wheel.py @@ -168,9 +168,9 @@ def __str__(self) -> str: else: return f"{self.os}_{self.arch}" - prefix = "@//python/config_settings:is_python_3." + prefix = f"@//python/config_settings:is_python_3.{self.minor_version}" if self.arch is None and self.os is None: - return f"{prefix}{self.minor_version}" + return prefix os = self.os or "any" cpu = self.arch or "any" diff --git a/python/pip_install/tools/wheel_installer/wheel_test.py b/python/pip_install/tools/wheel_installer/wheel_test.py index f7c847fc9e..dfdb03caab 100644 --- a/python/pip_install/tools/wheel_installer/wheel_test.py +++ b/python/pip_install/tools/wheel_installer/wheel_test.py @@ -236,13 +236,24 @@ def test_can_get_version_select(self): ) got = deps.build() + self.maxDiff = None + self.assertEqual(["bar"], got.deps) self.assertEqual( { "@//python/config_settings:is_python_3.7": ["baz"], - "cp37_linux_anyarch": ["baz", "posix_dep"], - "cp38_linux_anyarch": ["posix_dep", "posix_dep_with_version"], - "cp39_linux_anyarch": ["posix_dep", "posix_dep_with_version"], + "@//python/config_settings:is_python_3.7_linux_any": [ + "baz", + "posix_dep", + ], + "@//python/config_settings:is_python_3.8_linux_any": [ + "posix_dep", + "posix_dep_with_version", + ], + "@//python/config_settings:is_python_3.9_linux_any": [ + "posix_dep", + "posix_dep_with_version", + ], }, got.deps_select, ) From 1241cdb70db438cf82f24c1f98df5ac4782f68f5 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 Feb 2024 13:24:12 +0900 Subject: [PATCH 04/12] wip --- .../generate_whl_library_build_bazel.bzl | 51 +++++++++++++- .../generate_build_bazel_tests.bzl | 66 +++---------------- 2 files changed, 59 insertions(+), 58 deletions(-) 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 f36544fb3b..edad8e27f1 100644 --- a/python/pip_install/private/generate_whl_library_build_bazel.bzl +++ b/python/pip_install/private/generate_whl_library_build_bazel.bzl @@ -107,7 +107,7 @@ def _plat_label(plat): elif plat.startswith("@"): return str(Label(plat)) else: - fail("unsupported platform label") + return ":is_" + plat def _render_list_and_select(deps, deps_by_platform, tmpl): deps = render.list([tmpl.format(d) for d in sorted(deps)]) @@ -140,6 +140,51 @@ def _render_list_and_select(deps, deps_by_platform, tmpl): else: return "{} + {}".format(deps, deps_by_platform) +def _render_config_settings(dependencies_by_platform): + plats = [] + for p in dependencies_by_platform: + # p can be one of the following formats: + # * @platforms//os:{value} + # * @platforms//cpu:{value} + # * @//python/config_settings:is_python_3.{minor_version} + # * @//python/config_settings:is_python_3.{minor_version}_{os}_{cpu} + # * @//python/config_settings:is_python_3.{minor_version}_{os}_any + # * @//python/config_settings:is_python_3.{minor_version}_any_{cpu} + # * {os}_{cpu} + if p.startswith("@"): + continue + + os, _, arch = p.partition("_") + os = "" if os == "any" else os + arch = "" if arch == "any" else arch + + # TODO @aignas 2024-02-03: simplify + plats.append((os, arch)) + + if not plats: + return None, None + + additional_content = [] + for (os, arch) in plats: + constraint_values = [ + "@platforms//os:{}".format(os), + "@platforms//cpu:{}".format(arch), + ] + + additional_content.append( + """\ +config_setting( + name = "is_{name}", + constraint_values = {values}, + visibility = ["//visibility:private"], +)""".format( + name = "{}_{}".format(os, arch), + values = render.indent(render.list(sorted([str(Label(c)) for c in constraint_values]))).strip(), + ), + ) + + return "\n\n".join(additional_content) + def generate_whl_library_build_bazel( *, repo_prefix, @@ -247,6 +292,10 @@ def generate_whl_library_build_bazel( """load("@bazel_skylib//rules:copy_file.bzl", "copy_file")""", ] + config_settings_content = _render_config_settings(dependencies_by_platform) + if config_settings_content: + additional_content.append(config_settings_content) + lib_dependencies = _render_list_and_select( deps = dependencies, deps_by_platform = dependencies_by_platform, 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 72423aaec4..66a09b5b85 100644 --- a/tests/pip_install/whl_library/generate_build_bazel_tests.bzl +++ b/tests/pip_install/whl_library/generate_build_bazel_tests.bzl @@ -120,12 +120,12 @@ filegroup( "@pypi_foo//:whl", ] + select( { + "@//python/config_settings:is_python_3.10_linux_ppc": ["@pypi_py310_linux_ppc_dep//:whl"], "@//python/config_settings:is_python_3.9": ["@pypi_py39_dep//:whl"], + "@//python/config_settings:is_python_3.9_anyos_aarch64": ["@pypi_py39_arm_dep//:whl"], + "@//python/config_settings:is_python_3.9_linux_anyarch": ["@pypi_py39_linux_dep//:whl"], "@platforms//cpu:aarch64": ["@pypi_arm_dep//:whl"], "@platforms//os:windows": ["@pypi_win_dep//:whl"], - ":is_cp310_linux_ppc": ["@pypi_py310_linux_ppc_dep//:whl"], - ":is_cp39_anyos_aarch64": ["@pypi_py39_arm_dep//:whl"], - ":is_cp39_linux_anyarch": ["@pypi_py39_linux_dep//:whl"], ":is_linux_x86_64": ["@pypi_linux_intel_dep//:whl"], "//conditions:default": [], }, @@ -154,12 +154,12 @@ py_library( "@pypi_foo//:pkg", ] + select( { + "@//python/config_settings:is_python_3.10_linux_ppc": ["@pypi_py310_linux_ppc_dep//:pkg"], "@//python/config_settings:is_python_3.9": ["@pypi_py39_dep//:pkg"], + "@//python/config_settings:is_python_3.9_anyos_aarch64": ["@pypi_py39_arm_dep//:pkg"], + "@//python/config_settings:is_python_3.9_linux_anyarch": ["@pypi_py39_linux_dep//:pkg"], "@platforms//cpu:aarch64": ["@pypi_arm_dep//:pkg"], "@platforms//os:windows": ["@pypi_win_dep//:pkg"], - ":is_cp310_linux_ppc": ["@pypi_py310_linux_ppc_dep//:pkg"], - ":is_cp39_anyos_aarch64": ["@pypi_py39_arm_dep//:pkg"], - ":is_cp39_linux_anyarch": ["@pypi_py39_linux_dep//:pkg"], ":is_linux_x86_64": ["@pypi_linux_intel_dep//:pkg"], "//conditions:default": [], }, @@ -178,54 +178,6 @@ alias( actual = "_whl", ) -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", - 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", - 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 = [ @@ -240,12 +192,12 @@ config_setting( whl_name = "foo.whl", dependencies = ["foo", "bar-baz"], dependencies_by_platform = { + "@//python/config_settings:is_python_3.10_linux_ppc": ["py310_linux_ppc_dep"], "@//python/config_settings:is_python_3.9": ["py39_dep"], + "@//python/config_settings:is_python_3.9_any_aarch64": ["py39_arm_dep"], + "@//python/config_settings:is_python_3.9_linux_any": ["py39_linux_dep"], "@platforms//cpu:aarch64": ["arm_dep"], "@platforms//os:windows": ["win_dep"], - "cp310_linux_ppc": ["py310_linux_ppc_dep"], - "cp39_anyos_aarch64": ["py39_arm_dep"], - "cp39_linux_anyarch": ["py39_linux_dep"], "linux_x86_64": ["linux_intel_dep"], }, data_exclude = [], From f5c36ddd113cc58eb8c0b836f70df9ed9cbfe2a4 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 Feb 2024 21:35:45 +0900 Subject: [PATCH 05/12] update --- .../private/generate_whl_library_build_bazel.bzl | 2 +- .../whl_library/generate_build_bazel_tests.bzl | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) 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 edad8e27f1..57897d35b5 100644 --- a/python/pip_install/private/generate_whl_library_build_bazel.bzl +++ b/python/pip_install/private/generate_whl_library_build_bazel.bzl @@ -162,7 +162,7 @@ def _render_config_settings(dependencies_by_platform): plats.append((os, arch)) if not plats: - return None, None + return None additional_content = [] for (os, arch) in plats: 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 66a09b5b85..40d0c06ca2 100644 --- a/tests/pip_install/whl_library/generate_build_bazel_tests.bzl +++ b/tests/pip_install/whl_library/generate_build_bazel_tests.bzl @@ -98,7 +98,6 @@ 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") package(default_visibility = ["//visibility:public"]) @@ -122,8 +121,8 @@ filegroup( { "@//python/config_settings:is_python_3.10_linux_ppc": ["@pypi_py310_linux_ppc_dep//:whl"], "@//python/config_settings:is_python_3.9": ["@pypi_py39_dep//:whl"], - "@//python/config_settings:is_python_3.9_anyos_aarch64": ["@pypi_py39_arm_dep//:whl"], - "@//python/config_settings:is_python_3.9_linux_anyarch": ["@pypi_py39_linux_dep//:whl"], + "@//python/config_settings:is_python_3.9_any_aarch64": ["@pypi_py39_arm_dep//:whl"], + "@//python/config_settings:is_python_3.9_linux_any": ["@pypi_py39_linux_dep//:whl"], "@platforms//cpu:aarch64": ["@pypi_arm_dep//:whl"], "@platforms//os:windows": ["@pypi_win_dep//:whl"], ":is_linux_x86_64": ["@pypi_linux_intel_dep//:whl"], @@ -156,8 +155,8 @@ py_library( { "@//python/config_settings:is_python_3.10_linux_ppc": ["@pypi_py310_linux_ppc_dep//:pkg"], "@//python/config_settings:is_python_3.9": ["@pypi_py39_dep//:pkg"], - "@//python/config_settings:is_python_3.9_anyos_aarch64": ["@pypi_py39_arm_dep//:pkg"], - "@//python/config_settings:is_python_3.9_linux_anyarch": ["@pypi_py39_linux_dep//:pkg"], + "@//python/config_settings:is_python_3.9_any_aarch64": ["@pypi_py39_arm_dep//:pkg"], + "@//python/config_settings:is_python_3.9_linux_any": ["@pypi_py39_linux_dep//:pkg"], "@platforms//cpu:aarch64": ["@pypi_arm_dep//:pkg"], "@platforms//os:windows": ["@pypi_win_dep//:pkg"], ":is_linux_x86_64": ["@pypi_linux_intel_dep//:pkg"], From b615a77b4faf78c76c7032a6b4c1ab1238203b1f Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 Feb 2024 22:39:45 +0900 Subject: [PATCH 06/12] refactor and split into multiple functions --- python/config_settings/config_settings.bzl | 182 +++++++++++---------- 1 file changed, 98 insertions(+), 84 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index cdbbc84615..3845c6599a 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -73,23 +73,78 @@ def construct_config_settings(name, python_versions): visibility = ["//visibility:public"], ) - _construct_config_settings_for_os_cpu( - minor_to_micro_versions = minor_to_micro_versions, - minor_to_plats = minor_to_plats, - micro_to_plats = micro_to_plats, - ) + for minor_version, micro_versions in minor_to_micro_versions.items(): + _config_settings_for_minor_version( + minor_version = minor_version, + micro_versions = { + v: micro_to_plats[v] + for v in micro_versions + }, + ) + +def _config_settings_for_minor_version(*, minor_version, micro_versions): + """Constructs a set of configs for all Python versions. + """ + matches_minor_version_names = {} + constraint_values_by_suffix = {} + + for micro_version, plats in micro_versions.items(): + for suffix, constraint_values in _constraint_values(plats).items(): + matches_micro_version = _micro_version_condition( + micro_version = micro_version, + suffix = suffix, + constraint_values = constraint_values, + ) + matches_minor_version_names.setdefault(suffix, []).append(matches_micro_version) + constraint_values_by_suffix[suffix] = constraint_values + + for suffix, constraint_values in constraint_values_by_suffix.items(): + # This matches the raw flag value, e.g. --//python/config_settings:python_version=3.8 + # It's private because matching the concept of e.g. "3.8" value is done + # using the `is_python_X.Y` config setting group, which is aware of the + # minor versions that could match instead. + equals_minor_version = "_python_version_flag_equals_" + minor_version + suffix + native.config_setting( + name = equals_minor_version, + flag_values = {":python_version": minor_version}, + constraint_values = constraint_values, + ) + matches_minor_version_names[suffix].append(equals_minor_version) + + # This is prefixed with an underscore to prevent confusion due to how + # config_setting_group is implemented and how our micro-version targets + # are named. config_setting_group will generate targets like + # "is_python_3.10_1" (where the `_N` suffix is len(match_any). + # Meanwhile, the micro-version tarets are named "is_python_3.10.1" -- + # just a single dot vs underscore character difference. + selects.config_setting_group( + name = "_is_python_" + minor_version + suffix, + match_any = matches_minor_version_names[suffix], + ) + + native.alias( + name = "is_python_" + minor_version + suffix, + actual = "_is_python_" + minor_version, + visibility = ["//visibility:public"], + ) def _constraint_values(plats): + """For a list of (os, cpu) tuples get all possible platform names and constraint values. + """ ret = { + # This is the no platform constraint values version "": [], } _plats = [] for (os, cpu) in plats: if (os, None) not in _plats: + # Add only the OS constraint value _plats.append((os, None)) if (None, cpu) not in _plats: + # Add only the CPU constraint value _plats.append((None, cpu)) + # Add both OS and CPU constraint values _plats.append((os, cpu)) for (os, cpu) in _plats: @@ -106,83 +161,42 @@ def _constraint_values(plats): return ret -def _construct_config_settings_for_os_cpu(*, minor_to_micro_versions, minor_to_plats, micro_to_plats): - """Constructs a set of configs for all Python versions. - - Args: - minor_to_micro_versions: Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] - minor_to_plats: TODO - micro_to_plats: TODO - """ - for minor_version, micro_versions in minor_to_micro_versions.items(): - matches_minor_version_names = {} - for name, constraint_values in _constraint_values(minor_to_plats[minor_version]).items(): - # This matches the raw flag value, e.g. --//python/config_settings:python_version=3.8 - # It's private because matching the concept of e.g. "3.8" value is done - # using the `is_python_X.Y` config setting group, which is aware of the - # minor versions that could match instead. - equals_minor_version_name = "_python_version_flag_equals_" + minor_version + name - native.config_setting( - name = equals_minor_version_name, - flag_values = {":python_version": minor_version}, - constraint_values = constraint_values, - ) - matches_minor_version_names[name] = [equals_minor_version_name] - - for micro_version in micro_versions: - for name, constraint_values in _constraint_values(micro_to_plats[micro_version]).items(): - is_micro_version_name = "is_python_" + micro_version + name - if MINOR_MAPPING[minor_version] != micro_version: - native.config_setting( - name = is_micro_version_name, - flag_values = {":python_version": micro_version}, - constraint_values = constraint_values, - visibility = ["//visibility:public"], - ) - matches_minor_version_names[name].append(is_micro_version_name) - continue - - # Ensure that is_python_3.9.8 is matched if python_version is set - # to 3.9 if MINOR_MAPPING points to 3.9.8 - equals_micro_name = "_python_version_flag_equals_" + micro_version + name - native.config_setting( - name = equals_micro_name, - flag_values = {":python_version": micro_version}, - constraint_values = constraint_values, - ) - - # An alias pointing to an underscore-prefixed config_setting_group - # is used because config_setting_group creates - # `is_{minor}_N` targets, which are easily confused with the - # `is_{minor}.{micro}` (dot) targets. - selects.config_setting_group( - name = "_" + is_micro_version_name, - match_any = [ - equals_micro_name, - matches_minor_version_names[name][0], - ], - ) - native.alias( - name = is_micro_version_name, - actual = "_" + is_micro_version_name, - visibility = ["//visibility:public"], - ) - matches_minor_version_names[name].append(equals_micro_name) - - for name in _constraint_values(minor_to_plats[minor_version]).keys(): - # This is prefixed with an underscore to prevent confusion due to how - # config_setting_group is implemented and how our micro-version targets - # are named. config_setting_group will generate targets like - # "is_python_3.10_1" (where the `_N` suffix is len(match_any). - # Meanwhile, the micro-version tarets are named "is_python_3.10.1" -- - # just a single dot vs underscore character difference. - selects.config_setting_group( - name = "_is_python_" + minor_version + name, - match_any = matches_minor_version_names[name], - ) +def _micro_version_condition(*, micro_version, suffix, constraint_values): + minor_version, _, _ = micro_version.rpartition(".") + is_micro_version_name = "is_python_" + micro_version + suffix + if MINOR_MAPPING[minor_version] != micro_version: + native.config_setting( + name = is_micro_version_name, + flag_values = {":python_version": micro_version}, + constraint_values = constraint_values, + visibility = ["//visibility:public"], + ) + return is_micro_version_name + + # Ensure that is_python_3.9.8 is matched if python_version is set + # to 3.9 if MINOR_MAPPING points to 3.9.8 + equals_micro_name = "_python_version_flag_equals_" + micro_version + suffix + equals_minor_name = "_python_version_flag_equals_" + minor_version + suffix + native.config_setting( + name = equals_micro_name, + flag_values = {":python_version": micro_version}, + constraint_values = constraint_values, + ) - native.alias( - name = "is_python_" + minor_version + name, - actual = "_is_python_" + minor_version, - visibility = ["//visibility:public"], - ) + # An alias pointing to an underscore-prefixed config_setting_group + # is used because config_setting_group creates + # `is_{minor}_N` targets, which are easily confused with the + # `is_{minor}.{micro}` (dot) targets. + selects.config_setting_group( + name = "_" + is_micro_version_name, + match_any = [ + equals_micro_name, + equals_minor_name, + ], + ) + native.alias( + name = is_micro_version_name, + actual = "_" + is_micro_version_name, + visibility = ["//visibility:public"], + ) + return equals_micro_name From c59b985774d5f01b2c9df240efeb89127fc58251 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 Feb 2024 22:46:06 +0900 Subject: [PATCH 07/12] more cleanup --- python/config_settings/config_settings.bzl | 41 ++++++++++------------ 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 3845c6599a..0ae877573d 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -32,33 +32,17 @@ def construct_config_settings(name, python_versions): # Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] minor_to_micro_versions = {} - minor_to_plats = {} - micro_to_plats = {} + micro_version_to_platforms = {} allowed_flag_values = [] for micro_version, plats in python_versions.items(): minor, _, _ = micro_version.rpartition(".") minor_to_micro_versions.setdefault(minor, []).append(micro_version) allowed_flag_values.append(micro_version) - - for plat in plats: - cpu, _, os = plat.partition("-") - if "linux" in os: - os = "linux" - elif "darwin" in os: - os = "osx" - elif "windows" in os: - os = "windows" - else: - fail("unknown os: {}".format(os)) - - p = (os, cpu) - - # TODO @aignas 2024-02-03: use bazel skylib sets - if minor not in minor_to_plats or p not in minor_to_plats[minor]: - minor_to_plats.setdefault(minor, []).append(p) - if micro_version not in micro_to_plats or (os, cpu) not in micro_to_plats[micro_version]: - micro_to_plats.setdefault(micro_version, []).append(p) + micro_version_to_platforms[micro_version] = [ + _parse_platform(plat) + for plat in plats + ] allowed_flag_values.extend(list(minor_to_micro_versions)) @@ -77,11 +61,24 @@ def construct_config_settings(name, python_versions): _config_settings_for_minor_version( minor_version = minor_version, micro_versions = { - v: micro_to_plats[v] + v: micro_version_to_platforms[v] for v in micro_versions }, ) +def _parse_platform(plat): + cpu, _, os = plat.partition("-") + if "linux" in os: + os = "linux" + elif "darwin" in os: + os = "osx" + elif "windows" in os: + os = "windows" + else: + fail("unknown os: {}".format(os)) + + return (os, cpu) + def _config_settings_for_minor_version(*, minor_version, micro_versions): """Constructs a set of configs for all Python versions. """ From f697e915fdd0933027d352d47a1b78ae83e16e09 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 3 Feb 2024 22:50:19 +0900 Subject: [PATCH 08/12] more cleanup --- python/config_settings/config_settings.bzl | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 0ae877573d..2c4f5d1f32 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -32,17 +32,12 @@ def construct_config_settings(name, python_versions): # Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] minor_to_micro_versions = {} - micro_version_to_platforms = {} allowed_flag_values = [] - for micro_version, plats in python_versions.items(): + for micro_version in python_versions: minor, _, _ = micro_version.rpartition(".") minor_to_micro_versions.setdefault(minor, []).append(micro_version) allowed_flag_values.append(micro_version) - micro_version_to_platforms[micro_version] = [ - _parse_platform(plat) - for plat in plats - ] allowed_flag_values.extend(list(minor_to_micro_versions)) @@ -61,7 +56,10 @@ def construct_config_settings(name, python_versions): _config_settings_for_minor_version( minor_version = minor_version, micro_versions = { - v: micro_version_to_platforms[v] + v: [ + _parse_platform(plat) + for plat in python_versions[v] + ] for v in micro_versions }, ) From b12ce9666691c99a062bd98e487225f1cb7c64c4 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 4 Feb 2024 12:59:00 +0900 Subject: [PATCH 09/12] Use a better naming scheme for os/arch only config settings --- python/config_settings/config_settings.bzl | 8 ++++---- python/pip_install/tools/wheel_installer/wheel.py | 9 ++++++--- .../pip_install/tools/wheel_installer/wheel_test.py | 6 +++--- .../whl_library/generate_build_bazel_tests.bzl | 12 ++++++------ 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 2c4f5d1f32..42df80249d 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -144,15 +144,15 @@ def _constraint_values(plats): for (os, cpu) in _plats: constraint_values = [] + parts = [""] if os: constraint_values.append("@platforms//os:{}".format(os)) + parts.append(os) if cpu: constraint_values.append("@platforms//cpu:{}".format(cpu)) + parts.append(cpu) - os = os or "any" - cpu = cpu or "any" - - ret["_".join(["", os, cpu])] = constraint_values + ret["_".join(parts)] = constraint_values return ret diff --git a/python/pip_install/tools/wheel_installer/wheel.py b/python/pip_install/tools/wheel_installer/wheel.py index b093323d35..832a728726 100644 --- a/python/pip_install/tools/wheel_installer/wheel.py +++ b/python/pip_install/tools/wheel_installer/wheel.py @@ -172,10 +172,13 @@ def __str__(self) -> str: if self.arch is None and self.os is None: return prefix - os = self.os or "any" - cpu = self.arch or "any" + suffix = [prefix] + if self.os: + suffix.append(str(self.os)) + if self.arch: + suffix.append(str(self.arch)) - return f"{prefix}_{os}_{cpu}" + return "_".join(suffix) @classmethod def from_string(cls, platform: Union[str, List[str]]) -> List["Platform"]: diff --git a/python/pip_install/tools/wheel_installer/wheel_test.py b/python/pip_install/tools/wheel_installer/wheel_test.py index dfdb03caab..5e51f8405f 100644 --- a/python/pip_install/tools/wheel_installer/wheel_test.py +++ b/python/pip_install/tools/wheel_installer/wheel_test.py @@ -242,15 +242,15 @@ def test_can_get_version_select(self): self.assertEqual( { "@//python/config_settings:is_python_3.7": ["baz"], - "@//python/config_settings:is_python_3.7_linux_any": [ + "@//python/config_settings:is_python_3.7_linux": [ "baz", "posix_dep", ], - "@//python/config_settings:is_python_3.8_linux_any": [ + "@//python/config_settings:is_python_3.8_linux": [ "posix_dep", "posix_dep_with_version", ], - "@//python/config_settings:is_python_3.9_linux_any": [ + "@//python/config_settings:is_python_3.9_linux": [ "posix_dep", "posix_dep_with_version", ], 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 40d0c06ca2..7667ceeabb 100644 --- a/tests/pip_install/whl_library/generate_build_bazel_tests.bzl +++ b/tests/pip_install/whl_library/generate_build_bazel_tests.bzl @@ -121,8 +121,8 @@ filegroup( { "@//python/config_settings:is_python_3.10_linux_ppc": ["@pypi_py310_linux_ppc_dep//:whl"], "@//python/config_settings:is_python_3.9": ["@pypi_py39_dep//:whl"], - "@//python/config_settings:is_python_3.9_any_aarch64": ["@pypi_py39_arm_dep//:whl"], - "@//python/config_settings:is_python_3.9_linux_any": ["@pypi_py39_linux_dep//:whl"], + "@//python/config_settings:is_python_3.9_aarch64": ["@pypi_py39_arm_dep//:whl"], + "@//python/config_settings:is_python_3.9_linux": ["@pypi_py39_linux_dep//:whl"], "@platforms//cpu:aarch64": ["@pypi_arm_dep//:whl"], "@platforms//os:windows": ["@pypi_win_dep//:whl"], ":is_linux_x86_64": ["@pypi_linux_intel_dep//:whl"], @@ -155,8 +155,8 @@ py_library( { "@//python/config_settings:is_python_3.10_linux_ppc": ["@pypi_py310_linux_ppc_dep//:pkg"], "@//python/config_settings:is_python_3.9": ["@pypi_py39_dep//:pkg"], - "@//python/config_settings:is_python_3.9_any_aarch64": ["@pypi_py39_arm_dep//:pkg"], - "@//python/config_settings:is_python_3.9_linux_any": ["@pypi_py39_linux_dep//:pkg"], + "@//python/config_settings:is_python_3.9_aarch64": ["@pypi_py39_arm_dep//:pkg"], + "@//python/config_settings:is_python_3.9_linux": ["@pypi_py39_linux_dep//:pkg"], "@platforms//cpu:aarch64": ["@pypi_arm_dep//:pkg"], "@platforms//os:windows": ["@pypi_win_dep//:pkg"], ":is_linux_x86_64": ["@pypi_linux_intel_dep//:pkg"], @@ -193,8 +193,8 @@ config_setting( dependencies_by_platform = { "@//python/config_settings:is_python_3.10_linux_ppc": ["py310_linux_ppc_dep"], "@//python/config_settings:is_python_3.9": ["py39_dep"], - "@//python/config_settings:is_python_3.9_any_aarch64": ["py39_arm_dep"], - "@//python/config_settings:is_python_3.9_linux_any": ["py39_linux_dep"], + "@//python/config_settings:is_python_3.9_aarch64": ["py39_arm_dep"], + "@//python/config_settings:is_python_3.9_linux": ["py39_linux_dep"], "@platforms//cpu:aarch64": ["arm_dep"], "@platforms//os:windows": ["win_dep"], "linux_x86_64": ["linux_intel_dep"], From 12b8ff13153fa74cda3bde99735e75538ed788c5 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 5 Feb 2024 11:24:43 +0900 Subject: [PATCH 10/12] add tests and bugfix --- .bazelrc | 4 +- python/config_settings/config_settings.bzl | 20 ++++---- tests/config_settings/BUILD.bazel | 8 ++++ .../construct_config_settings_tests.bzl | 46 +++++++++++++++++-- 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/.bazelrc b/.bazelrc index c911ea5378..eb00db9bb1 100644 --- a/.bazelrc +++ b/.bazelrc @@ -4,8 +4,8 @@ # (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it) # To update these lines, execute # `bazel run @rules_bazel_integration_test//tools:update_deleted_packages` -build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered -query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered +build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered +query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered test --test_output=errors diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 42df80249d..a84f10e604 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -119,7 +119,7 @@ def _config_settings_for_minor_version(*, minor_version, micro_versions): native.alias( name = "is_python_" + minor_version + suffix, - actual = "_is_python_" + minor_version, + actual = "_is_python_" + minor_version + suffix, visibility = ["//visibility:public"], ) @@ -130,19 +130,23 @@ def _constraint_values(plats): # This is the no platform constraint values version "": [], } - _plats = [] + os_cpus = [] for (os, cpu) in plats: - if (os, None) not in _plats: + # Normalize the cpu names to the ones present in the `@platforms//cpu` + if cpu == "ppc64le": + cpu = "ppc" + + if (os, None) not in os_cpus: # Add only the OS constraint value - _plats.append((os, None)) - if (None, cpu) not in _plats: + os_cpus.append((os, None)) + if (None, cpu) not in os_cpus: # Add only the CPU constraint value - _plats.append((None, cpu)) + os_cpus.append((None, cpu)) # Add both OS and CPU constraint values - _plats.append((os, cpu)) + os_cpus.append((os, cpu)) - for (os, cpu) in _plats: + for (os, cpu) in os_cpus: constraint_values = [] parts = [""] if os: diff --git a/tests/config_settings/BUILD.bazel b/tests/config_settings/BUILD.bazel index 212e3f7b02..7fd1e0c093 100644 --- a/tests/config_settings/BUILD.bazel +++ b/tests/config_settings/BUILD.bazel @@ -17,3 +17,11 @@ load(":construct_config_settings_tests.bzl", "construct_config_settings_test_sui construct_config_settings_test_suite( name = "construct_config_settings_tests", ) + +platform( + name = "linux_aarch64", + constraint_values = [ + "@platforms//os:linux", + "@platforms//cpu:aarch64", + ], +) diff --git a/tests/config_settings/construct_config_settings_tests.bzl b/tests/config_settings/construct_config_settings_tests.bzl index 61beb9cd40..de0e6a162d 100644 --- a/tests/config_settings/construct_config_settings_tests.bzl +++ b/tests/config_settings/construct_config_settings_tests.bzl @@ -28,21 +28,49 @@ def _subject_impl(ctx): _subject = rule( implementation = _subject_impl, attrs = { + "match_cpu": attr.string(), "match_micro": attr.string(), "match_minor": attr.string(), + "match_os": attr.string(), + "match_os_cpu": attr.string(), "no_match": attr.string(), "no_match_micro": attr.string(), }, ) def _test_minor_version_matching(name): + minor_matches = { + "//python/config_settings:is_python_3.11": "matched-3.11", + "//conditions:default": "matched-default", + } + minor_cpu_matches = { + "//python/config_settings:is_python_3.11_aarch64": "matched-3.11-aarch64", + "//python/config_settings:is_python_3.11_ppc": "matched-3.11-ppc", + "//python/config_settings:is_python_3.11_s390x": "matched-3.11-s390x", + "//python/config_settings:is_python_3.11_x86_64": "matched-3.11-x86_64", + } + minor_os_matches = { + "//python/config_settings:is_python_3.11_linux": "matched-3.11-linux", + "//python/config_settings:is_python_3.11_osx": "matched-3.11-osx", + "//python/config_settings:is_python_3.11_windows": "matched-3.11-windows", + } + minor_os_cpu_matches = { + "//python/config_settings:is_python_3.11_linux_aarch64": "matched-3.11-linux-aarch64", + "//python/config_settings:is_python_3.11_linux_ppc": "matched-3.11-linux-ppc", + "//python/config_settings:is_python_3.11_linux_s390x": "matched-3.11-linux-s390x", + "//python/config_settings:is_python_3.11_linux_x86_64": "matched-3.11-linux-x86_64", + "//python/config_settings:is_python_3.11_osx_aarch64": "matched-3.11-osx-aarch64", + "//python/config_settings:is_python_3.11_osx_x86_64": "matched-3.11-osx-x86_64", + "//python/config_settings:is_python_3.11_windows_x86_64": "matched-3.11-windows-x86_64", + } + rt_util.helper_target( _subject, name = name + "_subject", - match_minor = select({ - "//python/config_settings:is_python_3.11": "matched-3.11", - "//conditions:default": "matched-default", - }), + match_minor = select(minor_matches), + match_cpu = select(minor_matches | minor_cpu_matches), + match_os = select(minor_matches | minor_os_matches), + match_os_cpu = select(minor_matches | minor_cpu_matches | minor_os_matches | minor_os_cpu_matches), match_micro = select({ "//python/config_settings:is_python_3.11": "matched-3.11", "//conditions:default": "matched-default", @@ -59,6 +87,7 @@ def _test_minor_version_matching(name): impl = _test_minor_version_matching_impl, config_settings = { str(Label("//python/config_settings:python_version")): "3.11.1", + "//command_line_option:platforms": str(Label("//tests/config_settings:linux_aarch64")), }, ) @@ -70,6 +99,15 @@ def _test_minor_version_matching_impl(env, target): target.attr("match_micro", factory = subjects.str).equals( "matched-3.11", ) + target.attr("match_cpu", factory = subjects.str).equals( + "matched-3.11-aarch64", + ) + target.attr("match_os", factory = subjects.str).equals( + "matched-3.11-linux", + ) + target.attr("match_os_cpu", factory = subjects.str).equals( + "matched-3.11-linux-aarch64", + ) target.attr("no_match", factory = subjects.str).equals( "matched-default", ) From 12a1e473b857436f0e5b656d02002382e523eb32 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 5 Feb 2024 21:18:53 +0900 Subject: [PATCH 11/12] doc: add CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51c7c6fdb8..a8c102895c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,12 @@ A brief description of the categories of changes: set to make repository rules log detailed information about what they're up to. +* (toolchain) The variants of `is_python` `config_setting` including the + target os and cpu values have been added for better ergonomics for matching + both, the python version and the os/cpu value in a single select statement. + They are added mainly for internal use and the API might change at any time + as needed by the internal repository rules. + ## 0.29.0 - 2024-01-22 [0.29.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.29.0 From 0f7da90d05076e03ad95a41cafe9eaa355b3769c Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Thu, 8 Feb 2024 16:16:30 +0900 Subject: [PATCH 12/12] Apply suggestions from self code review --- .../pip_install/private/generate_whl_library_build_bazel.bzl | 5 ++--- python/pip_install/tools/wheel_installer/wheel_test.py | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) 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 57897d35b5..077aff41e5 100644 --- a/python/pip_install/private/generate_whl_library_build_bazel.bzl +++ b/python/pip_install/private/generate_whl_library_build_bazel.bzl @@ -148,8 +148,8 @@ def _render_config_settings(dependencies_by_platform): # * @platforms//cpu:{value} # * @//python/config_settings:is_python_3.{minor_version} # * @//python/config_settings:is_python_3.{minor_version}_{os}_{cpu} - # * @//python/config_settings:is_python_3.{minor_version}_{os}_any - # * @//python/config_settings:is_python_3.{minor_version}_any_{cpu} + # * @//python/config_settings:is_python_3.{minor_version}_{os} + # * @//python/config_settings:is_python_3.{minor_version}_{cpu} # * {os}_{cpu} if p.startswith("@"): continue @@ -158,7 +158,6 @@ def _render_config_settings(dependencies_by_platform): os = "" if os == "any" else os arch = "" if arch == "any" else arch - # TODO @aignas 2024-02-03: simplify plats.append((os, arch)) if not plats: diff --git a/python/pip_install/tools/wheel_installer/wheel_test.py b/python/pip_install/tools/wheel_installer/wheel_test.py index 5e51f8405f..97f037a16e 100644 --- a/python/pip_install/tools/wheel_installer/wheel_test.py +++ b/python/pip_install/tools/wheel_installer/wheel_test.py @@ -236,8 +236,6 @@ def test_can_get_version_select(self): ) got = deps.build() - self.maxDiff = None - self.assertEqual(["bar"], got.deps) self.assertEqual( {