chore: remove mandatory default values for python version flag (#2217)
This is a different take on #2205.
Summary:
- Remove version constraints from the
`//python/config_settings:python_version`.
- Remove `is_python_config_setting` macro and use a different
implementation to
retain the same functionality.
This in general will improve the situation on how the toolchains are
registered
and hopefully is a step towards resolving issues for #2081.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 33aecf0..32e3902 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -79,6 +79,7 @@
### 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
@@ -274,7 +275,7 @@
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 a34623e..f0144e8 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 20bc506..c31d69f 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 @@
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 f1d2ff0..4410425 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 e0de7d3..1e05b92 100644
--- a/python/private/BUILD.bazel
+++ b/python/private/BUILD.bazel
@@ -78,8 +78,9 @@
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 301a97b..b15d6a8 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 @@
_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 d25f73a..0be6f9c 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 @@
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 b1b2e06..9e6b6e1 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: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 @@
# 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 @@
("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 3d4df14..a860681 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 @@
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 @@
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 @@
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"],
)
