diff --git a/CHANGELOG.md b/CHANGELOG.md index c4c99206b1..7207b30be0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ A brief description of the categories of changes: ### Removed * (toolchains): Removed accidentally exposed `http_archive` symbol from `python/repositories.bzl`. +* (toolchains): An internal _is_python_config_setting_ macro has been removed. ## [0.35.0] - 2024-08-15 @@ -273,7 +274,7 @@ A brief description of the categories of changes: be automatically deleted correctly. For example, if `python_generation_mode` is set to package, when `__init__.py` is deleted, the `py_library` generated for this package before will be deleted automatically. -* (whl_library): Use `is_python_config_setting` to correctly handle multi-python +* (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. The default python version case within the select is also now handled correctly, stabilizing the implementation. diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock index a34623ea08..f0144e82e0 100644 --- a/examples/bzlmod/MODULE.bazel.lock +++ b/examples/bzlmod/MODULE.bazel.lock @@ -1231,7 +1231,7 @@ }, "@@rules_python~//python/extensions:pip.bzl%pip": { "general": { - "bzlTransitiveDigest": "3vqCp6yUy32HDgpZG9L+zqedcsEnHZu0Y7hfoRk3owY=", + "bzlTransitiveDigest": "jb9c5l3dvSB2MHd/zfkT8Yr8efvg+K/YHtiRHU3aU6o=", "usagesDigest": "MChlcSw99EuW3K7OOoMcXQIdcJnEh6YmfyjJm+9mxIg=", "recordedFileInputs": { "@@other_module~//requirements_lock_3_11.txt": "a7d0061366569043d5efcf80e34a32c732679367cb3c831c4cdc606adc36d314", @@ -6140,7 +6140,7 @@ }, "@@rules_python~//python/private/pypi:pip.bzl%pip_internal": { "general": { - "bzlTransitiveDigest": "DdkE6u15ketmEdOGiZ1UcHX7sZV/xpVbFQLWBKjOAYM=", + "bzlTransitiveDigest": "cLqaqCEOdhle6//lX1Kcs2hfkmSerh2fk0izwhV8/GU=", "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=", "recordedFileInputs": { "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a", diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 20bc50660a..c31d69f202 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -1,4 +1,5 @@ load("@bazel_skylib//rules:common_settings.bzl", "string_flag") +load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS") load( "//python/private:flags.bzl", "BootstrapImplFlag", @@ -27,6 +28,8 @@ filegroup( construct_config_settings( name = "construct_config_settings", + minor_mapping = MINOR_MAPPING, + versions = TOOL_VERSIONS.keys(), ) string_flag( diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index f1d2ff0e06..44104259b7 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -18,13 +18,7 @@ load( "//python/private:config_settings.bzl", _construct_config_settings = "construct_config_settings", - _is_python_config_setting = "is_python_config_setting", ) -# This is exposed only for cases where the pip hub repo needs to use this rule -# to define hub-repo scoped config_settings for platform specific wheel -# support. -is_python_config_setting = _is_python_config_setting - # This is exposed for usage in rules_python only. construct_config_settings = _construct_config_settings diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index a35e2f7c2e..ab0604d53f 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -78,8 +78,9 @@ bzl_library( name = "config_settings_bzl", srcs = ["config_settings.bzl"], deps = [ - "//python:versions_bzl", + ":semver_bzl", "@bazel_skylib//lib:selects", + "@bazel_skylib//rules:common_settings", ], ) diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index 301a97b25d..b15d6a8e03 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -17,201 +17,95 @@ load("@bazel_skylib//lib:selects.bzl", "selects") load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") -load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS") +load(":semver.bzl", "semver") -_PYTHON_VERSION_FLAG = str(Label("//python/config_settings:python_version")) +_PYTHON_VERSION_FLAG = Label("//python/config_settings:python_version") +_PYTHON_VERSION_MAJOR_MINOR_FLAG = Label("//python/config_settings:_python_version_major_minor") -def _ver_key(s): - major, _, s = s.partition(".") - minor, _, s = s.partition(".") - micro, _, s = s.partition(".") - return (int(major), int(minor), int(micro)) - -def _flag_values(*, python_versions, minor_mapping): - """Construct a map of python_version to a list of toolchain values. - - This mapping maps the concept of a config setting to a list of compatible toolchain versions. - For using this in the code, the VERSION_FLAG_VALUES should be used instead. - - Args: - python_versions: {type}`list[str]` X.Y.Z` python versions. - minor_mapping: {type}`dict[str, str]` `X.Y` to `X.Y.Z` mapping. - - Returns: - A `map[str, list[str]]`. Each key is a python_version flag value. Each value - is a list of the python_version flag values that should match when for the - `key`. For example: - ``` - "3.8" -> ["3.8", "3.8.1", "3.8.2", ..., "3.8.19"] # All 3.8 versions - "3.8.2" -> ["3.8.2"] # Only 3.8.2 - "3.8.19" -> ["3.8.19", "3.8"] # The latest version should also match 3.8 so - as when the `3.8` toolchain is used we just use the latest `3.8` toolchain. - this makes the `select("is_python_3.8.19")` work no matter how the user - specifies the latest python version to use. - ``` - """ - ret = {} - - for micro_version in sorted(python_versions, key = _ver_key): - minor_version, _, _ = micro_version.rpartition(".") - - # 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. - ret.setdefault(minor_version, [minor_version]).append(micro_version) - - # 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 - default_micro_version = minor_mapping[minor_version] - ret[micro_version] = [micro_version, minor_version] if default_micro_version == micro_version else [micro_version] - - return ret - -VERSION_FLAG_VALUES = _flag_values(python_versions = TOOL_VERSIONS.keys(), minor_mapping = MINOR_MAPPING) - -def is_python_config_setting(name, *, python_version, reuse_conditions = None, **kwargs): - """Create a config setting for matching 'python_version' configuration flag. - - This function is mainly intended for internal use within the `whl_library` and `pip_parse` - machinery. - - The matching of the 'python_version' flag depends on the value passed in - `python_version` and here is the example for `3.8` (but the same applies - to other python versions present in @//python:versions.bzl#TOOL_VERSIONS): - * "3.8" -> ["3.8", "3.8.1", "3.8.2", ..., "3.8.19"] # All 3.8 versions - * "3.8.2" -> ["3.8.2"] # Only 3.8.2 - * "3.8.19" -> ["3.8.19", "3.8"] # The latest version should also match 3.8 so - as when the `3.8` toolchain is used we just use the latest `3.8` toolchain. - this makes the `select("is_python_3.8.19")` work no matter how the user - specifies the latest python version to use. - - Args: - name: name for the target that will be created to be used in select statements. - python_version: The python_version to be passed in the `flag_values` in the - `config_setting`. Depending on the version, the matching python version list - can be as described above. - reuse_conditions: A dict of version to version label for which we should - reuse config_setting targets instead of creating them from scratch. This - is useful when using is_python_config_setting multiple times in the - same package with the same `major.minor` python versions. - **kwargs: extra kwargs passed to the `config_setting`. - """ - if python_version not in name: - fail("The name '{}' must have the python version '{}' in it".format(name, python_version)) - - if python_version not in VERSION_FLAG_VALUES: - fail("The 'python_version' must be known to 'rules_python', choose from the values: {}".format(VERSION_FLAG_VALUES.keys())) - - python_versions = VERSION_FLAG_VALUES[python_version] - extra_flag_values = kwargs.pop("flag_values", {}) - if _PYTHON_VERSION_FLAG in extra_flag_values: - fail("Cannot set '{}' in the flag values".format(_PYTHON_VERSION_FLAG)) - - if len(python_versions) == 1: - native.config_setting( - name = name, - flag_values = { - _PYTHON_VERSION_FLAG: python_version, - } | extra_flag_values, - **kwargs - ) - return - - reuse_conditions = reuse_conditions or {} - create_config_settings = { - "_{}".format(name).replace(python_version, version): {_PYTHON_VERSION_FLAG: version} - for version in python_versions - if not reuse_conditions or version not in reuse_conditions - } - match_any = list(create_config_settings.keys()) - for version, condition in reuse_conditions.items(): - if len(VERSION_FLAG_VALUES[version]) == 1: - match_any.append(condition) - continue - - # Convert the name to an internal label that this function would create, - # so that we are hitting the config_setting and not the config_setting_group. - condition = Label(condition) - if hasattr(condition, "same_package_label"): - condition = condition.same_package_label("_" + condition.name) - else: - condition = condition.relative("_" + condition.name) - - match_any.append(condition) - - for name_, flag_values_ in create_config_settings.items(): - native.config_setting( - name = name_, - flag_values = flag_values_ | extra_flag_values, - **kwargs - ) - - # An alias pointing to an underscore-prefixed config_setting_group - # is used because config_setting_group creates - # `is_{version}_N` targets, which are easily confused with the - # `is_{minor}.{micro}` (dot) targets. - selects.config_setting_group( - name = "_{}_group".format(name), - match_any = match_any, - visibility = ["//visibility:private"], - ) - native.alias( - name = name, - actual = "_{}_group".format(name), - visibility = kwargs.get("visibility", []), - ) - -def construct_config_settings(name = None): # buildifier: disable=function-docstring +def construct_config_settings(*, name, versions, minor_mapping): # buildifier: disable=function-docstring """Create a 'python_version' config flag and construct all config settings used in rules_python. This mainly includes the targets that are used in the toolchain and pip hub repositories that only match on the 'python_version' flag values. Args: - name(str): A dummy name value that is no-op for now. + name: {type}`str` A dummy name value that is no-op for now. + versions: {type}`list[str]` A list of versions to build constraint settings for. + minor_mapping: {type}`dict[str, str]` A mapping from `X.Y` to `X.Y.Z` python versions. """ + _ = name # @unused _python_version_flag( - name = "python_version", + name = _PYTHON_VERSION_FLAG.name, # TODO: The default here should somehow match the MODULE config. Until # then, use the empty string to indicate an unknown version. This # also prevents version-unaware targets from inadvertently matching # a select condition when they shouldn't. build_setting_default = "", - values = [""] + VERSION_FLAG_VALUES.keys(), + visibility = ["//visibility:public"], + ) + + _python_version_major_minor_flag( + name = _PYTHON_VERSION_MAJOR_MINOR_FLAG.name, + build_setting_default = "", visibility = ["//visibility:public"], ) native.config_setting( name = "is_python_version_unset", - flag_values = { - Label("//python/config_settings:python_version"): "", - }, + flag_values = {_PYTHON_VERSION_FLAG: ""}, visibility = ["//visibility:public"], ) - for version, matching_versions in VERSION_FLAG_VALUES.items(): - is_python_config_setting( - name = "is_python_{}".format(version), - python_version = version, - reuse_conditions = { - v: native.package_relative_label("is_python_{}".format(v)) - for v in matching_versions - if v != version - }, + _reverse_minor_mapping = {full: minor for minor, full in minor_mapping.items()} + for version in versions: + minor_version = _reverse_minor_mapping.get(version) + if not minor_version: + native.config_setting( + name = "is_python_{}".format(version), + flag_values = {":python_version": version}, + visibility = ["//visibility:public"], + ) + continue + + # Also need to match the minor version when using + name = "is_python_{}".format(version) + native.config_setting( + name = "_" + name, + flag_values = {":python_version": version}, + visibility = ["//visibility:public"], + ) + + # An alias pointing to an underscore-prefixed config_setting_group + # is used because config_setting_group creates + # `is_{version}_N` targets, which are easily confused with the + # `is_{minor}.{micro}` (dot) targets. + selects.config_setting_group( + name = "_{}_group".format(name), + match_any = [ + ":_is_python_{}".format(version), + ":is_python_{}".format(minor_version), + ], + visibility = ["//visibility:private"], + ) + native.alias( + name = name, + actual = "_{}_group".format(name), + visibility = ["//visibility:public"], + ) + + # 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. + for minor in minor_mapping.keys(): + native.config_setting( + name = "is_python_{}".format(minor), + flag_values = {_PYTHON_VERSION_MAJOR_MINOR_FLAG: minor}, visibility = ["//visibility:public"], ) def _python_version_flag_impl(ctx): value = ctx.build_setting_value - if value not in ctx.attr.values: - fail(( - "Invalid --python_version value: {actual}\nAllowed values {allowed}" - ).format( - actual = value, - allowed = ", ".join(sorted(ctx.attr.values)), - )) - return [ # BuildSettingInfo is the original provider returned, so continue to # return it for compatibility @@ -227,9 +121,25 @@ def _python_version_flag_impl(ctx): _python_version_flag = rule( implementation = _python_version_flag_impl, build_setting = config.string(flag = True), + attrs = {}, +) + +def _python_version_major_minor_flag_impl(ctx): + input = ctx.attr._python_version_flag[config_common.FeatureFlagInfo].value + if input: + version = semver(input) + value = "{}.{}".format(version.major, version.minor) + else: + value = "" + + return [config_common.FeatureFlagInfo(value = value)] + +_python_version_major_minor_flag = rule( + implementation = _python_version_major_minor_flag_impl, + build_setting = config.string(flag = False), attrs = { - "values": attr.string_list( - doc = "Allowed values.", + "_python_version_flag": attr.label( + default = _PYTHON_VERSION_FLAG, ), }, ) diff --git a/python/private/pypi/generate_whl_library_build_bazel.bzl b/python/private/pypi/generate_whl_library_build_bazel.bzl index d25f73a049..0be6f9c409 100644 --- a/python/private/pypi/generate_whl_library_build_bazel.bzl +++ b/python/private/pypi/generate_whl_library_build_bazel.bzl @@ -157,14 +157,13 @@ def _render_config_settings(dependencies_by_platform): constraint_values_str = render.indent(render.list(constraint_values)).lstrip() if abi: - if not loads: - loads.append("""load("@rules_python//python/config_settings:config_settings.bzl", "is_python_config_setting")""") - additional_content.append( """\ -is_python_config_setting( +config_setting( name = "is_{name}", - python_version = "3.{minor_version}", + flag_values = {{ + "@rules_python//python/config_settings:_python_version_major_minor": "3.{minor_version}", + }}, constraint_values = {constraint_values}, visibility = ["//visibility:private"], )""".format( diff --git a/tests/config_settings/construct_config_settings_tests.bzl b/tests/config_settings/construct_config_settings_tests.bzl index b1b2e062f9..9e6b6e1fc3 100644 --- a/tests/config_settings/construct_config_settings_tests.bzl +++ b/tests/config_settings/construct_config_settings_tests.bzl @@ -18,7 +18,6 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:truth.bzl", "subjects") load("@rules_testing//lib:util.bzl", rt_util = "util") -load("//python/config_settings:config_settings.bzl", "is_python_config_setting") _tests = [] @@ -162,21 +161,25 @@ def construct_config_settings_test_suite(name): # buildifier: disable=function- # We have CI runners running on a great deal of the platforms from the list below, # hence use all of them within tests. for os in ["linux", "osx", "windows"]: - is_python_config_setting( + native.config_setting( name = "is_python_3.11_" + os, constraint_values = [ "@platforms//os:" + os, ], - python_version = "3.11", + flag_values = { + "//python/config_settings:_python_version_major_minor": "3.11", + }, ) for cpu in ["s390x", "ppc", "x86_64", "aarch64"]: - is_python_config_setting( + native.config_setting( name = "is_python_3.11_" + cpu, constraint_values = [ "@platforms//cpu:" + cpu, ], - python_version = "3.11", + flag_values = { + "//python/config_settings:_python_version_major_minor": "3.11", + }, ) for (os, cpu) in [ @@ -188,13 +191,15 @@ def construct_config_settings_test_suite(name): # buildifier: disable=function- ("osx", "x86_64"), ("windows", "x86_64"), ]: - is_python_config_setting( + native.config_setting( name = "is_python_3.11_{}_{}".format(os, cpu), constraint_values = [ "@platforms//cpu:" + cpu, "@platforms//os:" + os, ], - python_version = "3.11", + flag_values = { + "//python/config_settings:_python_version_major_minor": "3.11", + }, ) test_suite( diff --git a/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl b/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl index 3d4df14b5b..a860681ae9 100644 --- a/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl +++ b/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl @@ -87,7 +87,6 @@ _tests.append(_test_simple) def _test_dep_selects(env): want = """\ load("@bazel_skylib//rules:copy_file.bzl", "copy_file") -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,9 +157,11 @@ py_library( visibility = ["//visibility:public"], ) -is_python_config_setting( +config_setting( name = "is_python_3.10_linux_ppc", - python_version = "3.10", + flag_values = { + "@rules_python//python/config_settings:_python_version_major_minor": "3.10", + }, constraint_values = [ "@platforms//cpu:ppc", "@platforms//os:linux", @@ -168,16 +169,20 @@ is_python_config_setting( visibility = ["//visibility:private"], ) -is_python_config_setting( +config_setting( name = "is_python_3.9_anyos_aarch64", - python_version = "3.9", + flag_values = { + "@rules_python//python/config_settings:_python_version_major_minor": "3.9", + }, constraint_values = ["@platforms//cpu:aarch64"], visibility = ["//visibility:private"], ) -is_python_config_setting( +config_setting( name = "is_python_3.9_linux_anyarch", - python_version = "3.9", + flag_values = { + "@rules_python//python/config_settings:_python_version_major_minor": "3.9", + }, constraint_values = ["@platforms//os:linux"], visibility = ["//visibility:private"], )