You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2020/10/09 16:18:25 UTC
[impala] 02/02: IMPALA-8304: Generate JUnitXML if a command run by
CMake fails
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
commit 1f3160b4c07c8a5a146067222e6591d44bfa3c7d
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Sun Mar 3 16:50:24 2019 -0800
IMPALA-8304: Generate JUnitXML if a command run by CMake fails
This wraps each command executed by CMake with a wrapper that
generates a JUnitXML file if the command fails. If the command
succeeds, the wrapper does nothing. The wrapper applies to C++
compilation, linking, and custom shell commands (such as
building the frontend via maven). It does not apply to failures
coming from CMake itself. It can be disabled by setting
DISABLE_CMAKE_JUNITXML.
The command output can include Unicode (e.g. smart quotes for
g++), so this also updates generate_junitxml.py to handle
Unicode.
The wrapper interacts poorly with add_custom_command/add_custom_target
CMake commands that use 'cd directory && do_something', so this
switches those locations (in /docker) to use CMake's WORKING_DIRECTORY.
Testing:
- Verified it does not impact a successful build (including with
ccache and/or distcc).
- Verified it generates JUnitXML for C++ and Java compilation
failures.
- Verified it doesn't use the wrapper when DISABLE_CMAKE_JUNITXML
is set.
Change-Id: If71f2faf3ab5052b56b38f1b291fee53c390ce23
Reviewed-on: http://gerrit.cloudera.org:8080/12668
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
CMakeLists.txt | 16 ++++++
be/CMakeLists.txt | 11 +++-
bin/junitxml_command_wrapper.sh | 61 ++++++++++++++++++++++
docker/CMakeLists.txt | 8 +--
.../impala_py_lib/jenkins/generate_junitxml.py | 30 ++++++-----
5 files changed, 108 insertions(+), 18 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 34a5c61..f88a68d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -405,6 +405,22 @@ endif()
find_package(kuduClient REQUIRED NO_DEFAULT_PATH)
include_directories(SYSTEM ${KUDU_CLIENT_INCLUDE_DIR})
+# Run all commands with a wrapper that generates JUnitXML if the command fails.
+# Disabled if the DISABLE_CMAKE_JUNITXML environment variable is set
+# Note: There are known limitations for junitxml_command_wrapper.sh. The most
+# notable is that commands should not do "cd directory && do_something". Use
+# WORKING_DIRECTORY for add_custom_command/add_custom_target instead. See
+# junitxml_command_wrapper.sh for more details.
+if(NOT $ENV{DISABLE_CMAKE_JUNITXML} EQUAL "")
+ message(STATUS "DISABLE_CMAKE_JUNITXML is set, disabling JUnitXML Command Wrapper")
+else()
+ message(STATUS "Using JUnitXML Command Wrapper")
+ SET(JUNITXML_WRAPPER "$ENV{IMPALA_HOME}/bin/junitxml_command_wrapper.sh")
+ set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ${JUNITXML_WRAPPER})
+ set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ${JUNITXML_WRAPPER})
+ set_property(GLOBAL PROPERTY RULE_LAUNCH_CUSTOM ${JUNITXML_WRAPPER})
+endif()
+
# compile these subdirs using their own CMakeLists.txt
add_subdirectory(common/function-registry)
add_subdirectory(common/thrift)
diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt
index 507da19..638f0a7 100644
--- a/be/CMakeLists.txt
+++ b/be/CMakeLists.txt
@@ -214,8 +214,15 @@ if (CCACHE AND NOT DEFINED ENV{DISABLE_CCACHE})
endif()
endif()
-set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ${RULE_LAUNCH_PREFIX})
-set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ${RULE_LAUNCH_PREFIX})
+# There can be RULE_LAUNCH_COMPILE / RULE_LAUNCH_LINK settings already at the parent
+# level. The parent layer should wrap any launcher used here.
+get_property(PARENT_RULE_LAUNCH_COMPILE GLOBAL PROPERTY RULE_LAUNCH_COMPILE)
+get_property(PARENT_RULE_LAUNCH_LINK GLOBAL PROPERTY RULE_LAUNCH_LINK)
+
+set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE
+ "${PARENT_RULE_LAUNCH_COMPILE} ${RULE_LAUNCH_PREFIX}")
+set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK
+ "${PARENT_RULE_LAUNCH_LINK} ${RULE_LAUNCH_PREFIX}")
# Thrift requires these definitions for some types that we use
add_definitions(-DHAVE_INTTYPES_H -DHAVE_NETINET_IN_H -DHAVE_NETDB_H)
diff --git a/bin/junitxml_command_wrapper.sh b/bin/junitxml_command_wrapper.sh
new file mode 100755
index 0000000..401576e
--- /dev/null
+++ b/bin/junitxml_command_wrapper.sh
@@ -0,0 +1,61 @@
+#!/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.
+
+# This is a simple wrapper that runs the shell command specified by the arguments
+# and generates a JUnitXML file if the command fails. It incorporates the output
+# of the command into the JUnitXML file.
+#
+# This works best when it is invoking a single executable with arguments. There
+# are some known limitations when invoked from the shell (as it would be if invoked
+# by Make):
+# 1. For a string of commands 'junitxml_command_wrapper.sh A && B && C', it only sees
+# the first one (A). The command A runs in its own shell, so any state it sets for
+# the shell is not seen in B && C. For example, if A = "cd directory", then B and
+# C would not see the changed directory.
+# 2. For output piping 'junitxml_command_wrapper.sh A | B", again it only sees the
+# first one (A). It does leave the output unchanged (stdout remains stdout, stderr
+# remains stderr), but if command B fails, it will not generate JUnitXML.
+
+COMMAND=("$@")
+
+# Run the command, piping output to temporary files. Note that this output can
+# contain Unicode characters, because g++ (and likely others) can generate smart
+# quotes.
+STDOUT_TMP_FILE=$(mktemp)
+STDERR_TMP_FILE=$(mktemp)
+# The command's stdout and stderr need to remain separate, and we tee them to separate
+# files. Some CMake build steps have a command like "command1 | command2"
+# and command2 should not see stderr. That also means that this script must not produce
+# its own output when the command runs successfully.
+"${COMMAND[@]}" > >(tee "${STDOUT_TMP_FILE}") 2> >(tee "${STDERR_TMP_FILE}" >&2)
+COMMAND_RET_CODE=${PIPESTATUS[0]}
+if [[ ${COMMAND_RET_CODE} -ne 0 ]]; then
+ # Use a hash of the command to make sure multiple build failures generate distinct
+ # symptoms
+ # TODO: It would make sense to do some better parsing of the command to produce
+ # a better filename.
+ HASH=$(echo "${COMMAND[*]}" | md5sum | cut -d" " -f1)
+ "${IMPALA_HOME}"/bin/generate_junitxml.py --phase build --step "${HASH}" \
+ --error "Build command failed: ${COMMAND[*]}" \
+ --stdout "$(head -n 1000 ${STDOUT_TMP_FILE})" \
+ --stderr "$(head -n 1000 ${STDERR_TMP_FILE})"
+fi
+rm "${STDOUT_TMP_FILE}"
+rm "${STDERR_TMP_FILE}"
+exit "${COMMAND_RET_CODE}"
diff --git a/docker/CMakeLists.txt b/docker/CMakeLists.txt
index a455571..44f1329 100644
--- a/docker/CMakeLists.txt
+++ b/docker/CMakeLists.txt
@@ -57,10 +57,10 @@ if (NOT ${DISTRO_BASE_IMAGE} STREQUAL "UNSUPPORTED")
# Run docker build inside the build context directory so that all dependencies are
# sent to the docker daemon. This allows the Dockerfile built to copy all necessary
# dependencies.
- COMMAND cd ${IMPALA_BASE_BUILD_CONTEXT_DIR}/${build_type} &&
- tar cvh . -C ${CMAKE_SOURCE_DIR}/docker/impala_base/ . |
+ COMMAND tar cvh . -C ${CMAKE_SOURCE_DIR}/docker/impala_base/ . |
docker build -t impala_base_${build_type}
--build-arg BASE_IMAGE=${DISTRO_BASE_IMAGE} -
+ WORKING_DIRECTORY ${IMPALA_BASE_BUILD_CONTEXT_DIR}/${build_type}
DEPENDS impala_base_build_context_${build_type} ${CMAKE_SOURCE_DIR}/docker/impala_base/Dockerfile
DEPENDS ${CMAKE_SOURCE_DIR}/docker/daemon_entrypoint.sh
DEPENDS ${CMAKE_SOURCE_DIR}/bin/graceful_shutdown_backends.sh
@@ -87,10 +87,10 @@ if (NOT ${DISTRO_BASE_IMAGE} STREQUAL "UNSUPPORTED")
# Supply the appropriate base image as an argument for the Dockerfile. The same
# build context used for the base image is used for each daemon image. This allows
# each daemon image to only copy in the dependencies it requires.
- COMMAND cd ${IMPALA_BASE_BUILD_CONTEXT_DIR}/${build_type} &&
- tar cvh . -C ${CMAKE_SOURCE_DIR}/docker/${daemon_name}/ . |
+ COMMAND tar cvh . -C ${CMAKE_SOURCE_DIR}/docker/${daemon_name}/ . |
docker build --build-arg BASE_IMAGE=impala_base_${build_type}
-t ${image_name} -
+ WORKING_DIRECTORY ${IMPALA_BASE_BUILD_CONTEXT_DIR}/${build_type}
DEPENDS impala_base_image_${build_type} ${build_dir}/Dockerfile
COMMENT "Building ${image_name} docker image."
VERBATIM
diff --git a/lib/python/impala_py_lib/jenkins/generate_junitxml.py b/lib/python/impala_py_lib/jenkins/generate_junitxml.py
index 4a1f22f..285aa5d 100755
--- a/lib/python/impala_py_lib/jenkins/generate_junitxml.py
+++ b/lib/python/impala_py_lib/jenkins/generate_junitxml.py
@@ -22,6 +22,7 @@ These files will be consumed by jenkins.impala.io to generate reports for
easier triaging of build and setup errors.
"""
import argparse
+import codecs
import errno
import os
import textwrap
@@ -163,8 +164,8 @@ class JunitReport(object):
)
junit_log_file = os.path.join(junitxml_logdir, filename)
- with open(junit_log_file, 'w') as f:
- f.write(str(self))
+ with codecs.open(junit_log_file, encoding="UTF-8", mode='w') as f:
+ f.write(unicode(self))
return junit_log_file
@@ -175,29 +176,34 @@ class JunitReport(object):
If the supplied parameter is the path to a file, the contents will be inserted
into the XML report. If the parameter is just plain string, use that as the
- content for the report.
+ content for the report. For a file or a string passed in on the commandline,
+ this assumes it could contain Unicode content and converts it to a Unicode
+ object.
Args:
file_or_string: a path to a file, or a plain string
Returns:
- content as a string
+ content as a unicode object
"""
if file_or_string is None:
- content = ''
+ content = u''
elif os.path.exists(file_or_string):
- with open(file_or_string, 'r') as f:
+ with codecs.open(file_or_string, encoding="UTF-8", mode='r') as f:
content = f.read()
else:
- content = file_or_string
+ # This is a string passed in on the command line. Make sure to return it as
+ # a unicode string.
+ content = unicode(file_or_string, encoding="UTF-8")
return content
- def __str__(self):
+ def __unicode__(self):
"""
- Generate and return a pretty-printable XML string.
+ Generate and return a pretty-printable XML unicode string
"""
- root_node_str = minidom.parseString(ET.tostring(self.root_element))
- return root_node_str.toprettyxml(indent=' ' * 4)
+ root_node_unicode = ET.tostring(self.root_element)
+ root_node_dom = minidom.parseString(root_node_unicode)
+ return root_node_dom.toprettyxml(indent=' ' * 4)
def get_options():
@@ -221,7 +227,7 @@ def get_options():
parser.add_argument("--stdout",
help=textwrap.dedent(
"""Standard output to include in the XML report. Can be
- either a string or the path to a file..""")
+ either a string or the path to a file.""")
)
parser.add_argument("--stderr",
help=textwrap.dedent(