scripts: Break check_format up into functions

Change-Id: I3e3bd44826453e0ff5cc5b01769e81b5fdd660ff
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/4073
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
This commit is contained in:
Ben Walker 2020-09-04 14:29:22 -07:00 committed by Tomasz Zawadzki
parent 9f6f02f22e
commit d61da0ccda

View File

@ -33,9 +33,12 @@ function array_contains_string() {
rc=0 rc=0
echo -n "Checking file permissions..." function check_permissions() {
echo -n "Checking file permissions..."
while read -r perm _res0 _res1 path; do local rc=0
while read -r perm _res0 _res1 path; do
if [ ! -f "$path" ]; then if [ ! -f "$path" ]; then
continue continue
fi fi
@ -74,13 +77,19 @@ while read -r perm _res0 _res1 path; do
;; ;;
esac esac
done <<< "$(git grep -I --name-only --untracked -e . | git ls-files -s)" done <<< "$(git grep -I --name-only --untracked -e . | git ls-files -s)"
if [ $rc -eq 0 ]; then if [ $rc -eq 0 ]; then
echo " OK" echo " OK"
fi fi
if hash astyle; then return $rc
}
function check_c_style() {
local rc=0
if hash astyle; then
echo -n "Checking coding style..." echo -n "Checking coding style..."
if [ "$(astyle -V)" \< "Artistic Style Version 3" ]; then if [ "$(astyle -V)" \< "Artistic Style Version 3" ]; then
echo -n " Your astyle version is too old so skipping coding style checks. Please update astyle to at least 3.0.1 version..." echo -n " Your astyle version is too old so skipping coding style checks. Please update astyle to at least 3.0.1 version..."
@ -108,120 +117,159 @@ if hash astyle; then
fi fi
rm -f astyle.log rm -f astyle.log
fi fi
else else
echo "You do not have astyle installed so your code style is not being checked!" echo "You do not have astyle installed so your code style is not being checked!"
fi fi
return $rc
}
GIT_VERSION=$(git --version | cut -d' ' -f3) function check_comment_style() {
local rc=0
if version_lt "1.9.5" "${GIT_VERSION}"; then echo -n "Checking comment style..."
# git <1.9.5 doesn't support pathspec magic exclude
echo " Your git version is too old to perform all tests. Please update git to at least 1.9.5 version..."
exit 0
fi
echo -n "Checking comment style..." git grep --line-number -e '\/[*][^ *-]' -- '*.[ch]' > comment.log || true
git grep --line-number -e '[^ ][*]\/' -- '*.[ch]' ':!lib/rte_vhost*/*' >> comment.log || true
git grep --line-number -e '^[*]' -- '*.[ch]' >> comment.log || true
git grep --line-number -e '\s\/\/' -- '*.[ch]' >> comment.log || true
git grep --line-number -e '^\/\/' -- '*.[ch]' >> comment.log || true
git grep --line-number -e '\/[*][^ *-]' -- '*.[ch]' > comment.log || true if [ -s comment.log ]; then
git grep --line-number -e '[^ ][*]\/' -- '*.[ch]' ':!lib/rte_vhost*/*' >> comment.log || true
git grep --line-number -e '^[*]' -- '*.[ch]' >> comment.log || true
git grep --line-number -e '\s\/\/' -- '*.[ch]' >> comment.log || true
git grep --line-number -e '^\/\/' -- '*.[ch]' >> comment.log || true
if [ -s comment.log ]; then
echo " Incorrect comment formatting detected" echo " Incorrect comment formatting detected"
cat comment.log cat comment.log
rc=1 rc=1
else else
echo " OK" echo " OK"
fi fi
rm -f comment.log rm -f comment.log
echo -n "Checking for spaces before tabs..." return $rc
git grep --line-number $' \t' -- './*' ':!*.patch' > whitespace.log || true }
if [ -s whitespace.log ]; then
function check_spaces_before_tabs() {
local rc=0
echo -n "Checking for spaces before tabs..."
git grep --line-number $' \t' -- './*' ':!*.patch' > whitespace.log || true
if [ -s whitespace.log ]; then
echo " Spaces before tabs detected" echo " Spaces before tabs detected"
cat whitespace.log cat whitespace.log
rc=1 rc=1
else else
echo " OK" echo " OK"
fi fi
rm -f whitespace.log rm -f whitespace.log
echo -n "Checking trailing whitespace in output strings..." return $rc
}
git grep --line-number -e ' \\n"' -- '*.[ch]' > whitespace.log || true function check_trailing_whitespace() {
local rc=0
if [ -s whitespace.log ]; then echo -n "Checking trailing whitespace in output strings..."
git grep --line-number -e ' \\n"' -- '*.[ch]' > whitespace.log || true
if [ -s whitespace.log ]; then
echo " Incorrect trailing whitespace detected" echo " Incorrect trailing whitespace detected"
cat whitespace.log cat whitespace.log
rc=1 rc=1
else else
echo " OK" echo " OK"
fi fi
rm -f whitespace.log rm -f whitespace.log
echo -n "Checking for use of forbidden library functions..." return $rc
}
git grep --line-number -w '\(atoi\|atol\|atoll\|strncpy\|strcpy\|strcat\|sprintf\|vsprintf\)' -- './*.c' ':!lib/rte_vhost*/**' > badfunc.log || true function check_forbidden_functions() {
if [ -s badfunc.log ]; then local rc=0
echo -n "Checking for use of forbidden library functions..."
git grep --line-number -w '\(atoi\|atol\|atoll\|strncpy\|strcpy\|strcat\|sprintf\|vsprintf\)' -- './*.c' ':!lib/rte_vhost*/**' > badfunc.log || true
if [ -s badfunc.log ]; then
echo " Forbidden library functions detected" echo " Forbidden library functions detected"
cat badfunc.log cat badfunc.log
rc=1 rc=1
else else
echo " OK" echo " OK"
fi fi
rm -f badfunc.log rm -f badfunc.log
echo -n "Checking for use of forbidden CUnit macros..." return $rc
}
git grep --line-number -w 'CU_ASSERT_FATAL' -- 'test/*' ':!test/spdk_cunit.h' > badcunit.log || true function check_cunit_style() {
if [ -s badcunit.log ]; then local rc=0
echo -n "Checking for use of forbidden CUnit macros..."
git grep --line-number -w 'CU_ASSERT_FATAL' -- 'test/*' ':!test/spdk_cunit.h' > badcunit.log || true
if [ -s badcunit.log ]; then
echo " Forbidden CU_ASSERT_FATAL usage detected - use SPDK_CU_ASSERT_FATAL instead" echo " Forbidden CU_ASSERT_FATAL usage detected - use SPDK_CU_ASSERT_FATAL instead"
cat badcunit.log cat badcunit.log
rc=1 rc=1
else else
echo " OK" echo " OK"
fi fi
rm -f badcunit.log rm -f badcunit.log
echo -n "Checking blank lines at end of file..." return $rc
}
if ! git grep -I -l -e . -z './*' ':!*.patch' \ function check_eof() {
local rc=0
echo -n "Checking blank lines at end of file..."
if ! git grep -I -l -e . -z './*' ':!*.patch' \
| xargs -0 -P$(nproc) -n1 scripts/eofnl > eofnl.log; then | xargs -0 -P$(nproc) -n1 scripts/eofnl > eofnl.log; then
echo " Incorrect end-of-file formatting detected" echo " Incorrect end-of-file formatting detected"
cat eofnl.log cat eofnl.log
rc=1 rc=1
else else
echo " OK" echo " OK"
fi fi
rm -f eofnl.log rm -f eofnl.log
echo -n "Checking for POSIX includes..." return $rc
git grep -I -i -f scripts/posix.txt -- './*' ':!include/spdk/stdinc.h' ':!include/linux/**' ':!lib/rte_vhost*/**' ':!scripts/posix.txt' ':!*.patch' > scripts/posix.log || true }
if [ -s scripts/posix.log ]; then
function check_posix_includes() {
local rc=0
echo -n "Checking for POSIX includes..."
git grep -I -i -f scripts/posix.txt -- './*' ':!include/spdk/stdinc.h' ':!include/linux/**' ':!lib/rte_vhost*/**' ':!scripts/posix.txt' ':!*.patch' > scripts/posix.log || true
if [ -s scripts/posix.log ]; then
echo "POSIX includes detected. Please include spdk/stdinc.h instead." echo "POSIX includes detected. Please include spdk/stdinc.h instead."
cat scripts/posix.log cat scripts/posix.log
rc=1 rc=1
else else
echo " OK" echo " OK"
fi fi
rm -f scripts/posix.log rm -f scripts/posix.log
echo -n "Checking for proper function naming conventions..." return $rc
# commit_to_compare = HEAD - 1. }
commit_to_compare="$(git log --pretty=oneline --skip=1 -n 1 | awk '{print $1}')"
failed_naming_conventions=false
changed_c_libs=()
declared_symbols=()
# Build an array of all the modified C files. function check_naming_conventions() {
mapfile -t changed_c_libs < <(git diff --name-only HEAD $commit_to_compare -- lib/**/*.c module/**/*.c) local rc=0
# Matching groups are 1. qualifiers / return type. 2. function name 3. argument list / comments and stuff after that.
# Capture just the names of newly added (or modified) function definitions.
mapfile -t declared_symbols < <(git diff -U0 $commit_to_compare HEAD -- include/spdk*/*.h | sed -En 's/(^[+].*)(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
for c_file in "${changed_c_libs[@]}"; do echo -n "Checking for proper function naming conventions..."
# commit_to_compare = HEAD - 1.
commit_to_compare="$(git log --pretty=oneline --skip=1 -n 1 | awk '{print $1}')"
failed_naming_conventions=false
changed_c_libs=()
declared_symbols=()
# Build an array of all the modified C files.
mapfile -t changed_c_libs < <(git diff --name-only HEAD $commit_to_compare -- lib/**/*.c module/**/*.c)
# Matching groups are 1. qualifiers / return type. 2. function name 3. argument list / comments and stuff after that.
# Capture just the names of newly added (or modified) function definitions.
mapfile -t declared_symbols < <(git diff -U0 $commit_to_compare HEAD -- include/spdk*/*.h | sed -En 's/(^[+].*)(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
for c_file in "${changed_c_libs[@]}"; do
lib_map_file="mk/spdk_blank.map" lib_map_file="mk/spdk_blank.map"
defined_symbols=() defined_symbols=()
removed_symbols=() removed_symbols=()
@ -265,30 +313,42 @@ for c_file in "${changed_c_libs[@]}"; do
rc=1 rc=1
fi fi
done done
done done
if ! $failed_naming_conventions; then if ! $failed_naming_conventions; then
echo " OK" echo " OK"
fi fi
echo -n "Checking #include style..." return $rc
git grep -I -i --line-number "#include <spdk/" -- '*.[ch]' > scripts/includes.log || true }
if [ -s scripts/includes.log ]; then
function check_include_style() {
local rc=0
echo -n "Checking #include style..."
git grep -I -i --line-number "#include <spdk/" -- '*.[ch]' > scripts/includes.log || true
if [ -s scripts/includes.log ]; then
echo "Incorrect #include syntax. #includes of spdk/ files should use quotes." echo "Incorrect #include syntax. #includes of spdk/ files should use quotes."
cat scripts/includes.log cat scripts/includes.log
rc=1 rc=1
else else
echo " OK" echo " OK"
fi fi
rm -f scripts/includes.log rm -f scripts/includes.log
if hash pycodestyle 2> /dev/null; then return $rc
}
function check_python_style() {
local rc=0
if hash pycodestyle 2> /dev/null; then
PEP8=pycodestyle PEP8=pycodestyle
elif hash pep8 2> /dev/null; then elif hash pep8 2> /dev/null; then
PEP8=pep8 PEP8=pep8
fi fi
if [ -n "${PEP8}" ]; then if [ -n "${PEP8}" ]; then
echo -n "Checking Python style..." echo -n "Checking Python style..."
PEP8_ARGS+=" --max-line-length=140" PEP8_ARGS+=" --max-line-length=140"
@ -303,20 +363,26 @@ if [ -n "${PEP8}" ]; then
echo " OK" echo " OK"
fi fi
rm -f pep8.log rm -f pep8.log
else else
echo "You do not have pycodestyle or pep8 installed so your Python style is not being checked!" echo "You do not have pycodestyle or pep8 installed so your Python style is not being checked!"
fi fi
# find compatible shfmt binary return $rc
shfmt_bins=$(compgen -c | grep '^shfmt' || true) }
for bin in $shfmt_bins; do
function check_bash_style() {
local rc=0
# find compatible shfmt binary
shfmt_bins=$(compgen -c | grep '^shfmt' || true)
for bin in $shfmt_bins; do
if version_lt "$("$bin" --version)" "3.1.0"; then if version_lt "$("$bin" --version)" "3.1.0"; then
shfmt=$bin shfmt=$bin
break break
fi fi
done done
if [ -n "$shfmt" ]; then if [ -n "$shfmt" ]; then
shfmt_cmdline=() silly_plural=() shfmt_cmdline=() silly_plural=()
silly_plural[1]="s" silly_plural[1]="s"
@ -380,11 +446,17 @@ if [ -n "$shfmt" ]; then
printf ' OK\n' printf ' OK\n'
fi fi
fi fi
else else
echo "shfmt not detected, Bash style formatting check is skipped" echo "shfmt not detected, Bash style formatting check is skipped"
fi fi
if hash shellcheck 2> /dev/null; then return $rc
}
function check_bash_static_analysis() {
local rc=0
if hash shellcheck 2> /dev/null; then
echo -n "Checking Bash style..." echo -n "Checking Bash style..."
shellcheck_v=$(shellcheck --version | grep -P "version: [0-9\.]+" | cut -d " " -f2) shellcheck_v=$(shellcheck --version | grep -P "version: [0-9\.]+" | cut -d " " -f2)
@ -443,32 +515,38 @@ SC2119,SC2120,SC2148,SC2153,SC2154,SC2164,SC2174,SC2001,SC2206,SC2207,SC2223"
echo " OK" echo " OK"
fi fi
rm -f shellcheck.log rm -f shellcheck.log
else else
echo "You do not have shellcheck installed so your Bash style is not being checked!" echo "You do not have shellcheck installed so your Bash style is not being checked!"
fi fi
# Check if any of the public interfaces were modified by this patch. return $rc
# Warn the user to consider updating the changelog any changes }
# are detected.
echo -n "Checking whether CHANGELOG.md should be updated..." function check_changelog() {
staged=$(git diff --name-only --cached .) local rc=0
working=$(git status -s --porcelain --ignore-submodules | grep -iv "??" | awk '{print $2}')
files="$staged $working" # Check if any of the public interfaces were modified by this patch.
if [[ "$files" = " " ]]; then # Warn the user to consider updating the changelog any changes
# are detected.
echo -n "Checking whether CHANGELOG.md should be updated..."
staged=$(git diff --name-only --cached .)
working=$(git status -s --porcelain --ignore-submodules | grep -iv "??" | awk '{print $2}')
files="$staged $working"
if [[ "$files" = " " ]]; then
files=$(git diff-tree --no-commit-id --name-only -r HEAD) files=$(git diff-tree --no-commit-id --name-only -r HEAD)
fi fi
has_changelog=0 has_changelog=0
for f in $files; do for f in $files; do
if [[ $f == CHANGELOG.md ]]; then if [[ $f == CHANGELOG.md ]]; then
# The user has a changelog entry, so exit. # The user has a changelog entry, so exit.
has_changelog=1 has_changelog=1
break break
fi fi
done done
needs_changelog=0 needs_changelog=0
if [ $has_changelog -eq 0 ]; then if [ $has_changelog -eq 0 ]; then
for f in $files; do for f in $files; do
if [[ $f == include/spdk/* ]] || [[ $f == scripts/rpc.py ]] || [[ $f == etc/* ]]; then if [[ $f == include/spdk/* ]] || [[ $f == scripts/rpc.py ]] || [[ $f == etc/* ]]; then
echo "" echo ""
@ -476,12 +554,42 @@ if [ $has_changelog -eq 0 ]; then
needs_changelog=1 needs_changelog=1
fi fi
done done
fi
if [ $needs_changelog -eq 0 ]; then
echo " OK"
else
echo ""
fi
return $rc
}
rc=0
check_permissions || rc=1
check_c_style || rc=1
GIT_VERSION=$(git --version | cut -d' ' -f3)
if version_lt "1.9.5" "${GIT_VERSION}"; then
# git <1.9.5 doesn't support pathspec magic exclude
echo " Your git version is too old to perform all tests. Please update git to at least 1.9.5 version..."
exit $rc
fi fi
if [ $needs_changelog -eq 0 ]; then check_comment_style || rc=1
echo " OK" check_spaces_before_tabs || rc=1
else check_trailing_whitespace || rc=1
echo "" check_forbidden_functions || rc=1
fi check_cunit_style || rc=1
check_eof || rc=1
check_posix_includes || rc=1
check_naming_conventions || rc=1
check_include_style || rc=1
check_python_style || rc=1
check_bash_style || rc=1
check_bash_static_analysis || rc=1
check_changelog || rc=1
exit $rc exit $rc