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(whl_library): generate platform-specific dependency closures #1593

Merged
merged 20 commits into from
Dec 13, 2023

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Dec 4, 2023

Before this change, the dependency closures would be influenced by the
host-python interpreter, this removes the influence by detecting the
platforms against which the Requires-Dist wheel metadata is evaluated.
This functionality can be enabled via experimental_target_platforms
attribute to the pip.parse extension and is showcased in the bzlmod
example. The same attribute is also supported on the legacy pip_parse
repository rule.

The detection works in the following way:

  • Check if the python wheel is platform specific or cross-platform
    (i.e., ends with any.whl), if it is then platform-specific
    dependencies are generated, which will go through a select
    statement.
  • If it is platform specific, then parse the platform_tag and evaluate
    the Requires-Dist markers assuming the target platform rather than
    the host platform.

NOTE: The whl METADATA is now being parsed using the packaging
Python package instead of pkg_resources from setuptools.

Fixes #1591

@aignas aignas force-pushed the exp/os-specific-deps branch 2 times, most recently from 960ad4c to e85bd8b Compare December 4, 2023 22:58
python/config_settings/BUILD.bazel Outdated Show resolved Hide resolved
python/private/text_util.bzl Outdated Show resolved Hide resolved
python/pip_install/pip_repository.bzl Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/wheel.py Outdated Show resolved Hide resolved
continue

if not platforms:
# old behaviour, where we target the host platform
Copy link
Contributor

Choose a reason for hiding this comment

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

Something I learned last week: Bazel has a builtin repository that tells you the host system's constraints.

load("@local_config_platform//:constraints.bzl", "HOST_CONSTRAINTS")

(Not sure where we could use this, just passing along a TIL)

@aignas aignas requested a review from rickeylev December 8, 2023 06:04
@aignas aignas marked this pull request as ready for review December 8, 2023 06:04
@aignas
Copy link
Collaborator Author

aignas commented Dec 8, 2023

I think the resultant BUILD.bazel files are good enough for it to be considered for merging. @arrdem, cc-ing you as you've spent quite some time on this topic recently and would love to hear your opinion/feedback on the change.

This is to ensure that we can represent select of lists in a better way
Before this change, the dependency closures would be influenced by the
host-python interpreter, this removes the influence by detecting the
platforms against which the `Requires-Dist` wheel metadata is evaluated.
This functionality can be enabled via `target_platforms` attribute to
the `pip.parse` extension and is showcased in the `bzlmod` example. The
same attribute is also supported on the legacy `pip_parse` repository
rule.

The detection works in the following way:
- Check if the python wheel is platform specific or cross-platform
  (i.e., ends with `any.whl`), if it is then platform-specific
  dependencies are generated, which will go through a `select`
  statement.
- If it is platform specific, then parse the platform_tag and evaluate
  the `Requires-Dist` markers assuming the target platform rather than
  the host platform.

NOTE: The `whl` `METADATA` is now being parsed using the `packaging`
Python package instead of `pkg_resources` from `setuptools`.

Fixes bazelbuild#1591
CHANGELOG.md Outdated Show resolved Hide resolved
python/pip_install/pip_repository.bzl Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/wheel.py Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/wheel.py Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/wheel.py Outdated Show resolved Hide resolved
python/pip_install/pip_repository.bzl Outdated Show resolved Hide resolved
python/pip_install/pip_repository.bzl Outdated Show resolved Hide resolved
python/pip_install/pip_repository.bzl Outdated Show resolved Hide resolved
python/pip_install/pip_repository.bzl Outdated Show resolved Hide resolved
python/pip_install/pip_repository.bzl Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/arguments.py Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/wheel.py Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/wheel.py Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/wheel.py Outdated Show resolved Hide resolved
python/pip_install/tools/wheel_installer/wheel.py Outdated Show resolved Hide resolved
examples/bzlmod/MODULE.bazel Outdated Show resolved Hide resolved
examples/bzlmod/MODULE.bazel Outdated Show resolved Hide resolved
@rickeylev rickeylev added this pull request to the merge queue Dec 13, 2023
Merged via the queue into bazelbuild:main with commit 2b5447b Dec 13, 2023
3 checks passed
@aignas aignas deleted the exp/os-specific-deps branch May 13, 2024 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dependencies are not when using wheels on a different platform
2 participants