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] Major deviations from standard CMake Find Behaviour #12266

Closed
jpfeuffer opened this issue Oct 7, 2022 · 8 comments
Closed

[bug] Major deviations from standard CMake Find Behaviour #12266

jpfeuffer opened this issue Oct 7, 2022 · 8 comments
Assignees

Comments

@jpfeuffer
Copy link

Environment Details (include every applicable attribute)

  • Operating System+version: Ubuntu 20.04
  • Compiler+version: gcc9
  • Conan version: 1.53
  • Python version: 3.8

Steps to reproduce (Include if Applicable)

  • Build Qt5 with conan
  • Use e.g. IF (NOT Qt5Core_FOUND) message(FATAL_ERROR "not found") in your CMake code
  • Fail

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

not found

@jcar87 jcar87 self-assigned this Oct 7, 2022
@jpfeuffer
Copy link
Author

jpfeuffer commented Oct 8, 2022

The question is basically why you are in general deviating so much from standard CMake find modes when it gets to variables.
All the _FOUND variables are missing. All the _VERSION variables are missing. Why?
Ok the problem is that you are naming component-specific variables e.g., Qt5_Core_VARIABLE not the CMake standard Qt5Core_VARIABLE.
Also you are not exporting the usually always exported MAJOR/MINOR_VERSION variables for the packages.
https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-version-selection

@jpfeuffer jpfeuffer changed the title [bug] Qt5 generated FindQt5.cmake does not populate Qt5Component_FOUND variables [bug] Major deviations from standard CMake Find Behaviour Oct 8, 2022
@jcar87
Copy link
Contributor

jcar87 commented Oct 10, 2022

Hi @jpfeuffer - thanks for reporting this issue.

I have tried the following in a CMakeLists.txt:

find_package(Qt5 REQUIRED)

message("Qt5_FOUND: ${Qt5_FOUND}")
message("Qt5_VERSION: ${Qt5_VERSION}")

against a Qt installed from Conan Center with the CMakeDeps generator, and those two variables appear to be correctly defined.

Would you be able to provide more details about:

  • which Conan generator you are using (the current recommended are the newer CMakeToolchain and CMakeDeps)
  • How your find_package calls to locate Qt5 look like
  • Which specific variables you are expecting to be defined

Please note that we attempt to follow CMake guidelines and discourage the use of legacy variables defined as part of find_package altogether - and encourage the use of targets. To my knowledge, the targets defined by the Qt5 package in ConanCenter should be the same as the ones defined by a Qt distribution installed in a different way.

As for variables, I'm not entirely sure there is an actual standard with regards to naming variables in find modules or config files, or at least not one that is followed by everyone.
CMake documents these variables here: https://cmake.org/cmake/help/latest/manual/cmake-developer.7.html#standard-variable-names, and indeed they use the qualifier "standard", while at the same time discouraging their use in the last paragraph of that section.

Reading this document, for a package called Xxx, that has a component library Yy, CMake says this:

Xxx_Yy_LIBRARY
The path of library Yy provided by the module Xxx. Use this form when the module provides more than one library or where other modules may also provide a library of the same name.

So if Xxx = Qt5 and Yy = Core, would one not expect the variables to be of the form Qt5_Core_zzzz?

@jpfeuffer
Copy link
Author

jpfeuffer commented Oct 11, 2022

  • CMakeDeps
set(CONFIG_MODE "CONFIG")
set(QT_COMPONENTS Core Network Gui Sql)
find_package(Qt5 ${QT_MIN_VERSION} ${CONFIG_MODE} COMPONENTS ${QT_COMPONENTS} REQUIRED)

find_package(Boost ${CONFIG_MODE} COMPONENTS iostreams date_time regex)
  • e.g. Qt5Core_FOUND, Boost_MAJOR_VERSION, get_target_property(Qt5::WindowsIntegrationPlugin LOCATION). All these commands worked with my old CMake scripts (which relied on the libraries config files for some while) but do not work with your config files anymore.

Maybe Qt is wrong but they definitely do not follow this scheme:
https://github.com/mburakov/qt5/blob/master/qtbase/src/corelib/Qt5Config.cmake.in#L26

Also, you are not defining MAJOR MINOR VERSION (at least in Boost, whose config files did before. It is also recommended [not required] by CMake).

And you are missing the (IMPORTED)_LOCATION property in the targets. My CMake scripts do not work anymore. They depend on copying library files but this property is not there. THIS is actually a real blocker since I did not find a replacement or alternative property for this. For all others, I added ugly conditionals/parsed the versions myself.

@jpfeuffer
Copy link
Author

Any idea how I could proceed without the "LOCATION" properties while you decide on/implement a fix?
I.e. how can I find out in CMake where the library behind a corresponding target is located?

@jpfeuffer
Copy link
Author

After further investigation, you also miss the VERY important library type for your imported libraries!
How should CMake know how to propagate dependencies transitively without that? It makes a huge difference if you are linking privately to a static library or a shared library.

@Nerixyz
Copy link

Nerixyz commented Jan 28, 2023

I have tried the following in a CMakeLists.txt:

find_package(Qt5 REQUIRED)

message("Qt5_FOUND: ${Qt5_FOUND}")
message("Qt5_VERSION: ${Qt5_VERSION}")

This CMakeLists.txt would fail with a "regular" (installed through the Qt installer) installation with The Qt5 package requires at least one component. So Qt encourages the use of COMPONENTS ... in find_package. It's also mentioned in their documentation (e.g. QString).

A similar issue was mentioned for the boost package in conan-io/conan-center-index#11085.

@technic
Copy link

technic commented Aug 14, 2024

This is sad, I rely on Pkg_Component_FOUND variables to avoid duplicated calls to find_package()

@memsharded
Copy link
Member

After further investigation, you also miss the VERY important library type for your imported libraries!
How should CMake know how to propagate dependencies transitively without that? It makes a huge difference if you are linking privately to a static library or a shared library.

the CMakeDeps generator can control what libraries exists and what libraries are linked with the traits definitions and the Conan package_type definitions.

In any case, the new CMakeDeps generator (see https://docs.conan.io/2/incubating.html) will be creating always the right SHARED/STATIC/INTERFACE IMPORTED target library.

Also, to clarify it, it is perfectly fine to use the package xxxx-config.cmake files if they are inside the packages, see https://docs.conan.io/2/examples/tools/cmake/cmake_toolchain/use_package_config_cmake.html. It is only ConanCenter that is not packaging these files and requiring to define package_info() to ensure build-system portability and interoperability, not supporting only CMake.

So I am closing this ticket as solved by the new CMakeDeps generator, regarding the IMPORTED library types. Feedback about this generator is very appreciated, it will help to get it outside of "incubating" section.

For an updated discussion and possible features regarding the _FOUND variables, please follow this ticket: #17483. If anything changes, it is most likely that it will also be imported in the new incubating CMakeDeps generator. Thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants