Move check-style core logic from makefile to shell script (#1745)

* Move check-style core logic from source/Makefile:check-style target to deploy.sh:check_style_file function

* deploy.sh: fix shellcheck warning

* source/Makefile: restore removed-by-accident style target
This commit is contained in:
Ivan Zorin
2023-07-20 04:02:56 +03:00
committed by GitHub
parent bfbdf8c941
commit 93a18e5076
2 changed files with 74 additions and 49 deletions

View File

@@ -20,7 +20,8 @@ usage()
echo -e "\tclean - delete created docker container (but not pre-downloaded data for it)\n" echo -e "\tclean - delete created docker container (but not pre-downloaded data for it)\n"
echo "CMD (helper routines):" echo "CMD (helper routines):"
echo -e "\tdocs_readme - generate & OVERWRITE(!) README.md inside Documentation/ based on nav section from mkdocs.yml if it changed\n" echo -e "\tdocs_readme - generate & OVERWRITE(!) README.md inside Documentation/ based on nav section from mkdocs.yml if it changed\n"
echo -e "\tcheck_style - run clang-format using source/Makefile and generate gcc-compatible error log in source/check-style.log\n" echo -e "\tcheck_style_file SRC - run code style checks based on clang-format & custom parsers for source code file SRC\n"
echo -e "\tcheck_style_log - run clang-format using source/Makefile and generate gcc-compatible error log in source/check-style.log\n"
echo -e "STORAGE NOTICE: for \"shell\" and \"build\" commands extra files will be downloaded so make sure that you have ~5GB of free space.\n" echo -e "STORAGE NOTICE: for \"shell\" and \"build\" commands extra files will be downloaded so make sure that you have ~5GB of free space.\n"
} }
@@ -70,8 +71,43 @@ EOF
return "${ret}" return "${ret}"
} }
# Helper function to check code style using clang-format & grep/sed custom parsers:
# - basic logic moved from source/Makefile : `check-style` target for better maintainance since a lot of sh script involved;
# - output goes in gcc-like error compatible format for IDEs/editors.
check_style_file()
{
ret=0
src="${1}"
test ! -f "${src}" && echo "ERROR!!! Provided file ${src} is not available to check/read!!!" && exit 1
# count lines using diff between beauty-fied file & original file to detect format issue
var="$(clang-format "$src" | diff "$src" - | wc -l)"
if [ "${var}" -ne 0 ]; then
# show full log error or, if LIST=anything provided, then show only filename of interest (implemented for debug purposes mainly)
if [ -z "${LIST}" ]; then
# sed is here only for pretty logging
clang-format "${src}" | diff "${src}" - | sed 's/^---/-------------------------------------------------------------------------------/; s/^< /--- /; s/^> /+++ /; /^[0-9].*/ s/[acd,].*$/ERROR1/; /^[0-9].*/ s,^,\n\n\n\n'"${src}"':,; /ERROR1$/ s,ERROR1$,:1: error: clang-format code style mismatch:,; '
else
echo "${src}"
fi;
ret=1
fi;
# - clang-format has neat option for { } in condition blocks but it's available only since version 15:
# * https://clang.llvm.org/docs/ClangFormatStyleOptions.html#insertbraces
# - since reference env is alpine 3.16 with clang-format 13, implement custom parser to do the similar thing here with grep:
# it used to trace missing { and } for if/else/do/while/for BUT IT'S VERY SPECULATIVE, very-very hacky & dirty.
test -z "${LIST}" || silent_opt="-q"
# if file is problematic but filename only requested make final grep in pipe silent ...
grep -H -n -e "^ .*if .*)$" -e "^ .*else$" -e "^ .* do$" -e "^ .*while .*)$" -e "^ .*for .*)$" "${src}" | grep -v -e "^.*//" -e "^.*:.*: .*if ((.*[^)])$" | sed 's,^,\n\n,; s,: ,:1: error: probably missing { or } for conditional or loop block:\n>>>,;' | grep "${silent_opt}" -e "^.*$"
if [ "${?}" -ne 1 ]; then
# ... and only print the filename
test -z "${LIST}" || echo "${src}"
ret=1;
fi;
return "${ret}"
}
# check_style routine for those who too lazy to do it everytime manually # check_style routine for those who too lazy to do it everytime manually
check_style() check_style_log()
{ {
log="source/check-style.log" log="source/check-style.log"
make -C source check-style 2>&1 | tee "${log}" make -C source check-style 2>&1 | tee "${log}"
@@ -130,8 +166,13 @@ if [ "docs_readme" = "${cmd}" ]; then
exit "${?}" exit "${?}"
fi; fi;
if [ "check_style" = "${cmd}" ]; then if [ "check_style_file" = "${cmd}" ]; then
check_style check_style_file "${2}"
exit "${?}"
fi;
if [ "check_style_log" = "${cmd}" ]; then
check_style_log
exit "${?}" exit "${?}"
fi; fi;

View File

@@ -60,7 +60,7 @@ APP_INC_DIR=./Core/Inc
BRIEFLZ_INC_DIR=./Core/brieflz BRIEFLZ_INC_DIR=./Core/brieflz
MINIWARE_INC_CMSIS_DEVICE=./Core/BSP/Miniware/Vendor/CMSIS/Device/ST/STM32F1xx/Include MINIWARE_INC_CMSIS_DEVICE=./Core/BSP/Miniware/Vendor/CMSIS/Device/ST/STM32F1xx/Include
MINIWARE_CMSIS_CORE_INC_DIR=./Core/BSP/Miniware/Vendor/CMSIS/Include MINIWARE_CMSIS_CORE_INC_DIR=./Core/BSP/Miniware/Vendor/CMSIS/Include
MINIWARE_HAL_SRC_DIR= ./Core/BSP/Miniware/Vendor/STM32F1xx_HAL_Driver MINIWARE_HAL_SRC_DIR=./Core/BSP/Miniware/Vendor/STM32F1xx_HAL_Driver
MINIWARE_HAL_INC_DIR=./Core/BSP/Miniware/Vendor/STM32F1xx_HAL_Driver/Inc MINIWARE_HAL_INC_DIR=./Core/BSP/Miniware/Vendor/STM32F1xx_HAL_Driver/Inc
MINIWARE_HAL_LEGACY_INC_DIR=./Core/BSP/Miniware/Vendor/STM32F1xx_HAL_Driver/Inc/Legacy MINIWARE_HAL_LEGACY_INC_DIR=./Core/BSP/Miniware/Vendor/STM32F1xx_HAL_Driver/Inc/Legacy
MINIWARE_STARTUP_DIR=./Startup MINIWARE_STARTUP_DIR=./Startup
@@ -474,8 +474,8 @@ DEV_GLOBAL_DEFS=-DCFG_FREERTOS \
# -DCONFIG_BT_SMP # -DCONFIG_BT_SMP
# Required to be turned off due to their drivers tripping warnings # Required to be turned off due to their drivers tripping warnings
DEV_CFLAGS= -Wno-error=enum-conversion -Wno-type-limits -Wno-implicit-fallthrough DEV_CFLAGS=-Wno-error=enum-conversion -Wno-type-limits -Wno-implicit-fallthrough
DEV_CXXFLAGS= $(DEV_CFLAGS) DEV_CXXFLAGS=$(DEV_CFLAGS)
flash_size=128k flash_size=128k
bootldr_size=0x0 bootldr_size=0x0
@@ -852,52 +852,36 @@ clean:
clean-all: clean clean-all: clean
rm -Rf $(HEXFILE_DIR) rm -Rf $(HEXFILE_DIR)
# Code style checks using clang-format # Style formatting helper targets
# Show only list of affected files for debug purposes # Overwrite source files in your local repo copy according to IronOS code style rules (source/.clang-format) WITHOUT A WARNING!
check-style-list: # Use `git diff` or your favorite diff tool via `git difftool` before commit to make sure there are no false-negative changes.
@ret=0; for src in $(ALL_SOURCE) $(ALL_INCLUDES) ; do \ # If so, report an issue, please.
var=`clang-format "$$src" | diff "$$src" - | wc -l` ; \ style:
if [ $$var -ne 0 ] ; then \ @for src in $(ALL_SOURCE) $(ALL_INCLUDES); do echo "Formatting $$src" ; clang-format -i "$$src" ; done;
echo "$$src" ; \ @echo "Done! Please, check the changes before commit."
ret=1; test -n "$(STOP)" && break; \
fi ; \
grep -H -n -e "^ .*if .*)$$" -e "^ .*else$$" -e "^ .* do$$" -e "^ .*while .*)$$" -e "^ .*for .*)$$" "$$src" | grep -v -e "^.*//" -e "^.*:.*: .*if ((.*[^)])$$" | sed 's,^,\n\n,; s,: ,:1: error: probably missing { or } for conditional or loop block:\n>>>,;' | grep -q -e "^.*$$" ; \
if [ "$$?" -ne 1 ]; then \
echo "$$src" ; \
ret=1; test -n "$(STOP)" && break; \
fi ; \
done ; \
if [ $$ret -eq 0 ] ; then \
echo "Style check: PASS" && exit 0 ; \
else \
echo "Style check: FAIL!" && echo "Please, check the log above for the details." && exit 1 ; \
fi ;
# Show output in gcc-like error compatible format for IDEs/editors; call `make check-style STOP=1` to exit after first failed file # Code style checks using clang-format:
# NOTICE: # - show output in gcc-like error compatible format for IDEs/editors;
# - clang-format has neat option for { } in condition blocks but it's available only since version 15: # - external variables for debug purposes (can be used at the same time, i.e. STOP=1 LIST=1 ...):
# * https://clang.llvm.org/docs/ClangFormatStyleOptions.html#insertbraces # * call `make check-style STOP=1` to exit after first failed file;
# - grep block in the middle used to trace missing { and } for if/else BUT IT'S VERY SPECULATIVE, very-very hacky & dirty # * call `make check-style LIST=1` to show failed file names only;
# - here we process only list of files;
# - per-file check happens in scripts/deploy.sh : check_style_file - since shell commands involved, the check logic moved to shell script for better maintainance outside of makefile syntax crossing.
# - $? / error / STOP conditional logic needed to:
# * check errors in formatting from deploy.sh
# * process STOP env variable
check-style: check-style:
@ret=0; for src in $(ALL_SOURCE) $(ALL_INCLUDES) ; do \ @error=0; export LIST=$$LIST; for src in $(ALL_SOURCE) $(ALL_INCLUDES) ; do \
var=`clang-format "$$src" | diff "$$src" - | wc -l` ; \ ../scripts/deploy.sh check_style_file "$$src" ; \
if [ $$var -ne 0 ] ; then \ test "$${?}" -eq 1 && export error=1 ; \
clang-format "$$src" | diff "$$src" - | sed 's/^---/-------------------------------------------------------------------------------/; s/^< /--- /; s/^> /+++ /; /^[0-9].*/ s/[acd,].*$$/ERROR1/; /^[0-9].*/ s,^,\n\n\n\n'"$$src"':,; /ERROR1$$/ s,ERROR1$$,:1: error: clang-format code style mismatch:,; ' ; \ test "$${error}" -eq 1 && test -n "$${STOP}" && break; \
ret=1; test -n "$(STOP)" && break; \ done; \
fi ; \ if [ $$error -eq 0 ] ; then echo "" && echo "" && echo "Style check: PASS" && echo "" && echo "" && exit 0 ; \
grep -H -n -e "^ .*if .*)$$" -e "^ .*else$$" -e "^ .* do$$" -e "^ .*while .*)$$" -e "^ .*for .*)$$" "$$src" | grep -v -e "^.*//" -e "^.*:.*: .*if ((.*[^)])$$" | sed 's,^,\n\n,; s,: ,:1: error: probably missing { or } for conditional or loop block:\n>>>,;' | grep -e "^.*$$" ; \ else echo "" && echo "" && echo "Style check: FAIL! Please, check the log above for the details." && echo "If there is a false-negative trigger, please, report an issue attaching the log or link to the log!" && echo "" && echo "" && exit 1 ; \
if [ "$$?" -ne 1 ]; then \ fi;
ret=1; test -n "$(STOP)" && break; \
fi ; \
done ; \
if [ $$ret -eq 0 ] ; then \
echo "Style check: PASS" && exit 0 ; \
else \
echo "Style check: FAIL!" && echo "Please, check the log above for the details." && exit 1 ; \
fi ;
.PHONY: check-style-list check-style all clean default clean-all .PHONY: style check-style all clean default clean-all
.SECONDARY: .SECONDARY:
# Pull in dependency info for *existing* .o files # Pull in dependency info for *existing* .o files