CI: Add WinAPI header compatibility workflow

[annihilatorq]

This workflow contains a pretty simple check, which will ultimately eliminate the need to worry about guarding std::min/max and std::numeric_limits<T>::min/max, ensure naming conventions that do not conflict with Windows definitions, and address other minor issues that might occasionally slip through code reviews and make it into the main branch. This workflow will simply fail in the event of a forgotten guard or a conflict. The code will still need to use guards, but if this is forgotten, it won't pass CI

The workflow deliberately creates a poor environment for the library by generating a messy translation unit in a typical Windows project, where NOMINMAX, WIN32_LEAN_AND_MEAN, and NOGDI are not defined. The workflow also ensures that guard fixes via #undef of any Windows defines do not accidentally slip into the main branch, so as not to break a user's project whose code may rely on one of these defines.

DELETE is a special case in this workflow, as glaze/net actively uses #undef DELETE, so the presence of this define is not checked after including the Glaze header.

I used recursion and included each header separately instead of including the umbrella header (glaze.hpp), since I noticed that umbrella headers do not include the entire public Glaze API headers (is this by design, or does it require a fix?)

[stephenberry]

The workflow deliberately creates a poor environment for the library by generating a messy translation unit in a typical Windows project

This is a great idea!

[stephenberry]

I noticed that umbrella headers do not include the entire public Glaze API headers (is this by design, or does it require a fix?)

Glaze has so many features that I've been moving more and more away from umbrella headers. It's better for headers like "glaze/json.hpp" to handle the most common JSON utilities, but lots of headers are just for special use cases and it reduces compilation times by not including all features under huge umbrella headers.

When we finally get C++20 modules we should be able to have everything under a single module, or a few umbrella modules.

[annihilatorq]

This is a great idea!

Thanks, but it seems like the idea of building a separate target for each include was a bad one, since the workflow ended up taking too long, and it looks like I might have made some kind of mistake because the build is failing. I'll fix it when I wake up, since I was just about to go to sleep 😁

[annihilatorq]

The workflow is finally working as intended. The test run in which I removed the guard around std::numeric_limits<T>::max caught the error as expected (I cleared the history of that commit via rebase). I'll try to clean up the CMake script before the merge and make it easier to add headers or directories to the list for the WIN32_LEAN_AND_MEAN target, but functionally, the current version is already fully operational.

Currently, some headers, for example - any header from glaze/net, need to be built under WIN32_LEAN_AND_MEAN because the errors within them come from external dependencies (asio, erlang) that are outside of Glaze scope.
In such case, all we can do is try to provide user with better diagnostics (#2635)

[DockedFerret800]

The final decision regarding this is up to Stephen; however, the current workflow looks significantly unmanageable to me. I’d suggest creating a separate file for in the CMake folder, for example /cmake/ci/ and include from it. This would make things much cleaner. I would also use run-vcpkg action for the vcpkg installation step to leverage vcpkg binary caching and reduce CI runtime, although this is just a nit, given that the current step doesn’t take much time, so this can be omitted.

[annihilatorq]

The final decision regarding this is up to Stephen; however, the current workflow looks significantly unmanageable to me.

Yes, I was planning to refactor it today, as I mentioned in my previous comment. Yesterday, I made a lot of fixes to ensure it worked correctly, which cluttered up the CMake code with these quick patches.

As for adding run-vcpkg, I think Stephen would be against it, based on this comment, but I’m not sure if it makes sense to try to speed things up, considering that the workflow runs in ~2-3 minutes, while the longest workflows take about 20-30 minutes.

[DockedFerret800]

Yeah, this surely can be omitted in this case 👍🏻

[annihilatorq]

I think everything is ready for the merge now, pending your review @stephenberry

@DockedFerret800, could I get your thoughts on the current workflow structure?

[DockedFerret800]

LGTM, just a few small nits. 👍🏻

[packit-as-a-service]

One of the tests failed for 2a8e664. @admin check logs https://download.copr.fedorainfracloud.org/results/packit/stephenberry-glaze-2630/srpm-builds/10615142/builder-live.log, packit dashboard https://dashboard.packit.dev/jobs/srpm/620574 and external service dashboard https://copr.fedorainfracloud.org/coprs/build/10615142/

[stephenberry]

Thanks for this! It looks good!