-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Allow discovery of x86-64 managed Python builds on macOS #13474
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
Conversation
05782d0 to
b27ef41
Compare
| success: true | ||
| exit_code: 0 | ||
| ----- stdout ----- | ||
| [TEMP_DIR]/managed/cpython-3.13.3-macos-x86_64-none/bin/python3.13 |
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.
Unfortunately this is wrong. We'll need to change the ordering of the managed distributions per target platform so we prefer the native builds.
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.
I think this hints at a different design for #13701
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.
|
|
||
| // Windows ARM64 runs emulated x86_64 binaries transparently | ||
| if cfg!(windows) && matches!(self.family, target_lexicon::Architecture::Aarch64(_)) { | ||
| // macOS aarch64 and Windows ARM64 runs emulated x86_64 binaries transparently |
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.
I will note that isn't actually strictly true for macOS: Rosetta2 is not default-installed on Apple Silicon macOS. It is auto-installed the first time you run an intel app... but only if it's a full GUI App. If it's just a random CLI binary it actually won't bother!
So if we want to be fully bullet-proof we would have to check if Rosetta2 is already on the user's system :(
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.
In practice it tends to be fine though. Especially since we're finding these things on their system.
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.
Oh dear goodness. That's... good to know. I will at least add some commentary about that.
…tions (#13709) Resolves #13474 (comment) This kind of dynamic ordering freaks me out a little, but I think it's probably the best solution and is static at compile-time. Currently, we're just sorting by the stringified representation! which is just convenient for reproducibility, but we rely on these orderings for prioritization in discovery.
Replacement for #13474 (clobbered by a changed base) Once these are explicitly installed, they should be discoverable and usable. Currently, they are not.
No description provided.