unix: build test extensions as shared libraries; enable _testcapi#1025
Open
indygreg wants to merge 2 commits intogps-depot-larger-runnersfrom
Open
unix: build test extensions as shared libraries; enable _testcapi#1025indygreg wants to merge 2 commits intogps-depot-larger-runnersfrom
indygreg wants to merge 2 commits intogps-depot-larger-runnersfrom
Conversation
indygreg
force-pushed
the
gps-test-extension-shared-libraries
branch
8 times, most recently
from
9cd0276 to
1c77491
Compare
indygreg
force-pushed
the
gps-test-extension-shared-libraries
branch
from
1c77491 to
895a59c
Compare
Way back when we only supported statically linked extension modules. And we had to disable `_testcapi` because it wouldn't build statically. Now that we support building extension modules as shared libraries, it makes sense for extension modules in support of tests to not be part of libpython and to be shared libraries. This commit makes that change. And since `_testcapi` is buildable as a shared library, we enable it. In doing so, we eliminate the largest single source of test failures.
indygreg
force-pushed
the
gps-test-extension-shared-libraries
branch
from
895a59c to
15c329a
Compare
Collaborator
Author
|
I'm going to continue iterating on this to get musl shared builds to behave better. |
indygreg
force-pushed
the
gps-test-extension-shared-libraries
branch
from
fb5f7c3 to
77b14cd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Way back when we only supported statically linked extension modules. And we had to disable
_testcapiand_testlimitedcapibecause it wouldn't build statically.Now that we support building extension modules as shared libraries, we can enable
_testcapiand_testlimitedcapias shared libraries.This commit makes that change.
By making this change, we eliminate the largest single source of test failures. But this impact is not captured in code or CI since we currently don't run the stdlib tests in CI.
Enabling the new extensions allows tests - including PGO training tests - to exercise new code paths. This resulted in a new segfault appearing in test_bytes. Why, I'm not sure. (My best guess is it has something to do with calling variadic functions via ctypes.) This particular crash was likely a latent issue. We ignore this crashing test during PGO training as a workaround.
In order to get the extensions enabled on musl shared builds - but not static - we had to introduce some new logic to conditionally enable a shared extension on supported targets while automatically disabling on static targets. There may be a cleaner way to express this semantic intent. But the implemented solution does work.