Remove libcurl_vendor #64
Conversation
Instead rely on system provided version. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
|
| @@ -12,8 +12,7 @@ endif() | |||
|
|
|||
| find_package(ament_cmake_ros REQUIRED) | |||
| find_package(ament_index_cpp REQUIRED) | |||
| # TODO(wjwwood): remove libcurl_vendor and just use system curl when possible | |||
| find_package(libcurl_vendor REQUIRED) | |||
| find_package(CURL REQUIRED) | |||
There was a problem hiding this comment.
I'm fairly certain this is going to fail on Windows. At least, it did for me locally. I had to do the following:
> set CMAKE_PREFIX_PATH=C:\ProgramData\chocolatey\lib\curl\tools\curl-7.67.0-win64-mingw- Add
list(APPEND CMAKE_FIND_LIBRARY_SUFFIXES ".dll.a" ".a")right above thefind_package(CURL)line.
With both of those things, I was able to compile successfully. However, the tests failed because they couldn't then find the dll at runtime. That might be able to be solved by putting the environment hooks for Windows back in place, but I'm not entirely sure.
There was a problem hiding this comment.
I should also note that I got that list hint from: https://discourse.cmake.org/t/problem-with-findpackage-curl-required-and-precompiled-curl-7-68-0-for-windows/686/2
There was a problem hiding this comment.
Hmm, yeah adding C:\ProgramData\chocolatey\lib\curl\tools\curl-7.67.0-win64-mingw to PATH works as well. It's not very portable though, since the path changes with the version of curl (e.g. I have 7.77.0). I couldn't find any recommended solutions involving CMake online... maybe this is the motivation to keep our vendor package.
|
I don't have a good solution for Windows (or the time to iterate on this). |
|
Hi, i just ran into this issue here today. And changing to find_package(CURL REQUIRED) fixed it for me. Could a fix for windows just be a simple if-else here, so if windows find libcurl_vendor else find CURL? |
|
|
|
Ok will do that |
Instead rely on system provided version.
@clalancette I went ahead and did this anyways. I'll trigger CI on all platforms to see where we stand.