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 <package>_INCLUDE_DIR not set in 1.51 #11862

Closed
jsallay opened this issue Aug 12, 2022 · 5 comments
Closed

[bug] CMakeDeps <package>_INCLUDE_DIR not set in 1.51 #11862

jsallay opened this issue Aug 12, 2022 · 5 comments
Assignees
Milestone

Comments

@jsallay
Copy link
Contributor

jsallay commented Aug 12, 2022

Environment Details (include every applicable attribute)

  • Operating System+version:Ubuntu 20.04 (docker)
  • Compiler+version: gcc9
  • Conan version: 1.51.2
  • Python version: 3.8.10

Steps to reproduce (Include if Applicable)

It looks like a bug was introduced starting in conan 1.51.0. I work with some projects that don't always use CMake targets. They will
sometimes refer to variables like <package>_INCLUDE_DIR. This hasn't caused any problems up until this point. Today, I upgraded from 1.49.0 to 1.51.2 and my builds stopped working. Digging through the generated CMake files (using the 2.0 CMakeDeps/ToolChain stuff), and using gmp as an example, gmp-config.cmake contains:

set(gmp_INCLUDE_DIRS ${gmp_INCLUDE_DIRS_RELEASE} )

In conan version 1.50.2, gmp-release-x86-64-data.cmake contains

set(gmp_INCLUDE_DIRS_RELEASE "${gmp_PACKAGE_FOLDER_RELEASE}/include")

Starting in version 1.51.0, the gmp_INCLUDE_DIRS_RELEASE is not set (in fact many variables are no longer set at the top level in the *-data.cmake files).

Note also that this is not a gmp recipe specific issue. I am seeing the same behaviour in all the recipes I using (most of which are straight from conan center index). I am guessing that this is a result of #11673 but I haven't looked at the code close enough to be sure.

Logs (Executed commands with output) (Include/Attach if Applicable)

@memsharded memsharded added this to the 1.51.3 milestone Aug 13, 2022
@jcar87
Copy link
Contributor

jcar87 commented Aug 15, 2022

Hi @jsallay - thank you for reporting this issue! We will investigate this as a possible regression and aim to have a fix in version 1.51.3.

I work with some projects that don't always use CMake targets

Out of curiosity, we'd be interested to learn about technical limitations that prevent migration to using imported targets. Targets are more versatile and cover cases that the variable-based approach are not always able to satisfy, and are currently favoured by the CMake maintainers as well. The variables are kept for compatibility purposes for older projects, while targets are the preferred approach.

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 15, 2022

Out of curiosity, we'd be interested to learn about technical limitations that prevent migration to using imported targets. Targets are more versatile and cover cases that the variable-based approach are not always able to satisfy, and are currently favoured by the CMake maintainers as well. The variables are kept for compatibility purposes for older projects, while targets are the preferred approach.

I can give 2 reasons more or less connected:

  • some Find modules files provided by Kitware don't have imported targets, only variables. So a package manager agnostic CMakeLists would not use an unofficial imported target in this case.
  • when you package an existing library, the idea is to not patch upstream build files forever (patches can be very big), and sometimes library maintainer doesn't want or can't use targets (because an old CMake version must be supported for example, so it means that the official Find module file in CMake module folder may not yet provide targets).

@jsallay
Copy link
Contributor Author

jsallay commented Aug 15, 2022

Thanks.

My use case is wrapping an existing project. The next version of the project is going to use meson instead of CMake, but for the time being I have a bunch of old CMake code to deal with. I already have a handful of patches implemented to make it build with conan. If I had to replace every instance of old-style CMake with targets, that would be a huge effort.

@czoido
Copy link
Contributor

czoido commented Aug 16, 2022

Closed by #11874

@AlexanderSerov
Copy link

One more example which hold us with variables. Imagine we have library Foo and library Dependency. Normally we use Dependency library imported target to link against Main lib target and looks fine with conan.
In our case we also need to copy header files from Dependency lib to public interface of Main lib install tree. In other words, we need supply headers private headers of Dependecy to public interface of Main.
Before conan, we do so as following:

install(
     DIRECTORY ${Dependency_INCLUDE_DIR}/
     DESTINATION ${Main_INSTALL_PREFIX}/include
)

And I can't find a way to do so using conan imported target although we don't use conan enough yet. Of course we can get include_dir property from target but I not sure it also conan way. So is it true there is no way to handle such case in "right" conan way except using variables?

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

7 participants