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
Closed

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

wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner 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

Remove code specific to Windows Vista from os.cpu_count(): Python
requires Windows 7 or newer.
@vstinner
Copy link
Member Author

vstinner 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
Copy link
Member

zooba 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
Copy link
Member Author

vstinner 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 :-)

@zooba
Copy link
Member

zooba 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
Copy link
Member Author

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 cpu_count_vista branch March 15, 2019 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants