From f51480b22328e7ac605fd5e5f467737714b01189 Mon Sep 17 00:00:00 2001
From: Deepika Upadhyay <dupadhya@redhat.com>
Date: Wed, 21 Jul 2021 21:25:23 +0530
Subject: [PATCH] cmake: move ExternalProjectHelper & IncludeJaeger to
 BuildJaeger

set_library_properties_for_external_project assists with setting right target
properties for all jaeger dependencies.
IncludeJaeger would take care of linking and creating these targets
having them spread out when they are highly coupled seems not optimal.

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>
---
 cmake/modules/BuildJaeger.cmake           | 27 +++++++++++++++++++++++
 cmake/modules/ExternalProjectHelper.cmake | 17 --------------
 cmake/modules/IncludeJaeger.cmake         | 21 ------------------
 src/CMakeLists.txt                        |  3 +++
 4 files changed, 30 insertions(+), 38 deletions(-)
 delete mode 100644 cmake/modules/ExternalProjectHelper.cmake
 delete mode 100644 cmake/modules/IncludeJaeger.cmake

diff --git a/cmake/modules/BuildJaeger.cmake b/cmake/modules/BuildJaeger.cmake
index 826b58fd4dd21..8783a6fb7feb6 100644
--- a/cmake/modules/BuildJaeger.cmake
+++ b/cmake/modules/BuildJaeger.cmake
@@ -5,6 +5,28 @@
 # Find<package>.cmake
 # Boost Libraries: uses ceph build boost cmake/modules/BuildBoost.cmake
 
+include(BuildOpenTracing)
+
+# will do all linking and path setting
+function(set_library_properties_for_external_project _target _lib)
+  # Manually create the directory, it will be created as part of the build,
+  # but this runs in the configuration phase, and CMake generates an error if
+  # we add an include directory that does not exist yet.
+  set(_libfullname "${CMAKE_SHARED_LIBRARY_PREFIX}${_lib}${CMAKE_SHARED_LIBRARY_SUFFIX}")
+  set(_libpath "${CMAKE_BINARY_DIR}/external/lib/${_libfullname}")
+  set(_includepath "${CMAKE_BINARY_DIR}/external/include")
+  message(STATUS "Configuring ${_target} with ${_libpath}")
+  add_library(${_target} SHARED IMPORTED)
+  add_dependencies(${_target} ${_lib})
+
+  file(MAKE_DIRECTORY "${_includepath}")
+  set_target_properties(${_target} PROPERTIES
+    INTERFACE_LINK_LIBRARIES "${_libpath}"
+    IMPORTED_LINK_INTERFACE_LANGUAGES "CXX"
+    IMPORTED_LOCATION "${_libpath}"
+    INTERFACE_INCLUDE_DIRECTORIES "${_includepath}")
+endfunction()
+
 function(build_jaeger)
   set(Jaeger_SOURCE_DIR "${CMAKE_SOURCE_DIR}/src/jaegertracing/jaeger-client-cpp")
   set(Jaeger_INSTALL_DIR "${CMAKE_BINARY_DIR}/external")
@@ -55,4 +77,9 @@ function(build_jaeger)
     DEPENDS "${dependencies}"
     BUILD_BYPRODUCTS ${CMAKE_BINARY_DIR}/external/lib/libjaegertracing.so
     )
+
+  set_library_properties_for_external_project(opentracing::libopentracing
+  opentracing)
+  set_library_properties_for_external_project(jaegertracing::libjaegertracing
+  jaegertracing)
 endfunction()
diff --git a/cmake/modules/ExternalProjectHelper.cmake b/cmake/modules/ExternalProjectHelper.cmake
deleted file mode 100644
index 42beab197de10..0000000000000
--- a/cmake/modules/ExternalProjectHelper.cmake
+++ /dev/null
@@ -1,17 +0,0 @@
-function (set_library_properties_for_external_project _target _lib)
-  # Manually create the directory, it will be created as part of the build,
-  # but this runs in the configuration phase, and CMake generates an error if
-  # we add an include directory that does not exist yet.
-  set(_libfullname "${CMAKE_SHARED_LIBRARY_PREFIX}${_lib}${CMAKE_SHARED_LIBRARY_SUFFIX}")
-  set(_libpath "${CMAKE_BINARY_DIR}/external/lib/${_libfullname}")
-  set(_includepath "${CMAKE_BINARY_DIR}/external/include")
-  message(STATUS "Configuring ${_target} with ${_libpath}")
-
-  file(MAKE_DIRECTORY "${_includepath}")
-  set_target_properties(${_target} PROPERTIES
-    INTERFACE_LINK_LIBRARIES "${_libpath}"
-    IMPORTED_LINK_INTERFACE_LANGUAGES "CXX"
-    IMPORTED_LOCATION "${_libpath}"
-    INTERFACE_INCLUDE_DIRECTORIES "${_includepath}")
-  #  set_property(TARGET ${_target} APPEND PROPERTY IMPORTED_LINK_INTERFACE_LIBRARIES "CXX")
-endfunction ()
diff --git a/cmake/modules/IncludeJaeger.cmake b/cmake/modules/IncludeJaeger.cmake
deleted file mode 100644
index b62265c9757b8..0000000000000
--- a/cmake/modules/IncludeJaeger.cmake
+++ /dev/null
@@ -1,21 +0,0 @@
-include(BuildJaeger)
-include(BuildOpenTracing)
-
-include(ExternalProjectHelper)
-
-build_jaeger()
-
-add_library(opentracing::libopentracing SHARED IMPORTED)
-add_dependencies(opentracing::libopentracing opentracing)
-add_library(jaegertracing::libjaegertracing SHARED IMPORTED)
-add_dependencies(jaegertracing::libjaegertracing Jaeger)
-add_library(thrift::libthrift SHARED IMPORTED)
-add_dependencies(thrift::libthrift thrift)
-
-#(set_library_properties_for_external_project _target _lib)
-set_library_properties_for_external_project(opentracing::libopentracing
-  opentracing)
-set_library_properties_for_external_project(jaegertracing::libjaegertracing
-  jaegertracing)
-set_library_properties_for_external_project(thrift::libthrift
-  thrift)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b862da4ad700e..e0ecb15b1e618 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -431,6 +431,9 @@ if(WITH_JAEGER)
   list(APPEND jaeger_base
   opentracing::libopentracing
   jaegertracing::libjaegertracing)
+
+  build_jaeger()
+
   target_link_libraries(jaeger-base INTERFACE
   yaml-cpp::yaml-cpp
   opentracing::libopentracing
-- 
2.39.5