Fixes for mingw cross-compilation #2983
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
bluca
reviewed
Mar 12, 2018
| set_target_properties (libzmq-static PROPERTIES | ||
| PUBLIC_HEADER "${public_headers}" | ||
| COMPILE_DEFINITIONS "ZMQ_STATIC" | ||
| OUTPUT_NAME "zmq" | ||
| PREFIX "lib") | ||
| endif() | ||
|
|
||
| target_include_directories (objects |
There was a problem hiding this comment.
It's moved up to where the add_library(objects ... line is.
sigiesec
reviewed
Mar 12, 2018
tests/CMakeLists.txt
Outdated
| @@ -192,6 +194,7 @@ foreach(test ${tests}) | |||
| 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.
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.
Thanks, I'll have a look at PUBLIC.
There was a problem hiding this comment.
Nice! PUBLIC is a very handy feature.
sigiesec
reviewed
Mar 12, 2018
unittests/CMakeLists.txt
Outdated
| @@ -33,6 +34,7 @@ foreach(test ${unittests}) | |||
|
|
|||
| target_link_libraries(${test} libzmq-static ${OPTIONAL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} unity) | |||
|
|
|||
| set_target_properties(${test} PROPERTIES COMPILE_DEFINITIONS "${libzmq_DEFS}" ) | |||
Solution: remove objects optimisation in library build (similar to zeromq#2860) and set PUBLIC compile definitions on all static builds instead of MSVC only.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.