gh-102536: Added `_architecture` field to `sys.implementation`

[rruuaanng]

cc @zooba

• Issue: gh-102536

[rruuaanng]

I'd like to have tests as well please.

Okay, I have make change.

[rruuaanng]

Sorry but this requires more investigation. The value being added is not the correct one (sys.platform is already exposed so it does not make sense to have sys.implementation._architecture being the same IMO).

According tozooba description, it seems that adding this attribute can make the information of sys.implementation more comprehensive (although I personally don't think this is necessary. In fact, all fields in the implementation have corresponding Python APIs for obtaining.

[picnixz]

Yes but the attribute being added is not the one that you added. You need to get the one stored in python.props (but I don't know how to retrieve it).

[rruuaanng]

Yes but the attribute being added is not the one that you added. You need to get the one stored in python.props (but I don't know how to retrieve it).

Hmmm, I know how to get the architecture information in capi, but it seems that this is only supported on unix-like.

[picnixz]

On the issue:

sys.implementation._architecture and make it the value of $(ArchName)? from python.props

So we need to know how to recover $(ArchName). This is what we want to get and python.props is something that is Windows-only I think. You can have a look at the script: https://github.com/python/cpython/blob/abe5f799e6ce1d177f79554f1b84d348b6141045/PC/layout/support/props.py#L11.

I'll leave it to @zooba to explain exactly what he had in mind. But AFAICT, the value of sys.implementation._architecture should be the same as the one of $(ArchName).

[rruuaanng]

On the issue:

sys.implementation._architecture and make it the value of $(ArchName)? from python.props

So we need to know how to recover $(ArchName). This is what we want to get and python.props is something that is Windows-only I think. You can have a look at the script: https://github.com/python/cpython/blob/abe5f799e6ce1d177f79554f1b84d348b6141045/PC/layout/support/props.py#L11.

I'll leave it to @zooba to explain exactly what he had in mind. But AFAICT, the value of sys.implementation._architecture should be the same as the one of $(ArchName).

He probably needs a descriptive field for the current system architecture in the implementation (since ArchName is limited to this purpose). I think that's what he had in mind, so my PR applies to all targets that support the uname system call. This way, they can obtain information about their current platform through the implementation.

[zooba]

So we need to know how to recover $(ArchName). This is what we want to get and python.props is something that is Windows-only I think.

Yeah, this is compile-time information that isn't accessible at runtime (remembering that Windows supports CPU emulation, so an x64 machine can run a 32-bit build, and an ARM64 machine can run an x86 or x64 build). platform.machine() will tell you what the current hardware actually is, but $(ArchName) will tell you what your running Python was compiled for. The best we have right now for this is sys.winver.

As for how to get it at compile time, look in PCbuild/pythoncore.vcxproj for MS_DLL_ID, which is how we pass the value of sys.winver in. We should be able to use something simpler here (winver gets passed around all over the place, unfortunately), and just have sysmodule.c see if the preprocessor value is defined and if so, add it to the implementation struct.

[rruuaanng]

So we need to know how to recover $(ArchName). This is what we want to get and python.props is something that is Windows-only I think.

Yeah, this is compile-time information that isn't accessible at runtime (remembering that Windows supports CPU emulation, so an x64 machine can run a 32-bit build, and an ARM64 machine can run an x86 or x64 build). platform.machine() will tell you what the current hardware actually is, but $(ArchName) will tell you what your running Python was compiled for. The best we have right now for this is sys.winver.

As for how to get it at compile time, look in PCbuild/pythoncore.vcxproj for MS_DLL_ID, which is how we pass the value of sys.winver in. We should be able to use something simpler here (winver gets passed around all over the place, unfortunately), and just have sysmodule.c see if the preprocessor value is defined and if so, add it to the implementation struct.

In other words, what is needed is the MS_DLL_ID value, rather than the runtime machine architecture (platform.machine()). cc @zooba Need this

>>> import sys;sys.winver
'3.14'

Don't this

>>> import platform;platform.machine()
'AMD64'

[rruuaanng]

I found this in python.prop

    <ArchName Condition="'$(ArchName)' == '' and $(Platform) == 'x64'">amd64</ArchName>
    <ArchName Condition="'$(ArchName)' == '' and $(Platform) == 'ARM'">arm32</ArchName>
    <ArchName Condition="'$(ArchName)' == '' and $(Platform) == 'ARM64'">arm64</ArchName>
    <ArchName Condition="'$(ArchName)' == ''">win32</ArchName>

If what you mentioned by ArchName refers to this, then my current PR would not require any modifications to be merged. Because it is used for obtaining the runtime architecture(Sorry, I didn't really get what you guys are after from our conversation). I agree with picnixz's description, sys.implementation._architecture should be the same as the one of $(ArchName), Or perhapsI can change it to a tuple (MS_DLL_ID, ArchName). It's will

>> sys.implementation._architecture
('3.14', 'AMD64')
Equal to
>> print((sys.winver, platform.machine()))
('3.14', 'AMD64')

[rruuaanng]

I have another question to confirm (it seems to have been mentioned in the previous discussion, but I need to confirm it again), whether the _architecture only supports window?

[erlend-aasland]

The discussion on the issue did not conclude about which API to introduce; please discuss the API first, and then propose the change. I propose to close this premature PR until we know how to introduce this feature.

[rruuaanng]

Maybe this issue has already been discussed and is just waiting for a decision on whether to merge it because it has completed all the changes and testing.

[zooba]

Apart from the unnecessary V in VARCH_NAME, yeah, that's what I had in mind.

[ericsnowcurrently]

In case it got lost in the shuffle, I want to reiterate that sys.implementation is not the right place for this.

[zooba]

Does it belong in sysconfig then? As a compile-time option

[zooba]

Even if we make it available in sysconfig, it still has to be made available in a native module for sysconfig, as we don't have any other places to store it on Windows. There's no makefile or similar bundled with the runtime.

[rruuaanng]

Your idea is that if he is not in sysconfig, then we need it and he also provides it in sysconfig.

Edit

If I understand your idea correctly, then this question should open a new PR, and I can make him support it.

[rruuaanng]

Why does this test fail? test_no_stale_references (test.test_concurrent_futures.test_interpreter_pool.InterpreterPoolExecutorTest.test_no_stale_references) ... Fatal Python error: Segmentation fault

[vstinner]

@rruuaanng: Please follow what @erlend-aasland said:

The discussion on the issue did not conclude about which API to introduce; please discuss the API first, and then propose the change. I propose to close this premature PR until we know how to introduce this feature.

[vstinner]

I close the issue for now, I suggest to discuss on the issue and/or on the https://discuss.python.org/t/regarding-whether-we-should-add-py-currentarch-or-py-archname-function/65191 discussion.

[rruuaanng]

I think so. If there are concrete results in the community discussion, maybe I can reopen this PR (it was closed by someone else, so I can't reopen it).

[zooba]

No reason to close the PR yet.

[rruuaanng]

Thanks for zooba support. I agree. In fact, this issue has been discussed. And it has met all the requirements of the merger. I really like this function. It seems that only this can get the information at the time of build.

[ZeroIntensity]

@zooba I'm curious about the reason to put this in sys.implementation. I'm fine with putting it in sys (and that's probably the best place to put it), but implementation seems somewhat random. Why not just invent a new attribute e.g. sys._architecture?

[rruuaanng]

@zooba I'm curious about the reason to put this in sys.implementation. I'm fine with putting it in sys (and that's probably the best place to put it), but implementation seems somewhat random. Why not just invent a new attribute e.g. sys._architecture?

zooba comment from this PR of issue.

More importantly, and relevant for this case, is that platform.architecture() can't return the difference, even if it could detect it, because it's defined as only returning "64" or "32". And PEP 421 explicitly suggests sys.implementation is the place to store information that would be returned from platform about the Python implementation. The architecture the current runtime was compiled for sure seems to be related to the implementation, and since platform would be the obvious place to return such information (as it already is, albeit insufficiently), adding the field to sys.implementation seems fine.

[zooba]

@ZeroIntensity My rationale is on the issue: https://github.com/python/cpython/issues/102536#issuecomment-2420309303