-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
bpo-35433: Properly detect installed SDK versions #11009
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
Conversation
<_RegistryVersion Condition="$(_RegistryVersion) != '' and !$(_RegistryVersion.EndsWith('.0'))">$(_RegistryVersion).0</_RegistryVersion> | ||
|
||
<!-- The minimum allowed SDK version to use for building --> | ||
<DefaultWindowsSDKVersion>10.0.10586.0</DefaultWindowsSDKVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose this SDK version as the minimum as I remember an issue ~5 years ago that was solved by using this version instead of an earlier one (although possibly pre-release).
FYI it seems like PR #10245 introduced the conflict. |
Aha, @zooba approved the change in the bug tracker, but not here: @zooba: I would prefer that you explicitly approve this change, since there are conflicts with your https://bugs.python.org/issue34977 change. |
PCbuild/python.props
Outdated
|
||
<!-- The minimum allowed SDK version to use for building --> | ||
<DefaultWindowsSDKVersion>10.0.10586.0</DefaultWindowsSDKVersion> | ||
<DefaultWindowsSDKVersion Condition="$(_RegistryVersion.CompareTo($(DefaultWindowsSDKVersion))) >= 0">$(_RegistryVersion)</DefaultWindowsSDKVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just learned that a better way to spell this is:
$([System.Version]::Parse($(_RegistryVersion))) > $([System.Version]::Parse($(DefaultWindowsSDKVersion)))
For the SDK version it probably won't matter though. The numbering scheme is going to remain the same length for a while.
There are no conflicts. The only reason I didn't directly approve is because I was on my phone at the time and I can't post to GitHub properly from my phone (one of the reasons I voted against the move, and continue to vote against the move to put all issues on here). In fixing my other PR, I found a better way to do the version comparison. So approved pending that change. |
Your two PRs modified the same file and the Merge button was disabled because of that. It's back to green because I reverted your App Store change. |
Oh I see. In that case, this change will make mine unnecessary, but it's not directly related. |
Added computed SDK version to ShowVersionInfo.
(cherry picked from commit f46eccd) Co-authored-by: Jeremy Kloth <[email protected]>
GH-11058 is a backport of this pull request to the 3.7 branch. |
Sorry, @jkloth and @zooba, I could not cleanly backport this to |
GH-11059 is a backport of this pull request to the 3.6 branch. |
(cherry picked from commit f46eccd) Co-authored-by: Jeremy Kloth <[email protected]>
|
|
Without knowing what version of VS2015 is used on Davd Bolen's buildbots for 3.6 these errors are nigh impossible to fix... These changes now force VS2015 to use the Win10 SDK whereas prior it was defaulting to the 8.1 SDK. I have a message out to David regarding this. |
|
|
This commit broke the following (at least) the buildbots: https://buildbot.python.org/all/#/builders/38/builds/751 can someone work on a fix? Otherwise we would have to revert the commit per our policy with buildbot failures |
There is a bit of a catch-22 here, if the changes get reverted, a different buildbot will be broken. Note that this is also actively being investigated, |
|
https://bugs.python.org/issue35433