Skip to content

Add _test_simple_multiple_platforms_with_extras #2

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

Open
wants to merge 7 commits into
base: exp/pypi-simplify-freethreaded
Choose a base branch
from

Conversation

hartikainen
Copy link

As requested in bazel-contrib#2797 (comment).

aignas and others added 7 commits August 3, 2025 14:18
Before this PR we would be constructing slightly different environments
when the `env_marker_setting` is doing it in the analysis phase and when
we are doing it in the repo phase due to how the defaults are handled.

In this change we simply reuse the same select statements and add an extra
helper that is allowing us to process that.

Work towards bazel-contrib#2949
Prep for bazel-contrib#3058
This will be needed when we start selecting wheels entirely in the
bzlmod extension evaluation phase (bazel-contrib#3058).

This adds a few unit tests to just ensure that we conform to the spec
even though the code is very simple.

Work towards bazel-contrib#2747
Work towards bazel-contrib#2759
Work towards bazel-contrib#2849
DO NOT MERGE: stacked on bazel-contrib#3110

This PR only implements the selection algorithm where instead of
selecting all wheels that are compatible with the set of target
platforms, we select a single wheel that is most specialized for a
particular *single* target platform.

What is more, compared to the existing algorithm it does not assume
a particular list of supported platforms and just fully implements the
spec.

Work towards bazel-contrib#2747
Work towards bazel-contrib#2759
Work towards bazel-contrib#2849
Before this PR the configuration for platforms would be built
non-incrementally, making it harder for users to override particular
attributes of the already configured ones.

With this PR the new features introduced in bazel-contrib#3058 will be easier to
override.

Work towards bazel-contrib#2747
Before this we would pull all of the wheels that the user target
configuration would be compatible with and that meant that it was not
customizable. This also meant that there were a lot of footguns in the
configuration where the select statements were not really foolproof.

With this PR we select only those sources that need to be for the
declared configurations.

Freethreaded support is done by defining extra freethreaded platforms
using the new builder API (bazel-contrib#3063).

This is also changing the default platforms to be only the fully supported
platforms. This makes the testing easier and avoids us running into
compatibility issues during the roll out.

Work towards bazel-contrib#2747
Fixes bazel-contrib#2759
Fixes bazel-contrib#2849
DO NOT MERGE: stacked on bazel-contrib#3058

This is a continuation of bazel-contrib#3058 where we define freethreaded
platforms. They need to be used only for particular python versions
so I included an extra marker configuration attribute where we
are using pipstar marker evaluation before using the platform.

I think this in general will be a useful tool to configure only
particular platforms for particular python versions

Work towards bazel-contrib#2548, since this shows how we can define custom platforms
Work towards bazel-contrib#2747
Run with:
```console
bazel test //tests/pypi/extension:test_simple_multiple_platforms_with_extras
```
@hartikainen hartikainen requested a review from aignas as a code owner August 3, 2025 15:46
Comment on lines +482 to +484
# NOTE(hartikainen): Perhaps this is part of the problem?
# This should say `jax[cuda12]==0.7.0` for `linux_x86_64` platform and
# `jax==0.7.0` for `linux_arm64` and `osx_aarch64`.
Copy link
Author

@hartikainen hartikainen Aug 3, 2025

Choose a reason for hiding this comment

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

My guess is that this points to the main issue. We should probably have two entries under wheel libraries because one of them evaluate to jax[cuda12] and the other two to just jax.

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.

2 participants