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

bpo-32592: Remove Vista code from os.cpu_count() #11457

Closed
wants to merge 1 commit into from

Conversation

@vstinner
Copy link
Member

commented Jan 7, 2019

Remove code specific to Windows Vista from os.cpu_count(): Python
requires Windows 7 or newer.

https://bugs.python.org/issue32592

bpo-32592: Remove Vista code from os.cpu_count()
Remove code specific to Windows Vista from os.cpu_count(): Python
requires Windows 7 or newer.

@vstinner vstinner requested review from zooba, zware and python/windows-team Jan 7, 2019

@vstinner

This comment has been minimized.

Copy link
Member Author

commented Jan 7, 2019

Ah. Compilation failed on "Windows PR Tests win32" of Azure Pipelines PR:

2019-01-07T15:02:40.9331567Z d:\a\1\s\modules\posixmodule.c(11823): warning C4013: 'GetMaximumProcessorCount' undefined; assuming extern returning int [D:\a\1\s\PCbuild\pythoncore.vcxproj]

... but it pass on "Windows PR Tests win64".

Extract of pythoninfo on "Windows PR Tests win32" from a different PR:

  • platform.architecture: 32bit WindowsPE
  • platform.platform: Windows-10-10.0.14393-SP0
  • sys.version: 3.8.0a0 (remotes/pull/11455/merge:a07376fda, Jan 7 2019, 09:34:24) [MSC v.1915 32 bit (Intel)]
@zooba

This comment has been minimized.

Copy link
Member

commented Jan 8, 2019

That is a very strange error to get... I can't think of any reason other than a corrupt incremental build, but we don't do incremental builds here.

Perhaps the rebuild when you add a NEWS entry will work?

@vstinner

This comment has been minimized.

Copy link
Member Author

commented Jan 8, 2019

On IRC, @zware told me:

Looks like you need to set Py_WINVER to 0x0601 in PC/pyconfig.h

It tried to avoid puting this change in this PR to get a PR as small as possible, but it seems like it's not enough.

Note: I already failed to understand such failure in the past: https://bugs.python.org/issue32592#msg318103 Windows is still full of mystery for me :-)

@eamanu
eamanu approved these changes Jan 8, 2019
@auvipy
auvipy approved these changes Jan 9, 2019
@zooba

This comment has been minimized.

Copy link
Member

commented Jan 9, 2019

@vstinner Zachary is correct. You'll need to get that first change in before any of the rest will succeed reliably.

I'm okay with reviewing all of the changes together if it's simpler for you to do one PR. But the WINVER update is the critical part.

@vstinner

This comment has been minimized.

Copy link
Member Author

commented Mar 15, 2019

I don't understand how to remove Vista support. I give up on this PR. If someone else is interested, please go ahead and write a new PR :-)

@vstinner vstinner closed this Mar 15, 2019

@vstinner vstinner deleted the vstinner:cpu_count_vista branch Mar 15, 2019

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.