install_basic_package_files(EXPORT) does not cope well with targets at YCM 0.8.0

[PeterBowman]

The new EXPORT command to install_basic_package_files() and previously available TARGETS/TARGETS_PROPERTY/TARGETS_PROPERTIES are mutually exclusive (ref). However, there is an internal list of targets this command handles that defaults to the project name when using EXPORT (ref) and cannot be tweaked due to said exclusion. This list is used in two places across the code and ignores custom project targets:

• When no config template was passed, install_basic_package_files() generates a basic one.
• If BUILD_SHARED_LIBS=ON, all listed targets are tested against its type.

Regarding the former, generated config files are mostly useless and will lead to errors in downstreams. Check it with the following sample project:

cmake_minimum_required(VERSION 3.5 FATAL_ERROR)
project(ycm-0.8.0-is-broken LANGUAGES C)
find_package(YCM 0.8.0 REQUIRED)

file(WRITE "${CMAKE_BINARY_DIR}/mylib.c" "")
#set(BUILD_SHARED_LIBS ON)
add_library(my_test_target "${CMAKE_BINARY_DIR}/mylib.c")

install(TARGETS my_test_target
        EXPORT MY_TEST_PROJECT
        DESTINATION lib)

include(InstallBasicPackageFiles)
install_basic_package_files(MY_TEST_PROJECT
                            VERSION 0.1.0
                            COMPATIBILITY AnyNewerVersion
                            FIRST_TARGET my_test_target)

In ${CMAKE_BINARY_DIR}/MY_TEST_PROJECTConfig.cmake, there are several references to MY_TEST_PROJECT::MY_TEST_PROJECT while it should be MY_TEST_PROJECT::my_test_target.

Regarding the latter, and if BUILD_SHARED_LIBS is set (uncomment the line in my previous example), the configure step crashes with:

CMake Error at /usr/local/share/YCM/modules/InstallBasicPackageFiles.cmake:563 (get_property):
  get_property could not find TARGET MY_TEST_PROJECT.  Perhaps it has not yet
  been created.
Call Stack (most recent call first):
  CMakeLists.txt:14 (install_basic_package_files)

I don't think the BUILD_SHARED_LIBS variable should be checked against at all (see 0e0d0f5#r29318076), therefore this error would be raised at all times.

[drdanz]

@PeterBowman I think b05ed7e fixes the problem.
Can you confirm this?

Regarding the comment about BUILD_SHARED_LIBS concern, I agree, but I think it belongs to a separate issue... Can you please open a separate one, so that we can close this?

[PeterBowman]

Thanks, @drdanz, I can confirm the target passed to FIRST_TARGET is both correctly exported in the <pkg>Config.cmake file (first concern) and successfully checked against any private dependencies in case it's a static library (second concern). This will not work as expected when more than one target shall be exported by the project, though. More precisely:

• If the COMPATIBILITY_VARS option is specified, two variables are generated within this file: <pkg>_INCLUDE_DIRS and <pkg>_LIBRARIES. All targets beyond the first one will be skipped.
• If any other target describing a static library depends on some private package, this module will ignore it.

I don't think there is a simple solution for this (apart from accepting TARGETS/TARGETS_PROPERTY/TARGETS_PROPERTIES along with EXPORT, which somehow defeats the purpose of the latter).

Regarding the comment about BUILD_SHARED_LIBS concern, I agree, but I think it belongs to a separate issue... Can you please open a separate one, so that we can close this?

See #150.

[drdanz]

Thanks @PeterBowman.

Unfortunately, the optimal solution would be to be able to "read" the targets defined in one EXPORT, but I couldn't find any way to do that, I don't think this is allowed by CMake... It would be probably a good idea to open an issue upstream in CMake.

• If the COMPATIBILITY_VARS option is specified, two variables are generated within this file: <pkg>_INCLUDE_DIRS and <pkg>_LIBRARIES. All targets beyond the first one will be skipped.

The COMPATIBILITY_VARS option is instigating a deprecated behaviour, i.e. using variables instead of targets, therefore I'm not really concerned about that...

• If any other target describing a static library depends on some private package, this module will ignore it.

True. On the other hand, it will bring in the dependencies even if the target that depends on them is shared and there is at least one other target that is static.

I don't think there is an easy solution to that.

Anyway, I think that all of this goes a little bit beyond the goal of this module, the Basic in the name is supposed to mean that it handles "simple" cases, for more elaborate cases, it is probably better not to use this module and write the files yourself instead.

I'm closing this, but feel free to reopen this or to continue the discussion (either here or in #150).