Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions conan/tools/cmake/cmakedeps2/cmakedeps.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ def _content(self):
direct_deps.append((require, dep))
config = ConfigTemplate2(self, dep)
ret[config.filename] = config.content()
extra_file_names = (self.get_property("cmake_extra_file_names", dep, check_type=list)
or [])
old = self.get_cmake_filename(dep)
for f in extra_file_names:
filename = f"{f}-config.cmake" if f == f.lower() else f"{f}Config.cmake"
ret[filename] = f'include(CMakeFindDependencyMacro)\n' \
f"find_dependency({old})"
Comment on lines +76 to +79
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure the find_dependency() approach is the best one?
Why not:

  • Use a plain include(config.filename)?
  • Why not duplicating the contents of config.content() in each file?

I am a bit concerned the extra find_dependency() might have corner cases, it might kind of change the actual dependency graph of cmake files?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @jcar87 who suggested this approach

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think theres' a few conflicting use cases.

Currently we have a gap between foobar-config.cmake and the ability to locate it via find_package().

The CMake documentation for find_package states that:

In this mode, CMake searches for a file called <lowercasePackageName>-config.cmake or <PackageName>Config.cmake.

That is, if the package is called FooBar, FOOBAR, FooBAR FOOBar, CMake will find it correctly via foobar-config.cmake`, example:

find_package(FooBar)
find_package(FOOBAR)
find_package(FooBAR)
find_package(FOOBar)

all of the above are all satisfied by foobar-config.cmake, and CMake sets <name-you-searched-for>_FOUND) to `True.

However, that is does NOT currently work with CMakeConfigDeps, as we don't place the file in the search path, but define <name>_DIR instead.
The advantage of this approach is that we avoid the search altogether, so we minimise the risk of finding the wrong config file (e.g. when crossbuilding in some instances), the disadvantage is that we lose the ability to use names with different upper/lower case letters than the generated filename.

An example of this is libxml2-config.cmake (generated by the upstream project) which happily works with find_package(LibXml2) (using the name of the legacy built-in module), but does not work with CMakeConfigDeps (yet).

So a possible way to support this, would be for cmake_extra_file_names to ensure that every "alternative" name has a xxx_DIR entry in conan_cmakedeps_paths.cmake.
That would eliminate the need of generating additional files, on the condition that the file generated is all lowercase (<lowercaseName>-config.cmake) and the extra filenames are all variations that match the name but with alternative upper/lower case combinations. I would personally consider always generating the filenames with lower case but we do need to know the intended package name.

if we want to support generating files with different names altogether, that is, GTest and GoogleTest or similar - then yes, we would need to either:

  • generate multiple files (with full contents)
    or
  • have files like proposed in this PR that call find_package on the actual filename

I think in the past there were some cases in Conan Center where generating alternative names would have been convenient - however I'm not 100% sure this is justified unless it is to mirror what upstream does (and it is very unlikely that upstream generates the same package with different names) - so I wouldn't consider this unless we had very well motivated use cases. Having 2 different names for the same package is what caused most of the corner case headaches in CMakeDeps.

config_version = ConfigVersionTemplate2(self, dep)
ret[config_version.filename] = config_version.content()

Expand Down Expand Up @@ -257,6 +264,10 @@ def generate(self):
# If CMakeDeps generated, the folder is this one
# content.append(f'set({pkg_name}_ROOT "{gen_folder}")')
pkg_paths[pkg_name] = "${CMAKE_CURRENT_LIST_DIR}"
extra_file_names = (self._cmakedeps.get_property("cmake_extra_file_names", dep,
check_type=list) or [])
for f in extra_file_names:
pkg_paths[f] = pkg_paths[pkg_name]

# CMAKE_PROGRAM_PATH | CMAKE_LIBRARY_PATH | CMAKE_INCLUDE_PATH
cmake_program_path = self._get_cmake_paths([(req, dep) for req, dep in all_reqs if req.direct], "bindirs")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1572,3 +1572,47 @@ def build(self):
assert "find_package(matrix)" in c.out
assert "target_link_libraries(... matrix::matrix)" in c.out
assert "Conan: Target declared imported INTERFACE library 'matrix::matrix'" in c.out


@pytest.mark.tool("cmake", "3.27")
def test_dependendecy_multi_filename():
tc = TestClient()
conanfile = textwrap.dedent(f"""
from conan import ConanFile

class Pkg(ConanFile):
name = "pkg"
version = "0.1"
def package_info(self):
self.cpp_info.set_property("cmake_extra_file_names", ["foo", "extra_name"])
""")
cml = textwrap.dedent(f"""
cmake_minimum_required(VERSION 3.27)
project(app)
find_package(foo CONFIG REQUIRED)
if (foo_FOUND)
message("Found foo!")
if (TARGET foo::foo)
# This won't ever be found, because the target is pkg::pkg
# the default target is constructed from the conanfile name,
# not from the final cmake file name
message(FATAL_ERROR "Found target foo::foo!")
endif()

if (TARGET pkg::pkg)
message("Found original target pkg::pkg!")
endif()
endif()
""")
tc.save({"conanfile.py": conanfile,
"CMakeLists.txt": cml})
tc.run("create .")
tc.run("install --requires=pkg/0.1 -g CMakeDeps -g CMakeToolchain"
f" -c tools.cmake.cmakedeps:new={new_value}")
preset = "conan-default" if platform.system() == "Windows" else "conan-release"
tc.run_command(f"cmake --preset {preset}")
assert f"Found foo!" in tc.out
assert f"Found target foo::foo!" not in tc.out
assert "Found original target pkg::pkg!" in tc.out
assert "extra_name-config.cmake" in os.listdir(tc.current_folder)
assert "set(extra_name_DIR" in tc.load("conan_cmakedeps_paths.cmake")
Loading