@lawrence-danna-apple We currently redistributelibffi along with Python to support Mac OS X 10.10 as our deployment target. Your change implies that there were changes made tolibffi (and the copy that ships with macOS 11) for arm64 macOS. Are these upstreamed and available?

@lawrence-danna-apple We currently redistributelibffi along with Python to support Mac OS X 10.10 as our deployment target. Your change implies that there were changes made tolibffi (and the copy that ships with macOS 11) for arm64 macOS. Are these upstreamed and available?

We also distributelibffi as part of Mac OS. Our changes for 10.15, which cover the necessary API changes are available here:https://opensource.apple.com/source/libffi/libffi-25/

I'm not sure what the status is as to upstreaming, but I'll do a review of it and see if anything necessary is still missing from upstream.

Noe that in the changes I'm proposing here, the Mac OS systemlibffi is only used if the deployment target is set to 10.15 or better.

@lawrence-danna-apple We currently redistributelibffi along with Python to support Mac OS X 10.10 as our deployment target. Your change implies that there were changes made tolibffi (and the copy that ships with macOS 11) for arm64 macOS. Are these upstreamed and available?

We also distributelibffi as part of Mac OS. Our changes for 10.15, which cover the necessary API changes are available here:https://opensource.apple.com/source/libffi/libffi-25/

I'm not sure what the status is as to upstreaming, but I'll do a review of it and see if anything necessary is still missing from upstream.

Noe that in the changes I'm proposing here, the Mac OS systemlibffi is only used if the deployment target is set to 10.15 or better.

Got it, thanks for clarifying. It looks like there may be some issues inlibffi w/ XC12 + arm64:libffi/libffi#571. Good to know that we could rely onlibffi-25, though.

I'm a bit conflicted about the "variadic" attribute for _FuncPtr. I agree that it is necessary, but this is an API change and as such is a lot harder to back port.

I'd prefer to drop the copy of libffi included with CPython and always use the system libffi

I'm not sure if this is possible. The libffi prior to 10.15 won't have the APIs we need to support arm64. However, we may be able to use ifdefs and availability to sort it out. I'll try.

Do you know why libffi was bundled into cpython in the first place? Do we have reason to believe the libffi on older releases of mac OS was broken in some way?

@ronaldoussoren

I'm a bit conflicted about the "variadic" attribute for _FuncPtr. I agree that it is necessary

Well, it looks like I misread the documentation. It turns out that in the case where a variadic function is called with zero variadic arguments, the two calling conventions coincide. So the flag is not necessary after all.

@ronaldoussoren

I found it is possible to use the system libffi even if the deployment target is set to before 10.15. I've added runtime checks forffi_prep_cif_var,ffi_prep_closure_loc andffi_closure_alloc. If they are present, it will use them. If not, it will fall back on the old way of doing things.

I've also enabled the system ffi if either the deployment target is 10.15 OR the arm64 is in the target arches. I'm not sure if Mac OS on PPC is still supported by python, but these changes shouldn't break it if it is.

The only thing that will not work is to build a super-fat binary that works on ppc, x86 and arm64 at the same time.

Should I add a commit deletinglibffi_osx altogether, or leave it in for ppc support?

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I have made the requested changes; please review again

Thanks for making the requested changes!

@ned-deily: please review the changes made to this pull request.

@ned-deily

Your comments have alerted me to a new problem. If I understand you correctly, the way you're doing the official builds for python is to build on an old version of Mac OS, with an old version of Xcode, in order to assure compatibility with a broad range of Mac OS versions.

This will work for x86_64, but if you want a universal build that works on both 10.10 and 11.0-arm64, it will not work, becaue the old Xcode doesn't support arm64 for Mac OS

On the other hand, if you build with the new Xcode using MACOSX_DEPLOYMENT_TARGET=10.10, that also does not work on 10.10, because autoconf will discover several functions such asgetentropy which are available in the SDK, but are not available at runtime on 10.10. This will cause-Wunguarded-availability warnings in the build and an abort at runtime if the symbol is used.

@lawrence-danna-apple quite right. This has been an issue for us at Dropbox in recent years. Python's official approach for backwards compatibility is not based on setting the deployment target, which is problematic for use in client-side software. We've fixed this internally by patchingpyconfig.h, but that's not a great fix (ideally we could do something like weak-link them).

Seehttps://bugs.python.org/issue31359.

@maxbelanger@ned-deily

I opened up a new PR to deal with the deployment target issue.

#21576
#21577

Thanks for the PR. We are aware of the issues with supporting multiple levels and architectures Being able to weaklink from a build on a newer version to an older version has been desirable but it hasn't been a high priority issue. Actively supporting multiple architectures again (something we haven't had to do since the retirement of PPC :) makes this a higher priority issue and@ronaldoussoren is now working on it. I've added him to the reviewers of the new PR (#21577).

Thanks for the PR. It has been included in and superseded byGH-22855.