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

gh-106320: Remove private pylifecycle.h functions #106400

Merged
merged 2 commits into from
Jul 4, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jul 4, 2023

Remove private pylifecycle.h functions: move them to the internal C API ( pycore_atexit.h, pycore_pylifecycle.h and pycore_signal.h). No longer export most of these functions.

Move _testcapi.test_atexit() to _testinternalcapi.

Remove private pylifecycle.h functions: move them to the internal C
API ( pycore_atexit.h, pycore_pylifecycle.h and pycore_signal.h). No
longer export most of these functions.

Move _testcapi.test_atexit() to _testinternalcapi.
@gvanrossum
Copy link
Member

This broke macOS build for me. In Python/specialize.c there's a use of _PyOS_URandomNonblock but the file doesn't #include "pycore_pylifecycle.h". How did this pass CI? Anyway, I'll fix it.

gvanrossum added a commit that referenced this pull request Jul 4, 2023
…le.h (#106434)

Compilation of Python/specialize.c was broken on macOS for me by gh-106400.
@vstinner vstinner deleted the pycore_pylifecycle branch July 5, 2023 06:17
@vstinner
Copy link
Member Author

vstinner commented Jul 5, 2023

It seems like the macOS CI uses gcc. I suppose that you use clang. And clang is more strict about undefined functions. Maybe the CI should use clang?

cc @corona10

1 similar comment
@vstinner
Copy link
Member Author

vstinner commented Jul 5, 2023

It seems like the macOS CI uses gcc. I suppose that you use clang. And clang is more strict about undefined functions. Maybe the CI should use clang?

cc @corona10

@corona10
Copy link
Member

corona10 commented Jul 5, 2023

@vstinner
Basically, the default gcc of macOS is clang.
We need to check what's happening on GA for this use.
If you meant the compiler option, it would be worth doing it.
(Change the compiler to the clang and use clang options)

cc @ned-deily

$> gcc --version
Apple clang version 14.0.0 (clang-1400.0.29.202)

@vstinner
Copy link
Member Author

vstinner commented Jul 5, 2023

$> gcc --version
Apple clang version 14.0.0 (clang-1400.0.29.202)

Aha, maybe @gvanrossum LLVM is more recent. LLVM clang 15 made this change: https://releases.llvm.org/15.0.0/tools/clang/docs/ReleaseNotes.html

  • The -Wimplicit-function-declaration warning diagnostic now defaults to an error in C99 and later

@dgrisby
Copy link
Contributor

dgrisby commented Jul 28, 2023

omniORBpy (which I maintain, https://www.omniorb.net/ ) is using _Py_IsFinalizing to avoid crashes in threads created from C++ during interpreter shutdown, as documented at https://docs.python.org/3.13/c-api/init.html#c.PyEval_RestoreThread

Is there an alternative?

@vstinner
Copy link
Member Author

Would you mind to open a new separated issue to request a public function replacing the removed private function? Thanks in advance.

@dgrisby
Copy link
Contributor

dgrisby commented Aug 16, 2023

Would you mind to open a new separated issue to request a public function replacing the removed private function? Thanks in advance.

Here it is: #108014

Thanks.

befeleme added a commit to befeleme/uvloop that referenced this pull request Apr 30, 2024
private _Py_RestoreSignals() has been moved to CPython internals as of Python 3.13
See: python/cpython#106400
befeleme added a commit to befeleme/uvloop that referenced this pull request Apr 30, 2024
private _Py_RestoreSignals() has been moved to CPython internals as of Python 3.13
See: python/cpython#106400
befeleme added a commit to befeleme/uvloop that referenced this pull request Apr 30, 2024
private _Py_RestoreSignals() has been moved to CPython internals as of Python 3.13
See: python/cpython#106400
befeleme added a commit to befeleme/uvloop that referenced this pull request Apr 30, 2024
private _Py_RestoreSignals() has been moved to CPython internals as of Python 3.13
See: python/cpython#106400
Its implementation has been the same in all supported by uvloop Pythons
(3.8+), so the inlining was not conditionalized.
befeleme added a commit to befeleme/uvloop that referenced this pull request Apr 30, 2024
private _Py_RestoreSignals() has been moved to CPython internals as of Python 3.13
See: python/cpython#106400
Its implementation has been the same in all supported by uvloop Pythons
(3.8+), so the inlining was not conditionalized.
edgarrmondragon pushed a commit to edgarrmondragon/uvloop that referenced this pull request Jul 5, 2024
private _Py_RestoreSignals() has been moved to CPython internals as of Python 3.13
See: python/cpython#106400
Its implementation has been the same in all supported by uvloop Pythons
(3.8+), so the inlining was not conditionalized.
fantix pushed a commit to MagicStack/uvloop that referenced this pull request Aug 15, 2024
private _Py_RestoreSignals() has been moved to CPython internals as of Python 3.13
See: python/cpython#106400
Its implementation has been the same in all supported by uvloop Pythons
(3.8+), so the inlining was not conditionalized.
ludwigschwardt added a commit to ludwigschwardt/python-gnureadline that referenced this pull request Oct 17, 2024
Python 3.13 moved some private C API functions to internal header files.
See python/cpython#106320 for a general discussion on this (glad to see
that gnureadline was at least listed as an affected PyPI package :-) ).

These private functions affect us:
  - _Py_SetLocaleFromEnv -> pycore_pylifecycle.h
    (see python/cpython#106400)
  - _PyArg_CheckPositional, _PyArg_BadArgument -> pycore_modsupport.h
    (see python/cpython#110964)

Since we can't include these anymore, patch the relevant declarations into
the module code. The alternative (add the internal headers to this package)
is much messier, as we would have to pull in most of those headers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants