impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5659: Begin standardizing treatment of thirdparty libraries
Date Tue, 18 Jul 2017 19:43:09 GMT
Hello Impala Public Jenkins, Michael Ho, Tim Armstrong,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7418

to look at the new patch set (#6).

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
  handled by IMPALA_ADD_THIRDPARTY_LIB.

* 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/libprotoc.so
-- ----------> 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/libev.so

* 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, 155 insertions(+), 301 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/7418/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7418
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>

Mime
View raw message