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

cffi fails to build on Python 3.13 #23

Closed
alex opened this issue Oct 16, 2023 · 19 comments · Fixed by #24
Closed

cffi fails to build on Python 3.13 #23

alex opened this issue Oct 16, 2023 · 19 comments · Fixed by #24

Comments

@alex
Copy link
Contributor

alex commented Oct 16, 2023


    Traceback (most recent call last):
      File "/home/runner/work/cryptography/cryptography/src/rust/cryptography-cffi/../../_cffi_src/build_openssl.py", line 18, in <module>
        ffi = build_ffi_for_binding(
              ^^^^^^^^^^^^^^^^^^^^^^
      File "/home/runner/work/cryptography/cryptography/src/rust/cryptography-cffi/../../_cffi_src/utils.py", line 49, in build_ffi_for_binding
        return build_ffi(
               ^^^^^^^^^^
      File "/home/runner/work/cryptography/cryptography/src/rust/cryptography-cffi/../../_cffi_src/utils.py", line 61, in build_ffi
        ffi = FFI()
              ^^^^^
      File "/tmp/pip-build-env-r0dha1wc/overlay/lib/python3.13/site-packages/cffi/api.py", line 48, in __init__
        import _cffi_backend as backend
    ImportError: /tmp/pip-build-env-r0dha1wc/overlay/lib/python3.13/site-packages/_cffi_backend.cpython-313-x86_64-linux-gnu.so: undefined symbol: _PyThreadState_UncheckedGet
@nitzmahone
Copy link
Member

nitzmahone commented Oct 16, 2023

Looks like a previously private API is now public, so we probably just need to add another conditional for the new API:

#if PY_VERSION_HEX >= 0x03060000
return _PyThreadState_UncheckedGet();
#elif defined(_Py_atomic_load_relaxed)
return (PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current);
#else
return (PyThreadState*)_PyThreadState_Current; /* assume atomic read */
#endif

(though was it purposeful that the old macros are now missing?)

@alex
Copy link
Contributor Author

alex commented Oct 16, 2023

That looks right to me.

@arigo
Copy link
Contributor

arigo commented Oct 16, 2023

I'm happy if CPython introduces this kind of backward incompatibility, as opposed to more annoying ones.

Someone with CPython 3.13 should submit and test a pull request.

@nitzmahone
Copy link
Member

Yeah, as soon as cibuildwheel and PyPA's containers have 3.13 alpha support (I'm assuming within the next few weeks), this should be trivial to fix/test now that we've got proper CI testing on PRs.

@alex
Copy link
Contributor Author

alex commented Oct 16, 2023

It looks like cffi's CI doesn't use it, but FWIW github action's actions/setup-python now supports 3.13-dev (which is how I noticed this :-))

@nitzmahone
Copy link
Member

nitzmahone commented Oct 16, 2023

Yeah, I just built it locally with pyenv to try it out- there are several other ancient private API aliases that were removed in 3.13.0a1 that also needed tending. I'll put up a WIP PR in a bit. I forgot that PyPA doesn't typically put alphas in the manylinux containers, so I might have to throw together a standalone 3.13 canary job or something, since we probably won't have the "one-liner CI additions" we need until ~ May 2024 when 3.13 hits beta.

@alex
Copy link
Contributor Author

alex commented Nov 24, 2023

It looks like with 3.13-alpha2, several functions may have been re-introduced. Now the error I see is resolved by 49127c6

@illia-v
Copy link

illia-v commented Dec 7, 2023

Thanks for fixing the issue! Is it possible to release this at the current stage of Python 3.13 development to enable testing with 3.13 for dependent packages?

@nitzmahone
Copy link
Member

We likely won't start shipping pre-releases until 3.13 is in beta, and probably not a final release until 3.13.0rc1. There are just too many issues with people accidentally installing CFFI pre-releases with pip install --pre for other projects that depend on CFFI to ship them so early. That said, there are some options:

  • Grab a wheel build artifact zip from the bottom of a passing scheduled CI run and manually install the right wheel for your environment from it (or host it in a local index cache). These are the same fully-tested wheels we'll be releasing to PyPI when the time comes.
  • Update your dependency requirements files (or augment them with a constraints file) to have Python > 3.12 build CFFI from source on our main branch, eg:

requirements.txt

# other deps
...

# your normal CFFI requirement for released Pythons
cffi>1.15 ; python_version <= "3.12"

# a new one for pre-release Pythons to install from the current HEAD of the `main` branch; this could also just be added to a separate constraint file and passed to pip via `PIP_CONSTRAINT=(constraint file)` or `-c (constraint file)` 
cffi @ https://github.com/python-cffi/cffi/archive/refs/heads/main.zip; python_version > "3.12"

@illia-v
Copy link

illia-v commented Jan 19, 2024

We likely won't start shipping pre-releases until 3.13 is in beta, and probably not a final release until 3.13.0rc1. There are just too many issues with people accidentally installing CFFI pre-releases with pip install --pre for other projects that depend on CFFI to ship them so early.

I am not sure I understand a reason for not having an actual release until CPython 3.13 is RC.

That said, there are some options

Thank you for the suggestions!

@icemac
Copy link

icemac commented Jan 26, 2024

Sadly the provided options are not enough in all cases.

In https://github.com/zopefoundation/zope.index/actions/runs/7665156609/job/20890566920?pr=46 I am installing a cffi version directly from the GitHub main branch right before installing the package which needs it.
But that package has a dependency (the package persistent) which has cffi listed under setup_requires. Sadly pip does not use the already installed cffi version but instead grabs the latest release from PyPI which then breaks during complilation.

So having an new alpha release, even if it is just an sdist, would be really helpful.

@domdfcoding
Copy link

I wish there was a way to shoehorn a modified wheel into pip's temporary build environments, but I don't think there is. It would solve the issue @icemac and I are facing.

@nitzmahone
Copy link
Member

nitzmahone commented Feb 1, 2024

I wish there was a way to shoehorn a modified wheel into pip's temporary build environments

@icemac @domdfcoding Interesting- we rely heavily on exactly that pip wheel cache hack in our own CI for roughly the same reasons, but trying it with persistent, it looks like it doesn't behave the same way for build deps (eg setup_requires or PEP517 [build-system] requires). I can't find any pip docs that describe how the wheel cache is consulted for build deps, but there's clearly something different in that case than for regular runtime deps. It's somewhat further complicated by persistent using old setup_requires instead of pyproject.toml (so it sometimes falls back to old easy_install instead of pip for build deps)... Overall though, this looks like a pip/resolvelib bug, or at least a very misleading error message when specifying a constraint for a build dep that can only be resolved by the wheel cache:

  ERROR: Cannot install cffi because these package versions have conflicting dependencies.

  The conflict is caused by:
      The user requested cffi
      The user requested (constraint) cffi>=1.17.0.dev0

That said, just specifying a constraints file that specifies the git ref that CFFI should come from worked for me for the entire chain in one shot using a clean 3.13.0a3 venv with an empty pip wheel cache, eg:

echo "cffi @ git+https://github.com/nitzmahone/cffi.git@4692fed625076f5571b81fe031777de6ad1b4b02
" > constraint.txt

PIP_CONSTRAINT=constraint.txt pip install zope.index
(everything builds successfully, wheels get cached, and the runtime-installed CFFI is the one from git)

This also has the additional benefit of not relying on any undocumented behavior for the pip wheel cache- if your constraint specifies an immutable git ref (eg a specific SHA), the created cffi wheel is cached, and any future pip invocations that also include the same constraint will use the cache for everything. It works fine with a source git ref constraint like @main too, but the generated wheel is only cached for that pip invocation if you don't specify an immutable ref, so it'll rebuild cffi every time.

Does that cover what you need?

icemac added a commit to zopefoundation/zope.index that referenced this issue Feb 2, 2024
icemac added a commit to zopefoundation/meta that referenced this issue Feb 2, 2024
@icemac
Copy link

icemac commented Feb 2, 2024

@nitzmahone Thank you for your guidance. It helped me to successfully build zope.index for Python 3.13. I highly appreciate your help.

@icemac
Copy link

icemac commented Mar 5, 2024

After some successsful runs on GitHub Actions for some other projects which have cffi in their setup-requires I get on the MacOS runners the error that a Directory not empty, see https://github.com/zopefoundation/BTrees/actions/runs/8092673962/job/22114107857?pr=200 at line 341.

This problem is probably related to zopefoundation/meta#181. The last time it helped to have a proper release on PyPI to get rid of the error. If the wheel – which is needed to satisfy setup-requires – has to be build from sources, it seems to behave differently.

@alex
Copy link
Contributor Author

alex commented May 9, 2024

With 3.13's first beta out, I think it'd be a good time to look at getting this into a release.

@nitzmahone
Copy link
Member

Yep, I've been updating the various bits to do a 3.13-supporting pre-release this week.

@AraHaan
Copy link

AraHaan commented Jul 5, 2024

This still happens to some 3.13 testing builds for aiohttp:

ImportError: /tmp/pip-build-env-hr56q2nd/overlay/lib/python3.13/site-packages/_cffi_backend.cpython-313-x86_64-linux-gnu.so: undefined symbol: _PyErr_WriteUnraisableMsg

@domdfcoding
Copy link

The fixes for Python 3.13 are only in cffi 1.17.0rc1 so far. Pip needs the --pre option (or pip_pre=true in tox.ini) to install prereleases.

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 a pull request may close this issue.

7 participants