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

[WIP] bpo-32592: Drop support for Windows Vista #5231

Closed
wants to merge 5 commits into from

Conversation

@vstinner
Copy link
Member

commented Jan 18, 2018

Windows Vista extended support ended in April 2017. As stated in PEP
11, Python 3.7 drops support for Windows Vista and now requires
Windows 7 or newer.

https://bugs.python.org/issue32592

@vstinner vstinner requested review from zooba and zware Jan 18, 2018

@vstinner vstinner requested a review from python/windows-team as a code owner Jan 18, 2018

bpo-32592: Drop support for Windows Vista
Windows Vista extended support ended in April 2017. As stated in PEP
11, Python 3.7 drops support for Windows Vista and now requires
Windows 7 or newer.

@vstinner vstinner force-pushed the vstinner:drop_vista branch from 0f27f41 to 1a83ced Jan 18, 2018

@vstinner

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2018

Hum, the change should be mentionned in Doc/whatsnew/3.7.rst.

@vstinner

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2018

AppVeyor compilation failed with: "C:\Program Files (x86)\Windows Kits\10\Include\10.0.16299.0\shared\sdkddkver.h(289): fatal error C1189: #error: NTDDI_VERSION setting conflicts with _WIN32_WINNT setting [C:\projects\cpython\PCbuild\pythoncore.vcxproj]"

@@ -34,58 +34,13 @@
#define WIN32_LEAN_AND_MEAN
#include <windows.h>

/* options */

This comment has been minimized.

Copy link
@matrixise

matrixise Jan 18, 2018

Member

Maybe you could change the comment before #define Py_HAVE_CONDVAR ?

This comment has been minimized.

Copy link
@matrixise

matrixise Jan 18, 2018

Member

in fact, should we remove all the references to Vista when we drop the support of Vista ?

This comment has been minimized.

Copy link
@vstinner

vstinner Jan 18, 2018

Author Member

I updated the comment.

@matrixise

This comment has been minimized.

Copy link
Member

commented Jan 18, 2018

@vstinner There is some references to Vista in the Tools/msi/ directory.

Remove condvar emulation (_PY_EMULATED_WIN_CV)
Rephrase also a comment in condvar.h.
@vstinner

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2018

@vstinner There is some references to Vista in the Tools/msi/ directory.

I don't know this directory, so I prefer to leave it unchanged.

@matrixise

This comment has been minimized.

Copy link
Member

commented Jan 18, 2018

@vstinner ok then in this case, the PR seems to be right.

* example native support on VISTA and onwards.
*/

#if _PY_EMULATED_WIN_CV

This comment has been minimized.

Copy link
@zooba

zooba Jan 18, 2018

Member

See https://bugs.python.org/issue29871 before pulling this code out - our alternate code isn't 100% correct yet.

This comment has been minimized.

Copy link
@vstinner

vstinner Jan 18, 2018

Author Member

Oh, I can revert this change and propose it in https://bugs.python.org/issue29871.

My change should have no effect on Windows 7 and newer. Why do you prefer to keep the unused emulation code (until bpo-29871 is fixed)?

@vstinner

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2018

AppVeyor compilation failed with: "C:\Program Files (x86)\Windows Kits\10\Include\10.0.16299.0\shared\sdkddkver.h(289): fatal error C1189: #error: NTDDI_VERSION setting conflicts with _WIN32_WINNT setting [C:\projects\cpython\PCbuild\pythoncore.vcxproj]"

@zooba: Do you understand why my change breaks the compilation?

* to target Windows Vista. Modify this macro to enable them.
*/
#ifndef _PY_EMULATED_WIN_CV
#define _PY_EMULATED_WIN_CV 1 /* use emulated condition variables */

This comment has been minimized.

Copy link
@eryksun

eryksun Jan 18, 2018

Contributor

Isn't removing this a mistake? I think the SRWLOCK version was tried in another issue that was abandoned.

This comment has been minimized.

Copy link
@vstinner

vstinner Jan 18, 2018

Author Member

Hum, this code seems to sensitive and my change may conflict with https://bugs.python.org/issue29871 so I reverted this change and will open a new dedicated PR.

@vstinner

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2018

"posixmodule.obj : error LNK2001: unresolved external symbol _GetMaximumProcessorCount [C:\projects\cpython\PCbuild\pythoncore.vcxproj]"

Oh, my posixmodule.c change doesn't work :-(

Extract of pythoninfo:

sys.windowsversion: sys.getwindowsversion(major=10, minor=0, build=14393, platform=2, service_pack='')
sys.winver: 3.7-32
sys.version: 3.7.0a4+ (heads/master-2-g1d7f51e:1d7f51e, Jan 18 2018, 05:42:21) [MSC v.1912 32 bit (Intel)]

I understand that AppVeyor runs Windows 10 and Python was built by Visual Studio 2017

https://msdn.microsoft.com/fr-fr/library/windows/desktop/dd405489(v=vs.85).aspx

"Remarks: To compile an application that uses this function, set _WIN32_WINNT >= 0x0601. For more information, see Using the Windows Headers."

Aha. I need to try to understand why changing _WIN32_WINNT failed. But my Windows VM is down :-)

@vstinner vstinner changed the title bpo-32592: Drop support for Windows Vista [WIP] bpo-32592: Drop support for Windows Vista Jan 18, 2018

@vstinner

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2018

Hum, I'm not sure that my PR is correct anymore. I change its status to WIP, I will rework on it, once I get again access to a Windows VM to fix my PR.

@matrixise

This comment has been minimized.

Copy link
Member

commented Jan 26, 2018

@vstinner vstinner closed this Feb 1, 2018

@vstinner vstinner deleted the vstinner:drop_vista branch May 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.