gh-102536: Added _architecture field to sys.implementation

[rruuaanng]

cc @zooba

• Issue: extend sys module on windows #102536

[picnixz]

I'd like to have tests as well please.

[rruuaanng]

I'd like to have tests as well please.

Okay, I have make change.

[picnixz]

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).

[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:

cpython/PC/layout/support/props.py

Line 11 in abe5f79

PYTHON_PROPS_NAME = "python.props"

.

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:

cpython/PC/layout/support/props.py

Line 11 in abe5f79

PYTHON_PROPS_NAME = "python.props"

.

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')

[vstinner]

The issue is called "Issue: extend sys module on windows" but your implementation is only for non-Windows platforms.

[ericsnowcurrently]

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

[ZeroIntensity]

Is this supposed to be a public interface (just prefixed with _ to denote platform incompatibility), or a private implementation detail that we need internally? If it's the former, then I agree with Eric.

[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: #102536 (comment)

[ZeroIntensity]

"current interpreter runtime system" is wrong--_architecture defines what architecture CPython was built with, not what it's currently running under.

[zooba]

It's close. It specifies the architecture it was built to run as, which is independent from both the one it was built on, and the one it's currently running on.

But it's the only one that is specific to the currently running interpreter. So at most, I'd only change this to "the native architecture of the current interpreter", and possibly add "which will usually match the architecture of the current system" (though that's probably more words than it's worth).