Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[question] CMakeToolchain: Support for find_library(), find_path(), find_file(), find_program() #9427

Closed
1 task done
SpaceIm opened this issue Aug 14, 2021 · 9 comments · Fixed by #10186
Closed
1 task done
Milestone

Comments

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 14, 2021

Do you plan to add proper packages paths in CMakeToolchain to support:

  • find_library() of libraries in host context (add lib_paths of host context packages in CMAKE_LIBRARY_PATH?)
  • find_path() and find_file() of headers in host context (add include_paths of host context packages in CMAKE_INCLUDE_PATH?)
  • find_program() of programs in build context (add bin_paths of build context packages in CMAKE_PROGRAM_PATH?)

Without this support, CMakeToolchain is unusable when:

  • you package a CMake project which must rely on find_library() or find_path() to discover dependencies without official CMake Find/config files or pkgconfig files.
  • you want to use build requirements in a transparent CMake integration (virtualrunenv generator is a workaround, but I find it very hard and unconvenient to use in some IDEs).
@memsharded
Copy link
Member

Hi @SpaceIm

Quick question, I guess it will be necessary to add the full paths to the "bin", "lib" subdirectories inside the packages, and it will not be enough to add the root package folders to CMAKE_LIBRARY_PATH et al?

This would feel a bit in the wrong place for CMakeToolchain, recall:

  • Toolchains are logic about the current configuration: mapping Conan settings, options, etc, of the current package to the native build system syntax:
  • XXXDeps are logic about dependencies: where to find them, locations of files, etc.

So this functionality seems to belong in the Deps objects. Now, the current CMakeDeps is focused on creating the find_package() functionality, but we didn't consider so far the use case that you are exposing (neither the previous generators, this would be a completely new feature, isnt it?).

I think we should consider this, together with the mapping of the current cmake_paths functionality, how to use xxx-config.cmake files packaged inside packages, etc.

@memsharded memsharded added this to the 1.41 milestone Aug 14, 2021
@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 14, 2021

Quick question, I guess it will be necessary to add the full paths to the "bin", "lib" subdirectories inside the packages, and it will not be enough to add the root package folders to CMAKE_LIBRARY_PATH et al?

Maybe, but root package folder is more appropriate for CMAKE_PREFIX_PATH. See for example https://cmake.org/cmake/help/latest/command/find_path.html, they explain where find_path() scans files. Basically they look in ${CMAKE_PREFIX_PATH}/include and CMAKE_INCLUDE_PATH.
I haven't suggested to add root package in CMAKE_PREFIX_PATH, because you likely want find_program() to discover build requirements, and find_library(), find_path(), find_file() to discover requirements. If you have the same recipe in requirements and build requirements, you may find executable from host context rather than build context.

This would feel a bit in the wrong place for CMakeToolchain, recall:

  • Toolchains are logic about the current configuration: mapping Conan settings, options, etc, of the current package to the native build system syntax:
  • XXXDeps are logic about dependencies: where to find them, locations of files, etc.

I thought that CMakeToolchain was a replacement for cmake generator. But I see nothing wrong to have a new generator (well maybe too much complexity), it's just that in a concrete case, I would expect at the end to just have to pass the toolchain file, and my CMakeLists with find_package(), find_path(), find_file(), find_program() would just work, and properly use libs/headers from my requirements, and programs from my build requirements.

CMakeDeps generates config files (dropping Find modules files, which might not be a good idea, I'll open an issue with some edge cases based on expat recipe). And it can work out of the box with toolchain file, only because CMAKE_CURRENT_LIST_DIR is added to CMAKE_PREFIX_PATH in toolchain file, and config files are generated in the same folder than the toolchain file.
But for find_lib() etc, there is no file to generate, just to add proper paths to few global CMake variables. Now what would happen with a new generator, would it generate some cmake files defining those variables? If so, toolchain file would include them automatically? Don't you fear to make things too complex for consumers?

Or maybe I miss something, and there is some interaction between CMakeToolchain and CMakeDeps when files are generated? If consumers use both, it could automatically set CMAKE_LIBRARY_PATH etc in toolchain file. Maybe an option could allow to have finer grained control, well I don't know, I lack some knowledge of current design for toolchain and CMakeDeps.

So this functionality seems to belong in the Deps objects. Now, the current CMakeDeps is focused on creating the find_package() functionality, but we didn't consider so far the use case that you are exposing (neither the previous generators, this would be a completely new feature, isnt it?).

Currently, find_library(), find_file() etc can work with cmake generator (+conan_basic_setup()), which is an intrusive generator. This generator adds root package folder of all requirements (and build requirements?) to CMAKE_PREFIX_PATH.

@memsharded
Copy link
Member

One of the issues that trying to add that information will be the lack of multi-config support. That is inevitable as all the CMake find_xxx do not support multi-config (or generator expressions). This is one of the reasons it could partially work with the single-config cmake generator.

Conan CMakeDeps generator will generate files, the xxxx-data.cmake files with all the folder informations, one file for every dependency. Maybe it would make sense to leverage this information for this. But honestly, I don't know what would be the best approach here.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Dec 14, 2021

Maybe one more suggestion if #9427 (comment) is implemented: root package folders shouldn't be added to cpp_info.builddirs by default, or at least they shouldn't be added to CMAKE_PREFIX_PATH. Only others folders of cpp_info.builddirs and CMAKE_CURRENT_LIST_DIR should be added to CMAKE_PREFIX_PATH.
Why? To avoid to indirectly make executables of host context discoverable by find_program().

So to summarize, in conan_toolchain.cmake, I would populate:

  • CMAKE_PROGRAM_PATH with bin_paths of build requirements.
  • CMAKE_LIBRARY_PATH with lib_paths of requirements.
  • CMAKE_FRAMEWORK_PATH with framework_paths of requirements.
  • CMAKE_INCLUDE_PATH with include_paths of requirements (and maybe res_paths?).
  • CMAKE_PREFIX_PATH & CMAKE_MODULE_PATH with CMAKE_CURRENT_LIST_DIR and build_paths of requirements (without root packages folders, so I would recommend to remove root package folder from default value of builddirs, but carefully since it might break expected behavior of old generators. Or two new properties could be introduced: cmake_prefix_path & cmake_module_path and therefore CMAKE_PREFIX_PATH & CMAKE_MODULE_PATH would be populated with content of these properties from requirements instead of build_paths).
  • CMAKE_MODULE_PATH with build_paths of build requirements (without root packages folders as well, even if it's harmless here since CMAKE_MODULE_PATH has less side effects than CMAKE_PREFIX_PATH).
  • (and eventually $ENV{<VARIABLE>} with the same env variables defined by VirtualBuildEnv, maybe it could fix some issues with SIP on Macos when build requirements with shared dependencies are called at configure time like pkgconf).

Moreover conan should ensure either:

  • CMAKE_FIND_ROOT_PATH_MODE_PROGRAM, CMAKE_FIND_ROOT_PATH_MODE_LIBRARY, CMAKE_FIND_ROOT_PATH_MODE_INCLUDE, and CMAKE_FIND_ROOT_PATH_MODE_PACKAGE are set to BOTH (or NEVER) when cross-building.
    Indeed CMAKE_FIND_ROOT_PATH doesn't work well with conan paradigm (libs and tools packaged together), specially for cross-build.
  • Or maybe experiment with a mix of CMAKE_FIND_ROOT_PATH and CMAKE_IGNORE_PATH? => doesn't work
    I mean add:
    • CMAKE_CURRENT_LIST_DIR and root package folders of requirements and build requirements to CMAKE_FIND_ROOT_PATH
    • bin_paths of requirements and lib_paths, framework_paths, include_paths of build requirements to CMAKE_IGNORE_PATH

@jasal82
Copy link
Contributor

jasal82 commented Dec 15, 2021

I think you can't require that CMAKE_FIND_ROOT_PATH_MODE_* is always set to BOTH or NEVER. Often user toolchain files are used, which in many cases are not under user control. Also, conversely, setting CMAKE_FIND_ROOT_PATH_MODE_* to BOTH or NEVER can trigger unwanted side effects in builds that can't always be fixed because their CMakeLists cannot be modified (3rd party components). Whatever solution you find it should work with ONLY. Did you check if CMake prefixes CMAKE_PROGRAM_PATH and the others with the find root path as well if it is set or is that just the case for CMAKE_PREFIX_PATH and CMAKE_MODULE_PATH?

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Dec 15, 2021

CMAKE_MODULE_PATH is not affected by any CMAKE_FIND_ROOT_PATH_MODE_*. CMAKE_PREFIX_PATH is. I'll try to see for other variables.
If the issue is only on CMAKE_PREFIX_PATH, it may be sufficient to add the same content to CMAKE_FIND_ROOT_PATH but it's very important to not add root package folders.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Dec 16, 2021

I've tested:
The solution described in #9427 (comment) can be robust to CMAKE_FIND_ROOT_PATH_MODE_PACKAGE set to ONLY by populating CMAKE_FIND_ROOT_PATH with the content of CMAKE_PREFIX_PATH (CMAKE_CURRENT_LIST_DIR + build_paths, except root package folders).
But for CMAKE_FIND_ROOT_PATH_MODE_PROGRAM, CMAKE_FIND_ROOT_PATH_MODE_LIBRARY and CMAKE_FIND_ROOT_PATH_MODE_INCLUDE conan must ensure that the value is not ONLY:

if(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM STREQUAL "ONLY")
    set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM "BOTH")
endif()
if(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY STREQUAL "ONLY")
    set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY "BOTH")
endif()
if(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE STREQUAL "ONLY")
    set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE "BOTH")
endif()

Here is the problem, let's take CMAKE_FIND_ROOT_PATH_MODE_INCLUDE set to ONLY as an example:
find_file() doesn't look into CMAKE_INCLUDE_PATH (or CMAKE_PREFIX_PATH) in this case, it iterates in paths of CMAKE_FIND_ROOT_PATH, but not the paths themselves, only <path>/include.
What does it mean? It means that you must add the root package folder to CMAKE_FIND_ROOT_PATH. But by doing this you may also make tools (in <path>/bin) of host context discoverable, and they may be found by find_program() before executables of build context.
If you think a little bit about this, you'll see that you have no other choice than populating CMAKE_FIND_ROOT_PATH with root package folders of requirements AND build requirements. So you mix build & host context libs, executables, header files, nothing good can happen here in a cross-build scenario.

Actually CMAKE_FIND_ROOT_PATH_MODE_PROGRAM set to ONLY even breaks the discovery of Ninja.

@jasal82
Copy link
Contributor

jasal82 commented Dec 16, 2021

Actually CMAKE_FIND_ROOT_PATH_MODE_PROGRAM set to ONLY even breaks the discovery of Ninja.

That's only true if there is no build sysroot defined. In the cases I have in mind (Yocto SDK) Ninja would be part of the build sysroot which is also set in CMAKE_FIND_ROOT_PATH, together with the target sysroot. It's a question of where to pick the build context binaries from. Sometimes you want to use the tools installed in the host system (NEVER) and sometimes you want to explicitly select the ones in the SDK (ONLY) to have a consistent version and full isolation from the host environment. I think we should not mix up discovery of libraries/programs/packages provided by Conan with those located in the sysroots and I would also strongly advise against enforcing requirements on the find root path modes.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Dec 16, 2021

I think we should not mix up discovery of libraries/programs/packages provided by Conan with those located in the sysroots and I would also strongly advise against enforcing requirements on the find root path modes.

Well, I guess in this case you want to make "CMake sysroot" libs/programs discoverable before conan ones, that's all.
Order of precedence is something like this:

  • CMAKE_FIND_ROOT_PATH
  • CMAKE_SYSROOT
  • <PackageName>_ROOT
  • CMAKE_STAGING_PREFIX
  • <PackageName>_DIR and CMAKE_PREFIX_PATH (or CMAKE_MODULE_PATH for find_package() of Find files & include())
  • CMAKE_<PROGRAM/INCLUDE/LIBRARY/FRAMEWORK>_PATH
  • PATH/INCLUDE/LIB
  • CMAKE_SYSTEM_PREFIX_PATH

So yes maybe not a good idea to set CMAKE_FIND_ROOT_PATH in conan. But therefore there is no other choice than forcing CMAKE_FIND_ROOT_PATH_MODE_<PROGRAM/INCLUDE/LIBRARY/PACKAGE> to BOTH if they have been set to ONLY by another toolchain file or CMake itself.
If your SDK is properly configured and you require necessary dependencies from conan, find_*() will never grab host system libs/programs you don't want, because they are located in the very last paths checked by those commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants