xref: /spdk/scripts/check_format.sh (revision 391dd1c26857b6ea5de3b4ee3c93f302a70a4304)
1#!/usr/bin/env bash
2
3readonly BASEDIR=$(readlink -f $(dirname $0))/..
4cd $BASEDIR
5
6# exit on errors
7set -e
8
9if ! hash nproc 2> /dev/null; then
10
11	function nproc() {
12		echo 8
13	}
14
15fi
16
17function version_lt() {
18	[ $(echo -e "$1\n$2" | sort -V | head -1) != "$1" ]
19}
20
21function array_contains_string() {
22	name="$1[@]"
23	array=("${!name}")
24
25	for element in "${array[@]}"; do
26		if [ "$element" = "$2" ]; then
27			return $(true)
28		fi
29	done
30
31	return $(false)
32}
33
34rc=0
35
36echo -n "Checking file permissions..."
37
38while read -r perm _res0 _res1 path; do
39	if [ ! -f "$path" ]; then
40		continue
41	fi
42
43	# Skip symlinks
44	if [[ -L $path ]]; then
45		continue
46	fi
47	fname=$(basename -- "$path")
48
49	case ${fname##*.} in
50		c | h | cpp | cc | cxx | hh | hpp | md | html | js | json | svg | Doxyfile | yml | LICENSE | README | conf | in | Makefile | mk | gitignore | go | txt)
51			# These file types should never be executable
52			if [ "$perm" -eq 100755 ]; then
53				echo "ERROR: $path is marked executable but is a code file."
54				rc=1
55			fi
56			;;
57		*)
58			shebang=$(head -n 1 $path | cut -c1-3)
59
60			# git only tracks the execute bit, so will only ever return 755 or 644 as the permission.
61			if [ "$perm" -eq 100755 ]; then
62				# If the file has execute permission, it should start with a shebang.
63				if [ "$shebang" != "#!/" ]; then
64					echo "ERROR: $path is marked executable but does not start with a shebang."
65					rc=1
66				fi
67			else
68				# If the file doesnot have execute permissions, it should not start with a shebang.
69				if [ "$shebang" = "#!/" ]; then
70					echo "ERROR: $path is not marked executable but starts with a shebang."
71					rc=1
72				fi
73			fi
74			;;
75	esac
76
77done <<< "$(git grep -I --name-only --untracked -e . | git ls-files -s)"
78
79if [ $rc -eq 0 ]; then
80	echo " OK"
81fi
82
83if hash astyle; then
84	echo -n "Checking coding style..."
85	if [ "$(astyle -V)" \< "Artistic Style Version 3" ]; then
86		echo -n " Your astyle version is too old so skipping coding style checks. Please update astyle to at least 3.0.1 version..."
87	else
88		rm -f astyle.log
89		touch astyle.log
90		# Exclude rte_vhost code imported from DPDK - we want to keep the original code
91		#  as-is to enable ongoing work to synch with a generic upstream DPDK vhost library,
92		#  rather than making diffs more complicated by a lot of changes to follow SPDK
93		#  coding standards.
94		git ls-files '*.[ch]' '*.cpp' '*.cc' '*.cxx' '*.hh' '*.hpp' \
95			| grep -v rte_vhost | grep -v cpp_headers \
96			| xargs -P$(nproc) -n10 astyle --options=.astylerc >> astyle.log
97		if grep -q "^Formatted" astyle.log; then
98			echo " errors detected"
99			git diff
100			sed -i -e 's/  / /g' astyle.log
101			grep --color=auto "^Formatted.*" astyle.log
102			echo "Incorrect code style detected in one or more files."
103			echo "The files have been automatically formatted."
104			echo "Remember to add the files to your commit."
105			rc=1
106		else
107			echo " OK"
108		fi
109		rm -f astyle.log
110	fi
111else
112	echo "You do not have astyle installed so your code style is not being checked!"
113fi
114
115GIT_VERSION=$(git --version | cut -d' ' -f3)
116
117if version_lt "1.9.5" "${GIT_VERSION}"; then
118	# git <1.9.5 doesn't support pathspec magic exclude
119	echo " Your git version is too old to perform all tests. Please update git to at least 1.9.5 version..."
120	exit 0
121fi
122
123echo -n "Checking comment style..."
124
125git grep --line-number -e '/[*][^ *-]' -- '*.[ch]' > comment.log || true
126git grep --line-number -e '[^ ][*]/' -- '*.[ch]' ':!lib/rte_vhost*/*' >> comment.log || true
127git grep --line-number -e '^[*]' -- '*.[ch]' >> comment.log || true
128git grep --line-number -e '\s//' -- '*.[ch]' >> comment.log || true
129git grep --line-number -e '^//' -- '*.[ch]' >> comment.log || true
130
131if [ -s comment.log ]; then
132	echo " Incorrect comment formatting detected"
133	cat comment.log
134	rc=1
135else
136	echo " OK"
137fi
138rm -f comment.log
139
140echo -n "Checking for spaces before tabs..."
141git grep --line-number $' \t' -- './*' ':!*.patch' > whitespace.log || true
142if [ -s whitespace.log ]; then
143	echo " Spaces before tabs detected"
144	cat whitespace.log
145	rc=1
146else
147	echo " OK"
148fi
149rm -f whitespace.log
150
151echo -n "Checking trailing whitespace in output strings..."
152
153git grep --line-number -e ' \\n"' -- '*.[ch]' > whitespace.log || true
154
155if [ -s whitespace.log ]; then
156	echo " Incorrect trailing whitespace detected"
157	cat whitespace.log
158	rc=1
159else
160	echo " OK"
161fi
162rm -f whitespace.log
163
164echo -n "Checking for use of forbidden library functions..."
165
166git grep --line-number -w '\(atoi\|atol\|atoll\|strncpy\|strcpy\|strcat\|sprintf\|vsprintf\)' -- './*.c' ':!lib/rte_vhost*/**' > badfunc.log || true
167if [ -s badfunc.log ]; then
168	echo " Forbidden library functions detected"
169	cat badfunc.log
170	rc=1
171else
172	echo " OK"
173fi
174rm -f badfunc.log
175
176echo -n "Checking for use of forbidden CUnit macros..."
177
178git grep --line-number -w 'CU_ASSERT_FATAL' -- 'test/*' ':!test/spdk_cunit.h' > badcunit.log || true
179if [ -s badcunit.log ]; then
180	echo " Forbidden CU_ASSERT_FATAL usage detected - use SPDK_CU_ASSERT_FATAL instead"
181	cat badcunit.log
182	rc=1
183else
184	echo " OK"
185fi
186rm -f badcunit.log
187
188echo -n "Checking blank lines at end of file..."
189
190if ! git grep -I -l -e . -z './*' ':!*.patch' \
191	| xargs -0 -P$(nproc) -n1 scripts/eofnl > eofnl.log; then
192	echo " Incorrect end-of-file formatting detected"
193	cat eofnl.log
194	rc=1
195else
196	echo " OK"
197fi
198rm -f eofnl.log
199
200echo -n "Checking for POSIX includes..."
201git grep -I -i -f scripts/posix.txt -- './*' ':!include/spdk/stdinc.h' ':!include/linux/**' ':!lib/rte_vhost*/**' ':!scripts/posix.txt' ':!*.patch' > scripts/posix.log || true
202if [ -s scripts/posix.log ]; then
203	echo "POSIX includes detected. Please include spdk/stdinc.h instead."
204	cat scripts/posix.log
205	rc=1
206else
207	echo " OK"
208fi
209rm -f scripts/posix.log
210
211echo -n "Checking for proper function naming conventions..."
212failed_naming_conventions=false
213changed_c_libs=()
214declared_symbols=()
215
216# Build an array of all the modified C files.
217mapfile -t changed_c_libs < <(git diff --name-only HEAD origin/master -- lib/**/*.c module/**/*.c)
218# Matching groups are 1. qualifiers / return type. 2. function name 3. argument list / comments and stuff after that.
219# Capture just the names of newly added (or modified) function definitions.
220mapfile -t declared_symbols < <(git diff -U0 origin/master HEAD -- include/spdk*/*.h | sed -En 's/(^[+].*)(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
221
222for c_file in "${changed_c_libs[@]}"; do
223	lib_map_file="mk/spdk_blank.map"
224	defined_symbols=()
225	exported_symbols=()
226	if ls "$(dirname $c_file)"/*.map &> /dev/null; then
227		lib_map_file="$(ls "$(dirname $c_file)"/*.map)"
228	fi
229	# Matching groups are 1. leading +sign. 2, function name 3. argument list / anything after that.
230	# Capture just the names of newly added (or modified) functions that start with "spdk_"
231	mapfile -t defined_symbols < <(git diff -U0 origin/master HEAD -- $c_file | sed -En 's/(^[+])(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
232	# It's possible that we just modified a functions arguments so unfortunately we can't just look at changed lines in this function.
233	# matching groups are 1. All leading whitespace 2. function name. Capture just the symbol name.
234	mapfile -t exported_symbols < <(sed -En 's/(^[[:space:]]*)(spdk[a-z,A-Z,0-9,_]*);/\2/p' < $lib_map_file)
235	for defined_symbol in "${defined_symbols[@]}"; do
236		not_exported=true
237		not_declared=true
238		if array_contains_string exported_symbols $defined_symbol; then
239			not_exported=false
240		fi
241
242		if array_contains_string declared_symbols $defined_symbol; then
243			not_declared=false
244		fi
245
246		if $not_exported || $not_declared; then
247			if ! $failed_naming_conventions; then
248				echo " found naming convention errors."
249			fi
250			echo "function $defined_symbol starts with spdk_ which is reserved for public API functions."
251			echo "Please add this function to its corresponding map file and a public header or remove the spdk_ prefix."
252			failed_naming_conventions=true
253			rc=1
254		fi
255	done
256done
257
258if ! $failed_naming_conventions; then
259	echo " OK"
260fi
261
262echo -n "Checking #include style..."
263git grep -I -i --line-number "#include <spdk/" -- '*.[ch]' > scripts/includes.log || true
264if [ -s scripts/includes.log ]; then
265	echo "Incorrect #include syntax. #includes of spdk/ files should use quotes."
266	cat scripts/includes.log
267	rc=1
268else
269	echo " OK"
270fi
271rm -f scripts/includes.log
272
273if hash pycodestyle 2> /dev/null; then
274	PEP8=pycodestyle
275elif hash pep8 2> /dev/null; then
276	PEP8=pep8
277fi
278
279if [ -n "${PEP8}" ]; then
280	echo -n "Checking Python style..."
281
282	PEP8_ARGS+=" --max-line-length=140"
283
284	error=0
285	git ls-files '*.py' | xargs -P$(nproc) -n1 $PEP8 $PEP8_ARGS > pep8.log || error=1
286	if [ $error -ne 0 ]; then
287		echo " Python formatting errors detected"
288		cat pep8.log
289		rc=1
290	else
291		echo " OK"
292	fi
293	rm -f pep8.log
294else
295	echo "You do not have pycodestyle or pep8 installed so your Python style is not being checked!"
296fi
297
298# find compatible shfmt binary
299shfmt_bins=$(compgen -c | grep '^shfmt' || true)
300for bin in $shfmt_bins; do
301	if [[ "$("$bin" --version)" > "v3.0.9" ]]; then
302		shfmt=$bin
303		break
304	fi
305done
306
307if [ -n "$shfmt" ]; then
308	shfmt_cmdline=() silly_plural=()
309
310	silly_plural[1]="s"
311
312	commits=() sh_files=() sh_files_repo=() sh_files_staged=()
313
314	mapfile -t sh_files_repo < <(git ls-files '*.sh')
315	# Fetch .sh files only from the commits that are targeted for merge
316	while read -r _ commit; do
317		commits+=("$commit")
318	done < <(git cherry -v origin/master)
319
320	mapfile -t sh_files < <(git diff --name-only HEAD origin/master "${sh_files_repo[@]}")
321	# In case of a call from a pre-commit git hook
322	mapfile -t sh_files_staged < <(
323		IFS="|"
324		git diff --cached --name-only "${sh_files_repo[@]}" | grep -v "${sh_files[*]}"
325	)
326
327	if ((${#sh_files[@]})); then
328		printf 'Checking .sh formatting style...'
329
330		if ((${#sh_files_staged[@]})); then
331			sh_files+=("${sh_files_staged[@]}")
332		fi
333
334		shfmt_cmdline+=(-i 0)     # indent_style = tab|indent_size = 0
335		shfmt_cmdline+=(-bn)      # binary_next_line = true
336		shfmt_cmdline+=(-ci)      # switch_case_indent = true
337		shfmt_cmdline+=(-ln bash) # shell_variant = bash (default)
338		shfmt_cmdline+=(-d)       # diffOut - print diff of the changes and exit with != 0
339		shfmt_cmdline+=(-sr)      # redirect operators will be followed by a space
340
341		diff=$PWD/$shfmt.patch
342
343		# Explicitly tell shfmt to not look for .editorconfig. .editorconfig is also not looked up
344		# in case any formatting arguments has been passed on its cmdline.
345		if ! SHFMT_NO_EDITORCONFIG=true "$shfmt" "${shfmt_cmdline[@]}" "${sh_files[@]}" > "$diff"; then
346			# In case shfmt detects an actual syntax error it will write out a proper message on
347			# its stderr, hence the diff file should remain empty.
348			if [[ -s $diff ]]; then
349				diff_out=$(< "$diff")
350			fi
351
352			cat <<- ERROR_SHFMT
353
354				* Errors in style formatting have been detected.
355				${diff_out:+* Please, review the generated patch at $diff
356
357				# _START_OF_THE_DIFF
358
359				${diff_out:-ERROR}
360
361				# _END_OF_THE_DIFF
362				}
363
364			ERROR_SHFMT
365			rc=1
366		else
367			rm -f "$diff"
368			printf ' OK\n'
369		fi
370	fi
371else
372	echo "shfmt not detected, Bash style formatting check is skipped"
373fi
374
375if hash shellcheck 2> /dev/null; then
376	echo -n "Checking Bash style..."
377
378	shellcheck_v=$(shellcheck --version | grep -P "version: [0-9\.]+" | cut -d " " -f2)
379
380	# SHCK_EXCLUDE contains a list of all of the spellcheck errors found in SPDK scripts
381	# currently. New errors should only be added to this list if the cost of fixing them
382	# is deemed too high. For more information about the errors, go to:
383	# https://github.com/koalaman/shellcheck/wiki/Checks
384	# Error descriptions can also be found at: https://github.com/koalaman/shellcheck/wiki
385	# SPDK fails some error checks which have been deprecated in later versions of shellcheck.
386	# We will not try to fix these error checks, but instead just leave the error types here
387	# so that we can still run with older versions of shellcheck.
388	SHCK_EXCLUDE="SC1117"
389	# SPDK has decided to not fix violations of these errors.
390	# We are aware about below exclude list and we want this errors to be excluded.
391	# SC1083: This {/} is literal. Check expression (missing ;/\n?) or quote it.
392	# SC1090: Can't follow non-constant source. Use a directive to specify location.
393	# SC1091: Not following: (error message here)
394	# SC2001: See if you can use ${variable//search/replace} instead.
395	# SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
396	# SC2015: Note that A && B || C is not if-then-else. C may run when A is true.
397	# SC2016: Expressions don't expand in single quotes, use double quotes for that.
398	# SC2034: foo appears unused. Verify it or export it.
399	# SC2046: Quote this to prevent word splitting.
400	# SC2086: Double quote to prevent globbing and word splitting.
401	# SC2119: Use foo "$@" if function's $1 should mean script's $1.
402	# SC2120: foo references arguments, but none are ever passed.
403	# SC2148: Add shebang to the top of your script.
404	# SC2153: Possible Misspelling: MYVARIABLE may not be assigned, but MY_VARIABLE is.
405	# SC2154: var is referenced but not assigned.
406	# SC2164: Use cd ... || exit in case cd fails.
407	# SC2174: When used with -p, -m only applies to the deepest directory.
408	# SC2206: Quote to prevent word splitting/globbing,
409	#         or split robustly with mapfile or read -a.
410	# SC2207: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
411	# SC2223: This default assignment may cause DoS due to globbing. Quote it.
412	SHCK_EXCLUDE="$SHCK_EXCLUDE,SC1083,SC1090,SC1091,SC2010,SC2015,SC2016,SC2034,SC2046,SC2086,\
413SC2119,SC2120,SC2148,SC2153,SC2154,SC2164,SC2174,SC2001,SC2206,SC2207,SC2223"
414
415	SHCK_FORMAT="diff"
416	SHCK_APPLY=true
417	if [ "$shellcheck_v" \< "0.7.0" ]; then
418		SHCK_FORMAT="tty"
419		SHCK_APPLY=false
420	fi
421	SHCH_ARGS=" -x -e $SHCK_EXCLUDE -f $SHCK_FORMAT"
422
423	error=0
424	git ls-files '*.sh' | xargs -P$(nproc) -n1 shellcheck $SHCH_ARGS &> shellcheck.log || error=1
425	if [ $error -ne 0 ]; then
426		echo " Bash formatting errors detected!"
427
428		# Some errors are not auto-fixable. Fall back to tty output.
429		if grep -q "Use another format to see them." shellcheck.log; then
430			SHCK_FORMAT="tty"
431			SHCK_APPLY=false
432			SHCH_ARGS=" -e $SHCK_EXCLUDE -f $SHCK_FORMAT"
433			git ls-files '*.sh' | xargs -P$(nproc) -n1 shellcheck $SHCH_ARGS > shellcheck.log || error=1
434		fi
435
436		cat shellcheck.log
437		if $SHCK_APPLY; then
438			git apply shellcheck.log
439			echo "Bash errors were automatically corrected."
440			echo "Please remember to add the changes to your commit."
441		fi
442		rc=1
443	else
444		echo " OK"
445	fi
446	rm -f shellcheck.log
447else
448	echo "You do not have shellcheck installed so your Bash style is not being checked!"
449fi
450
451# Check if any of the public interfaces were modified by this patch.
452# Warn the user to consider updating the changelog any changes
453# are detected.
454echo -n "Checking whether CHANGELOG.md should be updated..."
455staged=$(git diff --name-only --cached .)
456working=$(git status -s --porcelain --ignore-submodules | grep -iv "??" | awk '{print $2}')
457files="$staged $working"
458if [[ "$files" = " " ]]; then
459	files=$(git diff-tree --no-commit-id --name-only -r HEAD)
460fi
461
462has_changelog=0
463for f in $files; do
464	if [[ $f == CHANGELOG.md ]]; then
465		# The user has a changelog entry, so exit.
466		has_changelog=1
467		break
468	fi
469done
470
471needs_changelog=0
472if [ $has_changelog -eq 0 ]; then
473	for f in $files; do
474		if [[ $f == include/spdk/* ]] || [[ $f == scripts/rpc.py ]] || [[ $f == etc/* ]]; then
475			echo ""
476			echo -n "$f was modified. Consider updating CHANGELOG.md."
477			needs_changelog=1
478		fi
479	done
480fi
481
482if [ $needs_changelog -eq 0 ]; then
483	echo " OK"
484else
485	echo ""
486fi
487
488exit $rc
489