[util] Don't explode on unexpected format in fix_include_guard.py
The existing code assumed that there was an #ifndef line somewhere in
the file and exploded if not (calling None.group()). This patch
handles these cases more gracefully by having a notion of "unfixable"
files. It also changes the output format a bit to make the CI
wrapper's job a bit easier.
Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
diff --git a/ci/scripts/include-guard.sh b/ci/scripts/include-guard.sh
index 7577221..ffc6076 100755
--- a/ci/scripts/include-guard.sh
+++ b/ci/scripts/include-guard.sh
@@ -22,19 +22,11 @@
}
echo "Checking header guards on headers changed since $merge_base"
-TMPFILE="$(mktemp)" || {
- echo >&2 "Failed to create temporary file"
- exit 1
-}
-trap 'rm -f "$TMPFILE"' EXIT
-
set -o pipefail
git diff --name-only --diff-filter=ACMRTUXB "$merge_base" -- \
"*.h" ':!*/vendor/*' | \
- xargs -r util/fix_include_guard.py --dry-run | \
- tee "$TMPFILE"
-if [ -s "$TMPFILE" ]; then
+ xargs -r util/fix_include_guard.py --dry-run || {
echo -n "##vso[task.logissue type=error]"
echo "Include guard check failed. Please run util/fix_include_guard.py on the above files."
exit 1
-fi
+}
diff --git a/util/fix_include_guard.py b/util/fix_include_guard.py
index 2de10df..2bddda8 100755
--- a/util/fix_include_guard.py
+++ b/util/fix_include_guard.py
@@ -13,10 +13,10 @@
#
# This script will write the names of all affected files to stdout; if no such
# output is present, all files have the correct guards. This script is
-# idempotent.
+# idempotent. If it cannot fix a file, the script will write the file name to
+# stderr and will exit at the end with a nonzero status code.
import argparse
-import os
import sys
import re
@@ -42,7 +42,9 @@
help='headers to fix guards for')
args = parser.parse_args()
- total_fixes = 0
+ total_unfixable = 0
+ total_fixable = 0
+
for header_path in args.headers:
header = Path(header_path).resolve().relative_to(REPO_TOP)
if header.suffix != '.h' or 'vendor' in header.parts:
@@ -55,8 +57,27 @@
header_text = header.read_text()
header_original = header_text
+ # Find the first non-comment, non-whitespace line in the file
+ first_line_match = re.search(r'^\s*([^/].*)', header_text, re.MULTILINE)
+ if first_line_match is None:
+ total_unfixable += 1
+ print('No non-comment line found in `{}\'.'.format(header),
+ file=sys.stderr)
+ continue
+
+ first_line = first_line_match.group(1).rstrip()
+
# Find the old guard name, which will be the first #ifndef in the file.
- old_guard = re.search(r'#ifndef +(\w+)', header_text).group(1)
+ # This should be an #ifndef line. If it isn't, we can't fix things
+ ifndef_match = re.match(r'#ifndef\s+(\w+)$', first_line)
+ if ifndef_match is None:
+ total_unfixable += 1
+ print('Unsupported first non-comment line in `{}\': {!r}.'
+ .format(header, first_line),
+ file=sys.stderr)
+ continue
+
+ old_guard = ifndef_match.group(1)
# Fix the guards at the top, which are guaranteed to be there.
header_text = re.sub('#(ifndef|define) +%s' % (old_guard, ),
@@ -70,12 +91,21 @@
if header_text != header_original:
print('Fixing header: "%s"' % (header, ), file=sys.stdout)
- total_fixes += 1
+ total_fixable += 1
if not args.dry_run:
header.write_text(header_text)
- print('Fixed %d files.' % (total_fixes, ), file=sys.stderr)
+ if total_fixable:
+ verb = 'Would have fixed' if args.dry_run else 'Fixed'
+ print('{} {} files.'.format(verb, total_fixable), file=sys.stderr)
+ if total_unfixable:
+ print('Failed to fix {} files.'.format(total_unfixable),
+ file=sys.stderr)
+
+ # Pass if we fixed everything or there was nothing to fix.
+ unfixed = total_unfixable + (total_fixable if args.dry_run else 0)
+ return 1 if unfixed else 0
if __name__ == '__main__':
- main()
+ sys.exit(main())
