CMake: Fixes for generated config

[FtZPetruska]

While packing 0.8.0 in vcpkg, I noticed the following issues with the generated yaml-cpp-config.cmake:

• YAML_CPP_SHARED_LIBS_BUILT is set with a PATH_VAR, expanding to ${PACKAGE_PREFIX}/<actual value>, which is always true.
• yaml-cpp-targets.cmake is loaded using a PATH_VAR, which causes an issue with the way vcpkg handles CMake configs.
• YAML_CPP_LIBRARIES is set to yaml-cpp without the yaml-cpp:: namespace.

About vcpkg, after the package is installed, it moves all CMake configs to share/<package name> and patches the PACKAGE_PREFIX_DIR and _IMPORT_PREFIX. Using a PATH_VAR to include the targets file would break this relocation.

---

This PR addresses these issues as such:

• For YAML_CPP_SHARED_LIBS_BUILT, use @YAML_BUILD_SHARED_LIBS@, this correctly expands to a boolean value.
• For yaml-cpp-targets.cmake, instead of relying on the CONFIG_EXPORT_DIR PATH_VAR, use ${CMAKE_CURRENT_LIST_DIR}/yaml-cpp-targets.cmake. This is guaranteed to be always true as the config and targets are always installed to the same location.
• For YAML_CPP_LIBRARIES, I set EXPORT_TARGETS to yaml-cpp::yaml-cpp as it is the target name that is actually available to users, and pass to check_required_components yaml-cpp instead.
• Lastly, I added an option YAML_CPP_INSTALL_CMAKEDIR to control which directory to install the CMake configs to. This is a configuration option often wanted by package managers as they sort of all have their own convention.

[jbeder]

Is there any way to test this? A lot of this CMake stuff is pretty complex, and I don't have a lot of context on it. Could you, for example, add to build.yml a sequence of commands that would demonstrate that this change works? (Ideally those same commands would fail somehow before this change.)

[FtZPetruska]

Thank you for your feedback!

I've updated the CI to run the following steps:

• Configure
• Build
• Run tests
• Install
• Configure the CMake package test
• Build the CMake package test

The CMake package test can be found under test/cmake, it checks that you can call find_package(yaml-cpp CONFIG), that YAML_CPP_SHARED_LIBS_BUILT matches the library type, and links against the exported target.

I've run the CI on:

• this branch
• 0.8.0
• yaml-cpp-0.7.0

The tests on yaml-cpp-0.7.0 did not lead to anywhere relevant, CMake was not able to find the include directories even when setting target_include_directories(main PRIVATE "${YAML_CPP_INCLUDE_DIR}").

On 0.8.0, the CI initially fails with the library type mismatch:

 CMake Error at CMakeLists.txt:13 (message):
  Library type (STATIC_LIBRARY) contradicts config:
  /home/runner/work/yaml-cpp/yaml-cpp/install-prefix/OFF

Commenting it out, the next error is:

/home/runner/work/yaml-cpp/yaml-cpp/test/cmake/main.cpp:1:10: fatal error: yaml-cpp/yaml.h: No such file or directory
    1 | #include "yaml-cpp/yaml.h"
      |          ^~~~~~~~~~~~~~~~~

Since YAML_CPP_LIBRARIES contain the library name instead of the target name, the include directories are not added to the target.

Lastly, adding YAML_CPP_INCLUDE_DIR to the include paths still fails on debug builds:

/usr/bin/ld: cannot find -lyaml-cpp: No such file or directory

While this could have worked on a Release build, the Debug build has a postfix.

---

The one thing that cannot be tested outside of "that's how vcpkg works internally", is moving the CMake configs after install. vcpkg's CI used to fail on most platforms (no logs available), adding the patches from this PR fixed it for most platforms.

[tobim]

Would you mind adding something along the lines of this code to the config file?

# Add an alias for backward compatibility with <= v0.7.
add_library(yaml-cpp ALIAS yaml-cpp::yaml-cpp)

There is at least one instance where client code uses the yaml-cpp target directly, instead of using YAML_CPP_LIBRARIES, and adding this alias makes the transition a lot easier.

[varunagrawal]

Hi @jbeder, thanks for all of your amazing work on this library!

We just upgraded to 0.8.0 and encountered the same issues that @FtZPetruska has specified above. The main change for me was replacing YAML_CPP_LIBRARIES with yaml-cpp::yaml-cpp as a fix

I for one would really love to see this PR landed soon. Thanks!

[FtZPetruska]

Thank you everyone for your feedback!

I have rebased on master and addressed the merge conflicts so this PR should be good for review again.

Would you mind adding something along the lines of this code to the config file?

# Add an alias for backward compatibility with <= v0.7.
add_library(yaml-cpp ALIAS yaml-cpp::yaml-cpp)

There is at least one instance where client code uses the yaml-cpp target directly, instead of using YAML_CPP_LIBRARIES, and adding this alias makes the transition a lot easier.

It's up to @jbeder to decide whether to add back the yaml-cpp target for compatibility. An option could be to add it back temporarily and add a deprecation warning as such:

# yaml-cpp-config.cmake

add_library(yaml-cpp ALIAS yaml-cpp::yaml-cpp)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.17)
  set_target_properties(yaml-cpp PROPERTIES 
    DEPRECATION "The target yaml-cpp is deprecated and will be removed in version x.y.z"
  )
endif()

[jbeder]

This is fantastic, thank you for your hard work!

The one change I'll ask is the alias for yaml-cpp::yaml-cpp, and I like your suggestion to mark it deprecated. We may as well delete it in 0.10.0 (two minor versions from now), so you can put that in the warning.

[FtZPetruska]

Thank you for your feedback!

It looks like properties cannot be set on ALIAS libraries, so I had to make it an INTERFACE library.
Using yaml-cpp in target_link_libraries now prints the following message:

CMake Warning (dev) at CMakeLists.txt:19 (target_link_libraries):
  The library that is being linked to, yaml-cpp, is marked as being
  deprecated by the owner.  The message provided by the developer is:

  The target yaml-cpp is deprecated and will be removed in version 0.10.0.
  Use the yaml-cpp::yaml-cpp target instead.

This warning is for project developers.  Use -Wno-dev to suppress it.

[tobim]

@jbeder would you mind creating a 0.8.1 release after this is merged?

[jbeder]

@jbeder would you mind creating a 0.8.1 release after this is merged?

Will do