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.

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

Always build libcurl as shared library #12

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Always build libcurl as shared library #12

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmake/dependencies.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ include(FetchContent)
include(cmake/dependencies/kdutils.cmake)

if (BUILD_INTEGRATION_CURL)
# Ignore BUILD_SHARED_LIBS and build libcurl as shared library
Copy link
Contributor

Choose a reason for hiding this comment

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

Building curl as static library would be beneficial for easier deployment of the real application. Perhaps we could do this override only when tests are being built? And add a comment that it's due to faking the curl library for automated tests.

And btw. msvc is still not too happy about it:

[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_easy_init already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_easy_setopt already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_easy_cleanup already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_easy_getinfo already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_global_init already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_multi_init already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_multi_add_handle already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_multi_remove_handle already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_multi_info_read already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_multi_socket_action already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]
[build] libcurl-d_imp.lib(libcurl-d.dll) : error LNK2005: curl_multi_setopt already defined in tst_network_access_manager.obj [C:\kdab\rnd\mecaps\code\mecaps\build\src\network_access_manager\test_network_access_manager.vcxproj]

But let's not yet fix it here. I hope I'll be able to sit down to fixing some windows issues later this week. Possibly we'll be able to get away with /FORCE:MULTIPLE linker flag. We'll see

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Regarding the linker error using MSVC: that is indeed a bummer and sth I wouldn't have expected (at least from the conclusions I had drawn after my investigation).
I noticed that the linker does not throw an error and only throws these kind of warnings warning C4273: 'curl_global_init': inconsistent dll linkage on your latest GitHub actions windows build.
If I were to make a guess, I'd guess error LNK2005 occurred building on your local machine with the find_package() call in the first line of curl.cmake finding a shared libcurl on your machine and using this to link - even though you just build a static libcurl (at least I ran into this issue when testing the commit on linux).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, which build that succeeded you had in mind?

I took a stab at the problem again but couldn't make it work even with /FORCE:MULTIPLE. The network access manager library is always calling the original curl dll and not the mocked symbols. This approach probably won't fly on Windows.

set(BUILD_SHARED_LIBS_OLD ${BUILD_SHARED_LIBS})
set(BUILD_SHARED_LIBS ON)
include(cmake/dependencies/curl.cmake)
set(BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS_OLD})
endif()

if (BUILD_INTEGRATION_MQTT)
Expand Down