Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi target windows api #32

Merged
merged 4 commits into from
Nov 11, 2018
Merged

Conversation

gmbeard
Copy link
Contributor

@gmbeard gmbeard commented Nov 9, 2018

This PR addresses #21.

Summary
For Windows platforms only, the CMake targets are now split into 2; The default mio::mio target and a new mio::mio_full_winapi target. The default target defines WIN32_LEAN_AND_MEAN and NOMINMAX as compiler flags, while the mio::mio_full_winapi target defines neither of these. I've also updated the docs with instructions for Windows users.

A word of caution, this may be a breaking change for existing builds that incorporate Mio into applications using the Windows API. This is because the definitions have now been moved into the compiler flags (instead of being defined in-line, within the source). In this case, those applications would have to use the mio::mio_full_winapi target, instead.

I've tested this on MSVC MSVC 19.14.26429.4 (Windows), and Clang 6.0.1 / 8.0.0 (FreeBSD)

Greg Beard added 4 commits October 19, 2018 07:21
The extra target doesn't `#define` any of `WIN32_LEAN_AND_MEAN`,
`NOMINMAX`. The default `mio::mio` adds both of these. This commit also
removes these defines from the `.*pp` files; they're now inserted by the
compile commands.
@vimpunk
Copy link
Owner

vimpunk commented Nov 11, 2018

@gmbeard Wow, this is great. Thank you! I could only test on Linux but it looks good to me. The breaking change is obviously not great, but I think the note in the readme should clear up any confusion that may result from this change.

@vimpunk vimpunk merged commit d77add9 into vimpunk:master Nov 11, 2018
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.

None yet

2 participants