Skip to content

Commit

Permalink
fix(whl_library): stabilize dependency per python version handling
Browse files Browse the repository at this point in the history 
We start using the recently introduced `is_python_config_setting` to make
it possible to have a working select statement when multiple python
version selection needs to happen in a `whl_library`.

This adds further fixes so that the correct dependencies are pulled in when the
`python_version` string flag is unset thus making this implementation suitable
for `bzlmod` use case where we would use a single `whl_library` instance for
multiple python versions within the hub.

Work towards bazelbuild#735.
  • Loading branch information
@aignas
aignas committed May 18, 2024
1 parent 3744e4d commit 4f238a5
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 131 deletions.
4 changes: 2 additions & 2 deletions .bazelrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
# (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it)
# To update these lines, execute
# `bazel run @rules_bazel_integration_test//tools:update_deleted_packages`
build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered
query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered
build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered
query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered

test --test_output=errors

Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ A brief description of the categories of changes:
be automatically deleted correctly. For example, if `python_generation_mode`
is set to package, when `__init__.py` is deleted, the `py_library` generated
for this package before will be deleted automatically.
* (whl_library): Use `is_python_config_setting` to correctly handle multi-python
version dependency select statements when the `experimental_target_platforms`
includes the Python ABI. The default python version case within the select is
also now handled correctly, stabilizing the implementation.

### Added

Expand Down
6 changes: 4 additions & 2 deletions examples/bzlmod/MODULE.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,14 @@ pip.parse(
# to generate the dependency graph for.
experimental_target_platforms = [
# Specifying the target platforms explicitly
"cp39_linux_x86_64",
"cp39_linux_*",
"cp39_*",
"cp310_*",
"cp311_*",
"cp312_*",
],
hub_name = "pip",
python_version = "3.9",
quiet = False,
requirements_lock = "//:requirements_lock_3_9.txt",
requirements_windows = "//:requirements_windows_3_9.txt",
# These modifications were created above and we
Expand Down
89 changes: 33 additions & 56 deletions python/pip_install/private/generate_whl_library_build_bazel.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,14 @@ py_library(
"""

def _plat_label(plat):
if plat.endswith("default"):
return plat
if plat.startswith("@//"):
return "@@" + str(Label("//:BUILD.bazel")).partition("//")[0].strip("@") + plat.strip("@")
elif plat.startswith("@"):
return str(Label(plat))
else:
return ":is_" + plat
return ":is_" + plat.replace("cp3", "python_3.")

def _render_list_and_select(deps, deps_by_platform, tmpl):
deps = render.list([tmpl.format(d) for d in sorted(deps)])
Expand All @@ -115,14 +117,7 @@ def _render_list_and_select(deps, deps_by_platform, tmpl):

# Add the default, which means that we will be just using the dependencies in
# `deps` for platforms that are not handled in a special way by the packages
#
# FIXME @aignas 2024-01-24: This currently works as expected only if the default
# value of the @rules_python//python/config_settings:python_version is set in
# the `.bazelrc`. If it is unset, then the we don't get the expected behaviour
# in cases where we are using a simple `py_binary` using the default toolchain
# without forcing any transitions. If the `python_version` config setting is set
# via .bazelrc, then everything works correctly.
deps_by_platform["//conditions:default"] = []
deps_by_platform.setdefault("//conditions:default", [])
deps_by_platform = render.select(deps_by_platform, value_repr = render.list)

if deps == "[]":
Expand All @@ -131,81 +126,63 @@ def _render_list_and_select(deps, deps_by_platform, tmpl):
return "{} + {}".format(deps, deps_by_platform)

def _render_config_settings(dependencies_by_platform):
py_version_by_os_arch = {}
loads = []
additional_content = []
for p in dependencies_by_platform:
# p can be one of the following formats:
# * //conditions:default
# * @platforms//os:{value}
# * @platforms//cpu:{value}
# * @//python/config_settings:is_python_3.{minor_version}
# * {os}_{cpu}
# * cp3{minor_version}_{os}_{cpu}
if p.startswith("@"):
if p.startswith("@") or p.endswith("default"):
continue

abi, _, tail = p.partition("_")
if not abi.startswith("cp"):
tail = p
abi = ""

os, _, arch = tail.partition("_")
os = "" if os == "anyos" else os
arch = "" if arch == "anyarch" else arch

py_version_by_os_arch.setdefault((os, arch), []).append(abi)

if not py_version_by_os_arch:
return None, None

loads = []
additional_content = []
for (os, arch), abis in py_version_by_os_arch.items():
constraint_values = []
if os:
constraint_values.append("@platforms//os:{}".format(os))
if arch:
constraint_values.append("@platforms//cpu:{}".format(arch))
if os:
constraint_values.append("@platforms//os:{}".format(os))

os_arch = (os or "anyos") + "_" + (arch or "anyarch")
additional_content.append(
"""\
config_setting(
constraint_values_str = render.indent(render.list(constraint_values)).lstrip()

if abi:
if not loads:
loads.append("""load("@rules_python//python/config_settings:config_settings.bzl", "is_python_config_setting")""")

additional_content.append(
"""\
is_python_config_setting(
name = "is_{name}",
constraint_values = {values},
python_version = "3.{minor_version}",
constraint_values = {constraint_values},
visibility = ["//visibility:private"],
)""".format(
name = os_arch,
values = render.indent(render.list(sorted([str(Label(c)) for c in constraint_values]))).strip(),
),
)

if abis == [""]:
if not os or not arch:
fail("BUG: both os and arch should be set in this case")
continue

for abi in abis:
if not loads:
loads.append("""load("@bazel_skylib//lib:selects.bzl", "selects")""")
minor_version = int(abi[len("cp3"):])
setting = "@@{rules_python}//python/config_settings:is_python_3.{version}".format(
rules_python = str(Label("//:BUILD.bazel")).partition("//")[0].strip("@"),
version = minor_version,
name = p.replace("cp3", "python_3."),
minor_version = abi[len("cp3"):],
constraint_values = constraint_values_str,
),
)
settings = [
":is_" + os_arch,
setting,
]

plat = "{}_{}".format(abi, os_arch)

else:
additional_content.append(
"""\
selects.config_setting_group(
name = "{name}",
match_all = {values},
config_setting(
name = "is_{name}",
constraint_values = {constraint_values},
visibility = ["//visibility:private"],
)""".format(
name = _plat_label(plat).lstrip(":"),
values = render.indent(render.list(sorted(settings))).strip(),
name = p.replace("cp3", "python_3."),
constraint_values = constraint_values_str,
),
)

Expand Down Expand Up @@ -379,7 +356,7 @@ def generate_whl_library_build_bazel(
contents = "\n".join(
[
_BUILD_TEMPLATE.format(
loads = "\n".join(loads),
loads = "\n".join(sorted(loads)),
py_library_label = py_library_label,
dependencies = render.indent(lib_dependencies, " " * 4).lstrip(),
whl_file_deps = render.indent(whl_file_deps, " " * 4).lstrip(),
Expand Down
39 changes: 26 additions & 13 deletions python/pip_install/tools/wheel_installer/wheel.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class Arch(Enum):
ppc64le = ppc

@classmethod
def interpreter(cls) -> "OS":
def interpreter(cls) -> "Arch":
"Return the currently running interpreter architecture."
# FIXME @aignas 2023-12-13: Hermetic toolchain on Windows 3.11.6
# is returning an empty string here, so lets default to x86_64
Expand Down Expand Up @@ -94,12 +94,6 @@ class Platform:
arch: Optional[Arch] = None
minor_version: Optional[int] = None

def __post_init__(self):
if not self.os and not self.arch and not self.minor_version:
raise ValueError(
"At least one of os, arch, minor_version must be specified"
)

@classmethod
def all(
cls,
Expand All @@ -125,7 +119,13 @@ def host(cls) -> List["Platform"]:
A list of parsed values which makes the signature the same as
`Platform.all` and `Platform.from_string`.
"""
return [cls(os=OS.interpreter(), arch=Arch.interpreter())]
return [
Platform(
os=OS.interpreter(),
arch=Arch.interpreter(),
minor_version=host_interpreter_minor_version(),
)
]

def all_specializations(self) -> Iterator["Platform"]:
"""Return the platform itself and all its unambiguous specializations.
Expand Down Expand Up @@ -160,9 +160,9 @@ def __lt__(self, other: Any) -> bool:

def __str__(self) -> str:
if self.minor_version is None:
assert (
self.os is not None
), f"if minor_version is None, OS must be specified, got {repr(self)}"
if self.os is None and self.arch is None:
return "//conditions:default"

if self.arch is None:
return f"@platforms//os:{self.os}"
else:
Expand Down Expand Up @@ -207,6 +207,7 @@ def from_string(cls, platform: Union[str, List[str]]) -> List["Platform"]:
minor_version=minor_version,
)
)

else:
ret.update(
cls.all(
Expand Down Expand Up @@ -252,6 +253,8 @@ def platform_system(self) -> str:
return "Darwin"
elif self.os == OS.windows:
return "Windows"
else:
return ""

# derived from OS and Arch
@property
Expand Down Expand Up @@ -416,7 +419,9 @@ def _maybe_add_common_dep(self, dep):
if len(self._target_versions) < 2:
return

platforms = [Platform(minor_version=v) for v in self._target_versions]
platforms = [Platform()] + [
Platform(minor_version=v) for v in self._target_versions
]

# If the dep is targeting all target python versions, lets add it to
# the common dependency list to simplify the select statements.
Expand Down Expand Up @@ -534,14 +539,22 @@ def _add_req(self, req: Requirement, extras: Set[str]) -> None:
):
continue

if match_arch:
if match_arch and self._add_version_select:
self._add(req.name, plat)
if plat.minor_version == host_interpreter_minor_version():
self._add(req.name, Platform(plat.os, plat.arch))
elif match_arch:
self._add(req.name, plat)
elif match_os and self._add_version_select:
self._add(req.name, Platform(plat.os, minor_version=plat.minor_version))
if plat.minor_version == host_interpreter_minor_version():
self._add(req.name, Platform(plat.os))
elif match_os:
self._add(req.name, Platform(plat.os))
elif match_version and self._add_version_select:
self._add(req.name, Platform(minor_version=plat.minor_version))
if plat.minor_version == host_interpreter_minor_version():
self._add(req.name, Platform())
elif match_version:
self._add(req.name, None)

Expand Down
Loading

0 comments on commit 4f238a5

Please sign in to comment.