Skip to content

Commit a399830

Browse files
authored
cmake format target: add whitespace correction (apache#12696)
Our format target does formatting of C++ and Python files via clang-format and yapf. Annoyingly, it hasn't done some extra whitespace checks that CI does. So a PR can be made that passes `format` locally but then CI will reject it for trailing whitespace, or DOS line endinds. This adds a new tools/whitespace-format.sh script that automatically fixes trailing whitespace and DOS carriage returns, matching the checks performed by CI. This script is then integrated into the `format` cmake target so it gets run for that. I verified that the script works on both Linux and Mac.
1 parent 31f7f56 commit a399830

File tree

3 files changed

+103
-1
lines changed

3 files changed

+103
-1
lines changed

CMakeLists.txt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,10 +806,18 @@ add_custom_target(
806806
VERBATIM
807807
)
808808

809+
add_custom_target(
810+
whitespace-format
811+
${CMAKE_SOURCE_DIR}/tools/whitespace-format.sh
812+
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
813+
COMMENT "fixing whitespace issues (trailing whitespace, DOS line endings)"
814+
VERBATIM
815+
)
816+
809817
# Add a format target that runs all the formatters.
810818
add_custom_target(
811819
format
812-
DEPENDS clang-format yapf cmake-format
820+
DEPENDS clang-format yapf cmake-format whitespace-format
813821
COMMENT "formatting all files"
814822
)
815823

tools/git/pre-commit

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,32 @@ else
123123
echo
124124
fi
125125

126+
# Check for trailing whitespace in staged files.
127+
TRAILING_WS=$(git diff-index --cached --diff-filter=ACMR --name-only HEAD | grep -vE "lib/(yamlcpp|Catch2|systemtap)" | grep -vE "\.(gold|test_input)$" | xargs git diff --cached | grep -E '^\+.*[[:space:]]$' || true)
128+
if [ -n "$TRAILING_WS" ]; then
129+
echo "The commit is not accepted because it contains trailing whitespace."
130+
echo "The easiest way to fix this is to run:"
131+
echo
132+
echo " $ cmake --build <build_dir> --target format"
133+
exit 1
134+
else
135+
echo "This commit complies with the trailing whitespace rules."
136+
echo
137+
fi
138+
139+
# Check for DOS carriage returns in staged files.
140+
DOS_CR=$(git diff-index --cached --diff-filter=ACMR --name-only HEAD | grep -vE "lib/(yamlcpp|Catch2|systemtap)" | grep -vE "\.test_input$" | xargs git diff --cached | grep -E '^\+.*\r$' || true)
141+
if [ -n "$DOS_CR" ]; then
142+
echo "The commit is not accepted because it contains DOS carriage returns."
143+
echo "The easiest way to fix this is to run:"
144+
echo
145+
echo " $ cmake --build <build_dir> --target format"
146+
exit 1
147+
else
148+
echo "This commit complies with the DOS line ending rules."
149+
echo
150+
fi
151+
126152
# Cleanup before exit
127153
rm -f "$clang_patch_file" "$yapf_patch_file"
128154
exit 0

tools/whitespace-format.sh

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
#!/usr/bin/env bash
2+
#
3+
# Script to fix whitespace issues: trailing whitespace and DOS carriage returns.
4+
#
5+
# Licensed to the Apache Software Foundation (ASF) under one
6+
# or more contributor license agreements. See the NOTICE file
7+
# distributed with this work for additional information
8+
# regarding copyright ownership. The ASF licenses this file
9+
# to you under the Apache License, Version 2.0 (the
10+
# "License"); you may not use this file except in compliance
11+
# with the License. You may obtain a copy of the License at
12+
#
13+
# http://www.apache.org/licenses/LICENSE-2.0
14+
#
15+
# Unless required by applicable law or agreed to in writing, software
16+
# distributed under the License is distributed on an "AS IS" BASIS,
17+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
18+
# See the License for the specific language governing permissions and
19+
# limitations under the License.
20+
21+
set -e
22+
23+
REPO_ROOT=$(cd $(dirname $0) && git rev-parse --show-toplevel)
24+
echo "Fixing whitespace issues in ${REPO_ROOT}"
25+
26+
# macOS requires an empty string argument for in-place sed edits.
27+
if [[ "$OSTYPE" == "darwin"* ]]; then
28+
SED_INPLACE=(sed -i '')
29+
else
30+
SED_INPLACE=(sed -i)
31+
fi
32+
33+
FILES_MODIFIED=0
34+
35+
echo "Checking for trailing whitespace..."
36+
TRAILING_WS_FILES=$(git grep -IE ' +$' | eval "grep -F -v 'lib/yamlcpp' | grep -F -v 'lib/Catch2' | grep -F -v 'lib/systemtap' | grep -F -v '.gold:' | grep -F -v '.test_input'" | cut -d: -f1 | sort -u || true)
37+
38+
if [ -n "$TRAILING_WS_FILES" ]; then
39+
while IFS= read -r file; do
40+
if [ -f "$file" ]; then
41+
echo " Fixing trailing whitespace in: $file"
42+
"${SED_INPLACE[@]}" 's/[[:space:]]*$//' "$file"
43+
FILES_MODIFIED=$((FILES_MODIFIED + 1))
44+
fi
45+
done <<< "$TRAILING_WS_FILES"
46+
fi
47+
48+
echo "Checking for DOS carriage returns..."
49+
DOS_FILES=$(git grep -IE $'\r$' | eval "grep -F -v 'lib/yamlcpp' | grep -F -v 'lib/Catch2' | grep -F -v 'lib/systemtap' | grep -F -v '.test_input'" | cut -d: -f1 | sort -u || true)
50+
51+
if [ -n "$DOS_FILES" ]; then
52+
while IFS= read -r file; do
53+
if [ -f "$file" ]; then
54+
echo " Removing DOS carriage returns from: $file"
55+
"${SED_INPLACE[@]}" $'s/\r$//' "$file"
56+
FILES_MODIFIED=$((FILES_MODIFIED + 1))
57+
fi
58+
done <<< "$DOS_FILES"
59+
fi
60+
61+
if [ $FILES_MODIFIED -eq 0 ]; then
62+
echo "Success! No whitespace issues found."
63+
else
64+
echo "Fixed whitespace issues in $FILES_MODIFIED file(s)."
65+
fi
66+
67+
exit 0
68+

0 commit comments

Comments
 (0)