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

Winsock.h was included multiple times through windows.h causing compile errors #772

Merged
merged 2 commits into from
Dec 11, 2019

Conversation

antihax
Copy link
Contributor

@antihax antihax commented Dec 9, 2019

Added #define _WINSOCKAPI_ to prevent windows.h importing winsock.h and removed a few redundant windows.h imports.

@vsonnier
Copy link
Collaborator

vsonnier commented Dec 9, 2019

Hi @antihax , Visual Studio 2019 works for me out of the box, like 2017 and 2015 did before. What configuration are you using by any chance ?
I'm following the procedure here : https://github.com/cjcliffe/CubicSDR/wiki/Build-Windows

If it is something esoteric like Mingw32 or MSYS2 I'm no even sure it is supported anymore.

Anyway, some remarks about the PR:

  • We are not going to patch external OSS, even embedded like RtAudio or TinyXml are.
  • OK for Timer.
  • Sound strange to put WinSocket specific hack in source code that don't use it at all. Again something may be wrong with your configuration.

@antihax
Copy link
Contributor Author

antihax commented Dec 9, 2019

Interesting, I am using VS2019. Are you defining WIN32_LEAN_AND_MEAN anywhere?
I will take another look tonight where this is coming from.

@vsonnier
Copy link
Collaborator

vsonnier commented Dec 9, 2019

Are you defining WIN32_LEAN_AND_MEAN anywhere?

I'm not playing with low-level settings like that, because it is irrelevant with CMake. I just generate the adequate VS solution with CMake following the steps and settings of the Wiki.
Please note that this is the only supported way: VS solution generated with CMake as Wiki describes, with wxWidgets 3.1.3 statically build, while using the the config Release (or RelWithDebInfo if you want to debug CubicSDR)...etc.

For any other config variant you are on your own, and we are not going to dedicate any time to support such variants, sorry.

@antihax
Copy link
Contributor Author

antihax commented Dec 9, 2019

Unfortunately I also did the same, but CMake output will not build due to redefining winsock.h imports.

I will see if I can identify if I did something incorrect when generating the project from CMake.

@vsonnier
Copy link
Collaborator

vsonnier commented Dec 9, 2019

I use a recent CMake, like 3.15+. (3.16 at that moment). Try to delete the CMake cache (see CMake GUI) and start again from the begining.

@antihax
Copy link
Contributor Author

antihax commented Dec 10, 2019

I checked I have the latest versions of all components, latest Win10 SDK, and completely reset all MSVC 2019 settings. Problem persists on all imports of windows.h.

I create a quick test solution with nothing else and can successfully import windows.h with no issue. Not sure what in CubicSDR or a dependency is creating the conflict, but something is importing winsock out of order.

I've refactored to include WIN32_LEAN_AND_MEAN in the pre-processor options, which is a cleaner solution.

@vsonnier
Copy link
Collaborator

I've refactored to include WIN32_LEAN_AND_MEAN in the pre-processor options, which is a cleaner solution.

Great, works for me too ! Please re-add Timer.cpp Win32 include removal, and we are good to go.

@vsonnier vsonnier merged commit cd6df01 into cjcliffe:master Dec 11, 2019
@vsonnier
Copy link
Collaborator

@antihax Thanks for your contribution(s) !

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.

2 participants