Skip to content

fix(bzlmod): set the default_version to the python_version flag #2253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2c1d0f4
fix(bzlmod): set the default_version to the python_version flag
aignas Sep 26, 2024
dba9902
fixup! fix(bzlmod): set the default_version to the python_version flag
aignas Sep 26, 2024
88cad95
fixup! fixup! fix(bzlmod): set the default_version to the python_vers…
aignas Sep 26, 2024
c6e10d6
refactor: make the code readability better when constructing the sett…
aignas Sep 26, 2024
dde6f2f
fixup examples/bzlmod to use MINOR_MAPPING from elsewhere
aignas Sep 26, 2024
ac06a1e
test: fix pypy config_settings
aignas Sep 26, 2024
ab73e16
remove the default config setting from bzlmod in pypi hub
aignas Sep 26, 2024
e4cc572
fix: add pythons_hub to internal_setup
aignas Sep 26, 2024
7e50e9f
remove dead code
aignas Sep 26, 2024
6a9a53f
thread the MINOR_MAPPING and TOOL_VERSIONS.keys through the hub repo …
aignas Sep 28, 2024
6cbe12a
move some of the constants to a versions.bzl for better naming
aignas Sep 28, 2024
19f50d2
Merge branch 'main' of https://github.com/bazelbuild/rules_python int…
rickeylev Oct 3, 2024
0ee8ada
fix loading of minor_mapping
rickeylev Oct 3, 2024
3c770d0
Merge branch 'main' into feat/default-version-python-hub
aignas Oct 7, 2024
81c5e79
comment: fix comment in pypi config_settings module
aignas Oct 7, 2024
d53bc4a
comment: remove if bzlmod enabled from internal_setup because it is noop
aignas Oct 7, 2024
b2378ff
Merge branch 'main' into feat/default-version-python-hub
aignas Oct 7, 2024
b03beb1
fix the 'MINOR_MAPPING' import for the register_multi_toolchains
aignas Oct 7, 2024
e6bb17d
fixup the bzl_library deps
aignas Oct 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ A brief description of the categories of changes:
(previously it defaulted to None).

### Fixed
* (bzlmod) The `python.override(minor_mapping)` now merges the default and the
overridden versions ensuring that the resultant `minor_mapping` will always
have all of the python versions.
* (bzlmod) The default value for the {obj}`--python_version` flag will now be
always set to the default python toolchain version value.
* (bzlmod) correctly wire the {attr}`pip.parse.extra_pip_args` all the
way to {obj}`whl_library`. What is more we will pass the `extra_pip_args` to
{obj}`whl_library` for `sdist` distributions when using
Expand Down
4 changes: 2 additions & 2 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ load("//:internal_setup.bzl", "rules_python_internal_setup")

rules_python_internal_setup()

load("@pythons_hub//:versions.bzl", "MINOR_MAPPING", "PYTHON_VERSIONS")
load("//python:repositories.bzl", "python_register_multi_toolchains")
load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")

python_register_multi_toolchains(
name = "python",
default_version = MINOR_MAPPING.values()[-2],
# Integration tests verify each version, so register all of them.
python_versions = TOOL_VERSIONS.keys(),
python_versions = PYTHON_VERSIONS,
)

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file")
Expand Down
9 changes: 4 additions & 5 deletions examples/bzlmod/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,16 @@ python.toolchain(
python.override(
available_python_versions = [
"3.10.9",
"3.9.18",
"3.9.19",
# The following is used by the `other_module` and we need to include it here
# as well.
"3.11.8",
],
# Also override the `minor_mapping` so that the root module,
# instead of rules_python's defaults, controls what full version
# is used when `3.x` is requested.
# instead of rules_python's defaulting to the latest available version,
# controls what full version is used when `3.x` is requested.
minor_mapping = {
"3.10": "3.10.9",
"3.11": "3.11.8",
"3.9": "3.9.19",
},
)
Expand Down Expand Up @@ -90,7 +89,7 @@ python.single_version_platform_override(
# See the tests folder for various examples on using multiple Python versions.
# The names "python_3_9" and "python_3_10" are autmatically created by the repo
# rules based on the `python_version` arg values.
use_repo(python, "python_3_10", "python_3_9", "python_versions")
use_repo(python, "python_3_10", "python_3_9", "python_versions", "pythons_hub")

# EXPERIMENTAL: This is experimental and may be removed without notice
uv = use_extension("@rules_python//python/uv:extensions.bzl", "uv")
Expand Down
7 changes: 2 additions & 5 deletions examples/bzlmod/MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion examples/bzlmod/tests/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@python_versions//3.10:defs.bzl", py_binary_3_10 = "py_binary", py_test_3_10 = "py_test")
load("@python_versions//3.11:defs.bzl", py_binary_3_11 = "py_binary", py_test_3_11 = "py_test")
load("@python_versions//3.9:defs.bzl", py_binary_3_9 = "py_binary", py_test_3_9 = "py_test")
load("@pythons_hub//:versions.bzl", "MINOR_MAPPING")
load("@rules_python//python:defs.bzl", "py_binary", "py_test")
load("@rules_python//python:versions.bzl", "MINOR_MAPPING")
load("@rules_python//python/config_settings:transition.bzl", py_versioned_binary = "py_binary", py_versioned_test = "py_test")

py_binary(
Expand Down
12 changes: 12 additions & 0 deletions internal_setup.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,25 @@ load("@rules_bazel_integration_test//bazel_integration_test:deps.bzl", "bazel_in
load("@rules_bazel_integration_test//bazel_integration_test:repo_defs.bzl", "bazel_binaries")
load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")
load("//:version.bzl", "SUPPORTED_BAZEL_VERSIONS")
load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")
load("//python/private:internal_config_repo.bzl", "internal_config_repo") # buildifier: disable=bzl-visibility
load("//python/private:pythons_hub.bzl", "hub_repo") # buildifier: disable=bzl-visibility
load("//python/private/pypi:deps.bzl", "pypi_deps") # buildifier: disable=bzl-visibility

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

internal_config_repo(name = "rules_python_internal")
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()),
)

# Because we don't use the pip_install rule, we have to call this to fetch its deps
pypi_deps()
Expand Down
5 changes: 3 additions & 2 deletions python/config_settings/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -28,8 +28,9 @@ filegroup(

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(
Expand Down
3 changes: 3 additions & 0 deletions python/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ bzl_library(
deps = [
":bazel_tools_bzl",
":internal_config_repo_bzl",
":pythons_hub_bzl",
"//python:versions_bzl",
"//python/private/pypi:deps_bzl",
],
)
Expand Down Expand Up @@ -212,6 +214,7 @@ bzl_library(
srcs = ["pythons_hub.bzl"],
deps = [
":py_toolchain_suite_bzl",
":text_util_bzl",
],
)

Expand Down
9 changes: 3 additions & 6 deletions python/private/config_settings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,22 @@ load(":semver.bzl", "semver")
_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
repositories that only match on the 'python_version' flag values.

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"],
)

Expand Down
13 changes: 13 additions & 0 deletions python/private/py_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -32,6 +34,17 @@ def py_repositories():
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",
Expand Down
8 changes: 3 additions & 5 deletions python/private/pypi/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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__"])

Expand Down Expand Up @@ -59,22 +58,21 @@ bzl_library(
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",
":whl_library_bzl",
":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(
Expand Down
10 changes: 8 additions & 2 deletions python/private/pypi/config_settings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,15 @@ def config_settings(

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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, maybe we should expose python_version_major_minor as a public setting. Otherwise, users don't have a good way to perform e.g. select({"3.10": ..., "3.11": ...}), which is the most common type of selection on python version.

Don't have to address in this PR though.

},
visibility = visibility,
)

Expand Down
3 changes: 1 addition & 2 deletions python/private/pypi/extension.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -506,7 +506,6 @@ def _pip_impl(module_ctx):
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),
)
Expand Down
9 changes: 0 additions & 9 deletions python/private/pypi/hub_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ def _impl(rctx):
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():
Expand Down Expand Up @@ -67,13 +65,6 @@ def _impl(rctx):

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,
),
Expand Down
Loading