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.

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

Update libcurl to version 7.75 and add SSL support #8463

Merged

Conversation

Nir-Az
Copy link
Collaborator

@Nir-Az Nir-Az commented Feb 28, 2021

  • Update libcurl third-party to version 7.75
  • Build libcurl with SSL
  • Change online versions DB link to https link

Tracked on [DSO-16463]

target_link_libraries(curl INTERFACE ws2_32.lib)
target_link_libraries(curl INTERFACE ws2_32.lib crypt32.lib)
else()
find_package(OpenSSL REQUIRED)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If that's required then it need to be propagated into the installation guide/scripts as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure we can explain about the online update flag when it's doing nothing for now.
The default is false and the DB is empty

@@ -37,13 +44,17 @@ if(CHECK_FOR_UPDATES)
add_definitions(-DCURL_STATICLIB) # Mandatory for building libcurl as static lib

target_include_directories(curl INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/libcurl/libcurl_install/include>)

target_link_libraries(curl INTERFACE debug ${CMAKE_CURRENT_BINARY_DIR}/libcurl/libcurl_install/lib/${CURL_DEBUG_TARGET_NAME}${CMAKE_STATIC_LIBRARY_SUFFIX})
Copy link
Collaborator

@ev-mp ev-mp Mar 3, 2021

Choose a reason for hiding this comment

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

I think the keyword should be changed to PRIVATE as they are not supposed to be exposed in the DLL interface.
Same for the below entries

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was built same as libusb (the keyword is not part of this PR), ifI try to change it to private I get lots of cmake errors like:

CMake Error at CMake/external_libcurl.cmake:43 (add_library):
Cannot find source file:

PRIVATE

Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm

@maloel / @ev-mp , please say how much effort should be made for changing to PRIVATE.

According to CMAKE , INTERFACE is not like PUBLIC.

Libraries following INTERFACE are appended to the link interface and are not used for linking .

Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Add DSO reference

@Nir-Az
Copy link
Collaborator Author

Nir-Az commented Mar 3, 2021

Add DSO reference

Added

@ev-mp ev-mp merged commit 056b928 into IntelRealSense:development Mar 3, 2021
@Nir-Az Nir-Az deleted the update_libcurl_to_use_https branch April 8, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants