-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Fixes for mingw cross-compilation #2983
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
Conversation
Solution: change case of `WinSock2.h Iphlpapi.h Rpc.h` to match the files on disk. This is only noticeable when cross-compiling from a case-sensitive system so wouldn't get picked up in MSVC or mingw builds running on a windows machine. MSDN uses capitalised versions in prose and lowercase in code examples: https://msdn.microsoft.com/en-us/library/windows/desktop/ms737629(v=vs.85).aspx Fixes zeromq#2978, the missing library message is a little misleading.
| PREFIX "lib") | ||
| endif() | ||
|
|
||
| target_include_directories (objects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this being removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's moved up to where the add_library(objects ... line is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry, missed that
tests/CMakeLists.txt
Outdated
| link_directories(${test} PRIVATE "${CMAKE_SOURCE_DIR}/../lib") | ||
| endif() | ||
| endif() | ||
| set_target_properties(${test} PROPERTIES COMPILE_DEFINITIONS "${libzmq_DEFS}" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the adequate way to propagate compile definitions to dependent targets.
target_compiler_definitions(libzmq PUBLIC ...) should be used somewhere above instead, then they should be propagated automatically.
See https://cmake.org/cmake/help/v3.0/command/target_compile_definitions.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll have a look at PUBLIC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! PUBLIC is a very handy feature.
unittests/CMakeLists.txt
Outdated
|
|
||
| target_link_libraries(${test} libzmq-static ${OPTIONAL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} unity) | ||
|
|
||
| set_target_properties(${test} PROPERTIES COMPILE_DEFINITIONS "${libzmq_DEFS}" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
Solution: remove objects optimisation in library build (similar to zeromq#2860) and set PUBLIC compile definitions on all static builds instead of MSVC only.
Tested on both static/shared builds (untested runtime - no windows machine at hand).
make teston OSX native build has single failure44 - test_many_sockets (Timeout)both before and after changes.