Skip to content

gh-135645: Added supports_isolated_interpreters to sys.implementation #135667

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Jun 18, 2025

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@sobolevn
Copy link
Member Author

Thank you, @vstinner! I would like to get a review from:

  • @ericsnowcurrently as a code-owner
  • and from @hugovk as the RM, because this is techincally a feature, and now it is a feature freeze

@sobolevn sobolevn requested a review from hugovk June 18, 2025 14:55
@sobolevn sobolevn added the needs backport to 3.14 bugs and security fixes label Jun 18, 2025
@vstinner
Copy link
Member

and from @hugovk as the RM, because this is techincally a feature, and now it is a feature freeze

PEP 734 got an exception to be added to Python 3.14 after the feature freeze, no?

@bedevere-app
Copy link

bedevere-app bot commented Jun 18, 2025

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

sobolevn and others added 2 commits June 18, 2025 22:18
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@sobolevn sobolevn added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 18, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 028b498 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135667%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 18, 2025
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

@sobolevn sobolevn added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 19, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 5fe139c 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135667%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 19, 2025
@sobolevn
Copy link
Member Author

This test fails on iOS: https://buildbot.python.org/#/builders/1382/builds/539

def test_implementation(self):
for key, value in self.key('implementation').items():
with self.subTest(part=key):
if key == 'version':
self.assertEqual(len(value), len(sys.implementation.version))
for part_name, part_value in value.items():
self.assertEqual(getattr(sys.implementation.version, part_name), part_value)
else:
self.assertEqual(getattr(sys.implementation, key), value)

======================================================================
FAIL: test_implementation (test.test_build_details.CPythonBuildDetailsTests.test_implementation) (part='supports_isolated_interpreters')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/19518409-EE82-48CA-B217-EBE32ABD5E73/data/Containers/Bundle/Application/DBA97A4E-0ECF-4103-AA68-4BEF211EB6BE/iOSTestbed.app/python/lib/python3.15/test/test_build_details.py", line 84, in test_implementation
    self.assertEqual(getattr(sys.implementation, key), value)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: False != True

@freakboy3742 can you please help me with understanding why? I don't know enough details about our iOS build process :(

@sobolevn
Copy link
Member Author

sobolevn commented Jun 19, 2025

Looks like that we generate build-details.json via generate-build-details.py on the emulator first (regular linux), then use this file on ios 🤔

@freakboy3742
Copy link
Contributor

@sobolevn It looks like this is a cross-compile issue. The build-details.json file is being seeded with the build python (macOS), but with _PYTHON_SYSCONFIGDATA_PATH defined to point at the host Python configuration (iOS) so that iOS-specific details are surfaced. That won't help with the contents of sys.implementation though, as none of those values are modified.

So - the fix will be some sort of override to pass cross-platform details for sys can be passed into the build. I'm not sure what @FFY00 had in mind for this - it's not obvious (to me, anyway) what the intended use of generate-build-details.py would be for setting properties that aren't overwritten by sysconfigdata - and in particular, the contents of sys.implementation. The handling of MULTIARCH is the only comparable value I can see - maybe analogous handling is needed for supports_isolated_interpreters?

That said - in this specific instance, I'm not clear why supports_isolated_interpreters is being set to False for iOS. iOS provides the _interpreters module, and runs (and passes) the test_interpreters and test_concurrent_futures.test_interpreter_pool tests... is there some other limitation I'm missing?

@freakboy3742
Copy link
Contributor

So - the fix will be some sort of override to pass cross-platform details for sys can be passed into the build. I'm not sure what @FFY00 had in mind for this - it's not obvious (to me, anyway) what the intended use of generate-build-details.py would be for setting properties that aren't overwritten by sysconfigdata - and in particular, the contents of sys.implementation. The handling of MULTIARCH is the only comparable value I can see - maybe analogous handling is needed for supports_isolated_interpreters?

@sobolevn Also - flagging that although this isn't an issue for iOS, it will be an issue for Emscripten and WASI. There's no buildbot for Emscripten yet (hopefully that will land early next week), but the WASI buildbot appears to be flagging the same problem, for exactly the same reason.

@sobolevn
Copy link
Member Author

!buildbot wasi

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 9e3fa38 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135667%2Fmerge

The command will test the builders whose names match following regular expression: wasi

The builders matched are:

  • wasm32-wasi Non-Debug PR
  • wasm32-wasi PR
  • wasm32 WASI 8Core PR

@sobolevn
Copy link
Member Author

sobolevn commented Jun 20, 2025

but the WASI buildbot appears to be flagging the same problem, for exactly the same reason.

🤔
@freakboy3742 looks like test_sys passes on all WASI buildbots.
test_capi failures are unrelated: #135721

@freakboy3742
Copy link
Contributor

but the WASI buildbot appears to be flagging the same problem, for exactly the same reason.

🤔 @freakboy3742 looks like test_sys passes on all WASI buildbots. test_capi failures are unrelated: #135721

Ah - the entire build-details.json test suite is excluded (and it looks like build-details.json isn't generate for Emscripten at least.) I guess that's more of an indication that those builds haven't got PEP 739 support yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants