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

[bug] CMakeDeps: <name>_LIBRARIES variable is not suitable for macros like check_symbol_exists #12180

Closed
SpaceIm opened this issue Sep 22, 2022 · 6 comments · Fixed by #16964
Closed
Assignees

Comments

@SpaceIm
Copy link
Contributor

SpaceIm commented Sep 22, 2022

Environment Details (include every applicable attribute)

  • Operating System+version: macOS Monterey
  • Compiler+version: AppleClang 14
  • Conan version: 1.52.0
  • Python version: 3.10.6
  • cmake version: 3.24.2

Steps to reproduce (Include if Applicable)

CMakeLists.txt

cmake_minimum_required(VERSION 3.15)
project(test_check_symbol_exists LANGUAGES C)

find_package(zstd REQUIRED)

include(CheckSymbolExists)
set(CMAKE_REQUIRED_INCLUDES_SAVE ${CMAKE_REQUIRED_INCLUDES})
set(CMAKE_REQUIRED_LIBRARIES_SAVE ${CMAKE_REQUIRED_LIBRARIES})
list(PREPEND CMAKE_REQUIRED_INCLUDES ${zstd_INCLUDE_DIRS})
list(PREPEND CMAKE_REQUIRED_LIBRARIES ${zstd_LIBRARIES})
check_symbol_exists(ZSTD_decompressStream "zstd.h" ZSTD_RECENT_ENOUGH)
set(CMAKE_REQUIRED_INCLUDES ${CMAKE_REQUIRED_INCLUDES_SAVE})
set(CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES_SAVE})

if(ZSTD_RECENT_ENOUGH)
    message(STATUS "Test succeeded")
else()
    message(FATAL_ERROR "Test failed")
endif()

test succeeds with cmake_find_package:

mkdir build1 && cd build1
conan install zstd/1.5.2@ -g CMakeToolchain -g cmake_find_package -c tools.cmake.cmaketoolchain:find_package_prefer_config="OFF"
cmake .. -DCMAKE_TOOLCHAIN_FILE=conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Release

test fails with CMakeDeps:

mkdir build2 && cd build2
conan install zstd/1.5.2@ -g CMakeToolchain -g CMakeDeps
cmake .. -DCMAKE_TOOLCHAIN_FILE=conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Release

CMAKE_REQUIRED_INCLUDES works fine here, but not CMAKE_REQUIRED_LIBRARIES, because zstd_LIBRARIES is set to zstd imported target instead of full paths of all libraries.
According to CMake documentation, check_c_source_compiles allows imported targets in CMAKE_REQUIRED_LIBRARIES, but they don't say anything for check_symbol_exists.

It's quite annoying for conan v2 migration of several CCI recipes.

Logs (Executed commands with output) (Include/Attach if Applicable)

CMakeError.log of CMakeDeps test:

Compiling the C compiler identification source file "CMakeCCompilerId.c" failed.
Compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc
Build flags: ;;;;-m64
Id flags:

The output was:
1
ld: library not found for -lSystem
clang: error: linker command failed with exit code 1 (use -v to see invocation)


Determining if the ZSTD_decompressStream exist failed with the following output:
Change Dir: /Users/spaceim/Documents/Personnel/test_check_symbol/build2/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/make -f Makefile cmTC_ed876/fast && /Applications/Xcode.app/Contents/Developer/usr/bin/make  -f CMakeFiles/cmTC_ed876.dir/build.make CMakeFiles/cmTC_ed876.dir/build
Building C object CMakeFiles/cmTC_ed876.dir/CheckSymbolExists.c.o
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc  -I/Users/spaceim/.conan/data/zstd/1.5.2/_/_/package/258f9d253d0e975fcb3ab38da19bbea0bc9bf506/include -m64  -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk -MD -MT CMakeFiles/cmTC_ed876.dir/CheckSymbolExists.c.o -MF CMakeFiles/cmTC_ed876.dir/CheckSymbolExists.c.o.d -o CMakeFiles/cmTC_ed876.dir/CheckSymbolExists.c.o -c /Users/spaceim/Documents/Personnel/test_check_symbol/build2/CMakeFiles/CMakeTmp/CheckSymbolExists.c
Linking C executable cmTC_ed876
/Applications/CMake.app/Contents/bin/cmake -E cmake_link_script CMakeFiles/cmTC_ed876.dir/link.txt --verbose=1
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -m64  -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk -Wl,-search_paths_first -Wl,-headerpad_max_install_names -m64  CMakeFiles/cmTC_ed876.dir/CheckSymbolExists.c.o -o cmTC_ed876
Undefined symbols for architecture x86_64:
  "_ZSTD_decompressStream", referenced from:
      _main in CheckSymbolExists.c.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [cmTC_ed876] Error 1
make: *** [cmTC_ed876/fast] Error 2


File /Users/spaceim/Documents/Personnel/test_check_symbol/build2/CMakeFiles/CMakeTmp/CheckSymbolExists.c:
/* */
#include <zstd.h>

int main(int argc, char** argv)
{
  (void)argv;
#ifndef ZSTD_decompressStream
  return ((int*)(&ZSTD_decompressStream))[argc];
#else
  (void)argc;
  return 0;
#endif
}
@SpaceIm
Copy link
Contributor Author

SpaceIm commented Nov 1, 2022

similar issue as #12012

@jcar87
Copy link
Contributor

jcar87 commented Nov 8, 2023

The CMake documentation specifies that targets are actually a valid input for anything that relies on try_compile, including check_symbol_exists. Docs here: https://cmake.org/cmake/help/latest/command/try_compile.html#options

The problem here is that the targets generated by CMakeDeps require a build type to be set (CMAKE_BUILD_TYPE for single config generators) - but the temporary CMake project that is created by try_compile, does not have any configuration set, which causes the issues.

A possible solution is to simply tell CMake to forward the current configuration to the try_compile generated projects: https://cmake.org/cmake/help/latest/variable/CMAKE_TRY_COMPILE_CONFIGURATION.html#variable:CMAKE_TRY_COMPILE_CONFIGURATION - in a recipe this could be done via:

tc.cache_variables["CMAKE_TRY_COMPILE_CONFIGURATION"] = str(self.settings.build_type)

Alternatively, this fix in upstream CMake https://gitlab.kitware.com/cmake/cmake/-/merge_requests/8016 might help - I believe it made it to CMake 3.26. I still need to test if it solves this particular issue, as the underlying cause has to do with the targets generated by CMakeDeps having gen-ex conditionals that depend on the build type.

@jcar87 jcar87 self-assigned this Nov 8, 2023
@SpaceIm
Copy link
Contributor Author

SpaceIm commented Nov 8, 2023

I can test with dcmtk.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Nov 8, 2023

It doesn't seem to work. I've tested in dcmtk recipe:

But still get this error during CMake configuration (macOS x86_64/Release/all shared/apple-clang 15/CMake 3.27.7/conan 2.0.13/Ninja generator):

-- Looking for prototype of SSL_CTX_get0_param
CMake Error at /Users/spaceim/.conan2/p/b/dcmtk278ca8b211711/b/build/Release/CMakeFiles/CMakeTmp/CMakeLists.txt:25 (target_link_libraries):
  Target "cmTC_4d7c0" links to:

    openssl::openssl

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.



CMake Error at CMake/CheckFunctionWithHeaderExists.cmake:30 (try_compile):
  Failed to generate test project build system.
Call Stack (most recent call first):
  CMake/dcmtkPrepare.cmake:671 (CHECK_FUNCTIONWITHHEADER_EXISTS)
  CMakeLists.txt:38 (include)

@valgur
Copy link
Contributor

valgur commented Jan 25, 2024

Adding CMAKE_TRY_COMPILE_CONFIGURATION very clearly fixed the try_compile() issues with OpenSSL symbol detection in conan-io/conan-center-index#21389.

I'll be adding this workaround just in case for any recipes that make use of compilation tests with CMAKE_REQUIRED_LIBRARIES in the future.

@memsharded
Copy link
Member

We are releasing in Conan 2.9 a completely new CMakeDeps generator in #16964 that has closed this ticket, with many pending features and fixes:

  • Allow defining cpp_info.default_components and using that information to generate CMake targets by default for packages
  • Allow defining cpp_info.exe to model executable imported targed generation
  • Creating always actual SHARED/STATIC/INTERFACE imported targets, with their IMPORTED_LOCATION, the IMPORTED_IMPLIB, the IMPORTED_CONFIGURATIONS
  • This solves issues with try_compile and other CMake source-compiling checks
  • No artificial library or package targets
  • Allow using the package languages attribute or a new cpp_info.languages information to define IMPORTED_LINK_INTERFACE_LANGUAGES
  • Define config files for executable targets for the "build" context automatically, without any special configuration
  • Use the correct CONFIGURATION from the package instead of the consumer one
  • Generate a new conan_cmakedeps_paths.cmake file, that can be used for integrations that cannot use a CMakeToolchain, like cmake-conan
  • Allow using executable targets from the host context if not cross-building (and not tool-requires that would prioritize those executable targets)

Current known pending functionality (to be added soon):

  • Not managing Apple frameworks
  • Not generating find modules, only CMake Config files

The new CMakeDeps generator is intended for testing and validation only, being a transparent replacement of the old one, so it is behind a new conf. To use it, use the -c tools.cmake.cmakedeps:new=will_break_next, and that will use the new generator instead of the old one. Note the will_break_next value means exactly that, that value will change in next release to force a break, so no one can depend on this generator in production yet.

Your feedback is very important

As this is a major change, we will only remove the conf gate when we get confirmation from users that it works and solve the issues. Please try the new generator for your project, and let us know if it works. If it doesn't, please re-open this ticket and let us know what failed. Thanks very much!

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

Successfully merging a pull request may close this issue.

4 participants