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

Implement PEP 489 - Multi-phase extension module initialization #2245

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

messense
Copy link
Member

@messense messense commented Mar 23, 2022

https://peps.python.org/pep-0489/

This is very much WIP, opening for discussion, previously: #1982

src/impl_/pymodule.rs Outdated Show resolved Hide resolved
@messense messense added the CI-no-fail-fast If one job fails, allow the rest to keep testing label Mar 25, 2022
@messense messense closed this Mar 25, 2022
@messense messense reopened this Mar 25, 2022
@messense
Copy link
Member Author

Looks like PyPy doesn't support PEP 489.

  = note: /usr/bin/ld: /home/runner/work/pyo3/pyo3/target/x86_64-unknown-linux-gnu/debug/deps/libpyo3-5c3820f48b335551.rlib(pyo3-5c3820f48b335551.pyo3.839e673f-cgu.2.rcgu.o): in function `pyo3_ffi::modsupport::PyModule_FromDefAndSpec':
          /home/runner/work/pyo3/pyo3/pyo3-ffi/src/modsupport.rs:146: undefined reference to `PyModule_FromDefAndSpec2'
          /usr/bin/ld: /home/runner/work/pyo3/pyo3/target/x86_64-unknown-linux-gnu/debug/deps/libpyo3-5c3820f48b335551.rlib(pyo3-5c3820f48b335551.pyo3.839e673f-cgu.5.rcgu.o): in function `pyo3::impl_::pymodule::ModuleDef::make_module':
          /home/runner/work/pyo3/pyo3/src/impl_/pymodule.rs:80: undefined reference to `PyModule_ExecDef'

@davidhewitt
Copy link
Member

:(

I guess we have to make a choice between making a new API that's not supported on PyPy, or just using different behaviour on CPython and PyPy. I think a separate API is probably better?

@messense
Copy link
Member Author

messense commented Mar 25, 2022

Maybe we should feature-gate it? See also cython/cython#2343:

Require users to explicitly enable this feature with a compiler directive, as Cython cannot safely determine that a module supports this (there might be global state in external C libraries or C function callbacks), and there will be a visible performance impact.

@davidhewitt
Copy link
Member

davidhewitt commented Mar 25, 2022

It would be interesting to explore. There's also still a possibility to have a #[pymodule(pep489)] or similar.

That Cython thread is really interesting in general.

@adamreichold
Copy link
Member

I guess we have to make a choice between making a new API that's not supported on PyPy, or just using different behaviour on CPython and PyPy. I think a separate API is probably better?

But isn't "same interface but slightly incompatible behaviour" (w.r.t. CPython) already something that PyPy users expect? I fear that having CPython-only API will only end up with extensions having feature-gated PyPy-compatible code paths any way, so I would prefer it if the thankless task of trying to paper this over was centralized in PyO3 (were we could also move the ecosystem over if/when PyPy adds support for this.)

@messense messense removed the CI-no-fail-fast If one job fails, allow the rest to keep testing label Mar 25, 2022
@davidhewitt
Copy link
Member

I think it might depend how different the PyPy behaviour is. I do agree if we can solve the problem for users transparently, that would be nicest.

@messense messense force-pushed the pep-489 branch 2 times, most recently from 18bc0ce to e842c78 Compare March 30, 2022 03:17
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.

4 participants