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

Missing dependencies on CMake exported target #1384

Open
wnts opened this issue Apr 5, 2020 · 6 comments · May be fixed by #1401
Open

Missing dependencies on CMake exported target #1384

wnts opened this issue Apr 5, 2020 · 6 comments · May be fixed by #1401

Comments

@wnts
Copy link

wnts commented Apr 5, 2020

The cpprestsdk exported target created during the build process of the libraries
does not carry all required dependencies.
For one, the Boost and openSSL dependencies are not defined on the target.

Commit cb7ca74 seems to have broken this for openSSL, by always defaulting to pkg-config for searching OpenSSL.
The issue leading to that commit has been solved in the find_package module by kitware: https://gitlab.kitware.com/cmake/cmake/-/issues/16885

For Boost this seems to be disabled deliberately...

# FindBoost continually breaks imported targets whenever boost updates.
if(1)
target_include_directories(cpprestsdk_boost_internal INTERFACE "$<BUILD_INTERFACE:${Boost_INCLUDE_DIR}>")
set(_prev)
set(_libs)
foreach(_lib ${Boost_LIBRARIES})
if(_lib STREQUAL "optimized" OR _lib STREQUAL "debug")
else()
if(_prev STREQUAL "optimized")
list(APPEND _libs "$<$<NOT:$<CONFIG:Debug>>:${_lib}>")
elseif(_prev STREQUAL "debug")
list(APPEND _libs "$<$<CONFIG:Debug>:${_lib}>")
else()
list(APPEND _libs "${_lib}")
endif()
endif()
set(_prev "${_lib}")
endforeach()
if (NOT IOS OR NOT EXISTS "${PROJECT_SOURCE_DIR}/../Build_iOS/boost")
target_link_libraries(cpprestsdk_boost_internal INTERFACE "$<BUILD_INTERFACE:${_libs}>")
endif()
else()

Running git blame on this file shows that that comment on line 54 was made 3 years ago...
Is it still relevant?

I saw that at the time serious effort was put into using modern CMake, so it would be nice to actually be able to see that in action.

Also, because of the above, the simple CMakeLists.txt file listed on the Readme is not working.

@letrthong
Copy link

let us know version of cmake on your pc and version of cpprestsdk

@wnts
Copy link
Author

wnts commented Apr 6, 2020

let us know version of cmake on your pc and version of cpprestsdk

CMake 3.14
cpprestsdk: latest commit on master branch

@letrthong
Copy link

my pc: cmake version 3.10.2
cpprest is boost-1.72.0

Using cmd as below to switch to boost-1.72.0
git reset --hard
git fetch
git checkout boost-1.72.0
git submodule update --init
cd boost/libs/asio
git checkout boost-1.69.0

I can cross compiler and push lib at https://github.com/letrthong/orangepi_zero_release/tree/master/x32 with toolchain gcc-linaro-arm-linux/gcc-linaro-7.3.1-2018.05-x86_64_arm-linux-gnueabihf/bin/arm-linux-gnueabihf-g++

@garethsb
Copy link
Contributor

Resolved by #1383?

c72578 added a commit to c72578/cpprestsdk that referenced this issue May 2, 2020
…)"

This reverts commit cb7ca74.

Issues concerning OpenSSL dependency and linking (microsoft#1384, microsoft#1388)
and cross compiling (microsoft#1378) have been reported.

- Fixes microsoft#1384, microsoft#1388
c72578 added a commit to c72578/cpprestsdk that referenced this issue May 2, 2020
…)"

This reverts commit cb7ca74.

Issues concerning OpenSSL linking dependency (microsoft#1384, microsoft#1388)
and cross compiling (microsoft#1378) have been reported.

- Fixes microsoft#1384, microsoft#1388
@wnts
Copy link
Author

wnts commented May 17, 2020

Resolved by #1383?

Not quite. #1383 fixes only part of the problem: now Boost is searched for through the find_dependency macro calls, but it's still not present as a transitive dependency on the exported targets.

What I want to be able to do (taken directly from the project's README):

cmake_minimum_required(VERSION 3.9)
project(main)

find_package(cpprestsdk REQUIRED)

add_executable(main main.cpp)
target_link_libraries(main PRIVATE cpprestsdk::cpprest)

But this does not work. the cpprestsdk::cpprest imported target does not carry the transitive dependency on Boost::system and OpenSSL::crypto.

What I need to do now is:

cmake_minimum_required(VERSION 3.9)
project(main)

find_package(cpprestsdk REQUIRED)

add_executable(main main.cpp)
target_link_libraries(main PRIVATE cpprestsdk::cpprest Boost::system OpenSSL::Crypto)

@wnts
Copy link
Author

wnts commented May 17, 2020

The problem is that the targets imported by the calls to find_dependency in the package configuration file (Release/cmake/cpprestsdk-config.in.cmake) do not match the actual targets linked against in Release/src/CMakeLists.txt. The latter creates targets different from those created by the find_dependency macro, and are created by the cpprest_find_* wrapper macros in the Release/cmake/cpprest_find_*.cmake files.

For example, find_dependency(Boost COMPONENTS system date_time regex) uses the CMake supplied FindBoost module, which creates the following imported targets:

Boost::system
Boost::date_time
Boost::regex

But the macro cpprest_find_boost creates the target cpprestsdk_boost_internal, which is what added to the INTERFACE requirements of the cpprestsdk::cpprest target, which is ultimately exported in the package configuration file. The cpprestsdk_boost_internal target has itself transitive dependencies on the various discovered boost library components.

Now one would expect this would still work, (assuming that cprest_find_* and the find_dependency macros end up finding the same files and directories for the given library).
But the cpprest_find_boost creates the cpprestsdk_boost_internal target with a $<BUILD_INTERFACE:...> generator expression, which expands to the empty string when exported by install(EXPORT ...)

The problem described above also occurs with the other dependencies (zlib, openssl, etc)

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 a pull request may close this issue.

3 participants