Skip to content

fix: prefer X.Y over X.Y.Z version aliases in python.toolchain #1340

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions examples/bzlmod/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ python.toolchain(
# work in progress.
python.toolchain(
configure_coverage_tool = True,
python_version = "3.10",
# One can also provide the full 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.

Because this isn't supported, please remove it from the examples. Having an example and saying "you can also..." implies its supported, which isn't true.

python_version = "3.10.9",
)

# You only need to load this repositories if you are using multiple Python versions.
Expand Down Expand Up @@ -102,7 +103,10 @@ pip.parse(
)
pip.parse(
hub_name = "pip",
python_version = "3.10",
# NOTE: if you are registering the python toolchain with the full version
# in 'python.toolchain' call, then you need to use the full version here as
# well or otherwise the resolution will fail.
python_version = "3.10.9",
requirements_lock = "//:requirements_lock_3_10.txt",
requirements_windows = "//:requirements_windows_3_10.txt",
# These modifications were created above and we
Expand Down
7 changes: 6 additions & 1 deletion examples/bzlmod/libs/my_lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,9 @@ def websockets_is_for_python_version(sanitized_version_check):
# We are checking that the name of the repository folders
# match the expexted generated names. If we update the folder
# structure or naming we will need to modify this test
return f"pip_{sanitized_version_check}_websockets" in websockets.__file__
if f"pip_{sanitized_version_check}_websockets" in websockets.__file__:
return True

raise RuntimeError(
f"Expected version '{sanitized_version_check}' was not in {websockets.__file__}"
)
2 changes: 1 addition & 1 deletion examples/bzlmod/other_module/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ PYTHON_NAME_311 = "python_3_11"
python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(
configure_coverage_tool = True,
python_version = "3.9",
python_version = "3.9.10",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here: please remove the patch level

)
python.toolchain(
configure_coverage_tool = True,
Expand Down
10 changes: 7 additions & 3 deletions examples/bzlmod/tests/my_lib_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
# If we update the folder structure or naming we need to modify this test.
sanitized_version_check = f"{sys.version_info.major}{sys.version_info.minor}"

if not my_lib.websockets_is_for_python_version(sanitized_version_check):
print("expected package for Python version is different than returned")
sys.exit(1)
try:
my_lib.websockets_is_for_python_version(sanitized_version_check)
except RuntimeError as e:
print(
f"Expected package for Python version is different than returned. Current version is {sys.version_info}"
)
raise e
6 changes: 3 additions & 3 deletions examples/multi_python_versions/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ python_register_multi_toolchains(
"3.8",
"3.9",
"3.10",
"3.11",
"3.11.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here: please remove the patch level

],
register_coverage_tool = True,
)
Expand All @@ -38,13 +38,13 @@ multi_pip_parse(
default_version = default_python_version,
python_interpreter_target = {
"3.10": interpreter_3_10,
"3.11": interpreter_3_11,
"3.11.1": interpreter_3_11,
"3.8": interpreter_3_8,
"3.9": interpreter_3_9,
},
requirements_lock = {
"3.10": "//requirements:requirements_lock_3_10.txt",
"3.11": "//requirements:requirements_lock_3_11.txt",
"3.11.1": "//requirements:requirements_lock_3_11.txt",
"3.8": "//requirements:requirements_lock_3_8.txt",
"3.9": "//requirements:requirements_lock_3_9.txt",
},
Expand Down
10 changes: 9 additions & 1 deletion examples/multi_python_versions/libs/my_lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,12 @@


def websockets_is_for_python_version(sanitized_version_check):
return f"pypi_{sanitized_version_check}_websockets" in websockets.__file__
# We are checking that the name of the repository folders
# match the expexted generated names. If we update the folder
# structure or naming we will need to modify this test
if f"pypi_{sanitized_version_check}_websockets" in websockets.__file__:
return True

raise RuntimeError(
f"Expected version '{sanitized_version_check}' was not in {websockets.__file__}"
)
3 changes: 2 additions & 1 deletion python/extensions/python.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

load("//python:repositories.bzl", "python_register_toolchains")
load("//python/extensions/private:pythons_hub.bzl", "hub_repo")
load("//python/private:full_version.bzl", "full_version")
load("//python/private:toolchains_repo.bzl", "multi_toolchain_aliases")

# This limit can be increased essentially arbitrarily, but doing so will cause a rebuild of all
Expand Down Expand Up @@ -74,7 +75,7 @@ def _python_impl(module_ctx):
module_toolchain_versions = []

for toolchain_attr in mod.tags.toolchain:
toolchain_version = toolchain_attr.python_version
toolchain_version, _, _ = full_version(toolchain_attr.python_version).rpartition(".")
toolchain_name = "python_" + toolchain_version.replace(".", "_")

# Duplicate versions within a module indicate a misconfigured module.
Expand Down
3 changes: 2 additions & 1 deletion python/pip.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ load("//python/pip_install:requirements.bzl", _compile_pip_requirements = "compi
load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED")
load("//python/private:full_version.bzl", "full_version")
load("//python/private:render_pkg_aliases.bzl", "NO_MATCH_ERROR_MESSAGE_TEMPLATE")
load("//python/private:version_label.bzl", "version_label")

compile_pip_requirements = _compile_pip_requirements
package_annotation = _package_annotation
Expand Down Expand Up @@ -369,7 +370,7 @@ def multi_pip_parse(name, default_version, python_versions, python_interpreter_t
if not python_version in requirements_lock:
fail("Missing requirements_lock for Python version %s in '%s'" % (python_version, name))

pip_parse_name = name + "_" + python_version.replace(".", "_")
pip_parse_name = name + "_" + version_label(full_version(python_version), sep = "_")
pip_parse(
name = pip_parse_name,
python_interpreter_target = python_interpreter_target[python_version],
Expand Down
17 changes: 12 additions & 5 deletions python/private/toolchains_repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ load(
"PLATFORMS",
"WINDOWS_NAME",
)
load(":full_version.bzl", "full_version")
load(":which.bzl", "which_with_fail")

def get_repository_name(repository_workspace):
Expand Down Expand Up @@ -252,8 +253,12 @@ def _multi_toolchain_aliases_impl(rctx):
rules_python = rctx.attr._rules_python_workspace.workspace_name

for python_version, repository_name in rctx.attr.python_versions.items():
file = "{}/defs.bzl".format(python_version)
rctx.file(file, content = """\
python_version = full_version(python_version)
for is_short in [False, True]:
python_version_short, _, _ = python_version.rpartition(".")
v = python_version_short if is_short else python_version

rctx.file("{}/defs.bzl".format(v), content = """\
# Generated by python/private/toolchains_repo.bzl

load(
Expand All @@ -266,16 +271,18 @@ load(
_py_test = "py_test",
)

{warning}
compile_pip_requirements = _compile_pip_requirements
host_platform = _host_platform
interpreter = _interpreter
py_binary = _py_binary
py_console_script_binary = _py_console_script_binary
py_test = _py_test
""".format(
repository_name = repository_name,
))
rctx.file("{}/BUILD.bazel".format(python_version), "")
repository_name = repository_name,
warning = "" if is_short else 'print("switch to importing from \"{}:defs.bzl\"")'.format(python_version_short),
))
rctx.file("{}/BUILD.bazel".format(v), "")

pip_bzl = """\
# Generated by python/private/toolchains_repo.bzl
Expand Down