Fix Cygwin building and add CI

[stahta01]

• I confirm that I am the author of this code and release it to the SDL project under the Zlib license. This contribution does not contain code from other sources, including code generated by a Large Language Model ("AI").
  This PR contains code from PR fix building on CYGWIN #15457 and PR Enables MSys2 MSYS environment to build SDL3 #15514

Fixes Cygwin building using native gcc.

Description

This PR is based on the PR #15457.
The warning reduction changes are very different in header "SDL_platform_defines.h".
The method to get "d3d12.h" to work is very different.
This PR adds Cygwin CI support and the CI run finishes

Existing Issue(s)

It might fix #15360
I did my testing with cmake/ninja under SDL3 the issue did it with cmake/makefiles

[stahta01]

There are two conversations that I think I can close but not certain on so leaving them open for a day or so waiting on feedback.

[sezero]

A few notes, mostly by comparison to old PR/15457:

• haptic/windows/SDL_dinputhaptic.c (at around line 94) and
  joystick/windows/SDL_dinputjoystick.c (at around line 432)
  need the same %lu -> new-macros changes for GetLastError()
  printing.

• This opengl header change seems missing?

diff --git a/include/SDL3/SDL_opengl.h b/include/SDL3/SDL_opengl.h
index 733f2b5..ba7d85b 100644
--- a/include/SDL3/SDL_opengl.h
+++ b/include/SDL3/SDL_opengl.h
@@ -84,7 +84,7 @@
 #  else
 #    define GLAPIENTRY __stdcall
 #  endif
-#elif defined(__CYGWIN__) && defined(USE_OPENGL32) /* use native windows opengl32 */
+#elif defined(__CYGWIN__)  /* && defined(USE_OPENGL32) */ /* use native windows opengl32 */
 #  define GLAPI extern
 #  define GLAPIENTRY __stdcall
 #elif (defined(__GNUC__) && __GNUC__ >= 4) || (defined(__SUNPRO_C) && (__SUNPRO_C >= 0x590))

• Question: You enable vulkan in the cmake script. Does the vulkan
  headers need any cygwin changes, e.g. calling convention or whatever?
  Iss it tested?

• It'd be nice to have the following in top-level cmake script:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e591a01..70f107c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -4068,12 +4055,13 @@ if(SDL_SHARED)
       )
     endif()
   elseif(UNIX AND NOT ANDROID)
+    # NOTE: the includes CYGWIN, too
     set_target_properties(SDL3-shared PROPERTIES
       VERSION "${SDL_SO_VERSION}"
       SOVERSION "${SDL_SO_VERSION_MAJOR}"
     )
   else()
-    if(WINDOWS OR CYGWIN)
+    if(WINDOWS)
       set_target_properties(SDL3-shared PROPERTIES
         PREFIX ""
       )

• It'd be good to have the following in cmake funcs (change by @madebr):

diff --git a/cmake/sdlplatform.cmake b/cmake/sdlplatform.cmake
index 6c60e0a..1fbce7f 100644
--- a/cmake/sdlplatform.cmake
+++ b/cmake/sdlplatform.cmake
@@ -4,6 +4,8 @@ function(SDL_DetectCMakePlatform)
     set(sdl_cmake_platform Windows)
   elseif(PSP)
     set(sdl_cmake_platform psp)
+  elseif(CYGWIN)
+    set(sdl_cmake_platform Cygwin)
   elseif(APPLE)
     if(CMAKE_SYSTEM_NAME MATCHES ".*(Darwin|MacOS).*")
       set(sdl_cmake_platform macOS)

• It'd be good to have the following in tests' cmake script:
  (I guess testnative would fail without this..)

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index b6e7b72..6167865 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -373,7 +373,7 @@ if(MACOS)
             testnativecocoa.m
             testnativex11.c
     )
-elseif(WINDOWS)
+elseif(WINDOWS OR CYGWIN)
     add_sdl_test_executable(testnative BUILD_DEPENDENT NEEDS_RESOURCES TESTUTILS SOURCES testnative.c testnativew32.c)
 elseif(HAVE_X11 OR HAVE_WAYLAND)
     add_sdl_test_executable(testnative BUILD_DEPENDENT NEEDS_RESOURCES TESTUTILS SOURCES testnative.c)

• Maybe add this to identify cygwin as itself?

diff --git a/src/SDL.c b/src/SDL.c
index 350ca34..11b8872 100644
--- a/src/SDL.c
+++ b/src/SDL.c
@@ -793,6 +793,8 @@ const char *SDL_GetPlatform(void)
     return "Solaris";
 #elif defined(SDL_PLATFORM_WIN32)
     return "Windows";
+#elif defined(SDL_PLATFORM_CYGWIN)
+    return "Cygwin";
 #elif defined(SDL_PLATFORM_WINGDK)
     return "WinGDK";
 #elif defined(SDL_PLATFORM_XBOXONE)

[stahta01]

I did not test anything, so, if you want me to remove vulkan I will do so. Remove the change and Pushed.

I will do the change to "SDL.c" that returns "Cygwin". Pushed Change.

I will try the testnative change. Pushed Change waiting for CI to finish without error.

I will do the sdlplatform.cmake change. Pushed edited change.

I will do the top-level cmake script change. Pushed Change.

I did not understand the reason for opengl header change, so, I skipped it. Do you think it is correct? Pushed Change.

I do not see a warning or error related to "GetLastError()" so I think that should be done in a separate PR.

[sezero]

I did not test anything, so, if you want me to remove vulkan I will do so. I am planning to test it under MSYS in the future.

IMO, dropping it for now would be better if not tested. If calling conventions (for e.g.) do need editing, it probably won't matter for 64 bits but it would matter a lot for 32 bits.

I did not understand the reason for opengl header change, so, I skipped it. Do you think it is correct?

It think it is: It was @TrueCat17's original change.

[sezero]

I do not see a warning or error related to "GetLastError()"

Because dinput.h isn't present for who knows why, and those two don't get built

so I think that should be done in a separate PR.

As you wish. I noted it for completeness.

[sezero]

I did not understand the reason for opengl header change, so, I skipped it. Do you think it is correct?

It think it is: It was @TrueCat17's original change.

Yes, it is correct: Without it, # define GLAPIENTRY __stdcall wouldn't take effect, and it is critical for 32 bits.

[stahta01]

I did not understand the reason for opengl header change, so, I skipped it. Do you think it is correct?

It think it is: It was @TrueCat17's original change.

Yes, it is correct: Without it, # define GLAPIENTRY __stdcall wouldn't take effect, and it is critical for 32 bits.

I have pushed that change.

[sezero]

The two DINPUT source changes should be as trivial as this:

diff --git a/src/haptic/windows/SDL_dinputhaptic.c b/src/haptic/windows/SDL_dinputhaptic.c
index fa374e4..ce0bd30 100644
--- a/src/haptic/windows/SDL_dinputhaptic.c
+++ b/src/haptic/windows/SDL_dinputhaptic.c
@@ -90,7 +90,7 @@ bool SDL_DINPUT_HapticInit(void)
     instance = GetModuleHandle(NULL);
     if (!instance) {
         SDL_SYS_HapticQuit();
-        return SDL_SetError("GetModuleHandle() failed with error code %lu.",
+        return SDL_SetError("GetModuleHandle() failed with error code %" SDL_PRIuULONG ".",
                             GetLastError());
     }
     ret = IDirectInput8_Initialize(dinput, instance, DIRECTINPUT_VERSION);
diff --git a/src/joystick/windows/SDL_dinputjoystick.c b/src/joystick/windows/SDL_dinputjoystick.c
index 62ffbb4..b9e80e6 100644
--- a/src/joystick/windows/SDL_dinputjoystick.c
+++ b/src/joystick/windows/SDL_dinputjoystick.c
@@ -429,7 +429,7 @@ bool SDL_DINPUT_JoystickInit(void)
     if (!instance) {
         IDirectInput8_Release(dinput);
         dinput = NULL;
-        return SDL_SetError("GetModuleHandle() failed with error code %lu.", GetLastError());
+        return SDL_SetError("GetModuleHandle() failed with error code %" SDL_PRIuULONG ".", GetLastError());
     }
     result = IDirectInput8_Initialize(dinput, instance, DIRECTINPUT_VERSION);
 

As for VULKAN: Looking at the headers, the following change is critical IMO, for __stdcall,
and with that I think you can re-enable vulkan:

diff --git a/src/video/khronos/vulkan/vk_platform.h b/src/video/khronos/vulkan/vk_platform.h
index 0ecd4f6..5624636 100644
--- a/src/video/khronos/vulkan/vk_platform.h
+++ b/src/video/khronos/vulkan/vk_platform.h
@@ -36,7 +36,7 @@ extern "C"
  * Function declaration:  VKAPI_ATTR void VKAPI_CALL vkCommand(void);
  * Function pointer type: typedef void (VKAPI_PTR *PFN_vkCommand)(void);
  */
-#if defined(_WIN32)
+#if defined(_WIN32) || defined(__CYGWIN__)
     // On Windows, Vulkan commands use the stdcall convention
     #define VKAPI_ATTR
     #define VKAPI_CALL __stdcall

[stahta01]

I have not edited "vk_platform.h" in this PR.
Are you sure it needs edited?
And I am guessing you want me to remove the "|| defined(CYGWIN)".

[stahta01]

I did not test anything, so, if you want me to remove vulkan I will do so. I am planning to test it under MSYS in the future.

IMO, dropping it for now would be better if not tested. If calling conventions (for e.g.) do need editing, it probably won't matter for 64 bits but it would matter a lot for 32 bits.

I did not understand the reason for opengl header change, so, I skipped it. Do you think it is correct?

It think it is: It was @TrueCat17's original change.

I did drop my vulkan change so it should no longer default to enabled. It might still allow someone to manually enable it.

[sezero]

I have not edited "vk_platform.h" in this PR. Are you sure it needs edited?

To enable vulkan, yes

And I am guessing you want me to remove the || defined(__CYGWIN__)

No, it should be added so that 32 bit works correctly as __stdcall

P.S.: Speaking of __stdcall, your cygwin compiler targets 64 bit, yes?
Do the cygwin headers undefine __stdcall and redefine it as a blank macro
for _WIN64 case? (My mingw headers does that in _mingw.h for example.)

[stahta01]

I have not edited "vk_platform.h" in this PR. Are you sure it needs edited?

To enable vulkan, yes

And I am guessing you want me to remove the || defined(__CYGWIN__)

No, it should be added so that 32 bit works correctly as __stdcall

P.S.: Speaking of __stdcall, your cygwin compiler targets 64 bit, yes? Do the cygwin headers undefine __stdcall and redefine it as a blank macro for _WIN64 case? (My mingw headers does that in _mingw.h for example.)

Cygwin has separate GCC compilers for mingw32 and mingw64 and a separate set of support folders that goes with them.
And the 64-bit native GCC does target 64 bits. I have never tried to see if it can target 32 bits likely no.

Edit2: Just got done searching did not find "_WIN64" in any header and the native GCC does not define it or related macros.

The below is from grep of native GCC predefined dump

#define _stdcall __attribute__((__stdcall__))
#define __stdcall __attribute__((__stdcall__))

$ cat /tmp/O0.txt | grep 'WIN'
#define __WINT_MAX__ 0xffffffffU
#define __WINT_MIN__ 0U
#define __SIZEOF_WINT_T__ 4
#define __CYGWIN__ 1
#define __WINT_TYPE__ unsigned int
#define __WINT_WIDTH__ 32

Tim S.

[sezero]

The below is from grep of native GCC predefined dump

#define _stdcall __attribute__((__stdcall__))
#define __stdcall __attribute__((__stdcall__))

That's right, and x86_64 mingw compiler does that too. However, the headers (_mingw.h in my case) undefines it and redefines it as a blank macro, so that __stdcall has no effect in all win64 apis. Does any of your cygwin headers do that? (I'm guessing that cygwin actually uses certain mingw headers, including _mingw.h, but I don't have a cygwin or msys setup, therefore I'm not certain.)

[stahta01]

Inside "C:/cygwin64/usr/include/w32api/_cygwin.h"

I found

#ifdef __x86_64__
#define _WIN64
#endif

I have not found anywhere that undefines _stdcall

[stahta01]

Manually edited the CI commits and did another forced push

[stahta01]

I think I made a mistake with the DLL name change will find out if CI fails. It failed.

[slouken]

Thank you for all your work on this!