Skip to content

Try to avoid invoking a new process to determine the Windows version in the platform module #129294

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
tomergert opened this issue Jan 25, 2025 · 2 comments
Labels
OS-windows performance Performance or resource usage type-feature A feature request or enhancement

Comments

@tomergert
Copy link

tomergert commented Jan 25, 2025

Feature or enhancement

Proposal:

In Lib/platform.py, there's a code aimed to determine the Windows version python runs in.

    for cmd in ('ver', 'command /c ver', 'cmd /c ver'):
        try:
            info = subprocess.check_output(cmd,
                                           stdin=subprocess.DEVNULL,
                                           stderr=subprocess.DEVNULL,
                                           text=True,
                                           encoding="locale",
                                           shell=True)

As much as I understand, one shouldn't invoke whole new process just to question what OS version he runs at.

so just before invoking cmd.exe, we will try to question winapi directly.

# Load ntdll.dll and call RtlGetVersion
    ntdll = ctypes.WinDLL("ntdll")
    rtl_get_version = ntdll.RtlGetVersion
    rtl_get_version(ctypes.byref(os_version))

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

@tomergert tomergert added the type-feature A feature request or enhancement label Jan 25, 2025
@encukou encukou added the performance Performance or resource usage label Jan 27, 2025
@zooba
Copy link
Member

zooba commented Jan 27, 2025

This is already fallback code for if WMI doesn't work, and GetVersionEx is already provided by sys.getwindowsversion.

I don't think there's anything to change here.

@tomergert
Copy link
Author

This is already fallback code for if WMI doesn't work, and GetVersionEx is already provided by sys.getwindowsversion.

I don't think there's anything to change here.

Thank you,. I addressed your comment in the PR
#129295 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants