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

Use a separate build directory for free-threading #260

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Use a separate build directory for free-threading #260

merged 1 commit into from
Jun 26, 2024

Conversation

brenns10
Copy link
Contributor

Hello!

I work on the drgn debugger, and with the recently added 3.13t builds in the manylinux docker, which enable the --disable-gil option, we started getting some build failures when building wheels. It turned out that the standard and free-threading builds of 3.13 both use the same build_temp and build_lib directories.

Drgn has a bit of a customized build system which allows incremental builds within the build_temp directory, so the object files from 3.13 were getting reused for the free-threaded build, but the ABI is not compatible anymore. The result was an extension that segfaulted immediately on import.

It seems that a standard distutils extension doesn't have incremental build support, so maybe overlapping build_temp and build_lib directories isn't a huge problem. But looking through the code, it seems like distutils tries to avoid overlapping these build_temp and build_lib directories where the compiler could get confused or produce incorrect results. So I thought this might be a suitable fix for distutils itself, rather than just in drgn's build system.

Thanks!

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Comment on lines +28 to +29
if get_config_var('Py_GIL_DISABLED'):
plat_spec += 't'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that by adding this test (without the fix), the tests now fail, and tests pass with the fix. Nice.

@jaraco
Copy link
Member

jaraco commented Jun 26, 2024

I'm ignoring the diffcov failure - it only checks coverage on a single environment, so until we have a test suite that implements coverage across all supported environments and we have CI environments running with Python free-threading, these lines are going to be uncovered.

@jaraco jaraco merged commit e57f6f1 into pypa:main Jun 26, 2024
14 of 16 checks passed
@brenns10
Copy link
Contributor Author

Thank you for the quick review!

jaraco added a commit that referenced this pull request Jun 28, 2024
Use a separate build directory for free-threading
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.

2 participants