Skip to content

Commit

Permalink
fix(bzlmod): set the default_version to the python_version flag (#2253)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
aignas and rickeylev authored Oct 7, 2024
1 parent 5c3b71c commit 33fa845
Show file tree
Hide file tree
Showing 22 changed files with 238 additions and 348 deletions.
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,
},
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

0 comments on commit 33fa845

Please sign in to comment.