Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18939 )
Change subject: IMPALA-10262: RPM/DEB Packaging Support ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/18939/8/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/18939/8/CMakeLists.txt@614 PS8, Line 614: if ($ENV{STRIP_DEPLOYMENT_IMPALAD}) : set(CPACK_STRIP_FILES ${IMPALA_INSTALLDIR}/bin/impalad) : message("Binaries in the package will be stripped") : endif() One thing I tried out is using the CPACK_DEBIAN_DEBUGINFO_PACKAGE option. That would produce a package with stripped binaries along with a separate package for debuginfo. That might be nice. CPACK_STRIP_FILES is good for now. That requires us to build things with Build IDs. Linux distributions always do that for packages. That is doable, but we would need to build a new toolchain because our libgcc_s.so and libstdc++.so don't have Build IDs right now. http://gerrit.cloudera.org:8080/#/c/18939/8/CMakeLists.txt@622 PS8, Line 622: execute_process( : COMMAND $ENV{IMPALA_HOME}/docker/install_os_packages.sh --dry-run : COMMAND tail -1 : OUTPUT_VARIABLE PKG_LIST : OUTPUT_STRIP_TRAILING_WHITESPACE : ) When I run this on Ubuntu 20, somehow this doesn't work. It works on the commandline, but CMake has PKG_LIST empty. I messed around with this for a while, and this is a formula that seems to work: execute_process( COMMAND bash -c "${CMAKE_SOURCE_DIR}/docker/install_os_packages.sh --dry-run | tail -n1" OUTPUT_VARIABLE PKG_LIST OUTPUT_STRIP_TRAILING_WHITESPACE ) http://gerrit.cloudera.org:8080/#/c/18939/8/CMakeLists.txt@628 PS8, Line 628: message(STATUS "Get required package list: ${PKG_LIST}") As a sanity check, can we raise an error if PKG_LIST is empty: if ("${PKG_LIST}" STREQUAL "") message(FATAL_ERROR "Package list is empty: '${PKG_LIST}'") else() message(STATUS "Get required package list: '${PKG_LIST}'") endif() http://gerrit.cloudera.org:8080/#/c/18939/8/package/bin/impala-env.sh File package/bin/impala-env.sh: http://gerrit.cloudera.org:8080/#/c/18939/8/package/bin/impala-env.sh@32 PS8, Line 32: export LIBHDFS_OPTS="${LIBHDFS_OPTS:-} -Djava.library.path=${HADOOP_LIB_DIR}/native/" : echo "Using hadoop native libs in ${HADOOP_LIB_DIR}/native/" : else : echo "WARNING: HDFS short-circuit reads are not enabled due to HADOOP_HOME not set." > nit: Could we also pack hadoop native libs into the final package? Good point, it looks like we don't include that right now, and it is needed for short-circuit reads. -- To view, visit http://gerrit.cloudera.org:8080/18939 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64419fd400fe8d233dac016b6306157fe9461d82 Gerrit-Change-Number: 18939 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Xiang Yang <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Fri, 30 Jun 2023 03:05:52 +0000 Gerrit-HasComments: Yes
