-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
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
building extensions as builtins is broken in 3.7 #76413
Comments
building extensions statically in linking these into the python binary is currently broken on 3.7. I'm attaching the change that worked for me in 3.7alpha2, but doesn't work anymore with alpha3. Currently investigating. When building extensions as builtins, these should benefit from the knowledge about the core interpreter. x86_64-linux-gnu-gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -g -fdebug-prefix-map=/home/packages/python/3.7/python3.7-3.7.0~a3=. -fstack-protector-strong -Wformat -Werror=format-security -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -IObjects -IInclude -IPython -I. -I../Include -Wdate-time -D_FORTIFY_SOURCE=2 -DPy_BUILD_CORE -c ../Modules/_elementtree.c -o Modules/_elementtree.o |
that patch lets the build succeed. two additional includes and a little bit of shuffling |
Can someone provide a pull request and a test for this prior to beta 1? |
I have put together a PR with Matthias' patch and a test. |
It seems that Microsoft compiler does not handle well the circular dependencies between Include/pystate.h and Include/internal/pystate.h so I have included guards for Windows in the new include for Include/pystate.h and deactivated the test in that case. Some better ideas on how to handle that case? |
Thanks Pablo for taking up the challenge! I just took a quick look at the current state of PR 5256 and I noted numerous issues that need to be addressed with the test. I don't have time at the moment to review the non-test changes. It would be good if another core developer could review it. Since it's not likely this will get resolved prior to the imminent 3.7.0b1 code freeze, I'm going to reluctantly defer this to b2 (unless someone else gets to it first). |
The compilation failure is a consequence of the changes made in bpo-30860. Simply adding '#include "internal/pystate.h"' in Modules/_elementtree.c fixes the compilation although this is not the correct fix. The modules configured in Modules/Setup keep being built with -DPy_BUILD_CORE while the refactoring done in bpo-30860 imposes new constraints on the way headers are handled for modules accessing the Py_BUILD_CORE API. Most modules configured in Modules/Setup do not use this API and none of the commented out modules in this file (normally built by setup.py [1]) does. PR 6489 fixes this by introducing yet another CFLAGS named PY_NO_CORE_CFLAGS to only use -DPy_BUILD_CORE with Setup modules that use the Py_BUILD_CORE API. [1] the _xxsubinterpreters module is the only one that sets -DPy_BUILD_CORE explicitly in setup.py |
Thanks, Xavier, for your analysis and your PR! We definitely meed to get this resolved before 3.7.0b4. Given the complexity and potential impact on this area and that we are approaching the final beta, I think we need a few pairs of eyes to review it. @Doko, does the PR work for you? @eric.snow and @ncoghlan, could you please take a look, too? Thanks! |
Even when statically linked, extension modules should NOT be getting built with |
Ah, oops - I commented before fully catching up on everything else, and now I see that Xavier already made exactly that point just above. I'll go review the PR now :) |
To answer my own comment in PR 6489 about the interference of the pyconfig.h macros with the build of std lib extension modules, the py_config_macros.py script prints the list of pyconfig.h.in macros that are being used by the standard library extension modules that do not access the interpreter internals. The script prints: errnomodule.c: const So the conclusion is that this is not a problem. |
I also resolved my own puzzlement from the PR comments regarding why _weakrefs.c was fine with being built as a regular extension module: it's because the C level access API for __weakrefs__ is published through the regular C API, not as a core-only internal API. So it wouldn't work if you tried to build it against the stable ABI (since it needs access to object internals), but it's fine without Py_BUILD_CORE. |
Xavier's changes should fix the reported compile error, while keeping the increased isolation of the interpreter core from the optional extension modules. If the latter change causes a detectable performance regression, then I think that would make more sense as a separate performance issue. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: