xref: /spdk/scripts/check_format.sh (revision 6a9e923da298dda69cbdaf73eed6fb0e85ec2ad2)
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 --ignore-submodules=all
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..."
212# commit_to_compare = HEAD - 1.
213commit_to_compare="$(git log --pretty=oneline --skip=1 -n 1 | awk '{print $1}')"
214failed_naming_conventions=false
215changed_c_libs=()
216declared_symbols=()
217
218# Build an array of all the modified C files.
219mapfile -t changed_c_libs < <(git diff --name-only HEAD $commit_to_compare -- lib/**/*.c module/**/*.c)
220# Matching groups are 1. qualifiers / return type. 2. function name 3. argument list / comments and stuff after that.
221# Capture just the names of newly added (or modified) function definitions.
222mapfile -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')
223
224for c_file in "${changed_c_libs[@]}"; do
225	lib_map_file="mk/spdk_blank.map"
226	defined_symbols=()
227	removed_symbols=()
228	exported_symbols=()
229	if ls "$(dirname $c_file)"/*.map &> /dev/null; then
230		lib_map_file="$(ls "$(dirname $c_file)"/*.map)"
231	fi
232	# Matching groups are 1. leading +sign. 2, function name 3. argument list / anything after that.
233	# Capture just the names of newly added (or modified) functions that start with "spdk_"
234	mapfile -t defined_symbols < <(git diff -U0 $commit_to_compare HEAD -- $c_file | sed -En 's/(^[+])(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
235	# Capture the names of removed symbols to catch edge cases where we just move definitions around.
236	mapfile -t removed_symbols < <(git diff -U0 $commit_to_compare HEAD -- $c_file | sed -En 's/(^[-])(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
237	for symbol in "${removed_symbols[@]}"; do
238		defined_symbols=("${defined_symbols[@]/$symbol/}")
239	done
240	# It's possible that we just modified a functions arguments so unfortunately we can't just look at changed lines in this function.
241	# matching groups are 1. All leading whitespace 2. function name. Capture just the symbol name.
242	mapfile -t exported_symbols < <(sed -En 's/(^[[:space:]]*)(spdk[a-z,A-Z,0-9,_]*);/\2/p' < $lib_map_file)
243	for defined_symbol in "${defined_symbols[@]}"; do
244		# if the list of defined symbols is equal to the list of removed symbols, then we are left with a single empty element. skip it.
245		if [ "$defined_symbol" = '' ]; then
246			continue
247		fi
248		not_exported=true
249		not_declared=true
250		if array_contains_string exported_symbols $defined_symbol; then
251			not_exported=false
252		fi
253
254		if array_contains_string declared_symbols $defined_symbol; then
255			not_declared=false
256		fi
257
258		if $not_exported || $not_declared; then
259			if ! $failed_naming_conventions; then
260				echo " found naming convention errors."
261			fi
262			echo "function $defined_symbol starts with spdk_ which is reserved for public API functions."
263			echo "Please add this function to its corresponding map file and a public header or remove the spdk_ prefix."
264			failed_naming_conventions=true
265			rc=1
266		fi
267	done
268done
269
270if ! $failed_naming_conventions; then
271	echo " OK"
272fi
273
274echo -n "Checking #include style..."
275git grep -I -i --line-number "#include <spdk/" -- '*.[ch]' > scripts/includes.log || true
276if [ -s scripts/includes.log ]; then
277	echo "Incorrect #include syntax. #includes of spdk/ files should use quotes."
278	cat scripts/includes.log
279	rc=1
280else
281	echo " OK"
282fi
283rm -f scripts/includes.log
284
285if hash pycodestyle 2> /dev/null; then
286	PEP8=pycodestyle
287elif hash pep8 2> /dev/null; then
288	PEP8=pep8
289fi
290
291if [ -n "${PEP8}" ]; then
292	echo -n "Checking Python style..."
293
294	PEP8_ARGS+=" --max-line-length=140"
295
296	error=0
297	git ls-files '*.py' | xargs -P$(nproc) -n1 $PEP8 $PEP8_ARGS > pep8.log || error=1
298	if [ $error -ne 0 ]; then
299		echo " Python formatting errors detected"
300		cat pep8.log
301		rc=1
302	else
303		echo " OK"
304	fi
305	rm -f pep8.log
306else
307	echo "You do not have pycodestyle or pep8 installed so your Python style is not being checked!"
308fi
309
310# find compatible shfmt binary
311shfmt_bins=$(compgen -c | grep '^shfmt' || true)
312for bin in $shfmt_bins; do
313	if version_lt "$("$bin" --version)" "3.1.0"; then
314		shfmt=$bin
315		break
316	fi
317done
318
319if [ -n "$shfmt" ]; then
320	shfmt_cmdline=() silly_plural=()
321
322	silly_plural[1]="s"
323
324	commits=() sh_files=() sh_files_repo=() sh_files_staged=()
325
326	mapfile -t sh_files_repo < <(git ls-files '*.sh')
327	# Fetch .sh files only from the commits that are targeted for merge
328	while read -r _ commit; do
329		commits+=("$commit")
330	done < <(git cherry -v origin/master)
331
332	mapfile -t sh_files < <(git diff --name-only HEAD origin/master "${sh_files_repo[@]}")
333	# In case of a call from a pre-commit git hook
334	mapfile -t sh_files_staged < <(
335		IFS="|"
336		git diff --cached --name-only "${sh_files_repo[@]}" | grep -v "${sh_files[*]}"
337	)
338
339	if ((${#sh_files[@]})); then
340		printf 'Checking .sh formatting style...'
341
342		if ((${#sh_files_staged[@]})); then
343			sh_files+=("${sh_files_staged[@]}")
344		fi
345
346		shfmt_cmdline+=(-i 0)     # indent_style = tab|indent_size = 0
347		shfmt_cmdline+=(-bn)      # binary_next_line = true
348		shfmt_cmdline+=(-ci)      # switch_case_indent = true
349		shfmt_cmdline+=(-ln bash) # shell_variant = bash (default)
350		shfmt_cmdline+=(-d)       # diffOut - print diff of the changes and exit with != 0
351		shfmt_cmdline+=(-sr)      # redirect operators will be followed by a space
352
353		diff=${output_dir:-$PWD}/$shfmt.patch
354
355		# Explicitly tell shfmt to not look for .editorconfig. .editorconfig is also not looked up
356		# in case any formatting arguments has been passed on its cmdline.
357		if ! SHFMT_NO_EDITORCONFIG=true "$shfmt" "${shfmt_cmdline[@]}" "${sh_files[@]}" > "$diff"; then
358			# In case shfmt detects an actual syntax error it will write out a proper message on
359			# its stderr, hence the diff file should remain empty.
360			if [[ -s $diff ]]; then
361				diff_out=$(< "$diff")
362			fi
363
364			cat <<- ERROR_SHFMT
365
366				* Errors in style formatting have been detected.
367				${diff_out:+* Please, review the generated patch at $diff
368
369				# _START_OF_THE_DIFF
370
371				${diff_out:-ERROR}
372
373				# _END_OF_THE_DIFF
374				}
375
376			ERROR_SHFMT
377			rc=1
378		else
379			rm -f "$diff"
380			printf ' OK\n'
381		fi
382	fi
383else
384	echo "shfmt not detected, Bash style formatting check is skipped"
385fi
386
387if hash shellcheck 2> /dev/null; then
388	echo -n "Checking Bash style..."
389
390	shellcheck_v=$(shellcheck --version | grep -P "version: [0-9\.]+" | cut -d " " -f2)
391
392	# SHCK_EXCLUDE contains a list of all of the spellcheck errors found in SPDK scripts
393	# currently. New errors should only be added to this list if the cost of fixing them
394	# is deemed too high. For more information about the errors, go to:
395	# https://github.com/koalaman/shellcheck/wiki/Checks
396	# Error descriptions can also be found at: https://github.com/koalaman/shellcheck/wiki
397	# SPDK fails some error checks which have been deprecated in later versions of shellcheck.
398	# We will not try to fix these error checks, but instead just leave the error types here
399	# so that we can still run with older versions of shellcheck.
400	SHCK_EXCLUDE="SC1117"
401	# SPDK has decided to not fix violations of these errors.
402	# We are aware about below exclude list and we want this errors to be excluded.
403	# SC1083: This {/} is literal. Check expression (missing ;/\n?) or quote it.
404	# SC1090: Can't follow non-constant source. Use a directive to specify location.
405	# SC1091: Not following: (error message here)
406	# SC2001: See if you can use ${variable//search/replace} instead.
407	# SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
408	# SC2015: Note that A && B || C is not if-then-else. C may run when A is true.
409	# SC2016: Expressions don't expand in single quotes, use double quotes for that.
410	# SC2034: foo appears unused. Verify it or export it.
411	# SC2046: Quote this to prevent word splitting.
412	# SC2086: Double quote to prevent globbing and word splitting.
413	# SC2119: Use foo "$@" if function's $1 should mean script's $1.
414	# SC2120: foo references arguments, but none are ever passed.
415	# SC2148: Add shebang to the top of your script.
416	# SC2153: Possible Misspelling: MYVARIABLE may not be assigned, but MY_VARIABLE is.
417	# SC2154: var is referenced but not assigned.
418	# SC2164: Use cd ... || exit in case cd fails.
419	# SC2174: When used with -p, -m only applies to the deepest directory.
420	# SC2206: Quote to prevent word splitting/globbing,
421	#         or split robustly with mapfile or read -a.
422	# SC2207: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
423	# SC2223: This default assignment may cause DoS due to globbing. Quote it.
424	SHCK_EXCLUDE="$SHCK_EXCLUDE,SC1083,SC1090,SC1091,SC2010,SC2015,SC2016,SC2034,SC2046,SC2086,\
425SC2119,SC2120,SC2148,SC2153,SC2154,SC2164,SC2174,SC2001,SC2206,SC2207,SC2223"
426
427	SHCK_FORMAT="diff"
428	SHCK_APPLY=true
429	if [ "$shellcheck_v" \< "0.7.0" ]; then
430		SHCK_FORMAT="tty"
431		SHCK_APPLY=false
432	fi
433	SHCH_ARGS=" -x -e $SHCK_EXCLUDE -f $SHCK_FORMAT"
434
435	error=0
436	git ls-files '*.sh' | xargs -P$(nproc) -n1 shellcheck $SHCH_ARGS &> shellcheck.log || error=1
437	if [ $error -ne 0 ]; then
438		echo " Bash formatting errors detected!"
439
440		# Some errors are not auto-fixable. Fall back to tty output.
441		if grep -q "Use another format to see them." shellcheck.log; then
442			SHCK_FORMAT="tty"
443			SHCK_APPLY=false
444			SHCH_ARGS=" -e $SHCK_EXCLUDE -f $SHCK_FORMAT"
445			git ls-files '*.sh' | xargs -P$(nproc) -n1 shellcheck $SHCH_ARGS > shellcheck.log || error=1
446		fi
447
448		cat shellcheck.log
449		if $SHCK_APPLY; then
450			git apply shellcheck.log
451			echo "Bash errors were automatically corrected."
452			echo "Please remember to add the changes to your commit."
453		fi
454		rc=1
455	else
456		echo " OK"
457	fi
458	rm -f shellcheck.log
459else
460	echo "You do not have shellcheck installed so your Bash style is not being checked!"
461fi
462
463# Check if any of the public interfaces were modified by this patch.
464# Warn the user to consider updating the changelog any changes
465# are detected.
466echo -n "Checking whether CHANGELOG.md should be updated..."
467staged=$(git diff --name-only --cached .)
468working=$(git status -s --porcelain --ignore-submodules | grep -iv "??" | awk '{print $2}')
469files="$staged $working"
470if [[ "$files" = " " ]]; then
471	files=$(git diff-tree --no-commit-id --name-only -r HEAD)
472fi
473
474has_changelog=0
475for f in $files; do
476	if [[ $f == CHANGELOG.md ]]; then
477		# The user has a changelog entry, so exit.
478		has_changelog=1
479		break
480	fi
481done
482
483needs_changelog=0
484if [ $has_changelog -eq 0 ]; then
485	for f in $files; do
486		if [[ $f == include/spdk/* ]] || [[ $f == scripts/rpc.py ]] || [[ $f == etc/* ]]; then
487			echo ""
488			echo -n "$f was modified. Consider updating CHANGELOG.md."
489			needs_changelog=1
490		fi
491	done
492fi
493
494if [ $needs_changelog -eq 0 ]; then
495	echo " OK"
496else
497	echo ""
498fi
499
500exit $rc
501