From bb0d2426ba0d8d29e917a4ce4451eec886c72d63 Mon Sep 17 00:00:00 2001 From: John Bytheway <52664+jbytheway@users.noreply.github.com> Date: Fri, 23 Jul 2021 16:36:30 -0400 Subject: [PATCH] Minimize clang-tidy CI runtime (#50131) clang-tidy is one of our slowest CI jobs. It would be nice if it were faster. When only cpp files are changed, we can limit the checks to just those files. When a header or build file is changed, or a new check is added, we need to recheck everything. To make this work, update the logic in build.sh which previously prioritised changed files. Make it now only check changed files, provided that's appropriate. Also, make files_changed work correctly on GitHub, at least for PR jobs. It will simply fail on non-PR jobs. --- build-scripts/build.sh | 16 ++++++++++++--- build-scripts/files_changed | 40 +++++++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/build-scripts/build.sh b/build-scripts/build.sh index 573e7a4d74c9d..008794662c3d4 100755 --- a/build-scripts/build.sh +++ b/build-scripts/build.sh @@ -130,14 +130,17 @@ then cd .. ln -s build/compile_commands.json + ./build-scripts/files_changed + # We want to first analyze all files that changed in this PR, then as # many others as possible, in a random order. set +x all_cpp_files="$( \ grep '"file": "' build/compile_commands.json | \ sed "s+.*$PWD/++;s+\"$++")" + changed_files="$( ./build-scripts/files_changed || echo unknown )" changed_cpp_files="$( \ - ./build-scripts/files_changed | grep -F "$all_cpp_files" || true )" + echo "$changed_files" | grep -F "$all_cpp_files" || true )" if [ -n "$changed_cpp_files" ] then remaining_cpp_files="$( \ @@ -160,8 +163,15 @@ then echo "Analyzing changed files" analyze_files_in_random_order "$changed_cpp_files" - echo "Analyzing remaining files" - analyze_files_in_random_order "$remaining_cpp_files" + # Check for changes to any files that would require us to run clang-tidy across everything + changed_global_files="$( \ + echo "$changed_files" | egrep -i "\.h$|clang-tidy-plugin|cmake|unknown" || true )" + if [ -n "$changed_global_files" ] + then + first_changed_file="$(echo "$changed_global_files" | head -n 1)" + echo "Analyzing remaining files because $first_changed_file was changed" + analyze_files_in_random_order "$remaining_cpp_files" + fi set -x else # Regular build diff --git a/build-scripts/files_changed b/build-scripts/files_changed index 27bbd9ef097be..b522337362f13 100755 --- a/build-scripts/files_changed +++ b/build-scripts/files_changed @@ -3,15 +3,39 @@ # Helper script to get the list of changed files for this build. # If a PR it lists the changes from the target branch. # If a 'regular' branch build the changes since the last build. -# https://docs.travis-ci.com/user/environment-variables/ -if [ -n $TRAVIS_COMMIT_RANGE ] +set -e + +function files_list +{ + git diff --name-only --diff-filter ACMRT "$@" +} + +if [ "$TRAVIS" = "true" ] +then + # https://docs.travis-ci.com/user/environment-variables/ + if [ -n $TRAVIS_COMMIT_RANGE ] + then + # If this string is populated, it should work. + # But it mught fail, due to e.g. the shallow clone depth missing the + # relevant commit, in which case we fail + files_list $TRAVIS_COMMIT_RANGE || exit 1 + else + # The only time it isn't populated is on a new PR branch, where THIS will work. + files_list $TRAVIS_BRANCH + fi +elif [ -n "$GITHUB_BASE_REF" ] then - # If this string is populated, it should work. - # But it mught fail, due to e.g. the shallow clone depth missing the - # relevant commit, in which case we fail - git diff --name-only $TRAVIS_COMMIT_RANGE || exit 1 + base="origin/$GITHUB_BASE_REF" + # We need to fetch enough commits to be able to compute the merge-base + # between this PR and master + commits_to_fetch=10 + while [ -z "$( git merge-base "$base" HEAD^2 2>/dev/null )" ] + do + git fetch -q --deepen=$commits_to_fetch origin "$GITHUB_BASE_REF" HEAD + (( commits_to_fetch *= 2 )) + done + files_list "$( git merge-base "$base" HEAD^2 )..HEAD^2" else - # The only time it isn't populated is on a new PR branch, where THIS will work. - git diff --name-only $TRAVIS_BRANCH + exit 1 fi