Skip to content
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

BLD: allow building without Py_LIMITED_API #154

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

neutrinoceros
Copy link
Contributor

This is to address #152 (comment)
This changes nothing by default but it allows building pyerfa without Py_LIMITED_API for testing purposes. This is needed to enable testing astropy in free-threaded Python.

Note that I also ran the test suite locally with free-threading (using Python 3.13.0b3 + numpy 2.1.0dev0) and got no errors so this seems like a safe bet.

@@ -19,6 +19,16 @@
]


class bdist_wheel_abi3(bdist_wheel):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is inspired from https://github.com/joerick/python-abi3-package-sample/blob/main/setup.py, which is the repo that cibuildwheel's docs cites as a reference implementation.
This class is used in replacement of the static metadata from setup.cfg

@neutrinoceros neutrinoceros force-pushed the bld/limited_api_as_opt_out branch 4 times, most recently from d284eba to 815f94a Compare July 16, 2024 16:14
@neutrinoceros neutrinoceros marked this pull request as ready for review July 16, 2024 16:30
@neutrinoceros neutrinoceros force-pushed the bld/limited_api_as_opt_out branch 2 times, most recently from 1908779 to 9006a5d Compare July 17, 2024 11:13
@neutrinoceros
Copy link
Contributor Author

pypa/setuptools#4424 might supersede this.

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pypa/setuptools#4424 might supersede this.

If I understand pypa/setuptools#4424 (comment) correctly, we'll get an actual error for trying to use the limited API with no-GIL. Which means we probably should go with this PR.

I think it looks good - my main suggestion is to add a comment...

I also considered adding a CI run, but ended up thinking we probably better first make sure pyerfa is actually OK with threads. From the stuff that we added ourselves, my main worry is about the leap second list - which should almost always be identical between threads, but perhaps is not guaranteed to be so.

@@ -10,7 +10,6 @@
*/

#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
#define Py_LIMITED_API 0x030900f0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to leave a comment here that we use the limited API but by default set it insetup.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't hurt, I'll add it now

@neutrinoceros
Copy link
Contributor Author

If I understand pypa/setuptools#4424 (comment) correctly, we'll get an actual error for trying to use the limited API with no-GIL. Which means we probably should go with this PR.

Actually we already get one, it's just no very clear:

Example
❯ uv pip install .
Resolved 2 packages in 724ms
error: Failed to prepare distributions
  Caused by: Failed to fetch wheel: pyerfa @ file:///Users/clm/dev/astropy-project/liberfa/pyerfa
  Caused by: Failed to build: `pyerfa @ file:///Users/clm/dev/astropy-project/liberfa/pyerfa`
  Caused by: Build backend failed to build wheel through `build_wheel()` with exit status: 1
--- stdout:
running bdist_wheel
running build
running build_py
creating build/lib.macosx-14.6-arm64-cpython-313t
creating build/lib.macosx-14.6-arm64-cpython-313t/erfa
copying erfa/version.py -> build/lib.macosx-14.6-arm64-cpython-313t/erfa
copying erfa/_version.py -> build/lib.macosx-14.6-arm64-cpython-313t/erfa
copying erfa/__init__.py -> build/lib.macosx-14.6-arm64-cpython-313t/erfa
copying erfa/core.py -> build/lib.macosx-14.6-arm64-cpython-313t/erfa
copying erfa/helpers.py -> build/lib.macosx-14.6-arm64-cpython-313t/erfa
creating build/lib.macosx-14.6-arm64-cpython-313t/erfa/tests
copying erfa/tests/__init__.py -> build/lib.macosx-14.6-arm64-cpython-313t/erfa/tests
copying erfa/tests/test_ufunc.py -> build/lib.macosx-14.6-arm64-cpython-313t/erfa/tests
copying erfa/tests/test_erfa.py -> build/lib.macosx-14.6-arm64-cpython-313t/erfa/tests
running build_ext
building 'erfa.ufunc' extension
creating build/temp.macosx-14.6-arm64-cpython-313t
creating build/temp.macosx-14.6-arm64-cpython-313t/erfa
creating build/temp.macosx-14.6-arm64-cpython-313t/liberfa
creating build/temp.macosx-14.6-arm64-cpython-313t/liberfa/erfa
creating build/temp.macosx-14.6-arm64-cpython-313t/liberfa/erfa/src
clang -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall -DHAVE_CONFIG_H=1 -DPy_LIMITED_API=0x30900f0 -Iliberfa/erfa/src -Iliberfa/erfa -I/Users/clm/Library/Caches/uv/builds-v0/.tmpMCztxb/lib/python3.13t/site-packages/numpy/_core/include -I/Users/clm/Library/Caches/uv/builds-v0/.tmpMCztxb/include -I/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t -c erfa/ufunc.c -o build/temp.macosx-14.6-arm64-cpython-313t/erfa/ufunc.o
--- stderr:
WARNING setuptools_scm.pyproject_reading toml section missing 'pyproject.toml does not contain a tool.setuptools_scm section'
Traceback (most recent call last):
  File "/Users/clm/Library/Caches/uv/builds-v0/.tmpMCztxb/lib/python3.13t/site-packages/setuptools_scm/_integration/pyproject_reading.py", line 36, in read_pyproject
    section = defn.get("tool", {})[tool_name]
              ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
KeyError: 'setuptools_scm'
In file included from erfa/ufunc.c:13:
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/Python.h:51:4: error: "The limited API is not currently supported in the free-threaded build"
#  error "The limited API is not currently supported in the free-threaded build"
   ^
In file included from erfa/ufunc.c:13:
In file included from /Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/Python.h:68:
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:213:5: error: unknown type name 'PyMutex'
    PyMutex ob_mutex;           // per-object lock
    ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:314:22: error: call to undeclared function '_Py_atomic_load_uint32_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    uint32_t local = _Py_atomic_load_uint32_relaxed(&ob->ob_ref_local);
                     ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:318:25: error: call to undeclared function '_Py_atomic_load_ssize_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&ob->ob_ref_shared);
                        ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:352:13: error: call to undeclared function '_Py_atomic_load_uint32_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    return (_Py_atomic_load_uint32_relaxed(&op->ob_ref_local) ==
            ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:352:63: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
    return (_Py_atomic_load_uint32_relaxed(&op->ob_ref_local) ==
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:390:9: error: call to undeclared function '_Py_IsOwnedByCurrentThread'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    if (_Py_IsOwnedByCurrentThread(ob)) {
        ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:430:5: error: call to undeclared function '_Py_atomic_store_ssize_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    _Py_atomic_store_ssize_relaxed(&ob->ob_size, size);
    ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:804:22: error: call to undeclared function '_Py_atomic_load_uint32_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local);
                     ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:810:9: error: call to undeclared function '_Py_IsOwnedByCurrentThread'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    if (_Py_IsOwnedByCurrentThread(op)) {
        ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:811:9: error: call to undeclared function '_Py_atomic_store_uint32_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, new_local);
        ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:814:9: error: call to undeclared function '_Py_atomic_add_ssize'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        _Py_atomic_add_ssize(&op->ob_ref_shared, (1 << _Py_REF_SHARED_SHIFT));
        ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:898:22: error: call to undeclared function '_Py_atomic_load_uint32_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local);
                     ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:903:9: error: call to undeclared function '_Py_IsOwnedByCurrentThread'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    if (_Py_IsOwnedByCurrentThread(op)) {
        ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:905:9: error: call to undeclared function '_Py_atomic_store_uint32_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local);
        ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:907:13: error: call to undeclared function '_Py_MergeZeroLocalRefcount'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
            _Py_MergeZeroLocalRefcount(op);
            ^
/Users/clm/.pyenv/versions/3.13t-dev/include/python3.13t/object.h:911:9: error: call to undeclared function '_Py_DecRefShared'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        _Py_DecRefShared(op);
        ^
erfa/ufunc.c:32:1: warning: typedef requires a name [-Wmissing-declarations]
typedef struct _typeobject {
^~~~~~~
2 warnings and 16 errors generated.
error: command '/usr/bin/clang' failed with exit code 1
---

@neutrinoceros
Copy link
Contributor Author

neutrinoceros commented Aug 13, 2024

Whoops, looks like CI has been broken for a couple weeks. Let me see what I can do.

edit: resolved in #155 and #156

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, looks all OK now! I'll merge this, since it doesn't depend on the others.

@mhvk mhvk merged commit e17a680 into liberfa:main Aug 13, 2024
14 of 24 checks passed
@neutrinoceros neutrinoceros deleted the bld/limited_api_as_opt_out branch August 13, 2024 15:14
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