Assume only config files are generated in CMakeDeps generator when none is specified

[jcar87]

Changelog: Bugfix: Fix case where CMakeDeps assumes a module dependency when transitive dependencies do not define cmake_find_mode and fallback to a config one.

Docs: omit

Fixes #11201

The CMakeDeps generator explicitly specifies the search mode (config or module) for transitive dependencies. It does so by inspecting the value of the cmake_find_mode property defined in package_info() in the individual packages, as well as whether or not it is generating a config or a module file. When a dependency specifies that both should be generated, the CMakeDeps generator will try to align modules to "depend" on modules, and configs to depend on configs.

The issue was that when a transitive dependency does not define a cmake_find_mode explicitly, it should be assumed to be config, and thus, the transitive find_dependency calls should explicitly say CONFIG as that's the only one available to be searched for. The generator was working correctly in this case (generating the config when cmake_find_mode was not specified), but when encoding the dependency find mode in consumers, it was defaulting to MODULE instead of CONFIG. This was only causing issues when the direct dependency was consumed as a module instead of a config (as seen here)

A test has been modified to cover the case when a consumer depends on package B which depends on package A, and package B is consumed from a module when package A only generates a config.

[CLAassistant]

All committers have signed the CLA.

[jcar87]

This is the case that fails, that is fixed by the PR. It roughly replicates the issue from conan-io/conan-center-index#10698.

When:

• Consumer has find_package(MYPKGB) and MODULES are searched first by default (this either by specifying it in the find_package call, or by setting CMAKE_FIND_PACKAGE_PREFER_CONFIG to OFF (otherwise the CMakeToolchain generator sets it to ON).

• MYPKGB depends on MYPKGA

• MYPKGA only provides a config because it does not specify cmake_find_mode at all (when it does, it all works fine).

[memsharded]

Do you mean MYPKGB depends on MYPKGA? Seems typo, reversed.

[jcar87]

Oops, you're right. I've updated the comment !

[jcar87]

This is in line with the documentation.

Note that if the packages explicitly use the python None value like this (instead of a string):
self.cpp_info.set_property("cmake_find_mode", None)

it would be mapped to "config" with this change - I wonder if we need to protect ourselves against that case, or if we should always assume that the property is always a string if defined.

[lasote]

I don't think so. We try not to be overprotective, we end with a massive amount of checks that eventually make the things even worse 😓 So I wouldn't add a check here and maybe something to the documentation saying, please note that "none" has to be a string and not a None value that means "unset".

[paulharris]

Is "unset" a valid setting? If so then I'd like to suggest to not allow "None" or any non-strings, and have "unset" an explicit setting that someone can see in the code and documentation.

If the parameter should be a string, then surely that should be a pre-assert?
You have to do manual checks when using a language that isn't statically typed ... that is the penalty of the "convenience" of a language like Python.

Especially if the consequence of someone typing None instead of "None" is several hours/days of debugging and struggling (surprise = bad), instead of a simple error message.

[franramirez688]

I would update/review the docstring with the new issue solved by this PR.

[franramirez688]

In my opinion, I prefer to see all the inputs together instead of saving them in a variable (when the list of inputs is small enough), I think it's easier to read the test.

Suggested change

	@pytest.mark.parametrize("find_mode_PKGA, find_mode_PKGB, find_mode_consumer", find_modes)
	@pytest.mark.parametrize("find_mode_PKGA, find_mode_PKGB, find_mode_consumer", [
	("both", "both", ""),
	("config", "config", ""),
	("module", "module", ""),
	("both", None, ""),
	("both", None, "MODULE")
	])

[paulharris]

Sorry, about all the noise, I've cleaned up the PRs and bugreports.

This merged PR only goes half way to fixing the problems related to module-config confusions.

Test case: conan-io/conan-center-index#10825
Discussion on the reasons behind the bug: #11201
My proposed fix: #11217

Shortest description of further problem:
conan-io/conan-center-index#10825 (comment)