Winansi: Drop pre-Vista workaround#6109
Conversation
Git for Windows doesn't support anything prior to Windows 8.1 since 2.47.0 and Git followed along with commits like ce6ccba (mingw: drop Windows 7-specific work-around, 2025-08-04). There is no need to pretend to the compiler that we still support Windows Vista, just to lock us out of easy access to newer APIs. There is also no need to have conflicting and unused definitions claiming we support some versions of Windows XP or even Windows NT 4.0. Bump all definitions of _WIN32_WINNT to a realistic value of Windows 8.1. This will also simplify code for a followup commit that will improve cpu core detection on multi-socket systems. Signed-off-by: Matthias Aßhauer <mha1993@live.de>
1edeb9a (Win32: warn if the console font doesn't support Unicode, 2014-06-10) introduced both code to detect the current console font on Windows Vista and newer and a fallback for older systems to detect the default console font and issue a warning if that font doesn't support unicode. Since we haven't supported any Windows older than Vista in almost a decade, we don't need to keep the workaround. Signed-off-by: Matthias Aßhauer <mha1993@live.de>
I'd say this doesn't hurt, so let's keep it. It's safer that way, anyway.
Right. I guess we should upstream this ASAP anyway. And then update as soon as even Git LFS on Windows 8.1 is broken. |
| #define WIN32_LEAN_AND_MEAN | ||
| #ifndef _WIN32_WINNT | ||
| #define _WIN32_WINNT 0x403 | ||
| #define _WIN32_WINNT 0x603 |
There was a problem hiding this comment.
Technically, this is not ours to change, it's nedmalloc. We should probably upstream the mimalloc vendoring-in, and then drop compat/nedmalloc/ from upstream, too: It's woefully unsupported.
As far as this PR is concerned, let's keep this change, it makes it simpler to verify that we haven't missed any _WIN32_WINNT definition.
There was a problem hiding this comment.
We should probably upstream the mimalloc vendoring-in, and then drop compat/nedmalloc/ from upstream, too
That would be great, but somehow I have my doubts about Junio loving the frequency of our mimalloc updates, the churn of our 4-commit update method or the (very useful) commit message of 266c6eb.
I think he might prefer something more like contrib/update-unicode/update_unicode.sh and the corresponding commits.
There was a problem hiding this comment.
Heh. I can always try to upstream as-is 😁
| # define WIN32_NATIVE | ||
| # if defined (_MSC_VER) && !defined(_WIN32_WINNT) | ||
| # define _WIN32_WINNT 0x0502 | ||
| # define _WIN32_WINNT 0x0603 |
There was a problem hiding this comment.
Technically, this is not ours to change, but I guess it could come back to bite us if we don't. So let's keep this change.
There was a problem hiding this comment.
By "ours" do you mean Git for Windows or Git? Because it is a Git modification to the vendored code (git@56fb3dd), but I guess it makes sense to drop this all together, since git@e8dfcac made this obsolete.
There was a problem hiding this comment.
I mean both Git and Git for Windows; Vendored-in code is harder to maintain if it is modified from the original version.
There was a problem hiding this comment.
I could upstream a commit to drop this definition alltogether, which would reduce the modifications to vendored code, but I doubt we'll see many updates to the vendored code.
There was a problem hiding this comment.
I doubt we'll see many updates to the vendored code.
True.
| #ifndef FLUSH_FLAGS_FILE_DATA_ONLY | ||
| #define FLUSH_FLAGS_FILE_DATA_ONLY 1 | ||
| #endif |
There was a problem hiding this comment.
We could even drop this altogether, right? If we require a new-enough _WIN32_WINNT, we require this flag to be defined, no?
There was a problem hiding this comment.
I wasn't sure if that is always the case and we can reasonably expect this, so I thought wrapping it in #ifndef was the safe bet.
2 participants
1edeb9a (Win32: warn if the console font doesn't support Unicode,
2014-06-10) introduced both code to detect the current console font on
Windows Vista and newer and a fallback for older systems to detect the
default console font and issue a warning if that font doesn't support
unicode.
Since we haven't supported any Windows older than Vista in almost a
decade, we don't need to keep the workaround.
This more or less fell out of #6108, but didn't quite fit into that PR.
There are also some other version specific hacks and workarounds I considered dropping, but decided against: