[feature] Set CMAKE_FIND_PACKAGE_PREFER_CONFIG in conan_toolchain.cmake

[mpusz]

Please consider setting CMAKE_FIND_PACKAGE_PREFER_CONFIG in the conan_toolchain.cmake. This will allow CMake to prefer configuration files generated by CMakeDeps over the ones provided with CMake release (i.e. FindGTest.cmake).

• I've read the CONTRIBUTING guide.

[memsharded]

I think it makes sense, could you please have a look and propose a PR if looks good to you @SSE4 ?

[SSE4]

do we just need this one variable? sounds narrow, maybe we should allow arbitrary variables to be defined in toolchain instead?

[memsharded]

https://cmake.org/cmake/help/git-stage/variable/CMAKE_FIND_PACKAGE_PREFER_CONFIG.html

I read this one as special, it is a paradigm, that goes together with the xxx-config.cmake files that CMakeDeps is generating. It makes sense that the toolchain defines that it wants Config files and not Modules, doesn't it?

[SSE4]

https://cmake.org/cmake/help/git-stage/variable/CMAKE_FIND_PACKAGE_PREFER_CONFIG.html

I read this one as special, it is a paradigm, that goes together with the xxx-config.cmake files that CMakeDeps is generating. It makes sense that the toolchain defines that it wants Config files and not Modules, doesn't it?

I am not sure to be honest. there are so many CMake variables which alter CMake behavior in one way or another. I don't see why just CMAKE_FIND_PACKAGE_PREFER_CONFIG should be exceptional.

and okay, we may set, but how would users specify its value? is the following enough?

def generate(self):
    tc = CMakeToolchain(self)
    tc.definitions["CMAKE_FIND_PACKAGE_PREFER_CONFIG"] = some_value
    tc.generate()

[memsharded]

I understand this should be a default, if our toolchain is going to use Config files. Every recipe should define it (it is a boolean, IIUC, so it would be setting it to ON). There are other CMake variables that the toolchain is defining like

            set(CMAKE_EXPORT_NO_PACKAGE_REGISTRY ON)
            # To support the cmake_find_package generators
            {% if cmake_module_path -%}
            set(CMAKE_MODULE_PATH {{ cmake_module_path }} ${CMAKE_MODULE_PATH})
            {%- endif %}
            {% if cmake_prefix_path -%}
            set(CMAKE_PREFIX_PATH {{ cmake_prefix_path }} ${CMAKE_PREFIX_PATH})

We could do the counter argument and say that all recipes should do:

def generate(self):
    tc = CMakeToolchain(self)
    tc.definitions["CMAKE_FIND_PACKAGE_PREFER_CONFIG"] = some_value
    tc.definitions["CMAKE_MODULE_PATH "] = self.install_folder
    tc.definitions["CMAKE_PREFIX_PATH "] = self.install_folder
    tc.generate()

But that seems too verbose, and it would make impossible to use the simple generators = "CMakeToolchain" syntax.

[SSE4]

so, if we decide to always define it, how do we pass the value to the CMakeToolchain, should ctor accept new argument like

tc = CMakeToolchain(self, cmake_find_package_prefer_config=some_value)

or should it be just hard-coded to ON/OFF/whatever in toolchain?
in that case, how would users change this value if they want different priority?

I'd say it's similar to CMAKE_FIND_ROOT_PATH_MODE_INCLUDE (and friends), which we may set, but how to override it?

[solvingj]

Again, the first fundamental question is, is this a flag that should be defined by "conan", the "recipe author" or by the "caller of the CLI", or all three with precedence:

• Conan programmatic/generator default
• Author can override default in constructor
• Caller can override default with [conf]

I definitely agree that this is "part of a paradigm" where Conan should take a strong position on the default. It seems like this could REALLY help us provide better handling of this problem (upstream findxxx.cmake and conan's generated files) which has been discussed at-length for years. I am actually shocked that this didn't come up until now.

[solvingj]

Ok, i see it's new as of Cmake 3.15

[memsharded]

@SSE4 no need to add anything to the class, just hardcode the set(CMAKE_FIND_PACKAGE_PREFER_CONFIG ON) in the toolchain.

https://cmake.org/cmake/help/git-stage/variable/CMAKE_FIND_PACKAGE_PREFER_CONFIG.html

Given that the whole proposed CMake integration CMakeDeps is based in user generated xxxConfig.cmake files, it makes sense that our own toolchain prefers them by default.

If some user wants to disable that to prefer old modules, they can just change that value in their CMakeLists.txt, before calling the find_package(), or they can do find_package(XXX MODULE REQUIRED), I mean they have mechanisms to not be blocked, and we can consider adding the opt-out later.

[SSE4]

@memsharded okay, I've added it, as seems like you already have a confidence this is a good thing, so I trust your opinion.

just wanted to make sure my voice is heard, as I have some concerns:

• the flag significantly alters CMake's behavior, so it's different from default. it might be surprising, misleading and confusing for consumers.
• you mentioned CMakeDeps generator, but the changes are for different one - CMakeToolchain. probably, you have a feeling that these two generators will be used together frequently, but I guess we don't have enough evidence of that just yet, as both are still experimental.
• I don't know for sure, maybe @solvingj can confirm, but I guess many companies already have their own written FindSomeSDK.cmake scripts which they prefer to be used over conan-generated.
• we just pick one flag which is kinda random... probably, we should take a stop, and first define, what's the goal for CMakeToolchain? doc says These file will automatically manage the definition of cmake values according to current Conan settings. if it's the only goal, then CMAKE_FIND_PACKAGE_PREFER_CONFIG doesn't seem to be required for conan settings management. we probably should take a look at https://cmake.org/cmake/help/git-stage/manual/cmake-variables.7.html and understand what is vital for our goal, and everything else should be defined by consumer IMO. we also have an advantage listed: One of the advantages of using Conan toolchains is that they can help to achieve the exact same build with local development flows, than when the package is created in the cache.. that doesn't see to require CMAKE_FIND_PACKAGE_PREFER_CONFIG as well.

[memsharded]

Thanks @SSE4 I think those are valid concerns.

Please @mpusz if you have more compelling evidence why this should be added, please tell (like cases in which the current solution failed).

Team, please vote:

1. Add CMAKE_FIND_PACKAGE_PREFER_CONFIG=ON to CMakeToolchain, users can opt-out via their CMakeLists.txt
2. Add CMAKE_FIND_PACKAGE_PREFER_CONFIG=ON to CMakeToolchain, users can opt-out via CMakeToolchain code
3. Add CMAKE_FIND_PACKAGE_PREFER_CONFIG=ON as opt-in to CMakeToolchain
4. Do not add CMAKE_FIND_PACKAGE_PREFER_CONFIG=ON, users can opt-in in their CMakeLists.txt

[jgsogo]

My two cents. I agree with @SSE4 that this variable modifies CMake behavior substantially (like others we might consider) and I would prefer to have the full picture instead of adding them one by one. And, by default, I prefer to have the same defaults as CMake does unless it is needed for Conan to work properly.

IMHO, this CMAKE_FIND_PACKAGE_PREFER_CONFIG one is related not to the CMakeToolchain but to CMakeDeps generator. I would expect it to be an opt-in (explicit) in the recipes:

class Recipe:

    generators = 'CMakeDeps'

    def generate(self):
        # Because I know I'm using CMakeDeps I set it to TRUE
        tc = CMakeToolchain(self)
        tc.definitions["CMAKE_FIND_PACKAGE_PREFER_CONFIG"] = ON
        tc.generate()

... and/or a conf variable that the caller can define (wherever we can decide to use CMakeDeps generator: command-line, conanfile.txt, profiles,..).

[mpusz]

I agree with @SSE4 concerns as well. However, I believe that this variable was added to CMake in order to fix a long-standing issue with finding dependencies. If a user installs a custom package and provides a path to its configuration files (i.e. via CMAKE_INSTALL_PREFIX) this package should be found by default rather than the obsoleted system one. CMake cannot change the default behavior because they strive for long-time backward compatibility. However, I hope that at some point they will decide to do so with their policy mechanism.

With CMakeToolchain and CMakeDeps we can fix it right now. I would prefer to have it as a default but it does not have to be hardcoded. It can be a constructor parameter or any other way that allows to change it. Otherwise, we will always have to teach:

[memsharded]

So the opt-in is already there (note that it is called variables):

def generate(self):
        # Because I know I'm using CMakeDeps I set it to TRUE
        tc = CMakeToolchain(self)
        tc.variables["CMAKE_FIND_PACKAGE_PREFER_CONFIG"] = ON
        tc.generate()

But still, we need to educate users to (almost) always use this form, or change their CMakeLists to find_package(xxxx CONFIG)
We can define a [conf] tools.cmake:find_package_prefer_config=ON, but we still need to educate the users to activate it in their profiles for correct behavior in case some cmake system module is there.
I am fine with each approach, but both have pros and cons, this is why I am asking for votes.

[mpusz]

you mentioned CMakeDeps generator, but the changes are for different one - CMakeToolchain. probably, you have a feeling that these two generators will be used together frequently, but I guess we don't have enough evidence of that just yet, as both are still experimental.

BTW, do we have any important use cases to not use them together? I understand that separation of concerns is important and having the functionality divided into two different classes is great (for Conan developers and maintenance) but I not sure if it helps users. I am teaching C++ for living and I am already providing slides saying to always write generate() like this:

    def generate(self):
        tc = CMakeToolchain(self)
        tc.variables["SOMETHING"] = ...
        tc.generate()
        deps = CMakeDeps(self)
        deps.configurations ....
        deps.generate()

and that forgetting about one of the above will end up with problems. From a teachability point of view, I do not think it is the right approach. The better would be something like:

    def generate(self):
        cmake = CMake(self)
        cmake.toolchain.variables["SOMETHING"] = ...
        cmake.deps.configurations ....
        cmake.generate()

Well, but this is probably a discussion for a totally different thread...

[memsharded]

@mpusz check the CMakeGen in #8534. It is basically what you suggest. We are doing first the "building blocks", make sure they work fine, are configurable enough, etc, and we will do higher level abstractions later.

[solvingj]

@mpusz I would also suggest in your teachings that you be sure to emphasize which things (like these generators) are still experimental. It's great to teach the "cutting edge", especially when it's clear that it's going to be the "new way" soon enough. But, I suggest highlighting that Conan is a project that evolves, and set the expectations properly.

[mpusz]

Sure, this is not only for teaching, but mostly to inform customers what to do now to ease the process of migration to Conan 2.0 when it comes.

[mpusz]

One additional point for the discussion here. Of course, we can do tc.variables["CMAKE_FIND_PACKAGE_PREFER_CONFIG"] = ON but in most cases it is not necessary. By this I mean, Conan package creators are often well aware of the Conan "issues" and they will use find_package(A CONFIG REQUIRED) in their CMake files anyway. It means that those users do not need this feature at all.

The people that actually need it are "common" customers (mostly C++ developers) that are not familiar with Conan details. They expect that if they provide conanfile.txt and then do find_package(B) in their CMake file everything will just work. But it is not the case with current defaults. Please also note that we have way more "common users" than package creators.

Any solution discussed above as an alternative to my proposal does not address the "common" user case as it is specific to conanfile.py file which most of the C++ developers will not write even once in their career.

So as I stated before, please consider making this behavior a default one with an option to disable it by advanced users if needed.