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 all 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
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ A brief description of the categories of changes:

* (py_wheel) Added `requires_file` and `extra_requires_files` attributes.

* (whl_library) *experimental_target_platforms* now supports specifying the
Python version explicitly and the output `BUILD.bazel` file will be correct
irrespective of the python interpreter that is generating the file and
extracting the `whl` distribution. Multiple python target version can be
specified and the code generation will generate version specific dependency
closures but that is not yet ready to be used and may break the build if
the default python version is not selected using
`common --@rules_python//python/config_settings:python_version=X.Y.Z`.

## 0.29.0 - 2024-01-22

[0.29.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.29.0
Expand All @@ -59,6 +68,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
2 changes: 1 addition & 1 deletion examples/bzlmod/.bazelrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
common --experimental_enable_bzlmod
common --enable_bzlmod

coverage --java_runtime_version=remotejdk_11

Expand Down
14 changes: 10 additions & 4 deletions examples/bzlmod/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,10 @@ 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_*",
"host",
# Specifying the target platforms explicitly
"cp39_linux_x86_64",
"cp39_linux_*",
"cp39_*",
],
hub_name = "pip",
python_version = "3.9",
Expand Down Expand Up @@ -137,8 +138,13 @@ 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",
# Using host python version
aignas marked this conversation as resolved.
Show resolved Hide resolved
"linux_*",
"osx_*",
"windows_*",
# Or specifying an exact platform
"linux_x86_64",
# Or the following to get the `host` platform only
"host",
],
hub_name = "pip",
Expand Down
16 changes: 11 additions & 5 deletions python/pip_install/pip_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -493,13 +493,19 @@ WARNING: It may not work as expected in cases where the python interpreter
implementation that is being used at runtime is different between different platforms.
This has been tested for CPython only.

Special values: `all` (for generating deps for all platforms), `host` (for
generating deps for the host platform only). `linux_*` and other `<os>_*` values.
In the future we plan to set `all` as the default to this attribute.

For specific target platforms use values of the form `<os>_<arch>` where `<os>`
is one of `linux`, `osx`, `windows` and arch is one of `x86_64`, `x86_32`,
`aarch64`, `s390x` and `ppc64le`.

You can also target a specific Python version by using `cp3<minor_version>_<os>_<arch>`.
If multiple python versions are specified as target platforms, then select statements
of the `lib` and `whl` targets will include usage of version aware toolchain config
settings like `@rules_python//python/config_settings:is_python_3.y`.

Special values: `host` (for generating deps for the host platform only) and
`<prefix>_*` values. For example, `cp39_*`, `linux_*`, `cp39_linux_*`.

NOTE: this is not for cross-compiling Python wheels but rather for parsing the `whl` METADATA correctly.
""",
),
"extra_pip_args": attr.string_list(
Expand Down Expand Up @@ -749,7 +755,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
135 changes: 112 additions & 23 deletions python/pip_install/private/generate_whl_library_build_bazel.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ py_binary(
"""

_BUILD_TEMPLATE = """\
load("@rules_python//python:defs.bzl", "py_library", "py_binary")
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
{loads}

package(default_visibility = ["//visibility:public"])

Expand Down Expand Up @@ -102,22 +101,37 @@ alias(
)
"""

def _plat_label(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

def _render_list_and_select(deps, deps_by_platform, tmpl):
deps = render.list([tmpl.format(d) for d in deps])
deps = render.list([tmpl.format(d) for d in sorted(deps)])

if not deps_by_platform:
return deps

deps_by_platform = {
p if p.startswith("@") else ":is_" + p: [
_plat_label(p): [
tmpl.format(d)
for d in deps
for d in sorted(deps)
]
for p, deps in deps_by_platform.items()
for p, deps in sorted(deps_by_platform.items())
}

# 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 = render.select(deps_by_platform, value_repr = render.list)

Expand All @@ -126,6 +140,87 @@ def _render_list_and_select(deps, deps_by_platform, tmpl):
else:
return "{} + {}".format(deps, deps_by_platform)

def _render_config_settings(dependencies_by_platform):
py_version_by_os_arch = {}
for p in dependencies_by_platform:
# p can be one of the following formats:
# * @platforms//os:{value}
# * @platforms//cpu:{value}
# * @//python/config_settings:is_python_3.{minor_version}
# * {os}_{cpu}
# * cp3{minor_version}_{os}_{cpu}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI, I had to move the rendering of the config settings outside the main loop because it was getting complex. However, this PR can generate the right select for pylint:

    deps = [
        "@pip_39_astroid//:pkg",
        "@pip_39_dill//:pkg",
        "@pip_39_isort//:pkg",
        "@pip_39_mccabe//:pkg",
        "@pip_39_platformdirs//:pkg",
        "@pip_39_tomlkit//:pkg",
    ] + select(
        {
            "@@rules_python~override//python/config_settings:is_python_3.10": ["@pip_39_tomli//:pkg"],
            "@@rules_python~override//python/config_settings:is_python_3.9": [
                "@pip_39_tomli//:pkg",
                "@pip_39_typing_extensions//:pkg",
            ],
            ":is_cp310_windows_anyarch": [
                "@pip_39_colorama//:pkg",
                "@pip_39_dill//:pkg",
                "@pip_39_tomli//:pkg",
            ],
            ":is_cp311_windows_anyarch": [
                "@pip_39_colorama//:pkg",
                "@pip_39_dill//:pkg",
            ],
            ":is_cp39_windows_anyarch": [
                "@pip_39_colorama//:pkg",
                "@pip_39_dill//:pkg",
                "@pip_39_tomli//:pkg",
                "@pip_39_typing_extensions//:pkg",
            ],
            "//conditions:default": [],
        },
    ),

Disregard the fact that the conditions and the target repos should match - this can be done as a followup item. I am not sure yet what design would be best. This versioned select is only enabled if the user specifies multiple target python versions.

if p.startswith("@"):
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))

os_arch = (os or "anyos") + "_" + (arch or "anyarch")
additional_content.append(
"""\
config_setting(
name = "is_{name}",
constraint_values = {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,
)
settings = [
":is_" + os_arch,
setting,
]

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

additional_content.append(
"""\
selects.config_setting_group(
name = "{name}",
match_all = {values},
visibility = ["//visibility:private"],
)""".format(
name = _plat_label(plat).lstrip(":"),
values = render.indent(render.list(sorted(settings))).strip(),
),
)

return loads, "\n\n".join(additional_content)

def generate_whl_library_build_bazel(
*,
repo_prefix,
Expand Down Expand Up @@ -228,24 +323,17 @@ def generate_whl_library_build_bazel(
if deps
}

for p in dependencies_by_platform:
if p.startswith("@"):
continue

os, _, cpu = p.partition("_")
loads = [
"""load("@rules_python//python:defs.bzl", "py_library", "py_binary")""",
"""load("@bazel_skylib//rules:copy_file.bzl", "copy_file")""",
]

additional_content.append(
"""\
config_setting(
name = "is_{os}_{cpu}",
constraint_values = [
"@platforms//cpu:{cpu}",
"@platforms//os:{os}",
],
visibility = ["//visibility:private"],
)
""".format(os = os, cpu = cpu),
)
loads_, config_settings_content = _render_config_settings(dependencies_by_platform)
if config_settings_content:
for line in loads_:
if line not in loads:
loads.append(line)
additional_content.append(config_settings_content)

lib_dependencies = _render_list_and_select(
deps = dependencies,
Expand Down Expand Up @@ -277,6 +365,7 @@ config_setting(
contents = "\n".join(
[
_BUILD_TEMPLATE.format(
loads = "\n".join(loads),
py_library_public_label = PY_LIBRARY_PUBLIC_LABEL,
py_library_impl_label = PY_LIBRARY_IMPL_LABEL,
py_library_actual_label = library_impl_label,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ def test_platform_aggregation(self) -> None:
args=[
"--platform=host",
"--platform=linux_*",
"--platform=all",
"--platform=osx_*",
"--platform=windows_*",
"--requirement=foo",
]
)
Expand Down
Loading
Loading