search

bsnes-emu / bsnes

bsnes is a Super Nintendo (SNES) emulator focused on performance, features, and ease of use.
Other
1.69k stars 158 forks source link

Fix build failure on UCRT mingw64 #199

Closed Morilli closed 3 years ago

Morilli commented 3 years ago

closes #198.

Note that #define __MSVCRT_VERSION__ WINVER is the only thing that broke the build, but I've removed the others regardless because

  1. it doesn't feel right to define some of these and not others
  2. Looks like they're bound to cause issues in the future as well

Note that the resulting binary does change after removing #define _WIN32_IE WINVER... don't ask me why, I genuinely have no idea why the internet explorer is of any relevancy here. Chose to ignore and move on 👍

Screwtapello commented 3 years ago

Hmm, apparently this breaks the Windows builds on both GitHub and CirrusCI. I guess some of those defines must be important.

Morilli commented 3 years ago

From what I can tell so far, it is missing WSAPoll, which is defined in winsock2.h iff _WIN32_WINNT >= 0x0600. On my system, there is a _mingw.h header that does #define _WIN32_WINNT 0x601 and is included from windows.h which in turn gets included in winsock2.h iff _INC_WINDOWS is NOT defined. _INC_WINDOWS only gets defined if windows.h was already included from what I can see.

That means the only reasonable explanation for the failure is that on whatever system the CI is running, _WIN32_WINNT gets defined to a value lower than 0x0600 in _mingw.h or even not at all. According to microsoft's documentation, the value 0x0600 corresponds to windows vista. Is the build running on an even older system (or in some compatibility mode or whatever)?

Morilli commented 3 years ago

Okay, I have checked version 8.0.2 of mingw-w64 (even though the CI uses 8.1, but I couldn't find that online), which defines #define _WIN32_WINNT 0x502.

I'm using msys2/mingw, which uses the current trunk version of mingw-w64 (which is version 9+) and defines what I wrote above.

I guess for the sake of supporting this Windows Server 2003 define, I will re-add the WINNT and WINVER defines and see how it goes then.

Screwtapello commented 3 years ago

I think it's a good idea to explicitly ask for the version of Windows we intend to support, regardless of what the toolchain defaults might happen to be.

Thanks for figuring this out!

Screwtapello commented 3 years ago

I squashed these two commits and merged the result: 41f30f348a79387461a2091d1c1690a3c656396d