Skip to content
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

feat: support parsing whl METADATA on any python version #1693

Merged
merged 29 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
fc90b80
feat: support resolving deps for a non-host interpreter version
aignas Jan 14, 2024
6bde063
Make os optional and fix the specializations call
aignas Jan 19, 2024
8f6b8a3
Add a test with version and platform matching
aignas Jan 19, 2024
1a01f07
make minor_version optional instead
aignas Jan 19, 2024
1b93a5e
improve the from string method
aignas Jan 19, 2024
63e1ad6
fixup! improve the from string method
aignas Jan 19, 2024
22128af
changelog
aignas Jan 19, 2024
03e9c1e
fixup! fixup! improve the from string method
aignas Jan 19, 2024
a385b8a
update changelog
aignas Jan 19, 2024
237738d
address review comments
aignas Jan 20, 2024
8ab711f
test: add tests for the select rendering
aignas Jan 20, 2024
f9bc941
fixup: use cannonical labels
aignas Jan 20, 2024
40a7546
Merge branch 'main' into feat/python-impl-independent-2
aignas Jan 20, 2024
1ec13e9
update docs
aignas Jan 20, 2024
11bd71e
fixup: remove 'all' from args test
aignas Jan 20, 2024
20c5a30
fixup: simplify the simple test and make tests work without bzlmod
aignas Jan 20, 2024
8432361
test: integration test and bugfix the version-specific loads
aignas Jan 20, 2024
7047f66
add the rest of OSes to the 3.10 settings
aignas Jan 20, 2024
d8b80d1
fixup config setting group
aignas Jan 20, 2024
da112f2
merge dependencies present on all py target versions to the common list
aignas Jan 20, 2024
19ed77d
simplify example
aignas Jan 20, 2024
eca95cc
clarify changelog
aignas Jan 20, 2024
65c5542
set the default python_version string_flag to make the selects work
aignas Jan 21, 2024
f744d3c
Merge branch 'main' into feat/python-impl-independent-2
aignas Jan 24, 2024
02360ca
revert the test code from bzlmod example - not ready yet to be used
aignas Jan 24, 2024
db002f3
doc: move changelog
aignas Jan 24, 2024
c24a7a8
update the FIXME
aignas Jan 24, 2024
3e4e504
Merge branch 'main' into feat/python-impl-independent-2
aignas Jan 24, 2024
7c276f2
comment: rename
aignas Jan 24, 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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ A brief description of the categories of changes:
platform-specific content in `MODULE.bazel.lock` files; Follow
[#1643](https://github.com/bazelbuild/rules_python/issues/1643) for removing
platform-specific content in `MODULE.bazel.lock` files.

* (wheel) The stamp variables inside the distribution name are no longer
lower-cased when normalizing under PEP440 conventions.

Expand Down Expand Up @@ -68,6 +69,13 @@ A brief description of the categories of changes:
settings to match on minor Python version. These settings match any `X.Y`
version instead of just an exact `X.Y.Z` version.

* *EXPERIMENTAL* (whl_library) The whl_library now can parse the `whl`
`METADATA` correctly irrespectively of the python interpreter version used to
run the tool. This means that for wheel-only setups people can use the system
interpreter when creating the external code repositories. In the future the
code can be modified to add select statements on the python version in the
version aware toolchains.

## [0.28.0] - 2024-01-07

[0.28.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.28.0
Expand Down
9 changes: 6 additions & 3 deletions examples/bzlmod/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,12 @@ pip.parse(
# You can use one of the values below to specify the target platform
# to generate the dependency graph for.
experimental_target_platforms = [
"all",
"cp39_linux_x86_64",
"cp39_linux_*",
"cp39_*",
"linux_*",
"linux_x86_64",
# The following will get the interpreter version from the one passed to pip.parse
"host",
],
hub_name = "pip",
Expand Down Expand Up @@ -137,8 +141,7 @@ pip.parse(
# You can use one of the values below to specify the target platform
# to generate the dependency graph for.
experimental_target_platforms = [
"all",
"linux_*",
"cp310_*",
"host",
],
hub_name = "pip",
Expand Down
2 changes: 1 addition & 1 deletion python/pip_install/pip_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ def _whl_library_impl(rctx):
# NOTE @aignas 2023-12-04: if the wheel is a platform specific
# wheel, we only include deps for that target platform
target_platforms = [
"{}_{}".format(p.os, p.cpu)
"{}_{}_{}".format(parsed_whl.abi_tag, p.os, p.cpu)
for p in whl_target_platforms(parsed_whl.platform_tag)
]

Expand Down
21 changes: 15 additions & 6 deletions python/pip_install/private/generate_whl_library_build_bazel.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -232,19 +232,28 @@ def generate_whl_library_build_bazel(
if p.startswith("@"):
continue

os, _, cpu = p.partition("_")
abi, _, tail = p.partition("_")
aignas marked this conversation as resolved.
Show resolved Hide resolved
os, _, cpu = tail.partition("_")
if not abi.startswith("cp"):
# The first item is not an abi
abi, os, cpu = "", abi, os

constraint_values = [
"@platforms//cpu:{}".format(cpu),
"@platforms//os:{}".format(os),
]
if abi:
minor_version = int(abi[len("cp3"):])
constraint_values.append("@rules_python//python/config_settings:is_python_3.{}".format(minor_version))
aignas marked this conversation as resolved.
Show resolved Hide resolved

additional_content.append(
"""\
config_setting(
name = "is_{os}_{cpu}",
constraint_values = [
"@platforms//cpu:{cpu}",
"@platforms//os:{os}",
],
constraint_values = {},
visibility = ["//visibility:private"],
)
""".format(os = os, cpu = cpu),
""".format(render.indent(render.list(constraint_values)).strip()),
)

lib_dependencies = _render_list_and_select(
Expand Down
98 changes: 80 additions & 18 deletions python/pip_install/tools/wheel_installer/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,30 @@ def _as_int(value: Optional[Union[OS, Arch]]) -> int:
return int(value.value)


class MinorVersion(int):
aignas marked this conversation as resolved.
Show resolved Hide resolved
@classmethod
def host(cls) -> "MinorVersion":
version = platform.python_version()
aignas marked this conversation as resolved.
Show resolved Hide resolved
_, _, tail = version.partition(".")
minor, _, _ = tail.partition(".")
return cls(minor)


@dataclass(frozen=True)
class Platform:
os: OS
os: Optional[OS] = None
arch: Optional[Arch] = None
minor_version: Optional[MinorVersion] = None

@classmethod
def all(cls, want_os: Optional[OS] = None) -> List["Platform"]:
def all(
cls,
want_os: Optional[OS] = None,
minor_version: Optional[MinorVersion] = None,
) -> List["Platform"]:
return sorted(
[
cls(os=os, arch=arch)
cls(os=os, arch=arch, minor_version=minor_version)
for os in OS
for arch in Arch
if not want_os or want_os == os
Expand Down Expand Up @@ -121,7 +135,14 @@ def all_specializations(self) -> Iterator["Platform"]:
yield self
if self.arch is None:
for arch in Arch:
yield Platform(os=self.os, arch=arch)
yield Platform(os=self.os, arch=arch, minor_version=self.minor_version)
if self.os is None:
for os in OS:
yield Platform(os=os, arch=self.arch, minor_version=self.minor_version)
if self.arch is None and self.os is None:
for os in OS:
for arch in Arch:
yield Platform(os=os, arch=arch, minor_version=self.minor_version)

def __lt__(self, other: Any) -> bool:
"""Add a comparison method, so that `sorted` returns the most specialized platforms first."""
Expand All @@ -137,10 +158,23 @@ def __lt__(self, other: Any) -> bool:
return self_os < other_os

def __str__(self) -> str:
if self.minor_version is None:
assert self.os is not None, "if minor_version is None, OS must be specified"
if self.arch is None:
return f"@platforms//os:{self.os}"
else:
return f"{self.os}_{self.arch}"

if self.arch is None and self.os is None:
return f"@rules_python//python/config_settings:is_python_3.{self.minor_version}"

if self.arch is None:
return f"@platforms//os:{self.os}"
return f"cp3{self.minor_version}_{self.os}_anyarch"

if self.os is None:
return f"cp3{self.minor_version}_anyos_{self.arch}"

return f"{self.os}_{self.arch}"
return f"cp3{self.minor_version}_{self.os}_{self.arch}"

@classmethod
def from_string(cls, platform: Union[str, List[str]]) -> List["Platform"]:
Expand All @@ -150,14 +184,27 @@ def from_string(cls, platform: Union[str, List[str]]) -> List["Platform"]:
for p in platform:
if p == "host":
ret.update(cls.host())
elif p == "all":
ret.update(cls.all())
elif p.endswith("*"):
os, _, _ = p.partition("_")
ret.update(cls.all(OS[os]))
continue

abi, _, tail = p.partition("_")
os, _, arch = tail.partition("_")
if not abi.startswith("cp"):
# The first item is not an abi
abi, os, arch = "", abi, os
elif not arch and os == "*":
arch = "*"

minor_version = MinorVersion(abi[len("cp3") :]) if abi else None

if arch != "*":
ret.add(cls(os=OS[os], arch=Arch[arch], minor_version=minor_version))
else:
os, _, arch = p.partition("_")
ret.add(cls(os=OS[os], arch=Arch[arch]))
ret.update(
cls.all(
want_os=OS[os] if os != "*" else None,
minor_version=minor_version,
)
)

return sorted(ret)

Expand Down Expand Up @@ -227,6 +274,9 @@ def platform_machine(self) -> str:
return ""

def env_markers(self, extra: str) -> Dict[str, str]:
# If it is None, use the host version
minor_version = self.minor_version or MinorVersion.host()

return {
"extra": extra,
"os_name": self.os_name,
Expand All @@ -235,11 +285,14 @@ def env_markers(self, extra: str) -> Dict[str, str]:
"platform_system": self.platform_system,
"platform_release": "", # unset
"platform_version": "", # unset
"python_version": f"3.{minor_version}",
# FIXME @aignas 2024-01-14: is putting zero last a good idea? Maybe we should
# use `20` or something else to avoid having wheird issues where the full version is used for
aignas marked this conversation as resolved.
Show resolved Hide resolved
# matching and the author decides to only support 3.y.5 upwards.
"implementation_version": f"3.{minor_version}.0",
"python_full_version": f"3.{minor_version}.0",
# we assume that the following are the same as the interpreter used to setup the deps:
# "implementation_version": "X.Y.Z",
# "implementation_name": "cpython"
# "python_version": "X.Y",
# "python_full_version": "X.Y.Z",
# "platform_python_implementation: "CPython",
}

Expand All @@ -258,9 +311,11 @@ def __init__(
requires_dist: Optional[List[str]],
extras: Optional[Set[str]] = None,
platforms: Optional[Set[Platform]] = None,
add_version_select: bool = False,
):
self.name: str = Deps._normalize(name)
self._platforms: Set[Platform] = platforms or set()
self._add_version_select = add_version_select

# Sort so that the dictionary order in the FrozenDeps is deterministic
# without the final sort because Python retains insertion order. That way
Expand Down Expand Up @@ -400,8 +455,9 @@ def _add_req(self, req: Requirement, extras: Set[str]) -> None:
]
)
match_arch = "platform_machine" in marker_str
match_version = "version" in marker_str

if not (match_os or match_arch):
if not (match_os or match_arch or match_version):
if any(req.marker.evaluate({"extra": extra}) for extra in extras):
self._add(req.name, None)
return
Expand All @@ -414,8 +470,14 @@ def _add_req(self, req: Requirement, extras: Set[str]) -> None:

if match_arch:
self._add(req.name, plat)
else:
elif match_os and self._add_version_select:
self._add(req.name, Platform(plat.os, minor_version=plat.minor_version))
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))
elif match_version:
self._add(req.name, None)

def build(self) -> FrozenDeps:
return FrozenDeps(
Expand Down
Loading
Loading