diff --git a/.travis.yml b/.travis.yml index fbd3915b515..6af73e31878 100644 --- a/.travis.yml +++ b/.travis.yml @@ -40,7 +40,7 @@ matrix: compiler: clang env: COMPILER=clang++ - env: NAME="CPP-LINT" - script: scripts/run_lint.sh master HEAD || true + script: scripts/travis_lint.sh || true script: - if [ -L bin/gcc ] ; then export PATH=$PWD/bin:$PATH ; fi ; diff --git a/scripts/filter_lint_by_diff.py b/scripts/filter_lint_by_diff.py new file mode 100755 index 00000000000..a5900dd7e81 --- /dev/null +++ b/scripts/filter_lint_by_diff.py @@ -0,0 +1,33 @@ +#!/usr/bin/env python + +import sys +import unidiff +import os.path + +if len(sys.argv) != 3: + print >>sys.stderr, "Usage: filter_lint_by_diff.py diff.patch repository_root_directory < cpplint_warnings.txt" + sys.exit(1) + +added_lines = set() +repository_root = sys.argv[2] + +with open(sys.argv[1], "r") as f: + diff = unidiff.PatchSet(f) + for diff_file in diff: + filename = diff_file.target_file + assert filename.startswith("b/") + filename = os.path.join(repository_root, filename[2:]) + added_lines.add((filename, 0)) + for diff_hunk in diff_file: + for diff_line in diff_hunk: + if diff_line.line_type == "+": + added_lines.add((filename, diff_line.target_line_no)) + +for l in sys.stdin: + bits = l.split(":") + if len(bits) < 3: + continue + filename = os.path.join(repository_root, bits[0]) + linenum = int(bits[1]) + if (filename, linenum) in added_lines: + sys.stdout.write(l) diff --git a/scripts/run_lint.sh b/scripts/run_lint.sh index 8208c109c67..9fc724fb92f 100755 --- a/scripts/run_lint.sh +++ b/scripts/run_lint.sh @@ -3,14 +3,13 @@ set -e script_folder=`dirname $0` +absolute_repository_root=`readlink -f $script_folder/..` -if [[ "$#" -ne 2 ]] +if [[ "$#" -gt 2 ]] then - echo "Script for running the CPP linter only on modified lines." - echo "Requires two arguments the start and the end" - echo "start - a git reference that marks the first commit whose changes to consider" - echo "end - a git reference that marks the last commit whose changes to consider" - + echo "Script for running the CPP linter only on modified lines. Arguments:" + echo "target - a git reference to the branch we want to compare against (default: 'master')" + echo "tip - a git reference to the commit with changes (default: current working tree)" exit 1 fi @@ -21,75 +20,49 @@ then exit 1 fi -git_start=$1 -git_end=$2 +if [[ "$#" -gt 0 ]] +then + git_start=$1 +else + git_start="master" +fi -# Get the list of files that have changed -diff_files=`git diff --name-only $git_start $git_end` +if [[ "$#" -gt 1 ]] +then + git_end=$2 + git_merge_base_end=$2 +else + git_end="" + git_merge_base_end="HEAD" +fi + +git_start=`git merge-base $git_start $git_merge_base_end` -# Build a filter that will filter the blame output -# to only include lines that come from one of the relevant_commits -# We do this by making the blame tool output the same hash for all -# lines that are too old. -blame_grep_filter=`git rev-parse "$git_start"` +cleanup() +{ + rm -f $diff_file +} -# Build a regex for finding the line number of a given line inside blame -# First matches the 40 digit hash of the commi -# Then match an arbitary length number that represents the line in the original file -# Finally matches (and groups) another arbitary length digit which is the -# line in the final file -regex="[0-9a-f]{40} [0-9]+ ([0-9]+)" +trap cleanup EXIT -# We only split on lines or otherwise the git blame output is nonsense -IFS=$'\n' +diff_file=`mktemp` -are_errors=0 +git diff $git_start $git_end > $diff_file + +# Get the list of files that have changed +diff_files=`git diff --name-only $git_start $git_end` for file in $diff_files; do + file=$absolute_repository_root/$file # If the file has been deleted we don't want to run the linter on it if ! [[ -e $file ]] then continue fi - # We build another grep filter the output of the linting script - lint_grep_filter="^(" - - # Include line 0 errors (e.g. copyright) - lint_grep_filter+=$file - lint_grep_filter+=":0:" - - # We first filter only the lines that start with a commit hash - # Then we filter out the ones that come from the start commit - modified_lines=`git blame $git_start..$git_end --line-porcelain $file | grep -E "^[0-9a-f]{40}" | { grep -v "$blame_grep_filter" || true; }` - - # For each modified line we find the line number - for line in $modified_lines; do - - # Use the above regex to match the line number - if [[ $line =~ $regex ]] - then - # Some bash magic to get the first group from the regex (the line number) - LINENUM="${BASH_REMATCH[1]}" - - # The format from the linting script is filepath:linenum: [error type] - # So we build the first bit to filter out relevant lines - LINE_FILTER=$file:$LINENUM: - - # Add the line filter on to the grep expression as we want - # lines that match any of the line filters - lint_grep_filter+="|" - lint_grep_filter+=$LINE_FILTER - fi - done - - # Add the closing bracket - lint_grep_filter+=")" - - # Run the linting script and filter by the filter we've build - # of all the modified lines + # Run the linting script and filter: # The errors from the linter go to STDERR so must be redirected to STDOUT - result=`$script_folder/cpplint.py $file 2>&1 | { grep -E "$lint_grep_filter" || true; }` + result=`$script_folder/cpplint.py $file 2>&1 | $script_folder/filter_lint_by_diff.py $diff_file $absolute_repository_root` # Providing some errors were relevant we print them out if [ "$result" ] @@ -99,7 +72,5 @@ for file in $diff_files; do fi done -unset IFS - # Return an error code if errors are found exit $are_errors diff --git a/scripts/travis_lint.sh b/scripts/travis_lint.sh new file mode 100755 index 00000000000..dc1ffb0b81b --- /dev/null +++ b/scripts/travis_lint.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +set -e + +script_folder=`dirname $0` +pip install --user unidiff + +if [ "$TRAVIS_PULL_REQUEST" == "false" ]; then + $script_folder/run_lint.sh HEAD~1 # Check for errors introduced in last commit +else + $script_folder/run_lint.sh $TRAVIS_BRANCH # Check for errors compared to merge target +fi +