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 <joemcdonnell@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>

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)

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})
+# There can be RULE_LAUNCH_COMPILE / RULE_LAUNCH_LINK settings already at the parent
-set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ${RULE_LAUNCH_PREFIX})
+# 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)

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}"

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} &&
+      COMMAND tar cvh . -C ${CMAKE_SOURCE_DIR}/docker/impala_base/ . |
-              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} &&
+      COMMAND tar cvh . -C ${CMAKE_SOURCE_DIR}/docker/${daemon_name}/ . |
-              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

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:
+    with codecs.open(junit_log_file, encoding="UTF-8", mode='w') as f:
-      f.write(str(self))
+      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))
+    root_node_unicode = ET.tostring(self.root_element)
-    return root_node_str.toprettyxml(indent=' ' * 4)
+    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(