conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.27k stars 981 forks source link

[bug] CMAKE_MODULE_PATH incorrectly set #6219

Closed sztomi closed 4 years ago

sztomi commented 4 years ago

Environment Details (include every applicable attribute)

Steps to reproduce (Include if Applicable)

We have a project that uses Qt and Qt quick. For this reason, we are relying on being able to access the Qt CMake macros. We do find_package(Qt) in our CMake script. We recently upgraded from 1.4.4 to 1.19. I noticed that in this version, I get the following error when using the msvc multi-config generator:

CMake Error at CMakeLists.txt:51 (find_package):
  By not providing "FindQt5.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "Qt5", but
  CMake did not find one.

  Could not find a package configuration file provided by "Qt5" with any of
  the following names:

    Qt5Config.cmake
    qt5-config.cmake

  Add the installation prefix of "Qt5" to CMAKE_PREFIX_PATH or set "Qt5_DIR"
  to a directory containing one of the above files.  If "Qt5" provides a
  separate development package or SDK, be sure it has been installed.

The relevant parts in our CMake:

set(QT_COMPONENTS Core Concurrent Qml Test Network WebChannel WebEngine Widgets Svg Xml)
find_package(Qt5 COMPONENTS ${QT_COMPONENTS} REQUIRED)
find_package(Qt5QuickCompiler)

Our tooling calls

$ conan install . -g cmake_multi -s build_type=Release ...
$ conan install . -g cmake_multi -s build_type=Debug  ...

Switching back to 1.4.4 immediately solves the issue, without any other changes. I upgraded to 1.20 and 1.21, but those also exhibit the same error.

Then, I went in and bisected the conan releases to identify when this broke, grabbing each version and rerunning each time (and I hope this buys me a beer πŸΊπŸ˜‚):

  1. 1.4.4 - good, 91 versions left
  2. 1.13.2 - good, 46 versions left
  3. 1.18.0 - bad, 23 versions left
  4. 1.15.1 - good, 12 versions left
  5. 1.16.1.dev1574067531 - bad, 6 versions left
  6. 1.15.4 - good, 3 versions left
  7. 1.15.5 - good, 1 version left
  8. 1.16.0 - bad

So this started happening in 1.16.0. Diff: https://github.com/conan-io/conan/compare/release/1.15.5...release/1.16.0

This looks like the culprit: https://github.com/conan-io/conan/compare/release/1.15.5...release/1.16.0#diff-4ace401e8414c1a6556986dc05a638d8R720

We are building RelWithDebInfo, but conanbuildinfo_release.cmake never sets CONAN_CMAKE_MODULE_PATH_RELWITHDEBINFO. I assume we would be expected to pass RelWithDebInfo as the build_type, but that's not possible for us. We are building our dependencies in Release configuration and only the projects that consume the deps are built with RelWithDebInfo - this is very intentional and preferred. We don't even have a RelWithDebInfo value for build_type in our settings (nor do we want to add this config and build everything with it). For now, I'm going to patch the generated conanbuildinfo.cmake to make this work again in our tooling, but it would be nice to get a parameter to set the CMAKE_BUILD_TYPE independently from the value of the conan build_type setting. That would also remove assumptions about the values and spellings of the settings used.

jgsogo commented 4 years ago

Hi @sztomi Thanks for the effort and time spent investigating the issue, I'm sure that you are invited to a beer if you come to ConanDays πŸ˜„

We try to keep things under control, and that's probably the underlying reason for having only available the variables for the configurations that are actually installed. I really think this should stay this way.

What you can set the value of the CMAKE_BUILD_TYPE yourself if you are using the CMake helper: you can access the cmake.definitions and modify whatever you need or you can even instantiate the CMake class with a different build_type (CMake(build_type="whatever")). Have you tried it?

sztomi commented 4 years ago

if you are using the CMake helper

We are doing this:

  def build(self):
    build_type = "RelWithDebInfo" if self.settings.build_type == "Release" else "Debug"
    cm = CMake(self, build_type=build_type, cmake_system_name=False, parallel=False)

Unfortunately this fails because we call

$ conan install . -g cmake_multi -s build_type=Release

If I instead I did this in the beginning:

$ conan install . -g cmake_multi -s build_type=RelWithDebInfo

I would get an error because RelWithDebInfo is an invalid value in our setup; furthermore, we don't build the dependencies in RelWithDebInfo. We only want to build the application consumes the (Release) deps as RelWithDebInfo.

jgsogo commented 4 years ago

Yes, I see it. You need the variable CONAN_CMAKE_MODULE_PATH_RELWITHDEBINFO populated with the values of CONAN_CMAKE_MODULE_PATH_RELEASE, which is the one you have available with the paths to your dependencies.

In the issue liked by memsharded there is a lot of interesting stuff and probably the serious conversation should be moved to that one. Here I can tell you about a workaround that might work for your use case: you can create yourself the variable you need (CONAN_CMAKE_MODULE_PATH_RELWITHDEBINFO or the CMake one) with the corresponding values:

  def build(self):
    build_type = "RelWithDebInfo" if self.settings.build_type == "Release" else "Debug"
    cm = CMake(self, build_type=build_type, cmake_system_name=False, parallel=False)
    if self.settings.build_type == "Release":
         cm.definitions["CONAN_CMAKE_MODULE_PATH_RELWITHDEBINFO"] = self.deps_cpp_info.build_paths

Other alternatives could be to write your own generator on top of the cmake_multi, but I don't think it would be cleaner than adding this line into the recipes.

sztomi commented 4 years ago

Thanks. Since I have a script that calls conan anyway, I simply did this (after conan, but before the CMake invocation):

  conanbuildinfo_multi = open("conanbuildinfo_multi.cmake").read()
  conanbuildinfo_multi = conanbuildinfo_multi.replace("_RELWITHDEBINFO", "_RELEASE")
  with open("conanbuildinfo_multi.cmake", "w") as fp:
    fp.write(conanbuildinfo_multi)
sztomi commented 4 years ago

After reading the linked issue, I think this is a duplicate of that one, so feel free to close it. I'll comment there.