[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1166: MINIFICPP-1331 Upgrade yaml-cpp to eliminate issues with "NULL" string in config

[GitBox <gi...@apache.org>]

szaszm commented on a change in pull request #1166:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1166#discussion_r702833271



##########
File path: cmake/BundledYamlCpp.cmake
##########
@@ -16,39 +16,51 @@
 # under the License.
 
 function(use_bundled_yamlcpp SOURCE_DIR BINARY_DIR)
+    if (WIN32)
+        set(CMAKE_INSTALL_LIBDIR "lib")
+    else()
+        include(GNUInstallDirs)
+    endif()
+
     # Define byproducts
     if (WIN32)
-        if ("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
-            set(BYPRODUCT "lib/libyaml-cppmdd.lib")
-        else()
-            set(BYPRODUCT "lib/libyaml-cppmd.lib")
-        endif()
+        set(BYPRODUCT "${CMAKE_INSTALL_LIBDIR}/yaml-cpp.lib")
     else()
-        set(BYPRODUCT "lib/libyaml-cpp.a")
+        set(BYPRODUCT "${CMAKE_INSTALL_LIBDIR}/libyaml-cpp.a")
     endif()
 
     # Set build options
     set(YAMLCPP_CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}
-            "-DCMAKE_INSTALL_PREFIX=${BINARY_DIR}/thirdparty/yaml-cpp-install")
+            "-DCMAKE_INSTALL_PREFIX=${BINARY_DIR}/thirdparty/yaml-cpp-install"
+            "-DCMAKE_DEBUG_POSTFIX="
+            "-DBUILD_SHARED_LIBS=OFF"
+            "-DYAML_CPP_BUILD_TESTS=OFF"
+            "-DYAML_CPP_BUILD_TOOLS=OFF")
 
     # Build project
     ExternalProject_Add(
             yaml-cpp-external
-            SOURCE_DIR "${SOURCE_DIR}/thirdparty/yaml-cpp-yaml-cpp-20171024"
+            GIT_REPOSITORY "https://github.com/jbeder/yaml-cpp.git"
+            GIT_TAG "yaml-cpp-0.7.0"

Review comment:
       Please prefer downloading the tarball (or zip): https://github.com/jbeder/yaml-cpp/archive/refs/tags/yaml-cpp-0.7.0.tar.gz
   Git is slower and it's prone to patching issues, although the latter doesn't apply in this case. I'm also experimenting with a local caching http(s) proxy (to avoid excessive downloading when benchmarking clean build performance), and it works fine with archives, but doesn't play well with git over http(s).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org