From 3bf133525f4659d52e70acf0ebf77f6bd45dd8d8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Nov 2023 10:44:00 +0100 Subject: [PATCH 1/3] Make a wrong uncrustify version a fatal error We know that using a different version of uncrustify produces different results. So make that an error rather than a warning. Also make the error output more helpful if uncrustify is not found. Signed-off-by: Gilles Peskine --- scripts/code_style.py | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/scripts/code_style.py b/scripts/code_style.py index ef2750819..17b56968e 100755 --- a/scripts/code_style.py +++ b/scripts/code_style.py @@ -166,16 +166,18 @@ def get_submodule_hash(commit: str, submodule: str) -> str: def get_uncrustify_version() -> str: """ - Get the version string from Uncrustify + Get the version string from Uncrustify. + + Return an empty string if Uncrustify is not found. """ - result = subprocess.run([UNCRUSTIFY_EXE, "--version"], - stdout=subprocess.PIPE, stderr=subprocess.PIPE, - check=False) - if result.returncode != 0: - print_err("Could not get Uncrustify version:", str(result.stderr, "utf-8")) - return "" - else: - return str(result.stdout, "utf-8") + try: + output = subprocess.check_output([UNCRUSTIFY_EXE, "--version"], + stderr=subprocess.PIPE) + return str(output, "utf-8").strip() + except FileNotFoundError: + sys.stderr.write('Fatal: command {} not found in PATH.\n' + .format(UNCRUSTIFY_EXE)) + return '' def check_style_is_correct(src_file_list: List[str]) -> bool: """ @@ -246,12 +248,14 @@ def main() -> int: """ Main with command line arguments. """ - uncrustify_version = get_uncrustify_version().strip() + uncrustify_version = get_uncrustify_version() if UNCRUSTIFY_SUPPORTED_VERSION not in uncrustify_version: - print("Warning: Using unsupported Uncrustify version '" + - uncrustify_version + "'") - print("Note: The only supported version is " + - UNCRUSTIFY_SUPPORTED_VERSION) + if uncrustify_version != '': + sys.stderr.write('Fatal: wrong uncrustify version ({}).\n' + .format(uncrustify_version)) + sys.stderr.write('You need uncrustify {} for correct results.\n' + .format(UNCRUSTIFY_SUPPORTED_VERSION)) + return 2 parser = argparse.ArgumentParser() parser.add_argument('-f', '--fix', action='store_true', From eaf2b3e358af34f332223d49d341f446e31ab038 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Nov 2023 10:49:30 +0100 Subject: [PATCH 2/3] Allow specifying a diffent uncrustify command This makes it easier to run the script on a machine where the system-installed uncrustify is a different version. Signed-off-by: Gilles Peskine --- scripts/code_style.py | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/scripts/code_style.py b/scripts/code_style.py index 17b56968e..92da9d549 100755 --- a/scripts/code_style.py +++ b/scripts/code_style.py @@ -164,29 +164,30 @@ def get_submodule_hash(commit: str, submodule: str) -> str: output = subprocess.check_output(cmd, universal_newlines=True) return output.split()[2] -def get_uncrustify_version() -> str: +def get_uncrustify_version(uncrustify_exe: str) -> str: """ Get the version string from Uncrustify. Return an empty string if Uncrustify is not found. """ try: - output = subprocess.check_output([UNCRUSTIFY_EXE, "--version"], + output = subprocess.check_output([uncrustify_exe, "--version"], stderr=subprocess.PIPE) return str(output, "utf-8").strip() except FileNotFoundError: sys.stderr.write('Fatal: command {} not found in PATH.\n' - .format(UNCRUSTIFY_EXE)) + .format(uncrustify_exe)) return '' -def check_style_is_correct(src_file_list: List[str]) -> bool: +def check_style_is_correct(uncrustify_exe: str, + src_file_list: List[str]) -> bool: """ Check the code style and output a diff for each file whose style is incorrect. """ style_correct = True for src_file in src_file_list: - uncrustify_cmd = [UNCRUSTIFY_EXE] + UNCRUSTIFY_ARGS + [src_file] + uncrustify_cmd = [uncrustify_exe] + UNCRUSTIFY_ARGS + [src_file] result = subprocess.run(uncrustify_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=False) if result.returncode != 0: @@ -227,7 +228,7 @@ def fix_style_single_pass(src_file_list: List[str]) -> bool: return False return True -def fix_style(src_file_list: List[str]) -> int: +def fix_style(uncrustify_exe: str, src_file_list: List[str]) -> int: """ Fix the code style. This takes 2 passes of Uncrustify. """ @@ -238,7 +239,7 @@ def fix_style(src_file_list: List[str]) -> int: # Guard against future changes that cause the codebase to require # more passes. - if not check_style_is_correct(src_file_list): + if not check_style_is_correct(uncrustify_exe, src_file_list): print_err("Code style still incorrect after second run of Uncrustify.") return 1 else: @@ -248,15 +249,6 @@ def main() -> int: """ Main with command line arguments. """ - uncrustify_version = get_uncrustify_version() - if UNCRUSTIFY_SUPPORTED_VERSION not in uncrustify_version: - if uncrustify_version != '': - sys.stderr.write('Fatal: wrong uncrustify version ({}).\n' - .format(uncrustify_version)) - sys.stderr.write('You need uncrustify {} for correct results.\n' - .format(UNCRUSTIFY_SUPPORTED_VERSION)) - return 2 - parser = argparse.ArgumentParser() parser.add_argument('-f', '--fix', action='store_true', help=('modify source files to fix the code style ' @@ -272,11 +264,23 @@ def main() -> int: # way to restyle a possibly empty set of files. parser.add_argument('--subset', action='store_true', help='only check the specified files (default with non-option arguments)') + parser.add_argument('--uncrustify', + default='uncrustify', + help='uncrustify command to run (default: uncrustify)') parser.add_argument('operands', nargs='*', metavar='FILE', help='files to check (files MUST be known to git, if none: check all)') args = parser.parse_args() + uncrustify_version = get_uncrustify_version(args.uncrustify) + if UNCRUSTIFY_SUPPORTED_VERSION not in uncrustify_version: + if uncrustify_version != '': + sys.stderr.write('Fatal: wrong uncrustify version ({}).\n' + .format(uncrustify_version)) + sys.stderr.write('You need uncrustify {} for correct results.\n' + .format(UNCRUSTIFY_SUPPORTED_VERSION)) + return 2 + covered = frozenset(get_src_files(args.since)) # We only check files that are known to git if args.subset or args.operands: @@ -289,10 +293,10 @@ def main() -> int: if args.fix: # Fix mode - return fix_style(src_files) + return fix_style(args.uncrustify, src_files) else: # Check mode - if check_style_is_correct(src_files): + if check_style_is_correct(args.uncrustify, src_files): print("Checked {} files, style ok.".format(len(src_files))) return 0 else: From 9707b5c138f7f8de22f3ee59db1444b6385e331c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Nov 2023 17:22:33 +0100 Subject: [PATCH 3/3] Also apply --uncrustify to --fix mode Before this commit, verify mode still hard-coded "uncrustify" as the command name. Signed-off-by: Gilles Peskine --- scripts/code_style.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/code_style.py b/scripts/code_style.py index 92da9d549..dae4b6802 100755 --- a/scripts/code_style.py +++ b/scripts/code_style.py @@ -15,7 +15,6 @@ from mbedtls_framework import build_tree UNCRUSTIFY_SUPPORTED_VERSION = "0.75.1" CONFIG_FILE = ".uncrustify.cfg" -UNCRUSTIFY_EXE = "uncrustify" UNCRUSTIFY_ARGS = ["-c", CONFIG_FILE] CHECK_GENERATED_FILES = "tests/scripts/check-generated-files.sh" @@ -213,13 +212,14 @@ def check_style_is_correct(uncrustify_exe: str, return style_correct -def fix_style_single_pass(src_file_list: List[str]) -> bool: +def fix_style_single_pass(uncrustify_exe: str, + src_file_list: List[str]) -> bool: """ Run Uncrustify once over the source files. """ code_change_args = UNCRUSTIFY_ARGS + ["--no-backup"] for src_file in src_file_list: - uncrustify_cmd = [UNCRUSTIFY_EXE] + code_change_args + [src_file] + uncrustify_cmd = [uncrustify_exe] + code_change_args + [src_file] result = subprocess.run(uncrustify_cmd, check=False) if result.returncode != 0: print_err("Uncrustify with file returned: " + @@ -232,9 +232,9 @@ def fix_style(uncrustify_exe: str, src_file_list: List[str]) -> int: """ Fix the code style. This takes 2 passes of Uncrustify. """ - if not fix_style_single_pass(src_file_list): + if not fix_style_single_pass(uncrustify_exe, src_file_list): return 1 - if not fix_style_single_pass(src_file_list): + if not fix_style_single_pass(uncrustify_exe, src_file_list): return 1 # Guard against future changes that cause the codebase to require