From 8a29ec894588d254fed79a9eb0ae2b9e99be0ec7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Nov 2023 10:44:00 +0100 Subject: [PATCH 1/4] 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 08ec4af423d6..cbbb66ba6e8d 100755 --- a/scripts/code_style.py +++ b/scripts/code_style.py @@ -90,16 +90,18 @@ def get_src_files(since: Optional[str]) -> List[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: """ @@ -170,12 +172,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 67c682cc5c16828ff750f7190da76549912768bf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Nov 2023 10:49:30 +0100 Subject: [PATCH 2/4] 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 cbbb66ba6e8d..e2832309de99 100755 --- a/scripts/code_style.py +++ b/scripts/code_style.py @@ -88,29 +88,30 @@ def get_src_files(since: Optional[str]) -> List[str]: filename in generated_files)] return src_files -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: @@ -151,7 +152,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. """ @@ -162,7 +163,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: @@ -172,15 +173,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 ' @@ -196,11 +188,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: @@ -213,10 +217,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 03d003f3018a2ca1669be5604c00da691d1e7e7a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Nov 2023 10:51:00 +0100 Subject: [PATCH 3/4] Hide spurious error message if uncrustify is not present Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 086677a600d3..919872e72710 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -5689,7 +5689,7 @@ component_check_code_style () { } support_check_code_style() { - case $(uncrustify --version) in + case $(uncrustify --version 2>/dev/null) in *0.75.1*) true;; *) false;; esac From 0819854dea2a8a448419154e057c439b4005d3c2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Nov 2023 17:22:33 +0100 Subject: [PATCH 4/4] 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 e2832309de99..196453fd9422 100755 --- a/scripts/code_style.py +++ b/scripts/code_style.py @@ -14,7 +14,6 @@ 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" @@ -137,13 +136,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: " + @@ -156,9 +156,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