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

[bug] CMakeDeps creates targets with dependency cycles #15386

Open
valgur opened this issue Jan 4, 2024 · 4 comments
Open

[bug] CMakeDeps creates targets with dependency cycles #15386

valgur opened this issue Jan 4, 2024 · 4 comments

Comments

@valgur
Copy link
Contributor

valgur commented Jan 4, 2024

Environment details

  • Conan version: 2.0.16

Steps to reproduce

Build the conan-io/conan-center-index#19298 pull request without this patch: https://github.com/conan-io/conan-center-index/blob/c66c8508ebb45572645e8be9ada9d02dde9efa50/recipes/gdal/post_3.5.0/patches/3.7.3/2-allow-cycles-in-cmake-targets.patch

Logs

https://c3i.jfrog.io/c3i/misc-v2/logs/pr/19298/61-linux-gcc/gdal/3.5.3//248d2058404514fe8cc2a96075d6b66745ab0cea-build.txt

CMake Error at src/cmake/helpers/GdalDriverHelper.cmake:237 (foreach):
Maximum recursion depth of 1000 exceeded
Call Stack (most recent call first):
src/cmake/helpers/GdalDriverHelper.cmake:253 (gdal_target_interfaces)
src/cmake/helpers/GdalDriverHelper.cmake:253 (gdal_target_interfaces)
src/cmake/helpers/GdalDriverHelper.cmake:253 (gdal_target_interfaces)
src/cmake/helpers/GdalDriverHelper.cmake:253 (gdal_target_interfaces)
src/cmake/helpers/GdalDriverHelper.cmake:253 (gdal_target_interfaces)
src/cmake/helpers/GdalDriverHelper.cmake:253 (gdal_target_interfaces)
...

@valgur
Copy link
Contributor Author

valgur commented Jan 4, 2024

CMake copes with the dependency cycles, but any CMake scripts that try to process the targets are prone to choke on the dependency loops. I have been bitten by this previously when trying to integrate Conan with another domain-specific build tool as well.
It can be worked around, but could the cyclical deps be avoided somehow, maybe?

@valgur
Copy link
Contributor Author

valgur commented Jan 5, 2024

For more details, here's what the repeated target dependencies actually look like.

In the case of the GDAL recipe,

get_property(link_libraries TARGET PROJ::proj PROPERTY INTERFACE_LINK_LIBRARIES)

returns

$<$<CONFIG:Release>:>;$<$<CONFIG:Release>:CONAN_LIB::proj_PROJ_proj_proj_RELEASE>;PROJ::proj

as the INTERFACE_LINK_LIBRARIES value.

Note the repeated PROJ::proj at the end. This happens to all targets defined by CMakeDeps. Fortunately, the workaround is simple:

get_property(link_libraries TARGET ${target} PROPERTY INTERFACE_LINK_LIBRARIES)
list(REMOVE_ITEM link_libraries ${target})

However, this is not 100% reliable. In the rare case of a cyclical dependency between libraries, the common solution is to repeat one of the libraries, e.g. -lfoo -lbar -lfoo for libs foo and bar that depend on each other.

I hope this can be fixed in Conan itself instead of relying on hacks in the consuming CMake scripts.

@memsharded
Copy link
Member

Hi @valgur

I am trying to understand this issue, had a look to the gdal PR, but that is massive, I don't know enough about the specifics of gdal to understand it easily.

So it is not clear to me where the dependency cycles come from, what is PROJ::proj, etc. I thought that for such cycles using something like self.cpp_info.libs = ["foo", "bar", "foo"] could work. Would it be possible to reduce the issue to some simple conan new projects? Or at least something smaller than the gdal PR? That would really help.

@valgur
Copy link
Contributor Author

valgur commented Jan 8, 2024

Here's a minimal demo to reproduce the issue:

conanfile.txt

[requires]
fmt/10.2.1
libjpeg-turbo/3.0.1
[generators]
CMakeDeps

CMakeLists.txt

cmake_minimum_required(VERSION 3.15)
project(test CXX)

set(CMAKE_BUILD_TYPE Release)
set(CMAKE_PREFIX_PATH ${CMAKE_CURRENT_SOURCE_DIR})

find_package(fmt REQUIRED CONFIG)
find_package(libjpeg-turbo REQUIRED CONFIG)

get_property(link_libraries TARGET fmt::fmt PROPERTY INTERFACE_LINK_LIBRARIES)
message("fmt::fmt deps: ${link_libraries}")

get_property(link_libraries TARGET libjpeg-turbo::libjpeg-turbo PROPERTY INTERFACE_LINK_LIBRARIES)
message("libjpeg-turbo::libjpeg-turbo deps: ${link_libraries}")
get_property(link_libraries TARGET libjpeg-turbo::turbojpeg-static PROPERTY INTERFACE_LINK_LIBRARIES)
message("libjpeg-turbo::turbojpeg-static deps: ${link_libraries}")
$ conan install .
$ cmake .

Outputs

fmt::fmt deps: $<$<CONFIG:Release>:>;$<$<CONFIG:Release>:CONAN_LIB::fmt_fmt_fmt_fmt_RELEASE>;fmt::fmt
libjpeg-turbo::libjpeg-turbo deps: libjpeg-turbo::turbojpeg-static;libjpeg-turbo::jpeg-static
libjpeg-turbo::turbojpeg-static deps: $<$<CONFIG:Release>:>;$<$<CONFIG:Release>:CONAN_LIB::libjpeg-turbo_libjpeg-turbo_turbojpeg-static_turbojpeg_RELEASE>

That is, fmt::fmt target has a dependency on fmt::fmt, causing a dependency cycle.
This happens to all packages that do not make use of components. I included libjpeg-turbo as an example of a package that does.

This is caused by

########## AGGREGATED GLOBAL TARGET WITH THE COMPONENTS #####################
{%- for comp_variable_name, comp_target_name in components_names %}
set_property(TARGET {{root_target_name}} PROPERTY INTERFACE_LINK_LIBRARIES {{ comp_target_name }} APPEND)
{%- endfor %}

which gets translated to

    ########## AGGREGATED GLOBAL TARGET WITH THE COMPONENTS #####################
    set_property(TARGET fmt::fmt PROPERTY INTERFACE_LINK_LIBRARIES fmt::fmt APPEND)

in the case of fmt, for example.

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

No branches or pull requests

2 participants