Skip to content

Commit 33fa845

Browse files
aignasrickeylev
andauthored
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 <[email protected]>
1 parent 5c3b71c commit 33fa845

22 files changed

+238
-348
lines changed

CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ A brief description of the categories of changes:
2929
(previously it defaulted to None).
3030

3131
### Fixed
32+
* (bzlmod) The `python.override(minor_mapping)` now merges the default and the
33+
overridden versions ensuring that the resultant `minor_mapping` will always
34+
have all of the python versions.
35+
* (bzlmod) The default value for the {obj}`--python_version` flag will now be
36+
always set to the default python toolchain version value.
3237
* (bzlmod) correctly wire the {attr}`pip.parse.extra_pip_args` all the
3338
way to {obj}`whl_library`. What is more we will pass the `extra_pip_args` to
3439
{obj}`whl_library` for `sdist` distributions when using

WORKSPACE

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ load("//:internal_setup.bzl", "rules_python_internal_setup")
4141

4242
rules_python_internal_setup()
4343

44+
load("@pythons_hub//:versions.bzl", "MINOR_MAPPING", "PYTHON_VERSIONS")
4445
load("//python:repositories.bzl", "python_register_multi_toolchains")
45-
load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")
4646

4747
python_register_multi_toolchains(
4848
name = "python",
4949
default_version = MINOR_MAPPING.values()[-2],
5050
# Integration tests verify each version, so register all of them.
51-
python_versions = TOOL_VERSIONS.keys(),
51+
python_versions = PYTHON_VERSIONS,
5252
)
5353

5454
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file")

examples/bzlmod/MODULE.bazel

+4-5
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,16 @@ python.toolchain(
4242
python.override(
4343
available_python_versions = [
4444
"3.10.9",
45+
"3.9.18",
4546
"3.9.19",
4647
# The following is used by the `other_module` and we need to include it here
4748
# as well.
4849
"3.11.8",
4950
],
5051
# Also override the `minor_mapping` so that the root module,
51-
# instead of rules_python's defaults, controls what full version
52-
# is used when `3.x` is requested.
52+
# instead of rules_python's defaulting to the latest available version,
53+
# controls what full version is used when `3.x` is requested.
5354
minor_mapping = {
54-
"3.10": "3.10.9",
55-
"3.11": "3.11.8",
5655
"3.9": "3.9.19",
5756
},
5857
)
@@ -90,7 +89,7 @@ python.single_version_platform_override(
9089
# See the tests folder for various examples on using multiple Python versions.
9190
# The names "python_3_9" and "python_3_10" are autmatically created by the repo
9291
# rules based on the `python_version` arg values.
93-
use_repo(python, "python_3_10", "python_3_9", "python_versions")
92+
use_repo(python, "python_3_10", "python_3_9", "python_versions", "pythons_hub")
9493

9594
# EXPERIMENTAL: This is experimental and may be removed without notice
9695
uv = use_extension("@rules_python//python/uv:extensions.bzl", "uv")

examples/bzlmod/MODULE.bazel.lock

+2-5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

examples/bzlmod/tests/BUILD.bazel

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
load("@python_versions//3.10:defs.bzl", py_binary_3_10 = "py_binary", py_test_3_10 = "py_test")
22
load("@python_versions//3.11:defs.bzl", py_binary_3_11 = "py_binary", py_test_3_11 = "py_test")
33
load("@python_versions//3.9:defs.bzl", py_binary_3_9 = "py_binary", py_test_3_9 = "py_test")
4+
load("@pythons_hub//:versions.bzl", "MINOR_MAPPING")
45
load("@rules_python//python:defs.bzl", "py_binary", "py_test")
5-
load("@rules_python//python:versions.bzl", "MINOR_MAPPING")
66
load("@rules_python//python/config_settings:transition.bzl", py_versioned_binary = "py_binary", py_versioned_test = "py_test")
77

88
py_binary(

internal_setup.bzl

+12
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,25 @@ load("@rules_bazel_integration_test//bazel_integration_test:deps.bzl", "bazel_in
2222
load("@rules_bazel_integration_test//bazel_integration_test:repo_defs.bzl", "bazel_binaries")
2323
load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")
2424
load("//:version.bzl", "SUPPORTED_BAZEL_VERSIONS")
25+
load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")
2526
load("//python/private:internal_config_repo.bzl", "internal_config_repo") # buildifier: disable=bzl-visibility
27+
load("//python/private:pythons_hub.bzl", "hub_repo") # buildifier: disable=bzl-visibility
2628
load("//python/private/pypi:deps.bzl", "pypi_deps") # buildifier: disable=bzl-visibility
2729

2830
def rules_python_internal_setup():
2931
"""Setup for rules_python tests and tools."""
3032

3133
internal_config_repo(name = "rules_python_internal")
34+
hub_repo(
35+
name = "pythons_hub",
36+
minor_mapping = MINOR_MAPPING,
37+
default_python_version = "",
38+
toolchain_prefixes = [],
39+
toolchain_python_versions = [],
40+
toolchain_set_python_version_constraints = [],
41+
toolchain_user_repository_names = [],
42+
python_versions = sorted(TOOL_VERSIONS.keys()),
43+
)
3244

3345
# Because we don't use the pip_install rule, we have to call this to fetch its deps
3446
pypi_deps()

python/config_settings/BUILD.bazel

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
2-
load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")
2+
load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION", "MINOR_MAPPING", "PYTHON_VERSIONS")
33
load(
44
"//python/private:flags.bzl",
55
"BootstrapImplFlag",
@@ -28,8 +28,9 @@ filegroup(
2828

2929
construct_config_settings(
3030
name = "construct_config_settings",
31+
default_version = DEFAULT_PYTHON_VERSION,
3132
minor_mapping = MINOR_MAPPING,
32-
versions = TOOL_VERSIONS.keys(),
33+
versions = PYTHON_VERSIONS,
3334
)
3435

3536
string_flag(

python/private/BUILD.bazel

+3
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ bzl_library(
165165
deps = [
166166
":bazel_tools_bzl",
167167
":internal_config_repo_bzl",
168+
":pythons_hub_bzl",
169+
"//python:versions_bzl",
168170
"//python/private/pypi:deps_bzl",
169171
],
170172
)
@@ -212,6 +214,7 @@ bzl_library(
212214
srcs = ["pythons_hub.bzl"],
213215
deps = [
214216
":py_toolchain_suite_bzl",
217+
":text_util_bzl",
215218
],
216219
)
217220

python/private/config_settings.bzl

+3-6
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,22 @@ load(":semver.bzl", "semver")
2222
_PYTHON_VERSION_FLAG = Label("//python/config_settings:python_version")
2323
_PYTHON_VERSION_MAJOR_MINOR_FLAG = Label("//python/config_settings:_python_version_major_minor")
2424

25-
def construct_config_settings(*, name, versions, minor_mapping): # buildifier: disable=function-docstring
25+
def construct_config_settings(*, name, default_version, versions, minor_mapping): # buildifier: disable=function-docstring
2626
"""Create a 'python_version' config flag and construct all config settings used in rules_python.
2727
2828
This mainly includes the targets that are used in the toolchain and pip hub
2929
repositories that only match on the 'python_version' flag values.
3030
3131
Args:
3232
name: {type}`str` A dummy name value that is no-op for now.
33+
default_version: {type}`str` the default value for the `python_version` flag.
3334
versions: {type}`list[str]` A list of versions to build constraint settings for.
3435
minor_mapping: {type}`dict[str, str]` A mapping from `X.Y` to `X.Y.Z` python versions.
3536
"""
3637
_ = name # @unused
3738
_python_version_flag(
3839
name = _PYTHON_VERSION_FLAG.name,
39-
# TODO: The default here should somehow match the MODULE config. Until
40-
# then, use the empty string to indicate an unknown version. This
41-
# also prevents version-unaware targets from inadvertently matching
42-
# a select condition when they shouldn't.
43-
build_setting_default = "",
40+
build_setting_default = default_version,
4441
visibility = ["//visibility:public"],
4542
)
4643

python/private/py_repositories.bzl

+13
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
load("@bazel_tools//tools/build_defs/repo:http.bzl", _http_archive = "http_archive")
1818
load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
19+
load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")
1920
load("//python/private/pypi:deps.bzl", "pypi_deps")
2021
load(":internal_config_repo.bzl", "internal_config_repo")
22+
load(":pythons_hub.bzl", "hub_repo")
2123

2224
def http_archive(**kwargs):
2325
maybe(_http_archive, **kwargs)
@@ -32,6 +34,17 @@ def py_repositories():
3234
internal_config_repo,
3335
name = "rules_python_internal",
3436
)
37+
maybe(
38+
hub_repo,
39+
name = "pythons_hub",
40+
minor_mapping = MINOR_MAPPING,
41+
default_python_version = "",
42+
toolchain_prefixes = [],
43+
toolchain_python_versions = [],
44+
toolchain_set_python_version_constraints = [],
45+
toolchain_user_repository_names = [],
46+
python_versions = sorted(TOOL_VERSIONS.keys()),
47+
)
3548
http_archive(
3649
name = "bazel_skylib",
3750
sha256 = "74d544d96f4a5bb630d465ca8bbcfe231e3594e5aae57e1edbf17a6eb3ca2506",

python/private/pypi/BUILD.bazel

+3-5
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
# limitations under the License.
1414

1515
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
16-
load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED")
1716

1817
package(default_visibility = ["//:__subpackages__"])
1918

@@ -59,22 +58,21 @@ bzl_library(
5958
srcs = ["extension.bzl"],
6059
deps = [
6160
":attrs_bzl",
61+
":evaluate_markers_bzl",
6262
":hub_repository_bzl",
6363
":parse_requirements_bzl",
64-
":evaluate_markers_bzl",
6564
":parse_whl_name_bzl",
6665
":pip_repository_attrs_bzl",
6766
":simpleapi_download_bzl",
6867
":whl_library_bzl",
6968
":whl_repo_name_bzl",
7069
"//python/private:full_version_bzl",
7170
"//python/private:normalize_name_bzl",
72-
"//python/private:version_label_bzl",
7371
"//python/private:semver_bzl",
72+
"//python/private:version_label_bzl",
7473
"@bazel_features//:features",
75-
] + [
7674
"@pythons_hub//:interpreters_bzl",
77-
] if BZLMOD_ENABLED else [],
75+
],
7876
)
7977

8078
bzl_library(

python/private/pypi/config_settings.bzl

+8-2
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,15 @@ def config_settings(
109109

110110
for python_version in [""] + python_versions:
111111
is_python = "is_python_{}".format(python_version or "version_unset")
112-
native.alias(
112+
113+
# The aliases defined in @rules_python//python/config_settings may not
114+
# have config settings for the versions we need, so define our own
115+
# config settings instead.
116+
native.config_setting(
113117
name = is_python,
114-
actual = Label("//python/config_settings:" + is_python),
118+
flag_values = {
119+
Label("//python/config_settings:_python_version_major_minor"): python_version,
120+
},
115121
visibility = visibility,
116122
)
117123

python/private/pypi/extension.bzl

+1-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"pip module extension for use with bzlmod"
1616

1717
load("@bazel_features//:features.bzl", "bazel_features")
18-
load("@pythons_hub//:interpreters.bzl", "DEFAULT_PYTHON_VERSION", "INTERPRETER_LABELS")
18+
load("@pythons_hub//:interpreters.bzl", "INTERPRETER_LABELS")
1919
load("//python/private:auth.bzl", "AUTH_ATTRS")
2020
load("//python/private:normalize_name.bzl", "normalize_name")
2121
load("//python/private:repo_utils.bzl", "repo_utils")
@@ -506,7 +506,6 @@ def _pip_impl(module_ctx):
506506
key: json.encode(value)
507507
for key, value in whl_map.items()
508508
},
509-
default_version = _major_minor_version(DEFAULT_PYTHON_VERSION),
510509
packages = sorted(exposed_packages.get(hub_name, {})),
511510
groups = hub_group_map.get(hub_name),
512511
)

python/private/pypi/hub_repository.bzl

-9
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ def _impl(rctx):
3535
key: [whl_alias(**v) for v in json.decode(values)]
3636
for key, values in rctx.attr.whl_map.items()
3737
},
38-
default_version = rctx.attr.default_version,
39-
default_config_setting = "//_config:is_python_" + rctx.attr.default_version,
4038
requirement_cycles = rctx.attr.groups,
4139
)
4240
for path, contents in aliases.items():
@@ -67,13 +65,6 @@ def _impl(rctx):
6765

6866
hub_repository = repository_rule(
6967
attrs = {
70-
"default_version": attr.string(
71-
mandatory = True,
72-
doc = """\
73-
This is the default python version in the format of X.Y. This should match
74-
what is setup by the 'python' extension using the 'is_default = True'
75-
setting.""",
76-
),
7768
"groups": attr.string_list_dict(
7869
mandatory = False,
7970
),

0 commit comments

Comments
 (0)