Stop trying to skip lint checks if tools are missing (#13411)
This functionality is broken. The DEBUG trap runs *before* each command,
not after. Better to have a clear error message than confusing failures,
which is what we get now. The logic for doing this was also super
verbose and complicated.
We should really rewrite this whole thing in Python.
skip-ci: lint scripts don't affect CI
diff --git a/build_tools/scripts/lint.sh b/build_tools/scripts/lint.sh
index a5002e8..152e9ed 100755
--- a/build_tools/scripts/lint.sh
+++ b/build_tools/scripts/lint.sh
@@ -6,8 +6,8 @@
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-# Runs all the lint checks that we run on GitHub locally. Skips checks if the
-# relevant tool doesn't exist.
+# Runs all the lint checks that we run on GitHub locally. Will fail if any tool
+# is missing.
# Keep this in sync with .github/workflows/lint.yml
@@ -16,30 +16,22 @@
set -uo pipefail
FINAL_RET=0
-LATEST_RET=0
+PREV_CMD=""
scripts_dir="$(dirname $0)"
function update_ret() {
- LATEST_RET="$?"
- if [[ "${LATEST_RET}" -gt "${FINAL_RET}" ]]; then
- FINAL_RET="${LATEST_RET}"
+ local prev_ret="$?"
+ local cur_cmd="${BASH_COMMAND}"
+ if [[ "${prev_ret}" -gt "${FINAL_RET}" ]]; then
+ FINAL_RET="${prev_ret}"
+ FAILING_CMD="${PREV_CMD}"
fi
+ PREV_CMD="${cur_cmd}"
}
-# Update the exit code after every command
-function enable_update_ret() {
- trap update_ret DEBUG
-}
-
-function disable_update_ret() {
- trap - DEBUG
-}
-
-function exists() {
- command -v "${1}" > /dev/null
-}
-
+# Analyze the exit code of the previous command before every command
+trap update_ret DEBUG
echo "***** Uncommitted changes *****"
git add -A
@@ -52,84 +44,35 @@
exit 1
fi
-enable_update_ret
-
echo "***** Bazel -> CMake *****"
./build_tools/bazel_to_cmake/bazel_to_cmake.py
./build_tools/bazel_to_cmake/bazel_to_cmake.py --root_dir=integrations/tensorflow/e2e
git add -A
git diff HEAD --exit-code
-trap - DEBUG
echo "***** buildifier *****"
-# Don't fail script if condition is false
-disable_update_ret
-if exists buildifier; then
- enable_update_ret
- ${scripts_dir}/run_buildifier.sh
- git diff --exit-code
-else
- enable_update_ret
- echo "'buildifier' not found. Skipping check"
-fi
+${scripts_dir}/run_buildifier.sh
+git diff --exit-code
echo "***** yapf *****"
# Don't fail script if condition is false
-disable_update_ret
-if exists yapf; then
- enable_update_ret
- git diff -U0 main | ./third_party/format_diff/format_diff.py yapf -i
-else
- enable_update_ret
- echo "'yapf' not found. Skipping check"
-fi
+git diff -U0 main | ./third_party/format_diff/format_diff.py yapf -i
echo "***** pytype *****"
-# Don't fail script if condition is false
-disable_update_ret
-if exists pytype; then
- enable_update_ret
- ./build_tools/pytype/check_diff.sh
-else
- enable_update_ret
- echo "'pytype' not found. Skipping check"
-fi
+./build_tools/pytype/check_diff.sh
echo "***** clang-format *****"
-# Don't fail script if condition is false
-disable_update_ret
-if exists git-clang-format; then
- enable_update_ret
- git-clang-format --style=file main
- git diff --exit-code
-else
- enable_update_ret
- echo "'git-clang-format' not found. Skipping check"
-fi
+git-clang-format --style=file main
+git diff --exit-code
echo "***** tabs *****"
-${scripts_dir}/check_tabs.sh
+"${scripts_dir}/check_tabs.sh"
echo "***** yamllint *****"
-disable_update_ret
-if exists yamllint; then
- enable_update_ret
- ${scripts_dir}/run_yamllint.sh
-else
- enable_update_ret
- echo "'yamllint' not found. Skipping check"
-fi
+"${scripts_dir}/run_yamllint.sh"
echo "***** markdownlint *****"
-# Don't fail script if condition is false
-disable_update_ret
-if exists markdownlint; then
- enable_update_ret
- ${scripts_dir}/run_markdownlint.sh
-else
- enable_update_ret
- echo "'markdownlint' not found. Skipping check"
-fi
+"${scripts_dir}/run_markdownlint.sh"
echo "***** Path Lengths *****"
./build_tools/scripts/check_path_lengths.py
@@ -147,8 +90,9 @@
fi
if (( "${FINAL_RET}" != 0 )); then
- echo "Encountered failures. Check error messages and changes to the working" \
- "directory and git index (which may contain fixes) and try again."
+ echo "Encountered failures when running: '${FAILING_CMD}'. Check error" \
+ "messages and changes to the working directory and git index (which" \
+ "may contain fixes) and try again."
fi
exit "${FINAL_RET}"
diff --git a/build_tools/scripts/run_buildifier.sh b/build_tools/scripts/run_buildifier.sh
index 6edae4c..735d197 100755
--- a/build_tools/scripts/run_buildifier.sh
+++ b/build_tools/scripts/run_buildifier.sh
@@ -10,7 +10,7 @@
# reference commit (default "main")
# See https://github.com/bazelbuild/buildtools/tree/master/buildifier
-set -o pipefail
+set -euo pipefail
BASE_REF="${1:-main}"
diff --git a/build_tools/scripts/run_markdownlint.sh b/build_tools/scripts/run_markdownlint.sh
index 7a717da..652c7ea 100755
--- a/build_tools/scripts/run_markdownlint.sh
+++ b/build_tools/scripts/run_markdownlint.sh
@@ -9,7 +9,7 @@
# Runs Markdownlint on Markdown (.md) files
# https://github.com/igorshubovych/markdownlint-cli
-set -xuo pipefail
+set -euo pipefail
declare -a included_files_patterns=(
# All .md files (disabled while we decide how rigorously to apply lint checks)
diff --git a/build_tools/scripts/run_yamllint.sh b/build_tools/scripts/run_yamllint.sh
index 979fe0d..adfc6df 100755
--- a/build_tools/scripts/run_yamllint.sh
+++ b/build_tools/scripts/run_yamllint.sh
@@ -9,7 +9,7 @@
# Runs yamllint on files modified vs the specified reference commit
# (default "main")
-set -uo pipefail
+set -euo pipefail
BASE_REF="${1:-main}"
