search

aws / aws-sdk-cpp

AWS SDK for C++
Apache License 2.0
1.98k stars 1.06k forks source link

CMake Config mode support for Curl #2717

Closed pr0g closed 9 months ago

pr0g commented 1 year ago

Describe the bug

Right now it does not seem possible to use a custom build of Curl built with CMake when it is installed in Config mode.

When building aws-sdk-cpp, CURL will be found in Config mode, but the include directories and library variables are not populated.

...
-- Http client: Curl
-- Found CURL: /path/to/build/lib/cmake/CURL/CURLConfig.cmake (found version "8.2.1-DEV")  
--   Curl include directory: 
--   Curl library: 
...

This is because in Config mode the variables ${<library>_INCLUDE_DIRS} and ${<library>_LIBRARIES} are not populated (in this case ${CURL_INCLUDE_DIRS} and ${CURL_LIBRARIES}).

There is a workaround for this (please see this experimental draft PR - https://github.com/aws/aws-sdk-cpp/pull/2718) where we use the CMake find_package command to find Curl, and then link against the Curl target in aws-sdk-cpp/src/aws-cpp-sdk-core/CMakeLists.txt 

# cmake/external_dependencies.cmake
...
  find_package(CURL)
  if(NOT CURL_FOUND)
      message(FATAL_ERROR "Could not find curl")
  else()
...
# aws-cpp-sdk-core/CMakeLists.txt
...
target_link_libraries(
    ${PROJECT_NAME} PRIVATE 
    ${PLATFORM_DEP_LIBS} ${CLIENT_LIBS} ${CRYPTO_LIBS}
    ${AWS_SDK_ADDITIONAL_LIBRARIES}
    CURL::libcurl) # <-- added
...

I realize this is a super hacky change and is not fit for purpose as is, but it proves that this can work with a fairly minor modification.

Expected Behavior

aws-sdk-cpp can be built when using a build of CURL in Config mode (see CMake Finding packages for details).

Current Behavior

Currently aws-sdk-cpp does not build when using a build of CURL in Config mode.

Reproduction Steps

To reproduce this issue (tested on macOS) you can use this simple CMake project:

# CMakeLists.txt

cmake_minimum_required(VERSION 3.22)

project(aws-cpp-sdk-external)

include(ExternalProject)
include(ProcessorCount)

ProcessorCount(CPU_COUNT)

# Custom build of OpenSSL
ExternalProject_Add(
  openssl
  GIT_REPOSITORY https://github.com/openssl/openssl.git
  GIT_TAG openssl-3.2.0-alpha2
  GIT_SHALLOW ON
  UPDATE_COMMAND ""
  PREFIX ${CMAKE_CURRENT_BINARY_DIR}
  BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/openssl/build/Debug
  CONFIGURE_COMMAND
    perl <SOURCE_DIR>/Configure no-docs no-tests no-legacy
    --prefix=${CMAKE_CURRENT_BINARY_DIR}
    --openssldir=ssl
  BUILD_COMMAND make -j ${CPU_COUNT}
  INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}
  INSTALL_COMMAND make install_sw)

# Custom build of Curl
ExternalProject_Add(
  curl
  GIT_REPOSITORY https://github.com/curl/curl.git
  GIT_TAG curl-8_2_1
  DEPENDS openssl
  PREFIX ${CMAKE_CURRENT_BINARY_DIR}
  BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/curl/build/Debug
  INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}
  CMAKE_ARGS -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
             -DOPENSSL_ROOT_DIR=<INSTALL_DIR>
  CMAKE_CACHE_ARGS -DCMAKE_DEBUG_POSTFIX:STRING=d)

# Custom build of aws-cpp-sdk
ExternalProject_Add(
  aws-cpp-sdk
  GIT_REPOSITORY https://github.com/aws/aws-sdk-cpp.git
  GIT_TAG 1.11.181
  DEPENDS curl
  PREFIX ${CMAKE_CURRENT_BINARY_DIR}
  BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/aws-cpp-sdk/build/Debug
  INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}
  CMAKE_ARGS -DCMAKE_BUILD_TYPE=Debug
             -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
             -DBUILD_ONLY=cognito-identity
             -DENABLE_TESTING=OFF
             -DAUTORUN_UNIT_TESTS=OFF
             -DFORCE_CURL=ON)

And then build with...

cmake -B build
cmake --build build

Possible Solution

Please see this experimental draft PR - https://github.com/aws/aws-sdk-cpp/pull/2718

This is by no means a full and proper solution but shows the general idea to start a discussion.

Additional Information/Context

I'd be very grateful to hear how this could be resolved. I'm happy to have a stab at making the above change more robust (handling scenarios where CURL shouldn't be linked etc...). Or maybe there is another approach where it is possible to populate the variables mentioned earlier with a CMake library in Config mode.

AWS CPP SDK version used

main

Compiler and Version used

Apple clang version 15.0.0 (clang-1500.0.40.1) (CMake 3.26.1)

Operating System and version

macOS Ventura 13.5.2

SergeyRyabinin commented 1 year ago

Hi @pr0g ,

thank you for submitting this issue. Last time I had a need to build with a custom built curl, I used the following argument:

-DCMAKE_PREFIX_PATH=$HOME/curl-install

I'm not a 100% expert in cmake, so maybe wrong here, is there a reason you are not able to use CMAKE_PREFIX_PATH when building the SDK? We will review your PR too.

Best regards, Sergey

pr0g commented 1 year ago

Hi @SergeyRyabinin,

Thank you for getting back to me so quickly about this.

Ah yes, the CMAKE_PREFIX_PATH usage is essentially what the ExternalProject_Add command is doing under the hood as far as I know. It is also possible to reproduce this issue by using CMAKE_PREFIX_PATH as you describe. To to do this, make a custom build of Curl (using CMake) and install it somewhere, and then run a command something like this:

# from aws-sdk-cpp root directory
cmake -B build -G Ninja -DCMAKE_PREFIX_PATH=path/to/curl/install \
  -DCMAKE_BUILD_TYPE=Debug -DBUILD_ONLY=cognito-identity \
  -DENABLE_TESTING=OFF -DAUTORUN_UNIT_TESTS=OFF \
  -DFORCE_CURL=ON
cmake --build build

The build fails at link time because the Curl libraries cannot be found (this is for the reason mentioned in the issue where ${CURL_INCLUDE_DIRS} and ${CURL_LIBRARIES} are not populated).

For completeness this is the CMakeLists.txt I used to build and install Curl

cmake_minimum_required(VERSION 3.22)

project(curl-external)

include(ExternalProject)
include(ProcessorCount)

ProcessorCount(CPU_COUNT)

ExternalProject_Add(
  openssl
  GIT_REPOSITORY https://github.com/openssl/openssl.git
  GIT_TAG openssl-3.2.0-alpha2
  GIT_SHALLOW ON
  UPDATE_COMMAND ""
  PREFIX ${CMAKE_CURRENT_BINARY_DIR}
  BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/openssl/build/Debug
  CONFIGURE_COMMAND
    perl <SOURCE_DIR>/Configure no-docs no-tests no-legacy
    --prefix=${CMAKE_CURRENT_BINARY_DIR}
    --openssldir=ssl
  BUILD_COMMAND make -j ${CPU_COUNT}
  INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}
  INSTALL_COMMAND make install_sw)

ExternalProject_Add(
  curl
  GIT_REPOSITORY https://github.com/curl/curl.git
  GIT_TAG curl-8_2_1
  DEPENDS openssl
  PREFIX ${CMAKE_CURRENT_BINARY_DIR}
  BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/curl/build/Debug
  INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}
  CMAKE_ARGS -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
             -DOPENSSL_ROOT_DIR=<INSTALL_DIR>
  CMAKE_CACHE_ARGS -DCMAKE_DEBUG_POSTFIX:STRING=d)

I just created a new directory for this and ran

cmake -B build -G Ninja
cmake --build build

And that build folder is what I pointed CMAKE_PREFIX_PATH at in the earlier command.

pr0g commented 1 year ago

Also for reference the issue manifests as this:

ld: Undefined symbols:
  _curl_easy_cleanup, referenced from:
      Aws::Http::CurlHandleContainer::~CurlHandleContainer() in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::DestroyCurlHandle(void*) in CurlHandleContainer.cpp.o
  _curl_easy_getinfo, referenced from:
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      ...
  _curl_easy_init, referenced from:
      Aws::Http::CurlHandleContainer::CreateCurlHandleInPool() in CurlHandleContainer.cpp.o
  _curl_easy_pause, referenced from:
      CurlProgressCallback(void*, long, long, long, long) in CurlHttpClient.cpp.o
      CurlProgressCallback(void*, long, long, long, long) in CurlHttpClient.cpp.o
  _curl_easy_perform, referenced from:
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
  _curl_easy_reset, referenced from:
      Aws::Http::CurlHandleContainer::ReleaseCurlHandle(void*) in CurlHandleContainer.cpp.o
  _curl_easy_setopt, referenced from:
      Aws::Http::CurlHandleContainer::ReleaseCurlHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      Aws::Http::CurlHandleContainer::SetDefaultOptionsOnHandle(void*) in CurlHandleContainer.cpp.o
      ...
  _curl_easy_strerror, referenced from:
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
  _curl_global_cleanup, referenced from:
      Aws::Http::CurlHttpClient::CleanupGlobalState() in CurlHttpClient.cpp.o
  _curl_global_init, referenced from:
      Aws::Http::CurlHttpClient::InitGlobalState() in CurlHttpClient.cpp.o
  _curl_slist_append, referenced from:
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
  _curl_slist_free_all, referenced from:
      Aws::Http::CurlHttpClient::MakeRequest(std::__1::shared_ptr<Aws::Http::HttpRequest> const&, Aws::Utils::RateLimits::RateLimiterInterface*, Aws::Utils::RateLimits::RateLimiterInterface*) const in CurlHttpClient.cpp.o
  _curl_version_info, referenced from:
      Aws::Http::CurlHttpClient::InitGlobalState() in CurlHttpClient.cpp.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
[335/338] Building CXX object generated/src/aws-cpp-sdk-iam/CMakeFiles/aws-cpp-sdk-iam.dir/ub_IAM.cpp.o
ninja: build stopped: subcommand failed.

With my quick hacky change (see https://github.com/aws/aws-sdk-cpp/pull/2718 and https://github.com/pr0g/aws-sdk-cpp/tree/experiment-curl-cmake) I am unblocked, but it would be really cool if this could be fixed upstream.

Thanks very much for your time!

pr0g commented 1 year ago

FYI this Stack Overflow question/answer seems to describe the same issue (not in the context of aws-sdk-cpp, but it's the same situation described above).

sbiscigl commented 10 months ago

Hey @pr0g i opened a PR that optionally falls back to using find_package if we cannot find the missing include and lib directories as mentioned, give it a look and let me know what you think, it should solve your issue as well

pr0g commented 10 months ago

Hey @pr0g i opened a PR that optionally falls back to using find_package if we cannot find the missing include and lib directories as mentioned, give it a look and let me know what you think, it should solve your issue as well

Change https://github.com/aws/aws-sdk-cpp/pull/2817 looks good to me! Thanks very much for adding support for this! 🥳 🎉

jmklix commented 9 months ago

@pr0g with the above PR merged does this work for you? If so we can close this issue

pr0g commented 9 months ago

Hey @jmklix, I haven't had a chance to test this out yet but I think it's safe to close for now. When I do get to it, if there's any issues I'll reopen with a repro. Thanks!

github-actions[bot] commented 9 months ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.