From: Deepika Upadhyay <dupadhya@redhat.com>
Date: Wed, 21 Jul 2021 15:18:02 +0000 (+0530)
Subject: ceph.spec, cmake, debian: use thrift 0.13+ from distro pkg
X-Git-Tag: v17.1.0~1325^2~8
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=80e82686ebafe36fca6dfd21cb32e63ced94d5cd;p=ceph.git

ceph.spec, cmake, debian: use thrift 0.13+ from distro pkg

the change to build and ship libthift was added when we didn't have 0.13.0
version shipped via distro pkgs, now that centos 8 and F34 supports req.
version, we do not need to build and ship it with jaeger library.

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>
---

diff --git a/ceph.spec.in b/ceph.spec.in
index 0c096ea35010..c62599a8a411 100644
--- a/ceph.spec.in
+++ b/ceph.spec.in
@@ -269,6 +269,7 @@ BuildRequires:  libzbd-devel
 %if 0%{with jaeger}
 BuildRequires:  bison
 BuildRequires:  flex
+BuildRequires:  thrift-devel >= 0.13.0
 %if 0%{?fedora} || 0%{?rhel}
 BuildRequires:  json-devel
 %endif
@@ -1064,7 +1065,6 @@ Group:		System/Libraries
 %endif
 Provides: 	libjaegertracing.so.0()(64bit)
 Provides:	libopentracing.so.1()(64bit)
-Provides:	libthrift.so.0.13.0()(64bit)
 %description -n libjaeger
 This package contains libraries needed to provide distributed
 tracing for Ceph.
@@ -2294,7 +2294,6 @@ fi
 %if %{with jaeger}
 %files -n libjaeger
 %{_libdir}/libopentracing.so.*
-%{_libdir}/libthrift.so.*
 %{_libdir}/libjaegertracing.so.*
 %post -n libjaeger -p /sbin/ldconfig
 %postun -n libjaeger -p /sbin/ldconfig
diff --git a/cmake/modules/BuildJaeger.cmake b/cmake/modules/BuildJaeger.cmake
index 76b9e6989e2b..826b58fd4dd2 100644
--- a/cmake/modules/BuildJaeger.cmake
+++ b/cmake/modules/BuildJaeger.cmake
@@ -1,10 +1,9 @@
 # This module builds Jaeger after it's dependencies are installed and discovered
 # opentracing: is built using cmake/modules/Buildopentracing.cmake
-# Thrift: build using cmake/modules/Buildthrift.cmake
+# Thrift: found using cmake/modules/Findthrift.cmake (not by default)
 # yaml-cpp, nlhomann-json: are installed locally and then discovered using
 # Find<package>.cmake
-# Boost Libraries used for building thrift are build and provided by
-# cmake/modules/BuildBoost.cmake
+# Boost Libraries: uses ceph build boost cmake/modules/BuildBoost.cmake
 
 function(build_jaeger)
   set(Jaeger_SOURCE_DIR "${CMAKE_SOURCE_DIR}/src/jaegertracing/jaeger-client-cpp")
diff --git a/cmake/modules/Buildthrift.cmake b/cmake/modules/Buildthrift.cmake
deleted file mode 100644
index 6d9579a35212..000000000000
--- a/cmake/modules/Buildthrift.cmake
+++ /dev/null
@@ -1,54 +0,0 @@
-function(build_thrift)
-  set(thrift_SOURCE_DIR "${CMAKE_SOURCE_DIR}/src/jaegertracing/thrift")
-  set(thrift_BINARY_DIR "${CMAKE_BINARY_DIR}/external/thrift")
-
-  set(thrift_CMAKE_ARGS  -DCMAKE_BUILD_TYPE=Release
-			 -DCMAKE_POSITION_INDEPENDENT_CODE=ON
-			 -DBUILD_JAVA=OFF
-			 -DBUILD_PYTHON=OFF
-			 -DBUILD_TESTING=OFF
-			 -DBUILD_TUTORIALS=OFF
-			 -DBUILD_C_GLIB=OFF
-			 -DBUILD_HASKELL=OFF
-			 -DWITH_LIBEVENT=OFF
-			 -DWITH_ZLIB=OFF
-			 -DBoost_INCLUDE_DIRS=${CMAKE_BINARY_DIR}/boost/include
-			 -DCMAKE_INSTALL_PREFIX="${CMAKE_BINARY_DIR}/boost;${CMAKE_BINARY_DIR}/boost/include;${CMAKE_BINARY_DIR}/external"
-			 -DCMAKE_FIND_ROOT_PATH="${CMAKE_BINARY_DIR}/boost;${CMAKE_BINARY_DIR}/boost/include;${CMAKE_BINARY_DIR}/external"
-			 -DCMAKE_INSTALL_RPATH=${CMAKE_BINARY_DIR}/external/lib
-			 -DCMAKE_INSTALL_RPATH_USE_LINK_PATH=TRUE
-			 -DCMAKE_INSTALL_PREFIX=${CMAKE_BINARY_DIR}/external
-			 -DCMAKE_INSTALL_LIBDIR=${CMAKE_BINARY_DIR}/external/lib)
-
-  if(WITH_SYSTEM_BOOST)
-    message(STATUS "thrift will be using system boost")
-    set(dependencies "")
-    list(APPEND thrift_CMAKE_ARGS -DBOOST_ROOT=/opt/ceph)
-    list(APPEND thrift_CMAKE_ARGS -DCMAKE_FIND_ROOT_PATH=/opt/ceph)
-  else()
-    message(STATUS "thrift will be using external build boost")
-    set(dependencies Boost)
-    list(APPEND thrift_CMAKE_ARGS  -DCMAKE_FIND_ROOT_PATH=${CMAKE_BINARY_DIR}/boost)
-    list(APPEND thrift_CMAKE_ARGS  -DCMAKE_PREFIX_PATH=${CMAKE_BINARY_DIR}/external)
-  endif()
-
-  if(CMAKE_MAKE_PROGRAM MATCHES "make")
-    # try to inherit command line arguments passed by parent "make" job
-    set(make_cmd $(MAKE) thrift)
-  else()
-    set(make_cmd ${CMAKE_COMMAND} --build <BINARY_DIR> --target thrift)
-  endif()
-  set(install_cmd ${CMAKE_MAKE_PROGRAM} install)
-
-  include(ExternalProject)
-  ExternalProject_Add(thrift
-    SOURCE_DIR ${thrift_SOURCE_DIR}
-    PREFIX "${CMAKE_BINARY_DIR}/external/thrift"
-    CMAKE_ARGS ${thrift_CMAKE_ARGS}
-    BINARY_DIR ${thrift_BINARY_DIR}
-    BUILD_COMMAND ${make_cmd}
-    INSTALL_COMMAND ${install_cmd}
-    DEPENDS ${dependencies}
-    BUILD_BYPRODUCTS ${CMAKE_BINARY_DIR}/external/lib/libthrift.so
-    )
-endfunction()
diff --git a/debian/control b/debian/control
index a364b7c56116..2495ce7494dd 100644
--- a/debian/control
+++ b/debian/control
@@ -69,6 +69,7 @@ Build-Depends: automake,
                librabbitmq-dev,
                librdkafka-dev,
                luarocks,
+               libthrift-dev (>= 0.13.0) <pkg.ceph.jaeger>,
                libyaml-cpp-dev (>= 0.6) <pkg.ceph.crimson> <pkg.ceph.jaeger>,
                libzstd-dev <pkg.ceph.check>,
                libxmlsec1 <pkg.ceph.check>,
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 22cc4e1b8b51..9d51c2e70a68 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -425,11 +425,11 @@ add_library(common-objs OBJECT ${libcommon_files})
 add_dependencies(common-objs legacy-option-headers)
 
 if(WITH_JAEGER)
-  include(BuildJaeger)
+  find_package(thrift 0.13.0)
   find_package(yaml-cpp 0.6.0 REQUIRED)
+  include(BuildJaeger)
   list(APPEND jaeger_base
   opentracing::libopentracing
-  thrift::libthrift
   jaegertracing::libjaegertracing)
   target_link_libraries(jaeger-base INTERFACE
   yaml-cpp::yaml-cpp