impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5659: Begin standardizing treatment of thirdparty libraries
Date Mon, 17 Jul 2017 02:47:57 GMT
Henry Robinson has uploaded a new patch set (#3).

Change subject: IMPALA-5659: Begin standardizing treatment of thirdparty libraries

IMPALA-5659: Begin standardizing treatment of thirdparty libraries

If Impala was built with --build_shared_libs, some thirdparty libraries
were still statically linked; this could cause runtime errors if the
libraries were also linked into a .so. This patch fixes that issue (for
gflags, glog and protobuf at least) by ensuring that build_shared_libs
is respected for those libraries.

* Standardize thirdparty library handling w/CMake by adding
  IMPALA_ADD_THIRDPARTY_LIB. This creates a symbolic name for each
  library, allowing us to switch the underlying library
  files (e.g. change from static to dynamic linking) without having to
  individually change the link clauses for each target.

* Remove most cases of add_library() from cmake_modules/* - that is all

* Add shared library detection for a couple of thirdparty
  dependencies (many only detect static libraries), just to prove the concept.

* All thirdparty libraries now print a standard set of messages. For example:

-- ----------> Adding thirdparty library protoc. <----------
-- Header files: /data/henry/src/cloudera/impala-toolchain/protobuf-2.6.1/include
-- Added shared library dependency protoc: /data/henry/src/cloudera/impala-toolchain/protobuf-2.6.1/lib/
-- ----------> Adding thirdparty library libev. <----------
-- Header files: /data/henry/src/cloudera/impala-toolchain/libev-4.20/include
-- Added shared library dependency libev: /data/henry/src/cloudera/impala-toolchain/libev-4.20/lib/

* Some libraries don't quite fit this pattern (LLVM and Boost) - leave
  them as is for now.

* Remove FindOpenSSL.cmake - the toolchain one is more modern.

Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
M CMakeLists.txt
M be/CMakeLists.txt
M cmake_modules/FindAvro.cmake
M cmake_modules/FindBreakpad.cmake
M cmake_modules/FindFlatBuffers.cmake
M cmake_modules/FindGFlags.cmake
M cmake_modules/FindGLog.cmake
M cmake_modules/FindGTest.cmake
M cmake_modules/FindHDFS.cmake
M cmake_modules/FindLdap.cmake
M cmake_modules/FindLz4.cmake
D cmake_modules/FindOpenSSL.cmake
M cmake_modules/FindRe2.cmake
M cmake_modules/FindSasl.cmake
M cmake_modules/FindSnappy.cmake
M cmake_modules/FindThrift.cmake
M cmake_modules/FindZlib.cmake
M cmake_modules/kudu_cmake_fns.txt
18 files changed, 126 insertions(+), 270 deletions(-)

  git pull ssh:// refs/changes/18/7418/3
To view, visit
To unsubscribe, visit

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Tim Armstrong <>

View raw message