GeezerVison is showing github.com — Mimalloc header is not installed · Issue #116984 · python/cpython
Mimalloc header is not installed #116984DescriptionBug report
Bug description:
Mimalloc is introduced in #109914.
This will cause a header not found when building any extension that includes a pycore_*.h header as:
[build] In file included from /Users/yyc/repo/py/install/include/python3.13d/internal/pycore_long.h:13:
[build] In file included from /Users/yyc/repo/py/install/include/python3.13d/internal/pycore_runtime.h:17:
[build] In file included from /Users/yyc/repo/py/install/include/python3.13d/internal/pycore_interp.h:30:
[build] /Users/yyc/repo/py/install/include/python3.13d/internal/pycore_mimalloc.h:39:10: fatal error: 'mimalloc.h' file not found
[build] #include "mimalloc.h"
[build] ^~~~~~~~~~~~
[build] 1 error generated.
See following PR for detail.
CPython versions tested on:
3.13, CPython main branch
Operating systems tested on:
Linux, macOS
Linked PRs
Activitypitrou commented on Mar 19, 2024 Slightly unrelated, but I see that mimalloc types are embedded directly in structures such as PyInterpreterState or PyThreadState.
Unless mimalloc has a stable ABI, does it risk breaking if an extension module or an application embedding Python uses its own different version of mimalloc?
It's also weird that a third-party library is included from CPython headers. Perhaps it would be better to hide this behind opaque pointers.
@encukou @colesbury oraluben commented on Mar 19, 2024 ContributorAuthor More actions I walked through the discussions in #109914 and seems this issue has been already discussed (at least mentioned, but forgotten later? Or not correctly installed after moved to Internal/? #109914 (review), #109914 (comment), #109914 (comment)). It's somehow intended that this header is not installed, but since cpython decides to install pycore_*.h headers, the mimalloc headers need to be installed as well (otherwise the core headers are all broken).
I also see some related about this:
Unless mimalloc has a stable ABI, does it risk breaking if an extension module or an application embedding Python uses its own different version of mimalloc?
If I'm not mistaken, no symbol is exported (#109914 (comment)), and the reason mimalloc headers are included is that there's extra patches on top of upstream. But the longterm plan is to upstream those (I cannot find the link to this but I believe saw Sam mentioned it somewhere).
Hope it helped @pitrou pitrou commented on Mar 19, 2024
If I'm not mistaken, no symbol is exported
Sure, but:
mimalloc.h is included, and it could come from whatever install of mimalloc is used by third-party extension code, not necessarily the mimalloc used for building CPython- the binary layout of
PyInterpreterState and PyThreadState depends on the size of mimalloc types, which may vary from one mimalloc version to another.
colesbury commented on Mar 19, 2024 @pitrou:
mimalloc.h is only included in internal headers (pycore_xxx.h). Not in any of the public APIs (Include or Include/cpython).PyInterpreterState is opaque. The mimalloc structs are included in _PyThreadStateImpl, not PyThreadState. _PyThreadStateImpl is not in any of the public headers. pitrou commented on Mar 19, 2024 @colesbury This issue shows that "internal" headers are included by third-party code. Presumably, PyInterpreterState and PyThreadState are also consumed by third-party code. colesbury commented on Mar 19, 2024 Yes, people sometimes include our internal headers. People also sometimes copy-paste our internal headers. We don't make attempts to avoid language level name conflicts in these cases. pitrou commented on Mar 19, 2024 I certainly don't have a horse in this race, but I find it weird to on the one hand fix this issue by installing the mimalloc headers, and on the other hand to not care about the potential consequences of including said headers in third-party code. oraluben commented on Mar 20, 2024 ContributorAuthor More actions I'd also prefer to not need to add Internal/mimalloc as extra include path, even after the headers are installed, if that's possible. colesbury commented on May 8, 2024 Reopening this for the Internal/mimalloc include path issue. added 3 commits that reference this issue on May 8, 2024gh-116984: Make mimalloc header includes relative to the current file (… oraluben commented on May 11, 2024 ContributorAuthor More actions Closing for the PRs from @colesbury have been landed, thanks for make this more friendly!
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|