tools: heuristically skip proto and Python format checks. (envoyproxy…

…#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 envoyproxy#10340 Signed-off-by: Harvey Tuch <[email protected]>
bestjss · Mar 25, 2020 · 5978ff3 · 5978ff3
1 parent fa91212
commit 5978ff3
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 <[email protected]>" --max-count=1 HEAD
diff --git a/tools/git/modified_since_last_github_commit.sh 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
@@ -6,6 +6,17 @@ set -e
 
 [[ "$1" == "check" || "$1" == "fix" ]] || (echo "Usage: $0 <check|fix>"; 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