This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 4cf0bfa83 IMPALA-12059: Deduplicate and filter warnings from Thrift
compilation
4cf0bfa83 is described below
commit 4cf0bfa83f9641eb95d83c76af7962e6a3f1e064
Author: Joe McDonnell <[email protected]>
AuthorDate: Fri Apr 28 16:35:35 2023 -0700
IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
This wraps Thrift compilation commands with the
bin/thrift-quiet-wrapper.sh shell script. This script runs the
Thrift compilation command, collecting the output. If the
command fails, it dumps the output as-is. If the command succeeds,
it deduplicates the warnings and filters out a couple warnings
that we have no intention of fixing.
Testing:
- Ran "make gen-deps" on a clean build and verified that
the Thrift warnings are deduplicated and filtered
- Modified a Thrift file and verified errors are printed as-is.
Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Reviewed-on: http://gerrit.cloudera.org:8080/19818
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
---
bin/thrift-quiet-wrapper.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++
common/thrift/CMakeLists.txt | 19 ++++++++-------
2 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/bin/thrift-quiet-wrapper.sh b/bin/thrift-quiet-wrapper.sh
new file mode 100755
index 000000000..aa08d3cad
--- /dev/null
+++ b/bin/thrift-quiet-wrapper.sh
@@ -0,0 +1,56 @@
+#!/bin/bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Utility script that invokes the passed in thrift command but deduplicates
+# and filters the output if the command is successful.
+
+COMMAND=("$@")
+
+TMP_FILE=$(mktemp -q)
+
+"${COMMAND[@]}" > ${TMP_FILE} 2>&1
+COMMAND_RET_CODE=${PIPESTATUS[0]}
+
+if [[ ${COMMAND_RET_CODE} -ne 0 ]]; then
+ # If the command failed, print all the output without sorting or filtering.
+ cat ${TMP_FILE}
+else
+ # If the command succeeded, then it is safe to deduplicate the output and
+ # filter out warnings that are not useful.
+ #
+ # Thrift can print some warnings from included Thrift files, not just
+ # the top-level Thrift file. Some commonly used Thrift files can be
+ # included several times by several other included Thrift files, leading
+ # to many copies of the same warning. Sorting and deduplicating reduces
+ # the output considerably (i.e. '| sort | uniq' below)
+ #
+ # Ignored output:
+ # 1. '64-bit constant "34359738368" may not work in all languages.'
+ # 2. 'The "byte" type is a compatibility alias for "i8". Use "i8" to
+ # emphasize the signedness of this type.'
+ # 3. Empty lines ("^$")
+ cat ${TMP_FILE} \
+ | sort | uniq \
+ | grep -v "64-bit constant.* may not work in all language." \
+ | grep -v 'The "byte" type is a compatibility alias for "i8".' \
+ | grep -v "^$"
+fi
+
+rm -f ${TMP_FILE}
+exit "${COMMAND_RET_CODE}"
diff --git a/common/thrift/CMakeLists.txt b/common/thrift/CMakeLists.txt
index 617113789..23fc1fac3 100644
--- a/common/thrift/CMakeLists.txt
+++ b/common/thrift/CMakeLists.txt
@@ -64,8 +64,8 @@ function(THRIFT_GEN VAR)
# from Hive or Parquet.
add_custom_command(
OUTPUT ${OUTPUT_BE_FILE}
- COMMAND ${THRIFT_CPP_COMPILER} ${CPP_ARGS} ${THRIFT_FILE}
- COMMAND ${THRIFT_PY_COMPILER} ${PYTHON_ARGS} ${THRIFT_FILE}
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_CPP_COMPILER} ${CPP_ARGS}
${THRIFT_FILE}
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_PY_COMPILER} ${PYTHON_ARGS}
${THRIFT_FILE}
DEPENDS ${ABS_THRIFT_FILE}
COMMENT "Running thrift compiler on ${THRIFT_FILE}"
VERBATIM
@@ -73,7 +73,7 @@ function(THRIFT_GEN VAR)
ELSEIF (THRIFT_FILE STREQUAL "ImpalaService.thrift")
add_custom_command(
OUTPUT ${OUTPUT_BE_FILE}
- COMMAND ${THRIFT_CPP_COMPILER} ${CPP_ARGS} ${THRIFT_FILE}
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_CPP_COMPILER} ${CPP_ARGS}
${THRIFT_FILE}
# Because of some CMake bug we can't just use
# sed -i.bak "'s|\\(dispatchCallTemplated.*\\));|\\1) override;|'"
# here because for some reason CMake doesn't remove the quotes and
generates
@@ -81,8 +81,8 @@ function(THRIFT_GEN VAR)
COMMAND ${CMAKE_SOURCE_DIR}/bin/cmake_aux/add_override.sh
${BE_OUTPUT_DIR}/gen-cpp/ImpalaHiveServer2Service.h
${BE_OUTPUT_DIR}/gen-cpp/ImpalaService.h
- COMMAND ${THRIFT_JAVA_COMPILER} ${JAVA_FE_ARGS} ${THRIFT_FILE}
- COMMAND ${THRIFT_PY_COMPILER} ${PYTHON_ARGS} ${THRIFT_FILE}
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_JAVA_COMPILER}
${JAVA_FE_ARGS} ${THRIFT_FILE}
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_PY_COMPILER} ${PYTHON_ARGS}
${THRIFT_FILE}
DEPENDS ${ABS_THRIFT_FILE}
COMMENT "Running thrift compiler on ${THRIFT_FILE}"
VERBATIM
@@ -90,9 +90,9 @@ function(THRIFT_GEN VAR)
ELSE (THRIFT_FILE STREQUAL ${TCLI_SERVICE_THRIFT} OR THRIFT_FILE STREQUAL
"parquet.thrift")
add_custom_command(
OUTPUT ${OUTPUT_BE_FILE}
- COMMAND ${THRIFT_CPP_COMPILER} ${CPP_ARGS} ${THRIFT_FILE}
- COMMAND ${THRIFT_JAVA_COMPILER} ${JAVA_FE_ARGS} ${THRIFT_FILE}
- COMMAND ${THRIFT_PY_COMPILER} ${PYTHON_ARGS} ${THRIFT_FILE}
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_CPP_COMPILER} ${CPP_ARGS}
${THRIFT_FILE}
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_JAVA_COMPILER}
${JAVA_FE_ARGS} ${THRIFT_FILE}
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_PY_COMPILER} ${PYTHON_ARGS}
${THRIFT_FILE}
DEPENDS ${ABS_THRIFT_FILE}
COMMENT "Running thrift compiler on ${THRIFT_FILE}"
VERBATIM
@@ -125,7 +125,7 @@ function(THRIFT_GEN_DS VAR)
list(APPEND ${VAR} ${OUTPUT_FILE})
add_custom_command(
OUTPUT ${OUTPUT_FILE}
- COMMAND ${THRIFT_JAVA_COMPILER} ${JAVA_EXT_DS_ARGS} ${THRIFT_FILE} &&
+ COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_JAVA_COMPILER}
${JAVA_EXT_DS_ARGS} ${THRIFT_FILE} &&
mkdir -p ${OUTPUT_DIR} && echo ${OUTPUT_FILE_MESSAGE} >
${OUTPUT_FILE}
DEPENDS ${ABS_THRIFT_FILE}
COMMENT "Running thrift compiler for ext-data-source on ${THRIFT_FILE}"
@@ -140,6 +140,7 @@ set(TCLI_SERVICE_THRIFT
"${HIVE_THRIFT_SOURCE_DIR}/TCLIService.thrift")
message("Using Thrift CPP compiler: ${THRIFT_CPP_COMPILER}")
message("Using Thrift JAVA compiler: ${THRIFT_JAVA_COMPILER}")
message("Using Thrift PY compiler: ${THRIFT_PY_COMPILER}")
+set(THRIFT_QUIET_WRAPPER "${CMAKE_SOURCE_DIR}/bin/thrift-quiet-wrapper.sh")
set(THRIFT_CPP_INCLUDE_DIR_OPTION -I ${THRIFT_CPP_CONTRIB_DIR}
-I $ENV{HIVE_METASTORE_THRIFT_DIR} -I ${HIVE_THRIFT_SOURCE_DIR})
set(THRIFT_JAVA_INCLUDE_DIR_OPTION -I ${THRIFT_JAVA_CONTRIB_DIR}
