Skip to content

feat: support arbitrary target_settings in our platforms 3/n #2990

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

Merged
merged 2 commits into from
Jun 20, 2025

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Jun 15, 2025

With this PR we can support arbitrary target settings instead of just
plain constraint_values. We still have custom logic to ensure that all
of the tests pass. However, the plan is to remove those tests once we
have simplified the wheel selection mechanisms and the pkg_aliases
macro. I.e. if we have at most 1 wheel per platform that the pypi
bzlmod extension passes to the pkg_aliases macro, then we can just
have a simple selects.with_or where we list out all of the target
platform values.

This PR may result in us creating more targets but that is the price
that we have to pay if we want to do this incrementally.

Work towards #2747
Work towards #2548
Work towards #260

@rickeylev
Copy link
Collaborator

I gave this a skim, but started to lose track of the diff, so I'll hold off on reviewing in depth. Mostly LGTM, though.

With this PR we can support arbitrary target settings instead of just
plain `constraint_values`. We still have custom logic to ensure that all
of the tests pass. However, the plan is to remove those tests once we
have simplified the wheel selection mechanisms and the `pkg_aliases`
macro. I.e. if we have at most 1 wheel per platform that the `pypi`
bzlmod extension passes to the `pkg_aliases` macro, then we can just
have a simple `selects.with_or` where we list out all of the target
platform values.

This PR may result in us creating more targets but that is the price
that we have to pay if we want to do this incrementally.

The last remaining thing for bazel-contrib#2548 is to make sure that we are not
parsing the user-friendly strings to get the `os` and the `cpu`.

Work towards bazel-contrib#2747
Work towards bazel-contrib#2548
Work towards bazel-contrib#260
@aignas aignas force-pushed the feat/pypi/default-3 branch from df5865e to 19ecb6f Compare June 20, 2025 02:26
@aignas aignas marked this pull request as ready for review June 20, 2025 02:27
@aignas aignas requested review from rickeylev and groodt as code owners June 20, 2025 02:27
@rickeylev rickeylev added this pull request to the merge queue Jun 20, 2025
Merged via the queue into bazel-contrib:main with commit 6fd4c0b Jun 20, 2025
3 checks passed
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