Skip `find_package` calls if required targets are already available during configuration

[GregTheMadMonk]

Hello!

I am using CPM.cmake to handle my dependencies and I wanted to use reflectcpp with pugixml to (de)serialize XML in addition to JSON. The problem I've encountered is: find_package call for pugixml during refletcpp configuration is performed regardless of whether or not pugixml::pugixml target is alredy available to the build system (and in my case it is, since I've already added pugixml to my project). In a way, that requires that all refletcpp dependencies are already built before the configuration step start.

I propose skipping the find_package call if the target already exists. For example, replacing https://github.com/getml/reflect-cpp/blob/37484d1681a7b8f89d3060f4481d902e1d4c5a91/CMakeLists.txt#L154 with

if (NOT TARGET pugixml::pugixml)
    find_package(pugixml CONFIG REQUIRED)
endif ()

For pugixml and other dependencies discovered via find_package. I can submit a PR, but the only one I'm not yet sure how to handle is msgpack since it doesn't use CMake targets... I guess it could be left as is, since it relies on vcpkg anyway?

[liuzicheng1987]

OK. Do you want to draft a PR for that? I think we can sort out the issues with msgpack and other libraries that don't use find_package together.

[apirogov]

I would also be very happy to see this fixed, so that reflect-cpp works better with CPM. I'm currently struggling to work around this with yaml-cpp.

[apirogov]

My current workaround:

CPMAddPackage(
  NAME "yaml-cpp"
  VERSION "0.7.0"
  GITHUB_REPOSITORY "jbeder/yaml-cpp"
  GIT_TAG "yaml-cpp-0.7.0"
  OPTIONS
    "YAML_CPP_BUILD_TESTS OFF"
    "YAML_CPP_BUILD_CONTRIB OFF"
    "YAML_CPP_BUILD_TOOLS OFF"
)
CPMAddPackage(
  NAME "reflect-cpp"
  VERSION "0.16.0"
  GITHUB_REPOSITORY "getml/reflect-cpp"
  OPTIONS "REFLECTCPP_YAML ON"
  PATCHES ./reflect-cpp-cmake.patch
)

With the patch being:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2e16468..726cc58 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -182,7 +182,7 @@ if (REFLECTCPP_YAML)
     list(APPEND REFLECT_CPP_SOURCES
         src/reflectcpp_yaml.cpp
     )
-    find_package(yaml-cpp CONFIG REQUIRED)
+    #find_package(yaml-cpp CONFIG REQUIRED)
     target_link_libraries(reflectcpp PUBLIC yaml-cpp::yaml-cpp)
 endif ()

@@ -232,8 +232,8 @@ install(
     FILE_SET reflectcpp_headers DESTINATION ${INCLUDE_INSTALL_DIR}
     )

-install(
-    EXPORT reflectcpp-exports
-    DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/reflectcpp
-    NAMESPACE reflectcpp::
-)
+# install(
+#     EXPORT reflectcpp-exports
+#     DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/reflectcpp
+#     NAMESPACE reflectcpp::
+# )

I have no idea what I am doing, but this worked for me. This way I could add reflectcpp via target_link_libraries and it seems to compile fine including the YAML features.

[GregTheMadMonk]

You can try changes from the PR, should work with your case

[apirogov]

Works for me, thanks! Hope this is merged soon.