BLD/WHL: declare Python 3 wheels as abi3-compliant#151
BLD/WHL: declare Python 3 wheels as abi3-compliant#151neutrinoceros wants to merge 1 commit intobrandon-rhodes:masterfrom
Conversation
668ce11 to
22d3d1d
Compare
extension/wrapper.cpp
Outdated
| @@ -1,3 +1,6 @@ | |||
| // keep in sync with setup.py setup(options=...) | |||
| #define Py_LIMITED_API 0x030800f0 | |||
There was a problem hiding this comment.
note that I didn't make this conditional because (supposedly) Python 2 isn't even aware of this pre-proc def so it shouldn't choke on it
setup.py
Outdated
| provides = ['sgp4'], | ||
| ext_modules = ext_modules, | ||
| # keep in sync with wrapper.cpp Py_LIMITED_API | ||
| options={"bdist_wheel": {"py_limited_api": f"cp38"}} if sys.version_info >= (3, 8) else {}, |
There was a problem hiding this comment.
however, I can't tell for sure what setuptools does when passed this option on Python 2 so I opted for a safer approach
extension/wrapper.cpp
Outdated
| @@ -1,3 +1,8 @@ | |||
| #ifndef | |||
There was a problem hiding this comment.
Don't you need to name the macro on the #ifndef line?
There was a problem hiding this comment.
of course. My C is a bit rusty, but in hindsight I'm mostly surprised to see this compiled in the first place... anyway, fixed now !
|
@brandon-rhodes, before I dive in and try to fix the failing build on macos, I noticed that the build matrix only started at Python 3.10, not 3.8 as I assumed. Is this expected ? I can easily adapt this to be dynamic on the version that's actually used to build, which should be more flexible. |
|
also: do you intend to keep compat with 2.7 ? |
I vaguely assumed that, when @mworion’s recent PR omitted 3.8 and 3.9, it was because the tooling was no longer available on CI to get them easily built. So it was not "expected" in the sense that I could have told you ahead of time that those two would be dropped; but, "expected" in the sense that Python tools seem to be dropping old versions and abandoning legacy users at an alarming rate, so I'm not surprised when contributer PRs drop them. Also,
If it's easy to do so, then, yes, it's nice to keep any remaining 2.7 folks supported. But if it's awkward and hard to maintain, then they can probably keep happily using a previous version of |
|
thanks for your quick answers. I tried rewriting this in a version agnostic fashion an preserve compat for Python 2, so now the infrastructure is the one source of truth for which versions should be built. |
|
switching to draft as I only now realize that the build is actually failing eventhough it completes "successfully" I missed that I should have been using |
|
Thanks for double-checking, and for taking another look at this. (I will soon embark on a quick vacation, so if I don't respond immediately when you circle back, just wait and I should return soon.) |
|
I've opened the following PRs in support of this patch:
the latter 3 should represent the entire set of changes required, and are all based of the first one so CI would fail if anything isn't working. |
this may overlap partially with #150 (I'll notify the author there)
In short, this makes it so that all supported Python 3 versions (including future ones) are served with a single wheel (per platform). The tradeoff with this technique is usually that some faster C APIs may not be available, but it looks like the extension here isn't relying on any of those, so it's actually free here.