-
Notifications
You must be signed in to change notification settings - Fork 3
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
Always build libcurl as shared library #12
base: main
Are you sure you want to change the base?
Conversation
Ignore BUILD_SHARED_LIBS and build libcurl as shared library. We cache current value of BUILD_SHARED_LIBS, set BUILD_SHARED_LIBS ON when including curl and reset BUILD_SHARED_LIBS to it's previous value afterwards.
@@ -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 |
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.
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
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.
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).
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.
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.
Ignore BUILD_SHARED_LIBS and always build libcurl as shared library. We cache current value of BUILD_SHARED_LIBS, set BUILD_SHARED_LIBS ON when including curl.cmake and reset BUILD_SHARED_LIBS to it's previous value after including curl.cmake.
@MiKom I tried to set BUILD_SHARED_LIBS for the curl target individually, using set_target_properties in curl.cmake but didn't succeed. My current solution does not appear very elegant OTOHS it is very unambiguous due to it's verbosity. :)