From 5978ff3cb90077c5d6b6067c0e100863cfb79683 Mon Sep 17 00:00:00 2001 From: htuch Date: Tue, 24 Mar 2020 20:21:34 -0400 Subject: [PATCH] tools: heuristically skip proto and Python format checks. (#10490) * tools: heuristically skip proto and Python format checks. This patch introduces a conservative heuristic to allow us to skip the time consuming proto and Python format checks. It does this by looking back to the last commit in the history introduced by GitHub, and if no relevant .proto or .py is present, skipping the respective check. The heuristic is conservative; a .proto may have been introduced, formatted and thus a redundant format occurs. Still, this should save many developer cycles waiting when their PRs have nothing to do with APIs or Python. Risk level: Low (developer tooling only) Testing: manual Fixes #10340 Signed-off-by: Harvey Tuch --- ci/check_and_fix_format.sh | 6 ++++++ ci/do_ci.sh | 4 +--- tools/code_format/format_python_tools.sh | 4 ++++ tools/git/last_github_commit.sh | 8 ++++++++ tools/git/modified_since_last_github_commit.sh | 7 +++++++ tools/proto_format/proto_format.sh | 11 +++++++++++ 6 files changed, 37 insertions(+), 3 deletions(-) create mode 100755 tools/git/last_github_commit.sh create mode 100755 tools/git/modified_since_last_github_commit.sh diff --git a/ci/check_and_fix_format.sh b/ci/check_and_fix_format.sh index 7ae4a7ca2d0b..7d5fe0a54d18 100755 --- a/ci/check_and_fix_format.sh +++ b/ci/check_and_fix_format.sh @@ -4,6 +4,12 @@ set -e DIFF_OUTPUT="${DIFF_OUTPUT:-/build/fix_format.diff}" +# We set this for two reasons. First, we want to ensure belt-and-braces that we check these formats +# in CI in case the skip-on-file-change heuristics in proto_format.sh etc. are buggy. Second, this +# prevents AZP cache weirdness. +export FORCE_PROTO_FORMAT=yes +export FORCE_PYTHON_FORMAT=yes + function fix { set +e ci/do_ci.sh fix_format diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 3194460a27c3..5495afaceded 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -303,12 +303,10 @@ elif [[ "$CI_TARGET" == "bazel.fuzzit" ]]; then elif [[ "$CI_TARGET" == "fix_format" ]]; then # proto_format.sh needs to build protobuf. setup_clang_toolchain - echo "protoxform_test..." - ./tools/protoxform/protoxform_test.sh echo "fix_format..." ./tools/code_format/check_format.py fix ./tools/code_format/format_python_tools.sh fix - ./tools/proto_format/proto_format.sh fix + ./tools/proto_format/proto_format.sh fix --test exit 0 elif [[ "$CI_TARGET" == "check_format" ]]; then # proto_format.sh needs to build protobuf. diff --git a/tools/code_format/format_python_tools.sh b/tools/code_format/format_python_tools.sh index 56587489a73a..ac755e3dff52 100755 --- a/tools/code_format/format_python_tools.sh +++ b/tools/code_format/format_python_tools.sh @@ -1,5 +1,9 @@ #!/bin/bash +"$(dirname "$0")"/../git/modified_since_last_github_commit.sh ./ py || \ + [[ "${FORCE_PYTHON_FORMAT}" == "yes" ]] || \ + { echo "Skipping format_python_tools.sh due to no Python changes"; exit 0; } + . tools/shell_utils.sh set -e diff --git a/tools/git/last_github_commit.sh b/tools/git/last_github_commit.sh new file mode 100755 index 000000000000..9746d259ac3b --- /dev/null +++ b/tools/git/last_github_commit.sh @@ -0,0 +1,8 @@ +#!/bin/bash + +# Looking back from HEAD, find the first commit that was merged onto master by GitHub. This is +# likely the last non-local change on a given branch. There may be some exceptions for this +# heuristic, e.g. when patches are manually merged for security fixes on master, but this is very +# rare. + +git rev-list --no-merges --committer="GitHub " --max-count=1 HEAD diff --git a/tools/git/modified_since_last_github_commit.sh b/tools/git/modified_since_last_github_commit.sh new file mode 100755 index 000000000000..bbb9d388a239 --- /dev/null +++ b/tools/git/modified_since_last_github_commit.sh @@ -0,0 +1,7 @@ +#!/bin/bash + +declare -r BASE="$(dirname "$0")" +declare -r TARGET_PATH=$1 +declare -r EXTENSION=$2 + +git diff --name-only $("${BASE}"/last_github_commit.sh)..HEAD | grep "\.${EXTENSION}$" diff --git a/tools/proto_format/proto_format.sh b/tools/proto_format/proto_format.sh index 9b2fe06b92a0..c899c284fb4b 100755 --- a/tools/proto_format/proto_format.sh +++ b/tools/proto_format/proto_format.sh @@ -6,6 +6,17 @@ set -e [[ "$1" == "check" || "$1" == "fix" ]] || (echo "Usage: $0 "; exit 1) +# Developers working on protoxform and other proto format tooling changes will need to override the +# following check by setting FORCE_PROTO_FORMAT=yes in the environment. +./tools/git/modified_since_last_github_commit.sh ./api/envoy proto || \ + [[ "${FORCE_PROTO_FORMAT}" == "yes" ]] || \ + { echo "Skipping proto_format.sh due to no API change"; exit 0; } + +if [[ "$2" == "--test" ]] +then + ./tools/protoxform/protoxform_test.sh +fi + # Clean up any stale files in the API tree output. Bazel remembers valid cached # files still. # rm -rf bazel-bin/external/envoy_api