-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
target: Implement OS version detection for Windows #4585
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
Address review comment by @rocksnest
I'm fine with this, if it's what makes sense to use for feature testing. Maybe we can have the WindowsVersion names as "canonical aliases" which are used when parsing and stringifying the target triple. |
Unless you have an argument not to, let's use the PEB to get the info |
I've already told ya why I don't want to pull this off the PEB, if you feel the need to do so even though there's a perfectly good API for this then feel free to close this PR. I don't want my name on that code. |
I agree with using the |
Ah, I see #4585 (comment) now - sorry 'bout that. missed it in my notifications. OK, ntdll.dll function call is fine by me. |
I've followed the direction you've set but building the version word by hand feels a bit awkward, maybe using the major.minor.build_no scheme works better...
Closes #4581