Use CURL::libcurl instead of variables #3030
Conversation
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
|
looks like it works on windows but not Ubuntu bionic
|
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
gazebo/common/CMakeLists.txt
Outdated
| @@ -237,7 +229,7 @@ target_compile_definitions(gazebo_common | |||
| target_link_libraries(gazebo_common | |||
| PUBLIC | |||
| ${Boost_LIBRARIES} | |||
| ${CURL_LIBRARIES} | |||
| CURL::libcurl | |||
There was a problem hiding this comment.
Not something introduced in this PR, but it seems that CURL is not used in public headers, so probably CURL::libcurl can be linked as a PRIVATE linked library, instead of a PUBLIC one.
There was a problem hiding this comment.
Made PRIVATE in 9057ef9
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
There was a problem hiding this comment.
I think most of the plugins don't need to link to curl; I only see 3 that should need it:
grep -rnI include\ .curl/ plugins
plugins/rest_web/RestUiLoginDialog.cc:19:#include <curl/curl.h>
plugins/rest_web/RestApi.cc:20:#include <curl/curl.h>
plugins/StaticMapPlugin.cc:18:#include <curl/curl.h>
In fact, I think we could remove the curl include from RestUiLoginDialog.cpp, since it doesn't actually use it for anything. I'll try testing some changes and suggest a patch
tools/CMakeLists.txt
Outdated
| @@ -53,6 +45,7 @@ if (WIN32) | |||
| endif() | |||
|
|
|||
| target_link_libraries(gz | |||
| CURL::libcurl | |||
There was a problem hiding this comment.
I don't see any usage of curl by gz; I would remove this line
I'm guessing the cmake code you've removed in this file is vestigial
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Only StaticMapPlugin and RestWebPlugin use CURL, so only add linking for these. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
|
I just pushed my changes directly; hope you don't mind |
|
All CI jobs appear to have built 馃帀 . I don't see any new CMake warnings. As for test failures, there are lots, but that doesn't seem unusual looking at the past jobs. Anything left to do before this can be merged? |
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org> Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Maybe this will fix the build failures in this job 馃し
https://build.osrfoundation.org/job/gazebo-ci-pr_any-windows7-amd64/ws/ws/
The trouble is
gazebo_commonis linking againstlibcurl.dllinstead oflibcurl_imp.lib. I have no idea why that happens. The exported CMake targets on Windows seem reasonable to me, so maybe using them will solve the issue.