[feature] CMakeToolchain: remove CMAKE_FIND_PACKAGE_PREFER_CONFIG ON

[SpaceIm]

• [x] I've read the CONTRIBUTING guide.

CMAKE_FIND_PACKAGE_PREFER_CONFIG ON in CMakeToolchain originally addressed this problem:

• CMakeDeps is supposed to be able to work in conjunction with CMakeDeps (cmake_find_package & cmake_find_package_multi removed in conan v2).
• CMakeDeps was supposed to generate only CMake config files.
• by default CMake looks first for Find files, then config files in a call like find_package(foo) (without CONFIG or NO_MODULE).
• it would have been an issue for consumers/recipes depending on recipes packaging libraries listed in https://cmake.org/cmake/help/latest/manual/cmake-modules.7.html#find-modules, since those modules would have take precedence over config files generated by CMakeDeps. Indeed, no one in C++ ecosystem has ever written in its CMakeLists find_package(ZLIB REQUIRED CONFIG).

But now:

• CMakeDeps has learned generation of Find module files when a recipe set cmake_find_mode property to both in package_info().
• all CCI recipes listed in https://cmake.org/cmake/help/latest/manual/cmake-modules.7.html#find-modules have been updated so that CMakeDeps will always generate both Find & config files.

There is no more reason to keep CMAKE_FIND_PACKAGE_PREFER_CONFIG ON. it may have undesirable side effects. For example an agnostic CMakeLists looking for one of the files in https://cmake.org/cmake/help/latest/manual/cmake-modules.7.html#find-modules, may ends up with the config file which have different target names than the Find file (there are few cases like freetype and expat, which have built-in CMake modules files in CMake installation, and also provide config files. Target names are different but not the Find & config file names).

[memsharded]

CMakeDeps was supposed to generate only CMake config files. CMakeDeps has learned generation of Find module files when a recipe set cmake_find_mode property to both in package_info().

I think the problem with removing this is that the config.cmake files are still preferred by Conan in the majority of cases. The find modules are generated for compatibility and some cases, but Conan should prefer and use by default the config.cmake ones, not the modules. And if we remove this, then Conan will start to use the find modules, right?

@mpusz can you please check? You were promoting this one.

[SpaceIm]

The find modules are generated for compatibility and some cases, but Conan should prefer and use by default the config.cmake ones, not the modules. And if we remove this, then Conan will start to use the find modules, right?

CMakeDeps doesn't generate Find module files if the related dependency has not cmake_find_mode set to both in its package_info() (very few recipes have it, it's not the default value). It means that even without CMAKE_FIND_PACKAGE_PREFER_CONFIG ON, for most dependencies (all that are not listed in https://cmake.org/cmake/help/latest/manual/cmake-modules.7.html#find-modules) a find_package(foo) (even without CONFIG) will find a config file.

Moreover it's neat to keep CMake state alteration through CMakeToolchain to the bare minimum (principle of least surprise).

[SpaceIm]

Here is an example:

I want to create a recipe foo (for CCI or my company) for a library depending on expat

In foo CMakeLists:

...
find_package(EXPAT REQUIRED)
target_link_libraries(foo PRIVATE EXPAT::EXPAT)
...

This CMakeLists is perfectly fine, I don't expect to patch it.

I'm a good boy and my foo recipe relies on CMakeToolchain & CMakeDeps:

When I run conan create I see that CMake configuration fails, it has found expat-config.cmake (due to CMAKE_FIND_PACKAGE_PREFER_CONFIG ON, find_package() has tried first to look at EXPATConfig.cmake, then expat-config.cmake), but target EXPAT::EXPAT doesn't exist.

What's going on? => expat official config file is expat-config.cmake but this config files provides expat::expat target, not EXPAT::EXPAT like https://cmake.org/cmake/help/latest/module/FindEXPAT.html

It's clear that foo author expected to find FindEXPAT.cmake, not expat-config.cmake, but CMAKE_FIND_PACKAGE_PREFER_CONFIG ON broke this expectation by introducing a non-default behavior in CMake.

[memsharded]

expat official config file is expat-config.cmake but this config files provides expat::expat target,

I don't fully understand it.

You are telling that expat official config file is expat-config.cmake and that it is providing an expat::expat target. Why is then EXPAT::EXPAT preferred, and why should it be the default? Don't we agree that config.cmake files are the modern ones and should be used when available? And you are saying that expat is providing those.

[SpaceIm]

Almost nobody uses expat::expat, most libraries still rely on https://cmake.org/cmake/help/latest/module/FindEXPAT.html, same for Freetype which is similar in its targets discrepancy. conan shouldn't force libraries which don't even know conan to change their CMakeLists to handle an issue which doesn't exist outside of conan.

Have you ever see a library depending on freetype using freetype target (from the config file, yes Freetype provides a config file)? No, you will always see Freetype::Freetype from the module file.

[memsharded]

But won't users using the expat official config.cmake module complaining about the opposite? They will say:

• I am using expat
• I am using the official config.cmake from expat
• CMake recommends to use the modern config.cmake instead of modules, it is the good modern approach
• I am using the expat::expat target
• Now Conan is forcing me to use the legacy find module syntax? Shouldn't it be the default?

I understand many recipes in the open source and ConanCenter will still have the syntax for the legacy expat find module, but I am facing this conversation with users using Conan in production, trying to do "the right, modern thing", like every day, too. (this is the reason I also summoned @mpusz)

[SpaceIm]

conan won't force anything. It's the opposite with my proposal, conan would allow you to write a good agnostic CMakeLists. If they want to use expat-config.cmake, they must write find_package(expat REQUIRED CONFIG), because it's what would work outside of conan.

[memsharded]

conan won't force anything. It's the opposite with my proposal, conan would allow you to write a good agnostic CMakeLists. If they want to use expat-config.cmake, they must write find_package(expat REQUIRED CONFIG), because it's what would work outside of conan.

Ok, yes, now I see what you mean, thanks for explaining!

[mpusz]

From your description, it seems that freetype and expat are clearly broken if they provide different CMake target names in CMake Find modules and their own config files. Shouldn't they be fixed?

By fixing them I mean both:

• config files in their own repos
• Conan recipes so it generates correct names in generated configuration files that are compatible with CMake find modules (if those should be really preferred).

[mpusz]

Nevertheless, it seems that with new support to generate config as well as module files by CMakeDeps it seems that CMAKE_FIND_PACKAGE_PREFER_CONFIG ON is no longer needed, indeed. Of course, assuming that all of the affected have (and will have in future) cmake_find_mode set to both set in package_info().

[mpusz]

However, please note that many corporations write custom find modules for some packages for legacy projects (at least two of my customers did it). In new projects they use Conan with config files. In such case cmake_find_mode will not be set to both and they will have to be aware to update the recipes by themselves. This also means that they must have their own Conan server and can not use ConanCenter at all which might be an issue for some.

[SpaceIm]

From your description, it seems that freetype and expat are clearly broken if they provide different CMake target names in CMake Find modules and their own config files. Shouldn't they be fixed?

By fixing them I mean both:

* config files in their own repos

* Conan recipes so it generates correct names in generated configuration files that are compatible with CMake find modules (if those should be really preferred).

Regarding expat, it seems that upstream author doesn't care: https://github.com/libexpat/libexpat/issues/407

I wouldn't say they are broken, nothing prevents a library to provide other names in their config files than built-in Find modules.

Nevertheless, it seems that with new support to generate config as well as module files by CMakeDeps it seems that CMAKE_FIND_PACKAGE_PREFER_CONFIG ON is no longer needed, indeed. Of course, assuming that all of the affected have (and will have in future) cmake_find_mode set to both set in package_info().

Almost all related CCI recipes (bzip2, expat, freetype, fontconfig, gdal, glew, giflib, icu, jasper, libalsa, libcurl, libarchive, libgettext, libjpeg, libpng, libpq, libtiff, libxslt, odbc, openal, openssl, protobuf, sqlite3, xerces-c, xz_utils, zlib), have been updated to generate a Find module file (and the config file is still generated if needed).

However, please note that many corporations write custom find modules for some packages for legacy projects (at least two of my customers did it). In new projects they use Conan with config files. In such case cmake_find_mode will not be set to both and they will have to be aware to update the recipes by themselves. This also means that they must have their own Conan server and can not use ConanCenter at all which might be an issue for some.

I would suggest to change these custom Find module files to use a module file included with include() which wraps a find_package(foo CONFIG), then eventually fallback to a find_package(foo), then custom logic if not found, and at the end provides a common imported target in other CMakeLists files, that would be neater I think.

But nothing prevent someone to inject CMAKE_FIND_PACKAGE_PREFER_CONFIG ON externally if they still want this behavior (or keep the option in CMakeToolchain but turns tools.cmake.cmaketoolchain:find_package_prefer_config to False by default). Since it's not the default in CMake, it shouldn't be opt-out in conan.

However it's a good point, and it's worth noting that several CCI recipes also rely on current cmake_find_package generator to take precedence over some fragile & custom Find module files of upstream.

[SpaceIm]

Here is a discussion on Slack related to packaging of VTK, and highlighting how CMAKE_FIND_PACKAGE_PREFER_CONFIG ON can break a build: https://cpplang.slack.com/archives/C41CWV9HA/p1651818019947719

[paulharris]

I need to disable that flag for some things, but disabling the flag causes other issues in other contexts. See #11203

[paulharris]

Here is a discussion on Slack related to packaging of VTK, and highlighting how CMAKE_FIND_PACKAGE_PREFER_CONFIG ON can break a build: https://cpplang.slack.com/archives/C41CWV9HA/p1651818019947719

I've done some more work on this, and in the end I've patched VTK to just work with conan:

• find_package(Freetype) + target=FreeType::Freetype --> find_package(freetype) + target=freetype
• find_package(EXPAT) + target=EXPAT::EXPAT --> find_package(expat) + target=expat::expat

It is annoying that I can't use the namespaced targets with these libraries, almost feels like I'm using my Grandma's cmake imports! It also feels weird to use lowercase::lowercase targets. I wish there was some consistency, perhaps since we are breaking things, now is the time to try and enforce some sensible defaults within CCI ?

There are other libraries that also have no namespaced targets in their CONFIG finders, or are simply different to what VTK had expected (ie different to the official cmake for each library, or whatever Kitware cooked up).

I wonder who is "right", perhaps this is just a difference from VTK's custom finders, or this is a difference in the recipe.

I know Kitware includes a lot of customised Finders, I wonder if we should adopt some of their changes?

• utf8cpp only has target utf8cpp (no utf8cpp::utf8cpp)
• lz4 uses lz4::lz4 (not LZ4::LZ4 as before)
• LibPROJ + LibPROJ::LibPROJ is now proj + PROJ::proj (with no LibPROJ_MAJOR_VERSION definition)
• Eigen3::Eigen3 --> Eigen3::Eigen
• JsonCpp::JsonCpp --> jsoncpp::jsoncpp

At least they are consistently inconsistent...

[SpaceIm]

I know Kitware includes a lot of customised Finders, I wonder if we should adopt some of their changes?

No. Custom Find module files written in a library for its dependencies are not the truth, otherwise we would have to provide all the custom target names that all downstream libraries in C/C++ ecosystem have manually created for their dependencies. The only source of truth is the config file provided by the dependecy itself, or https://cmake.org/cmake/help/latest/manual/cmake-modules.7.html#find-modules for module files. If VTK build system doesn't work out of the box, you have to patch it, not modify upstream recipes.

[paulharris]

Ok, how about the library naming conventions within the conan-cmake ecosystem ?

eg, typically for vtk you would do find_package(VTK) target_link... VTK::Common https://cmake.org/cmake/help/latest/module/FindVTK.html

But if the goal is to prefer CONFIG style finders, then should it become find_package(vtk) and VTK::Common ? ie cmake_target_name = vtk ? or vtk::vtk ? or cmake_target_name = VTK::VTK ?

And do I have to generate a VTK::VTK target? VTK is only components, and yet a VTK::VTK target is still generated.

Looking for some guidance here... thanks :)

[SpaceIm]

Just follow upstream convention? I don't understand all these questions. It's even explicitly described in https://cmake.org/cmake/help/latest/module/FindVTK.html

FindVTK.cmake is the module file VTKConfig.cmake is the config file

So a vtk recipe should follow these names in package_info(), it's the only criteria.

[paulharris]

I've found one problem with conan+cmake is knowing when cmake has picked up conan's finder, and when it has found a system/cmake finder.

It would be good if we could disable cmake's search for system cmake files, and create a whitelist for the system finders that we WANT cmake to find. That might mean making another dozen+ recipes of "system" libraries, like threads/system and compiler/system and other things that come with cmake. Those cmakes could enable a "yes you can search the system this once" permission and find the target finder.

Or maybe we can at least augment the find_package() so it very specifically states (prints out) when it finds a conan cmake, and when it finds a system cmake. Then at least we can eyeball the build log looking for things that shouldn't be allowed. That might also be the opportunity to check against a short whitelist, rather than needing to write lots of extra conan system recipes.

[SpaceIm]

Example of fix required now in vulkan-loader while replacing cmake generator by CMakeToolchain: https://github.com/conan-io/conan-center-index/blob/801b43dbe507de353a068e5a41bca404d2d4e4fe/recipes/vulkan-loader/all/conanfile.py#L153-L158