kudu git commit: [iwyu_tool.py] fix on the IWYU tool invocation

[al...@apache.org]

Repository: kudu
Updated Branches:
  refs/heads/master 4ee3efed5 -> 4935b1b29


[iwyu_tool.py] fix on the IWYU tool invocation

Fixed the way include-what-you-use binary is invoked by the
iwyu_tool.py wrapper script.  Prior to this fix, subprocess.Popen()
had the 'shell' argument set to 'True', so any issue with starting
the binary was silenced because there would be no OSError exception
raised.  Overall, if using 'shell=True' for subprocess.Popen(),
it wouldn't be feasible to detect that sort of error via examining
the exit code of the started process because include-what-you-use
always returns exit code 1 regardless of its findings.

I also updated the iwyu.sh script to be more robust in handling
possible errors, if any.

Prior to this fix, the 'iwyu' CMake target returned success if
include-what-you-use binary was not in place.  This patch fixes that
problem and introduces more robust handling of other possible issues
while working with the include-what-you-use tool.

Change-Id: Ifa4284857583fff6543f65a9c2a71a9e445f39e2
Reviewed-on: http://gerrit.cloudera.org:8080/7989
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/4935b1b2
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/4935b1b2
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/4935b1b2

Branch: refs/heads/master
Commit: 4935b1b298782aabcdac5b8a5c49c3c15d80d8f6
Parents: 4ee3efe
Author: Alexey Serbin <as...@cloudera.com>
Authored: Wed Sep 6 18:47:47 2017 -0700
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Thu Sep 7 04:21:55 2017 +0000

----------------------------------------------------------------------
 build-support/iwyu/iwyu.sh      | 29 +++++++++++++++++++++--------
 build-support/iwyu/iwyu_tool.py | 17 +++++++++--------
 2 files changed, 30 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/4935b1b2/build-support/iwyu/iwyu.sh
----------------------------------------------------------------------
diff --git a/build-support/iwyu/iwyu.sh b/build-support/iwyu/iwyu.sh
index 6c529fc..52e81eb 100755
--- a/build-support/iwyu/iwyu.sh
+++ b/build-support/iwyu/iwyu.sh
@@ -17,19 +17,26 @@
 # specific language governing permissions and limitations
 # under the License.
 
-ROOT=$(cd $(dirname $BASH_SOURCE)/../..; pwd)
+set -e
+set -o pipefail
 
-IWYU_LOG=$(mktemp -t kudu-iwyu.XXXXXX)
-trap "rm -f $IWYU_LOG" EXIT
+ROOT=$(cd $(dirname $BASH_SOURCE)/../..; pwd)
 
 # Build the list of updated files which are of IWYU interest.
+# Since '-e' is set, transform the exit code from the grep accordingly:
+# grep returns 1 if no lines were selected.
 file_list_tmp=$(git diff --name-only \
-    $($ROOT/build-support/get-upstream-commit.sh) | grep -E '\.(c|cc|h)$')
+    $($ROOT/build-support/get-upstream-commit.sh) | \
+    (grep -E '\.(c|cc|h)$' || [ $? -eq 1 ]))
 if [ -z "$file_list_tmp" ]; then
   echo "IWYU verification: no updates on related files, declaring success"
   exit 0
 fi
 
+IWYU_LOG=$(mktemp -t kudu-iwyu.XXXXXX)
+UNFILTERED_IWYU_LOG=${IWYU_LOG}.unfiltered
+trap "rm -f $IWYU_LOG $UNFILTERED_IWYU_LOG" EXIT
+
 # Adjust the path for every element in the list. The iwyu_tool.py normalizes
 # paths (via realpath) to match the records from the compilation database.
 IWYU_FILE_LIST=
@@ -47,13 +54,19 @@ IWYU_ARGS="\
     --mapping_file=$IWYU_MAPPINGS_PATH/gtest.imp \
     --mapping_file=$IWYU_MAPPINGS_PATH/libstdcpp.imp"
 
-PATH="$PATH:$PWD/../../thirdparty/clang-toolchain/bin" \
+if ! PATH="$PATH:$PWD/../../thirdparty/clang-toolchain/bin" \
     python $ROOT/build-support/iwyu/iwyu_tool.py -p . $IWYU_FILE_LIST -- \
-    $IWYU_ARGS | awk -f $ROOT/build-support/iwyu/iwyu-filter.awk | \
-    tee $IWYU_LOG
+    $IWYU_ARGS > $UNFILTERED_IWYU_LOG 2>&1; then
+  echo "IWYU verification: failed to run the tool, see below for details"
+  cat $UNFILTERED_IWYU_LOG
+  exit 2
+fi
 
+awk -f $ROOT/build-support/iwyu/iwyu-filter.awk $UNFILTERED_IWYU_LOG | \
+    tee $IWYU_LOG
 if [ -s "$IWYU_LOG" ]; then
-  # The output is not empty: the changelist needs correction.
+  # The output is not empty: IWYU finds the set of headers to be inconsistent.
+  echo "IWYU verification: changelist needs correction, see above for details"
   exit 1
 fi
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/4935b1b2/build-support/iwyu/iwyu_tool.py
----------------------------------------------------------------------
diff --git a/build-support/iwyu/iwyu_tool.py b/build-support/iwyu/iwyu_tool.py
index a3dd4a1..8943226 100755
--- a/build-support/iwyu/iwyu_tool.py
+++ b/build-support/iwyu/iwyu_tool.py
@@ -135,11 +135,10 @@ FORMATTERS = {
     'clang': clang_formatter
 }
 
-def get_output(cwd, command):
+def get_output(cwd, args):
     """ Run the given command and return its output as a string. """
-    process = subprocess.Popen(command,
+    process = subprocess.Popen(args=args,
                                cwd=cwd,
-                               shell=True,
                                stdout=subprocess.PIPE,
                                stderr=subprocess.STDOUT)
     return process.communicate()[0].decode("utf-8").splitlines()
@@ -154,14 +153,16 @@ def run_iwyu(cwd, compile_command, iwyu_args, verbose, formatter):
     else:
         clang_args = []
 
-    iwyu_args = ['-Xiwyu ' + a for a in iwyu_args]
-    command = ['include-what-you-use'] + clang_args + iwyu_args
-    command = '%s %s' % (' '.join(command), args.strip())
+    extra_args = []
+    for a in iwyu_args:
+        extra_args += ['-Xiwyu', a]
+
+    cmd_args = ['include-what-you-use'] + clang_args + extra_args + args.split()
 
     if verbose:
-        print('%s:' % command)
+        print('{}'.format(cmd_args))
 
-    formatter(get_output(cwd, command))
+    formatter(get_output(cwd, cmd_args))
 
 
 def main(compilation_db_path, source_files, verbose, formatter, iwyu_args):