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

Wrong package configuration file for aws-c-common #1695

Closed
2 tasks done
marco-ceriani opened this issue Jun 30, 2021 · 8 comments
Closed
2 tasks done

Wrong package configuration file for aws-c-common #1695

marco-ceriani opened this issue Jun 30, 2021 · 8 comments
Labels
bug This issue is a bug. Cmake Cmake related submissions p3 This is a minor priority issue

Comments

@marco-ceriani
Copy link

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug
In one of the products I work on I'm using the aws-encryption-sdk-c library to encrypt data, which is another library provided by AWS, which depends on the aws-sdk-cpp. Hence, I wrote a CMake file to build both libraries as shared libraries.
However, since a recent release, the aws-c-common is compiled as a static library, ignoring the variable BUILD_SHARED_LIB. This is fine for the compilation of aws-sdk-cpp. But when I try to use it inside aws-encryption-sdk-c, that build fails, with this message

[ 61%] Performing configure step for 'encryption_sdk'
CMake Error at <install_prefix>/aws-c-common/cmake/aws-c-common-config.cmake:8 (include):
  include could not find load file:

    <install_prefix>/aws-c-common/cmake/shared/aws-c-common-targets.cmake
Call Stack (most recent call first):
  CMakeLists.txt:38 (find_package)

I've looked at that file, and the issue is that it uses the variable BUILD_SHARED_LIBS to choose wether to include static/aws-c-common-targets.cmake or shared/aws-c-common-targets.cmake. But now aws-c-common is always built as a static library. Hence, the folder shared does not exist. Since the compilation of the common c libraries is now independent from the variable BUILD_SHARED_LIBS, then also the config.cmake should be modified accordingly.

SDK version number
1.9.39

Platform/OS/Hardware/Device
CentOS 7.9

To Reproduce (observed behavior)
Steps to reproduce the behavior (please share code)

Expected behavior
A clear and concise description of what you expected to happen.

@marco-ceriani marco-ceriani added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 30, 2021
@KaibaLopez
Copy link
Contributor

Hi @marco-ceriani ,
Can you share how you are trying to link the cpp sdk? It should be doing the linking correctly if you are using find_package() but it is kind of hard to see what is going on without knowing how it is being linked.

@KaibaLopez KaibaLopez added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jul 14, 2021
@KaibaLopez KaibaLopez self-assigned this Jul 14, 2021
@marco-ceriani
Copy link
Author

I'm building both aws-sdk-cpp and aws-encryption-sdk using a CMakeLIsts.txt with two external projects

ExternalProject_Add(aws-sdk-cpp
    PREFIX          ${SDK_ROOT}
    GIT_REPOSITORY  https://github.com/aws/aws-sdk-cpp.git
    GIT_TAG         ${AWS_SDK_VERSION}
    LIST_SEPARATOR  "|"
    CMAKE_ARGS      -DBUILD_ONLY=kms
                    -DBUILD_SHARED_LIBS=ON
                    -DENABLE_TESTING=OFF
                    -DENABLE_UNITY_BUILD=ON
                    -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
                    -DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
                    -DCMAKE_CXX_FLAGS=-Wno-stringop-overflow
    BUILD_COMMAND   make install
    TEST_COMMAND    ""
)

# Download Encryption SDK
ExternalProject_Add(encryption_sdk
    PREFIX          ${SDK_ROOT}
    GIT_REPOSITORY  https://github.com/aws/aws-encryption-sdk-c.git
    GIT_TAG         ${ENCRYPTION_SDK_VERSION}
    LIST_SEPARATOR  "|"
    CMAKE_ARGS      -DBUILD_SHARED_LIBS=ON
                    -DBUILD_DOC=${BUILD_DOC}
                    -DBUILD_AWS_ENC_SDK_CPP=ON
                    -DCMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH}
                    -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
                    -DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
    BUILD_COMMAND   make install ${DOC_TARGET}
    TEST_COMMAND    ""
)
add_dependencies(encryption_sdk aws-sdk-cpp)

I'm also doing other things to prepare a package with all the libraries, but it's irrelevant for this issue.

The complete build script for the aws-encryption-sdk is in their project. The piece that breaks down is indeed a find_package(aws-c-common CONFIG REQUIRED) at line 38.

list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

# Make sure we can pick up Cmake modules installed by dependencies
# both when they are in the CMAKE_INSTALL_PREFIX directory
# and in the CMAKE_PREFIX_PATH list.
list(APPEND CMAKE_MODULE_PATH "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/cmake")
foreach(prefix ${CMAKE_PREFIX_PATH})
    list(APPEND CMAKE_MODULE_PATH "${prefix}/${CMAKE_INSTALL_LIBDIR}/cmake")
endforeach(prefix)

# Including AwsSharedLibSetup depends on aws-c-common being installed already.
# Putting this ahead of that include line for a more meaningful error when missing.
find_package(aws-c-common CONFIG REQUIRED)

The build of aws-sdk-cpp completes correctly, and it produces the file <install_prefix>/lib/aws-c-common/cmake/aws-c-common-config.cmake, with this content

set(THREADS_PREFER_PTHREAD_FLAG ON)

if(WIN32 OR UNIX OR APPLE)
    find_package(Threads REQUIRED)
endif()

if (BUILD_SHARED_LIBS)
    include(${CMAKE_CURRENT_LIST_DIR}/shared/aws-c-common-targets.cmake)
else()
    include(${CMAKE_CURRENT_LIST_DIR}/static/aws-c-common-targets.cmake)
endif()

While building the other library, the variable BUILD_SHARED_LIBS is again ON, and I assume that this triggers the search on the path /shared/aws-c-common-targets.cmake that does not exist, since the aws-c-common library is now compiled statically.

I have two questions

  1. is it possible to use the find_package, if half the libraries are linked as dynamic objects and half as static ones, given that the same variable BUILD_SHARED_LIBS is used in all cmake files?
  2. I could try to build again aws-c-common as dynamic library. In that case, given that the cpp sdk will contain some of the functions of c-common, will there be issues when linking both libaws-cpp-sdk-core.so and libaws-c-common.so in my application?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Jul 20, 2021
@alex-chew
Copy link

Hi @KaibaLopez, any updates on this? I'm able to use a workaround on the aws-encryption-sdk-c side [1], but it's preferable if the aws-sdk-cpp fixes the root cause.

[1] aws/aws-encryption-sdk-c#719

@jmklix jmklix assigned jmklix and unassigned KaibaLopez Jul 15, 2022
@jmklix
Copy link
Member

jmklix commented Mar 8, 2023

We have been working on some updates to the build/configurations of this sdk. You can track the progress here

@jmklix jmklix added the p3 This is a minor priority issue label Mar 8, 2023
@jmklix jmklix removed their assignment Mar 8, 2023
@jmklix jmklix added the Cmake Cmake related submissions label Sep 18, 2023
@sbiscigl
Copy link
Contributor

Looks like this was fixed in aws-c-common/pull/924 now we fallback and try to load whatever library is present

# try to load the lib follow BUILD_SHARED_LIBS. Fall back if not exist.
if (BUILD_SHARED_LIBS)
    if (EXISTS "${CMAKE_CURRENT_LIST_DIR}/shared")
        aws_load_targets(shared)
    else()
        aws_load_targets(static)
    endif()
else()
    if (EXISTS "${CMAKE_CURRENT_LIST_DIR}/static")
        aws_load_targets(static)
    else()
        aws_load_targets(shared)
    endif()
endif()

with this fallback behavior it will now find the the any present lib, if you can replicate give a shout, but I do believe this to be fixed.

@marco-ceriani
Copy link
Author

Hello Sam.
Coincidentally I came back to update and recompile just recently. Currently we're not using the latest develop, but a version downloaded in late 2023. But since the pull was in 2022, it's included.

I didn't have any trouble building. I had no time to investigate if the relevant change was the pull 924 in aws-sdk-cpp, or some other change in the project aws-encryption-sdk-c. However, I'm still building the encryption SDK as dynamic library, like before, and can confirm that aws-c-common built as a static library is correctly consumed as dependency.

I'm in favour of closing the ticket, since my problem is solved (and the pull is as fine as it can be).

If you prefer a more in-depth analysis, I need some time, it's a busy period for at least a couple of weeks.

@sbiscigl
Copy link
Contributor

sbiscigl commented Feb 19, 2024

I'm in favour of closing the ticket, since my problem is solved (and the pull is as fine as it can be).
If you prefer a more in-depth analysis, I need some time, it's a busy period for at least a couple of weeks.

nope no need to do any investigation on your side, we can close it thanks for chiming in and sorry it took a while! if you start seeing anything like it again, give a shout.

@jmklix jmklix closed this as completed Feb 20, 2024
Copy link

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. Cmake Cmake related submissions p3 This is a minor priority issue
Projects
None yet
Development

No branches or pull requests

5 participants