Skip to content

Conversation

@tonytheodore
Copy link
Contributor

Tested on both static/shared builds (untested runtime - no windows machine at hand).

make test on OSX native build has single failure 44 - test_many_sockets (Timeout) both before and after changes.

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
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, missed that

link_directories(${test} PRIVATE "${CMAKE_SOURCE_DIR}/../lib")
endif()
endif()
set_target_properties(${test} PROPERTIES COMPILE_DEFINITIONS "${libzmq_DEFS}" )
Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.


target_link_libraries(${test} libzmq-static ${OPTIONAL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} unity)

set_target_properties(${test} PROPERTIES COMPILE_DEFINITIONS "${libzmq_DEFS}" )
Copy link
Member

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.
@bluca bluca merged commit d2293da into zeromq:master Mar 12, 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.

3 participants