-
Couldn't load subscription status.
- Fork 1.5k
Switch to Python Build Standalone for macOS wheels
#21716
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
base: master
Are you sure you want to change the base?
Switch to Python Build Standalone for macOS wheels
#21716
Conversation
f75db89 to
d03a228
Compare
d03a228 to
a2711b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
a2711b7 to
77d35ca
Compare
Python Build Standalone for macOS wheels
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one comment, lgtm otherwise
|
Good timing for this PR. We need to switch to a hermetic python for bazel. I am also sort of curious why we do this at all (for omnibus) when all the build machines and dev images have python 3.12 installed. But answering that should not block the this |
Looking at #21160, I guess the question would be better answered by @Kyle-Neale? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me pending removal of the Claude comment!
This is a follow-up of: - #21692 ... where a review comment rightfully suggested that: > Instead, we should run a normal single-arch Python distribution where > everything will just work by default and we can remove the renaming > logic. The source of distributions should be the > [python-build-standalone](https://github.com/astral-sh/python-build-standalone) > project which the makers of UV recently adopted. They are used by > everyone nowadays, even > [rules_python](https://github.com/bazel-contrib/rules_python/blob/cf594f780c91f13d48e77faad34df48ac57398da/python/versions.bzl#L29). The present change aims at embracing the above-recommended approach, which looks indeed promising because the need for bundling targeted architectures is then addressed without additional environment variables, while still allowing `delocate_wheel` to verify architecture requirements. Also, the PBS distribution happens to be pretty lightweight (16MB vs 71MB).
This is to address: > n00b claude user, does this need to stay in the source code? (#21716 (comment)) > Looks good to me pending removal of the Claude comment! (#21716 (review))
77d35ca to
a4238c3
Compare
Review from iliakur is dismissed. Related teams and files:
- agent-integrations
- .github/workflows/resolve-build-deps.yaml
Done in second commit (a4238c3). |
Motivation
This is a direct follow-up of:
... where a review comment rightfully suggested that:
What does this PR do?
The present change aims at embracing the above-recommended approach, which indeed looks simpler because the need for bundling targeted architectures is then addressed without additional environment variables, while still allowing
delocate_wheelto verify architecture requirements.Also, the PBS distribution happens to be pretty lightweight (16MB vs 71MB).