Skip to content

Conversation

@MarijnS95
Copy link

The build setup for this repository is incredibly simple, but unfortunately even recent changes to the CMake have made various platforms and compiler combinations unbuildable. Add a basic CI that at the very least builds the project to demonstrate all these problems, which should all go away when other PRs are merged.

Currently this highlights two errors that are both being solved by #45. One is the missing read()/write() functions declarations when compiling for Windows using clang. The other was an intuitive replacement for simple code improvement, that now also ends up being a fix for 84658df / b8b8bdc: adapt.c can no longer find #include "adapt.h" because (1) the include_directories("win32") was removed and (2) the two files are no longer residing in the same directory.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting this file because it's no longer being included since b8b8bdc#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aL9.

Maybe that's an error though...

Copy link
Author

@MarijnS95 MarijnS95 Jul 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my own testing, when adding #45 on top things now fail with ssize_t being missing: https://github.com/MarijnS95/GKlib/actions/runs/16099153358/job/45425988032

I assume because the compiler only defines _WIN32, whereas this now-unused GKlibSystem.cmake file used to set WIN32. But only for if(MSVC) of course, so cl.exe + clang-cl get it but clang would have missed out too.

EDIT: Same for the missing strerror_r, which is in the #else for #if defined(WIN32).

The build setup for this repository is incredibly simple, but
unfortunately even recent changes to the CMake have made various
platforms and compiler combinations unbuildable.  Add a basic CI that at
the very least _builds_ the project to demonstrate all these problems,
which should all go away when other PRs are merged.
MarijnS95 added 2 commits July 6, 2025 15:26
…fined through CMake)

Recent changes no longer include a CMake file that set `-DWIN32` (which
was only set for `MSVC` anyway, meaning Windows + `clang` would miss
out) resulting in lots of errors when compiling on/for Windows because
compatibility code is no longer enabled.  Switch all those `#ifdef`s to
`_WIN32` which will be compiled by both MSVC and `clang` when compiling
for Windows.
… MSVC)

When compiling on/to Windows via LLVM/clang(-cl), the compiler ID is
no longer `MSVC` even though `m.lib` should still not be used.  Read
`WIN32` instead which will always be set when targeting Windows
regardless of the compiler being used.
@MarijnS95
Copy link
Author

With the latest changes, almost all jobs are green now: https://github.com/MarijnS95/GKlib/actions/runs/16099464137

Except for a very simple native MSBuild / Visual Studio + MSVC compilation. That still fails with:

D:\a\GKlib\GKlib\apps\gkuniq.c(78,5): error C2065: '__asm__': undeclared identifier [D:\a\GKlib\GKlib\build\apps\gkuniq.vcxproj]
D:\a\GKlib\GKlib\apps\gkuniq.c(78,13): error C2143: syntax error: missing ';' before 'volatile' [D:\a\GKlib\GKlib\build\apps\gkuniq.vcxproj]
D:\a\GKlib\GKlib\apps\gkuniq.c(84,3): error C2065: '__asm__': undeclared identifier [D:\a\GKlib\GKlib\build\apps\gkuniq.vcxproj]
D:\a\GKlib\GKlib\apps\gkuniq.c(84,11): error C2143: syntax error: missing ';' before 'volatile' [D:\a\GKlib\GKlib\build\apps\gkuniq.vcxproj]

@MarijnS95
Copy link
Author

@karypis are you interested in this change? I'd like to use it to demonstrate #45, and prevent such issues from creeping in in the future :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant