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

Avoid setting CMAKE_FIND_ROOT_PATH_MODE_LIBRARY BOTH in CMakeToolchain #10513

Closed
puetzk opened this issue Feb 4, 2022 · 4 comments
Closed

Avoid setting CMAKE_FIND_ROOT_PATH_MODE_LIBRARY BOTH in CMakeToolchain #10513

puetzk opened this issue Feb 4, 2022 · 4 comments

Comments

@puetzk
Copy link

puetzk commented Feb 4, 2022

See also the thread started by by @SpaceIm in #10186 (comment)_
I didn't see this until looking at the 1.45 changelog, and I figure the review of a month-old merged PR is not best place to raise additional concerns.

setting CMAKE_FIND_ROOT_PATH_MODE_* to BOTH gives me a lot of heartburn. I get the argument that BOTH will still check the places that ONLY would have cheked first, so if ONLY would have succeeded, BOTH gives the same answer. But CMakeToolchain goes with CmakeDeps, which writes package-config.cmake files. So if I'm using find_library (or a FindLibFoo.cmake MODULE mode find_package), it's probably for something that does not come from a conan package. And I've been bitten quite a few times when a library was missing in the embedded sysroot, usually some transitive thing I didn't know I should think about, and BOTH resulted in CMake just finding something from the build environment. And in fact that may very well even link without error: if the host sysroot is x86-linux and the build environment is an ubuntu with 32-bit multiarch packages installed, gcc isn't necessarily going to see anything wrong. Wrong versions, wrong toolchains, mismatched glibc, etc are still quite like;,, but you might still produce a binary full of nasal demons without so much as a warning from the linker.

I see the problems with using CMAKE_FIND_ROOT_PATH, though - conan doesn't really want to re-root everybody else's path's either.
My current solution has been to bind mount the conan cache at the same location inside the sysroot, so that it's found prefixed, but that's noo ideal either.

I wonder if CMake could be convinced to implement something like a CMAKE_FIND_ONLY_PATH that was kind of the the opposite of CMAKE_IGNORE_PATH for conan 2.0 to use. It could just be a ;-list of prefixes that did not introduce any additional search locations, but made paths (from things like CMAKE_LIBRARY_PATH) eligible to match in CMAKE_FIND_ROOT_PATH_MODE_* ONLY if they intersected with it. If that existed, conan could list the root paths of packages found in the host context; and if necessary to support older CMake versions, you could still set "BOTH" mode for them (but then only old versions would have to take the risk of accidentally picking up non-conan libraries from the build environment).

@SpaceIm
Copy link
Contributor

SpaceIm commented Feb 4, 2022

Actually, I've proposed a more complex solution in #10186, allowing to always honor:

  • CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY
  • CMAKE_FIND_ROOT_PATH_MODE_<INCLUDE/LIBRARY/FRAMEWORK> ONLY as soon as CMAKE_FIND_ROOT_PATH_MODE_PROGRAM is NEVER (which is a common setup).

see https://github.com/SpaceIm/conan/blob/edff4ac9980a43c525363b36278267d5eab2522f/conan/tools/cmake/toolchain.py#L422-L468

But it was considered too complex, so a simpler solution has been implemented where ONLY is always overridden by BOTH.

@puetzk
Copy link
Author

puetzk commented Feb 4, 2022

CMAKE_FIND_ROOT_PATH_MODE_<INCLUDE/LIBRARY/FRAMEWORK> ONLY as soon as CMAKE_FIND_ROOT_PATH_MODE_PROGRAM is NEVER

Yep, that's precisely the usual setup for cross-compiling that I would like to retain.

@puetzk
Copy link
Author

puetzk commented Feb 4, 2022

it just did like there is kind of a CMake gap here - no good way to say "these build-system paths contain host binaries" without using CMAKE_FIND_ROOT_PATH, which has all kinds of other consequences (re-rooting all other searches)

@memsharded
Copy link
Member

memsharded commented Nov 11, 2024

The new CMakeDeps generator in Conan 2.9 (#16964), enabled with -c tools.cmake.cmakedeps:new=will_break_next uses a new approach, this variable will not be set, only the PkgName_DIR for the desired Conan packages will be created in a new file to be included in conan_toolchain.cmake, instead of the previous variables logic, for the desired Conan packages will be created.

I am closing this ticket as solved, please give it a try to the new generators and create new tickets as necessary against the new generator. The will_break_next is a flag that will check, as the feature is in dev-state, for testing only, we want to get enough users feedback before releasing it, to be able to iterate it safely. Many thanks for your feedback.

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

No branches or pull requests

3 participants