fix(bzlmod): set the default_version to the python_version flag (#2253)
With this change we set the default value of `--python_version` when
the `python.toolchain` is used in `bzlmod` and we generate the
appropriate config settings based on the registered toolchains and
given overrides by the root module.
This means that we expect the `--python_version` to be always set to
a non-empty value under `bzlmod` and we can cleanup code which was
handling `//conditions:default` case. This also means that we can
in theory drop the requirement for `python_version` in `pip.parse`
and just setup dependencies for all packages that we find in the
`requirements.txt` file and move on. This is left as future work
by myself or anyone willing to contribute. We can also start reusing
the same `whl_library` instance for multi-platform packages because
the python version flag is always set - this will simplify the layout
and makes the feature non-experimental anymore under bzlmod.
Summary:
* Add `@pythons_hub` to the `WORKSPACE` with dummy data to make
pythons_hub work.
* Add `MINOR_MAPPING` and `PYTHON_VERSIONS` to pythons_hub to
generate the config settings.
* Remove handling of the default version in `pypi` code under bzlmod.
Work towards #2081, #260, #1708
---------
Co-authored-by: Richard Levasseur <rlevasseur@google.com>
diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel
index c31d69f..9fb3957 100644
--- a/python/config_settings/BUILD.bazel
+++ b/python/config_settings/BUILD.bazel
@@ -1,5 +1,5 @@
load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
-load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")
+load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION", "MINOR_MAPPING", "PYTHON_VERSIONS")
load(
"//python/private:flags.bzl",
"BootstrapImplFlag",
@@ -28,8 +28,9 @@
construct_config_settings(
name = "construct_config_settings",
+ default_version = DEFAULT_PYTHON_VERSION,
minor_mapping = MINOR_MAPPING,
- versions = TOOL_VERSIONS.keys(),
+ versions = PYTHON_VERSIONS,
)
string_flag(
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
index f128742..cb0fd5c 100644
--- a/python/private/BUILD.bazel
+++ b/python/private/BUILD.bazel
@@ -165,6 +165,8 @@
deps = [
":bazel_tools_bzl",
":internal_config_repo_bzl",
+ ":pythons_hub_bzl",
+ "//python:versions_bzl",
"//python/private/pypi:deps_bzl",
],
)
@@ -212,6 +214,7 @@
srcs = ["pythons_hub.bzl"],
deps = [
":py_toolchain_suite_bzl",
+ ":text_util_bzl",
],
)
diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl
index b15d6a8..eceeb54 100644
--- a/python/private/config_settings.bzl
+++ b/python/private/config_settings.bzl
@@ -22,7 +22,7 @@
_PYTHON_VERSION_FLAG = Label("//python/config_settings:python_version")
_PYTHON_VERSION_MAJOR_MINOR_FLAG = Label("//python/config_settings:_python_version_major_minor")
-def construct_config_settings(*, name, versions, minor_mapping): # buildifier: disable=function-docstring
+def construct_config_settings(*, name, default_version, 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
@@ -30,17 +30,14 @@
Args:
name: {type}`str` A dummy name value that is no-op for now.
+ default_version: {type}`str` the default value for the `python_version` flag.
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_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 = "",
+ build_setting_default = default_version,
visibility = ["//visibility:public"],
)
diff --git a/python/private/py_repositories.bzl b/python/private/py_repositories.bzl
index 8ddcb5d..ff8a638 100644
--- a/python/private/py_repositories.bzl
+++ b/python/private/py_repositories.bzl
@@ -16,8 +16,10 @@
load("@bazel_tools//tools/build_defs/repo:http.bzl", _http_archive = "http_archive")
load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
+load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")
load("//python/private/pypi:deps.bzl", "pypi_deps")
load(":internal_config_repo.bzl", "internal_config_repo")
+load(":pythons_hub.bzl", "hub_repo")
def http_archive(**kwargs):
maybe(_http_archive, **kwargs)
@@ -32,6 +34,17 @@
internal_config_repo,
name = "rules_python_internal",
)
+ maybe(
+ hub_repo,
+ name = "pythons_hub",
+ minor_mapping = MINOR_MAPPING,
+ default_python_version = "",
+ toolchain_prefixes = [],
+ toolchain_python_versions = [],
+ toolchain_set_python_version_constraints = [],
+ toolchain_user_repository_names = [],
+ python_versions = sorted(TOOL_VERSIONS.keys()),
+ )
http_archive(
name = "bazel_skylib",
sha256 = "74d544d96f4a5bb630d465ca8bbcfe231e3594e5aae57e1edbf17a6eb3ca2506",
diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel
index 8cfd3d6..e76f9d3 100644
--- a/python/private/pypi/BUILD.bazel
+++ b/python/private/pypi/BUILD.bazel
@@ -13,7 +13,6 @@
# limitations under the License.
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
-load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED")
package(default_visibility = ["//:__subpackages__"])
@@ -59,9 +58,9 @@
srcs = ["extension.bzl"],
deps = [
":attrs_bzl",
+ ":evaluate_markers_bzl",
":hub_repository_bzl",
":parse_requirements_bzl",
- ":evaluate_markers_bzl",
":parse_whl_name_bzl",
":pip_repository_attrs_bzl",
":simpleapi_download_bzl",
@@ -69,12 +68,11 @@
":whl_repo_name_bzl",
"//python/private:full_version_bzl",
"//python/private:normalize_name_bzl",
- "//python/private:version_label_bzl",
"//python/private:semver_bzl",
+ "//python/private:version_label_bzl",
"@bazel_features//:features",
- ] + [
"@pythons_hub//:interpreters_bzl",
- ] if BZLMOD_ENABLED else [],
+ ],
)
bzl_library(
diff --git a/python/private/pypi/config_settings.bzl b/python/private/pypi/config_settings.bzl
index 9741217..9ccb646 100644
--- a/python/private/pypi/config_settings.bzl
+++ b/python/private/pypi/config_settings.bzl
@@ -109,9 +109,15 @@
for python_version in [""] + python_versions:
is_python = "is_python_{}".format(python_version or "version_unset")
- native.alias(
+
+ # The aliases defined in @rules_python//python/config_settings may not
+ # have config settings for the versions we need, so define our own
+ # config settings instead.
+ native.config_setting(
name = is_python,
- actual = Label("//python/config_settings:" + is_python),
+ flag_values = {
+ Label("//python/config_settings:_python_version_major_minor"): python_version,
+ },
visibility = visibility,
)
diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl
index 712d2fe..200aa43 100644
--- a/python/private/pypi/extension.bzl
+++ b/python/private/pypi/extension.bzl
@@ -15,7 +15,7 @@
"pip module extension for use with bzlmod"
load("@bazel_features//:features.bzl", "bazel_features")
-load("@pythons_hub//:interpreters.bzl", "DEFAULT_PYTHON_VERSION", "INTERPRETER_LABELS")
+load("@pythons_hub//:interpreters.bzl", "INTERPRETER_LABELS")
load("//python/private:auth.bzl", "AUTH_ATTRS")
load("//python/private:normalize_name.bzl", "normalize_name")
load("//python/private:repo_utils.bzl", "repo_utils")
@@ -506,7 +506,6 @@
key: json.encode(value)
for key, value in whl_map.items()
},
- default_version = _major_minor_version(DEFAULT_PYTHON_VERSION),
packages = sorted(exposed_packages.get(hub_name, {})),
groups = hub_group_map.get(hub_name),
)
diff --git a/python/private/pypi/hub_repository.bzl b/python/private/pypi/hub_repository.bzl
index f589dd4..7afb616 100644
--- a/python/private/pypi/hub_repository.bzl
+++ b/python/private/pypi/hub_repository.bzl
@@ -35,8 +35,6 @@
key: [whl_alias(**v) for v in json.decode(values)]
for key, values in rctx.attr.whl_map.items()
},
- default_version = rctx.attr.default_version,
- default_config_setting = "//_config:is_python_" + rctx.attr.default_version,
requirement_cycles = rctx.attr.groups,
)
for path, contents in aliases.items():
@@ -67,13 +65,6 @@
hub_repository = repository_rule(
attrs = {
- "default_version": attr.string(
- mandatory = True,
- doc = """\
-This is the default python version in the format of X.Y. This should match
-what is setup by the 'python' extension using the 'is_default = True'
-setting.""",
- ),
"groups": attr.string_list_dict(
mandatory = False,
),
diff --git a/python/private/pypi/render_pkg_aliases.bzl b/python/private/pypi/render_pkg_aliases.bzl
index 9e5158f..0086bff 100644
--- a/python/private/pypi/render_pkg_aliases.bzl
+++ b/python/private/pypi/render_pkg_aliases.bzl
@@ -73,7 +73,6 @@
def _render_whl_library_alias(
*,
name,
- default_config_setting,
aliases,
target_name,
**kwargs):
@@ -97,9 +96,6 @@
for alias in sorted(aliases, key = lambda x: x.version):
actual = "@{repo}//:{name}".format(repo = alias.repo, name = target_name)
selects.setdefault(actual, []).append(alias.config_setting)
- if alias.config_setting == default_config_setting:
- selects[actual].append("//conditions:default")
- no_match_error = None
return render.alias(
name = name,
@@ -121,7 +117,7 @@
**kwargs
)
-def _render_common_aliases(*, name, aliases, default_config_setting = None, group_name = None):
+def _render_common_aliases(*, name, aliases, group_name = None):
lines = [
"""load("@bazel_skylib//lib:selects.bzl", "selects")""",
"""package(default_visibility = ["//visibility:public"])""",
@@ -131,9 +127,7 @@
if aliases:
config_settings = sorted([v.config_setting for v in aliases if v.config_setting])
- if not config_settings or default_config_setting in config_settings:
- pass
- else:
+ if config_settings:
error_msg = NO_MATCH_ERROR_MESSAGE_TEMPLATE_V2.format(
config_settings = render.indent(
"\n".join(config_settings),
@@ -145,10 +139,6 @@
error_msg = error_msg,
))
- # This is to simplify the code in _render_whl_library_alias and to ensure
- # that we don't pass a 'default_version' that is not in 'versions'.
- default_config_setting = None
-
lines.append(
render.alias(
name = name,
@@ -159,7 +149,6 @@
[
_render_whl_library_alias(
name = name,
- default_config_setting = default_config_setting,
aliases = aliases,
target_name = target_name,
visibility = ["//_groups:__subpackages__"] if name.startswith("_") else None,
@@ -188,7 +177,7 @@
return "\n\n".join(lines)
-def render_pkg_aliases(*, aliases, default_config_setting = None, requirement_cycles = None):
+def render_pkg_aliases(*, aliases, requirement_cycles = None):
"""Create alias declarations for each PyPI package.
The aliases should be appended to the pip_repository BUILD.bazel file. These aliases
@@ -198,7 +187,6 @@
Args:
aliases: dict, the keys are normalized distribution names and values are the
whl_alias instances.
- default_config_setting: the default to be used for the aliases.
requirement_cycles: any package groups to also add.
Returns:
@@ -227,7 +215,6 @@
"{}/BUILD.bazel".format(normalize_name(name)): _render_common_aliases(
name = normalize_name(name),
aliases = pkg_aliases,
- default_config_setting = default_config_setting,
group_name = whl_group_mapping.get(normalize_name(name)),
).strip()
for name, pkg_aliases in aliases.items()
@@ -278,13 +265,12 @@
target_platforms = target_platforms,
)
-def render_multiplatform_pkg_aliases(*, aliases, default_version = None, **kwargs):
+def render_multiplatform_pkg_aliases(*, aliases, **kwargs):
"""Render the multi-platform pkg aliases.
Args:
aliases: dict[str, list(whl_alias)] A list of aliases that will be
transformed from ones having `filename` to ones having `config_setting`.
- default_version: str, the default python version. Defaults to None.
**kwargs: extra arguments passed to render_pkg_aliases.
Returns:
@@ -302,7 +288,6 @@
config_setting_aliases = {
pkg: multiplatform_whl_aliases(
aliases = pkg_aliases,
- default_version = default_version,
glibc_versions = flag_versions.get("glibc_versions", []),
muslc_versions = flag_versions.get("muslc_versions", []),
osx_versions = flag_versions.get("osx_versions", []),
@@ -317,14 +302,13 @@
contents["_config/BUILD.bazel"] = _render_config_settings(**flag_versions)
return contents
-def multiplatform_whl_aliases(*, aliases, default_version = None, **kwargs):
+def multiplatform_whl_aliases(*, aliases, **kwargs):
"""convert a list of aliases from filename to config_setting ones.
Args:
aliases: list(whl_alias): The aliases to process. Any aliases that have
the filename set will be converted to a list of aliases, each with
an appropriate config_setting value.
- default_version: string | None, the default python version to use.
**kwargs: Extra parameters passed to get_filename_config_settings.
Returns:
@@ -344,7 +328,6 @@
filename = alias.filename,
target_platforms = alias.target_platforms,
python_version = alias.version,
- python_default = default_version == alias.version,
**kwargs
)
@@ -511,8 +494,7 @@
glibc_versions,
muslc_versions,
osx_versions,
- python_version = "",
- python_default = True):
+ python_version):
"""Get the filename config settings.
Args:
@@ -522,7 +504,6 @@
muslc_versions: list[tuple[int, int]], list of versions.
osx_versions: list[tuple[int, int]], list of versions.
python_version: the python version to generate the config_settings for.
- python_default: if we should include the setting when python_version is not set.
Returns:
A tuple:
@@ -573,18 +554,9 @@
prefixes = ["sdist"]
suffixes = [_non_versioned_platform(p) for p in target_platforms or []]
- if python_default and python_version:
- prefixes += ["cp{}_{}".format(python_version, p) for p in prefixes]
- elif python_version:
- prefixes = ["cp{}_{}".format(python_version, p) for p in prefixes]
- elif python_default:
- pass
- else:
- fail("BUG: got no python_version and it is not default")
-
versioned = {
- ":is_{}_{}".format(p, suffix): {
- version: ":is_{}_{}".format(p, setting)
+ ":is_cp{}_{}_{}".format(python_version, p, suffix): {
+ version: ":is_cp{}_{}_{}".format(python_version, p, setting)
for version, setting in versions.items()
}
for p in prefixes
@@ -592,9 +564,9 @@
}
if suffixes or versioned:
- return [":is_{}_{}".format(p, s) for p in prefixes for s in suffixes], versioned
+ return [":is_cp{}_{}_{}".format(python_version, p, s) for p in prefixes for s in suffixes], versioned
else:
- return [":is_{}".format(p) for p in prefixes], setting_supported_versions
+ return [":is_cp{}_{}".format(python_version, p) for p in prefixes], setting_supported_versions
def _whl_config_setting_suffixes(
platform_tag,
diff --git a/python/private/python.bzl b/python/private/python.bzl
index cedf39a..83bc43f 100644
--- a/python/private/python.bzl
+++ b/python/private/python.bzl
@@ -236,6 +236,7 @@
name = "pythons_hub",
# Last toolchain is default
default_python_version = py.default_python_version,
+ minor_mapping = py.config.minor_mapping,
toolchain_prefixes = [
render.toolchain_prefix(index, toolchain.name, _TOOLCHAIN_INDEX_PAD_LENGTH)
for index, toolchain in enumerate(py.toolchains)
@@ -493,20 +494,23 @@
_fail = _fail,
)
- minor_mapping = default.pop("minor_mapping", {})
register_all_versions = default.pop("register_all_versions", False)
kwargs = default.pop("kwargs", {})
- if not minor_mapping:
- versions = {}
- for version_string in available_versions:
- v = semver(version_string)
- versions.setdefault("{}.{}".format(v.major, v.minor), []).append((int(v.patch), version_string))
+ versions = {}
+ for version_string in available_versions:
+ v = semver(version_string)
+ versions.setdefault("{}.{}".format(v.major, v.minor), []).append((int(v.patch), version_string))
- minor_mapping = {
- major_minor: max(subset)[1]
- for major_minor, subset in versions.items()
- }
+ minor_mapping = {
+ major_minor: max(subset)[1]
+ for major_minor, subset in versions.items()
+ }
+
+ # The following ensures that all of the versions will be present in the minor_mapping
+ minor_mapping_overrides = default.pop("minor_mapping", {})
+ for major_minor, full in minor_mapping_overrides.items():
+ minor_mapping[major_minor] = full
return struct(
kwargs = kwargs,
@@ -705,6 +709,10 @@
"3.11": "3.11.4",
}
```
+
+:::{versionchanged} 0.37.0
+The values in this mapping override the default values and do not replace them.
+:::
""",
default = {},
),
diff --git a/python/private/python_register_multi_toolchains.bzl b/python/private/python_register_multi_toolchains.bzl
index 68f5249..1c7138d 100644
--- a/python/private/python_register_multi_toolchains.bzl
+++ b/python/private/python_register_multi_toolchains.bzl
@@ -15,6 +15,10 @@
"""This file contains repository rules and macros to support toolchain registration.
"""
+# NOTE @aignas 2024-10-07: we are not importing this from `@pythons_hub` because of this
+# leading to a backwards incompatible change - the `//python:repositories.bzl` is loading
+# from this file and it will cause a circular import loop and an error. If the users in
+# WORKSPACE world want to override the `minor_mapping`, they will have to pass an argument.
load("//python:versions.bzl", "MINOR_MAPPING")
load(":python_register_toolchains.bzl", "python_register_toolchains")
load(":toolchains_repo.bzl", "multi_toolchain_aliases")
diff --git a/python/private/pythons_hub.bzl b/python/private/pythons_hub.bzl
index da6c80d..fdaad60 100644
--- a/python/private/pythons_hub.bzl
+++ b/python/private/pythons_hub.bzl
@@ -14,10 +14,8 @@
"Repo rule used by bzlmod extension to create a repo that has a map of Python interpreters and their labels"
-load(
- "//python/private:toolchains_repo.bzl",
- "python_toolchain_build_file_content",
-)
+load(":text_util.bzl", "render")
+load(":toolchains_repo.bzl", "python_toolchain_build_file_content")
def _have_same_length(*lists):
if not lists:
@@ -34,6 +32,12 @@
visibility = ["@rules_python//:__subpackages__"],
)
+bzl_library(
+ name = "versions_bzl",
+ srcs = ["versions.bzl"],
+ visibility = ["@rules_python//:__subpackages__"],
+)
+
{toolchains}
"""
@@ -82,6 +86,12 @@
"{name}_host": Label("@{name}_host//:python"),
"""
+_versions_bzl_template = """
+DEFAULT_PYTHON_VERSION = "{default_python_version}"
+MINOR_MAPPING = {minor_mapping}
+PYTHON_VERSIONS = {python_versions}
+"""
+
def _hub_repo_impl(rctx):
# Create the various toolchain definitions and
# write them to the BUILD file.
@@ -107,8 +117,22 @@
rctx.file(
"interpreters.bzl",
_interpreters_bzl_template.format(
- interpreter_labels = interpreter_labels,
+ # TODO @aignas 2024-09-28: before 1.0 remove the value from here
default_python_version = rctx.attr.default_python_version,
+ interpreter_labels = interpreter_labels,
+ ),
+ executable = False,
+ )
+
+ rctx.file(
+ "versions.bzl",
+ _versions_bzl_template.format(
+ default_python_version = rctx.attr.default_python_version,
+ minor_mapping = render.dict(rctx.attr.minor_mapping),
+ python_versions = rctx.attr.python_versions or render.list(sorted({
+ v: None
+ for v in rctx.attr.toolchain_python_versions
+ })),
),
executable = False,
)
@@ -125,6 +149,14 @@
doc = "Default Python version for the build in `X.Y` or `X.Y.Z` format.",
mandatory = True,
),
+ "minor_mapping": attr.string_dict(
+ doc = "The minor mapping of the `X.Y` to `X.Y.Z` format that is used in config settings.",
+ mandatory = True,
+ ),
+ "python_versions": attr.string_list(
+ doc = "The list of python versions to include in the `interpreters.bzl` if the toolchains are not specified. Used in `WORKSPACE` builds.",
+ mandatory = False,
+ ),
"toolchain_prefixes": attr.string_list(
doc = "List prefixed for the toolchains",
mandatory = True,
