Skip to content

[zeromq] Add sodium support and fix static build #4214

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 8, 2019
Merged

[zeromq] Add sodium support and fix static build #4214

merged 2 commits into from
Mar 8, 2019

Conversation

myd7349
Copy link
Contributor

@myd7349 myd7349 commented Sep 2, 2018

No description provided.

@myd7349 myd7349 mentioned this pull request Dec 17, 2018
Closed
@myd7349 myd7349 force-pushed the prs/libzmq-sodium-feature branch from 2440af1 to f8566ee Compare December 17, 2018 13:21
This was referenced Dec 18, 2018
Merged
Closed
@myd7349 myd7349 changed the title [zeromq] Add new feature [WIP][zeromq] Add new feature Dec 19, 2018
@myd7349
Copy link
Contributor Author

myd7349 commented Dec 19, 2018 •
edited by vicroms
Loading

Another issue related to a static library:

After installing zeromq with sodium feature via:

> C:\user\myd7349> vcpkg install zeromq[sodium]:x86-windows-static 

I tested it with a little CMake project: https://github.com/myd7349/Ongoing-Study/tree/master/c/ZeroMQ/zmqbench

> $ mkdir build
> $ cd build
> $ cmake .. -DVCPKG_TARGET_TRIPLET=x86-windows-static -DCMAKE_TOOLCHAIN_FILE=E:/vcpkg/scripts/buildsystems/vcpkg.cmake -DCMAKE_GENERATOR_PLATFORM=Win32
> $ cmake --build . --config Release
>libzmq-mt-s-4_3_1.lib(random.cpp.obj) : error LNK2001: unresolved external symbol _randombytes_close [C:\Users\myd7349\Desktop\Ongoing-Stud
y\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(random.cpp.obj) : error LNK2001: unresolved external symbol _sodium_init [C:\Users\myd7349\Desktop\Ongoing-Study\c\Ze
roMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(zmq_utils.cpp.obj) : error LNK2001: unresolved external symbol _crypto_scalarmult_base [C:\Users\myd7349\Desktop\Ongo
ing-Study\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(zmq_utils.cpp.obj) : error LNK2001: unresolved external symbol _crypto_box_keypair [C:\Users\myd7349\Desktop\Ongoing-
Study\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(curve_server.cpp.obj) : error LNK2001: unresolved external symbol _crypto_box_beforenm [C:\Users\myd7349\Desktop\Ongo
ing-Study\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(curve_server.cpp.obj) : error LNK2001: unresolved external symbol _crypto_secretbox_open [C:\Users\myd7349\Desktop\On
going-Study\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(curve_server.cpp.obj) : error LNK2001: unresolved external symbol _crypto_secretbox [C:\Users\myd7349\Desktop\Ongoing
-Study\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(curve_server.cpp.obj) : error LNK2001: unresolved external symbol _crypto_box_open [C:\Users\myd7349\Desktop\Ongoing-
Study\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(curve_server.cpp.obj) : error LNK2001: unresolved external symbol _randombytes [C:\Users\myd7349\Desktop\Ongoing-Stud
y\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(curve_server.cpp.obj) : error LNK2001: unresolved external symbol _crypto_box_afternm [C:\Users\myd7349\Desktop\Ongoi
ng-Study\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(curve_server.cpp.obj) : error LNK2001: unresolved external symbol _crypto_box [C:\Users\myd7349\Desktop\Ongoing-Study
\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]
libzmq-mt-s-4_3_1.lib(curve_client.cpp.obj) : error LNK2001: unresolved external symbol _crypto_box_open_afternm [C:\Users\myd7349\Desktop\
Ongoing-Study\c\ZeroMQ\zmqbench\build\zmqbench_server.vcxproj]

It is the same issue as #4312.

By the way, static build of nng[mbedtls] also has this issue.

@myd7349 myd7349 changed the title [WIP][zeromq] Add new feature [zeromq] Add new feature Dec 21, 2018
@myd7349 myd7349 force-pushed the prs/libzmq-sodium-feature branch from c639043 to 8f66067 Compare December 26, 2018 05:11
@myd7349 myd7349 force-pushed the prs/libzmq-sodium-feature branch from 8f66067 to 30821d2 Compare January 19, 2019 03:12
@myd7349 myd7349 changed the title [zeromq] Add new feature [zeromq] Add new feature and fix static build Jan 19, 2019
@myd7349 myd7349 force-pushed the prs/libzmq-sodium-feature branch from 30821d2 to 76fea31 Compare February 13, 2019 04:52
@myd7349 myd7349 changed the title [zeromq] Add new feature and fix static build [WIP][zeromq] Add sodium support and fix static build Feb 23, 2019
@LarryIII LarryIII added the wip label Mar 6, 2019
@vicroms
Copy link
Member

vicroms commented Mar 6, 2019

Hi @myd7349

Can you tell me what is the status of this PR?
As it currently stands, it causes no regressions ...but our CI doesn't test for non-default features 😅

@vicroms vicroms self-assigned this Mar 6, 2019
@myd7349
Copy link
Contributor Author

myd7349 commented Mar 7, 2019 •
edited
Loading

Hi! @vicroms

Since libsodium uses MSBuild instead of CMake, the sodium feature only works on Windows currently(#3627).

Another problem is about a static build of zeromq[sodium]. The users have to link to libsodium by themselves if they want to use this feature:

find_package(ZeroMQ CONFIG REQUIRED)
find_library(libsodium ...)

target_link_libraries(...)

I was considering to make a PR for libsodium port in vcpkg to migrate the build system from MSBuild to CMake. Once done, I am able to fix both of these two problems.

Except that, I am fine with removing the WIP tag, and maybe I can fix these two problems by another PR.

@myd7349 myd7349 changed the title [WIP][zeromq] Add sodium support and fix static build [zeromq] Add sodium support and fix static build Mar 7, 2019
@vicroms
Copy link
Member

vicroms commented Mar 8, 2019

Given our incremental philosopjy, I'm OK with merging it as it is, and creating a new PR to fix those two issues in the future.

Although, it would be good to first create a new issue to document them and not loose track of them.

@vicroms vicroms removed the wip label Mar 8, 2019
@vicroms vicroms merged commit adc005f into microsoft:master Mar 8, 2019
@myd7349 myd7349 deleted the prs/libzmq-sodium-feature branch March 8, 2019 10:04
@myd7349 myd7349 mentioned this pull request Mar 8, 2019
Merged
@myd7349
Copy link
Contributor Author

myd7349 commented Mar 8, 2019

@vicroms Thanks!

I have just created a PR for libsodium: #5600 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@vicroms vicroms

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@myd7349 @vicroms @LarryIII