clang-format and clang-tidy scripts: More robust algorithm to find correct executable (#6041)

* More robust algorithm to find correct executable

* Revise message wording

* Add clang-tidy and clang-format to requirements.txt.
Add to message explaining install process.

* Extracted get_binary to helpers.py. Use execptions for clean exit.

* Add parameter types

* clang-{tidy,format} in requirements_test.txt
clean up script exit

* Kill processes on ^C

* Move clang-tidy and clang-format into requirements_dev.txt

requirements_dev.txt

@@ -0,0 +1,3 @@
+# Useful stuff when working in a development environment
+clang-format==13.0.1
+clang-tidy==14.0.6

requirements_test.txt

@@ -11,5 +11,3 @@ pytest-mock==3.12.0
 pytest-asyncio==0.23.2
 asyncmock==0.4.2
 hypothesis==5.49.0
-
-clang-format==13.0.1 ; platform_machine != 'armv7l'

script/clang-format

@@ -1,6 +1,12 @@
 #!/usr/bin/env python3
 
-from helpers import print_error_for_file, get_output, git_ls_files, filter_changed
+from helpers import (
+    print_error_for_file,
+    get_output,
+    git_ls_files,
+    filter_changed,
+    get_binary,
+)
 import argparse
 import click
 import colorama
@@ -13,11 +19,12 @@ import sys
 import threading
 
 
-def run_format(args, queue, lock, failed_files):
+
+def run_format(executable, args, queue, lock, failed_files):
     """Takes filenames out of queue and runs clang-format on them."""
     while True:
         path = queue.get()
-        invocation = ["clang-format-13"]
+        invocation = [executable]
         if args.inplace:
             invocation.append("-i")
         else:
@@ -58,22 +65,6 @@ def main():
     )
     args = parser.parse_args()
 
-    try:
-        get_output("clang-format-13", "-version")
-    except:
-        print(
-            """
-        Oops. It looks like clang-format is not installed. 
-        
-        Please check you can run "clang-format-13 -version" in your terminal and install
-        clang-format (v13) if necessary.
-        
-        Note you can also upload your code as a pull request on GitHub and see the CI check
-        output to apply clang-format.
-        """
-        )
-        return 1
-
     files = []
     for path in git_ls_files(["*.cpp", "*.h", "*.tcc"]):
         files.append(os.path.relpath(path, os.getcwd()))
@@ -90,11 +81,12 @@ def main():
 
     failed_files = []
     try:
+        executable = get_binary("clang-format", 13)
         task_queue = queue.Queue(args.jobs)
         lock = threading.Lock()
         for _ in range(args.jobs):
             t = threading.Thread(
-                target=run_format, args=(args, task_queue, lock, failed_files)
+                target=run_format, args=(executable, args, task_queue, lock, failed_files)
             )
             t.daemon = True
             t.start()
@@ -109,13 +101,18 @@ def main():
         # Wait for all threads to be done.
         task_queue.join()
 
+    except FileNotFoundError as ex:
+        return 1
     except KeyboardInterrupt:
         print()
         print("Ctrl-C detected, goodbye.")
+        # Kill subprocesses (and ourselves!)
+        # No simple, clean alternative appears to be available.
         os.kill(0, 9)
+        return 2    # Will not execute.
 
-    sys.exit(len(failed_files))
+    return len(failed_files)
 
 
 if __name__ == "__main__":
-    main()
+    sys.exit(main())

script/clang-tidy

@@ -11,6 +11,7 @@ from helpers import (
     load_idedata,
     root_path,
     basepath,
+    get_binary,
 )
 import argparse
 import click
@@ -26,6 +27,7 @@ import tempfile
 import threading
 
 
+
 def clang_options(idedata):
     cmd = []
 
@@ -110,10 +112,12 @@ def clang_options(idedata):
     return cmd
 
 
-def run_tidy(args, options, tmpdir, queue, lock, failed_files):
+pids = set()
+
+def run_tidy(executable, args, options, tmpdir, queue, lock, failed_files):
     while True:
         path = queue.get()
-        invocation = ["clang-tidy-14"]
+        invocation = [executable]
 
         if tmpdir is not None:
             invocation.append("--export-fixes")
@@ -193,22 +197,6 @@ def main():
     )
     args = parser.parse_args()
 
-    try:
-        get_output("clang-tidy-14", "-version")
-    except:
-        print(
-            """
-        Oops. It looks like clang-tidy-14 is not installed.
-
-        Please check you can run "clang-tidy-14 -version" in your terminal and install
-        clang-tidy (v14) if necessary.
-
-        Note you can also upload your code as a pull request on GitHub and see the CI check
-        output to apply clang-tidy.
-        """
-        )
-        return 1
-
     idedata = load_idedata(args.environment)
     options = clang_options(idedata)
 
@@ -242,12 +230,13 @@ def main():
 
     failed_files = []
     try:
+        executable = get_binary("clang-tidy", 14)
         task_queue = queue.Queue(args.jobs)
         lock = threading.Lock()
         for _ in range(args.jobs):
             t = threading.Thread(
                 target=run_tidy,
-                args=(args, options, tmpdir, task_queue, lock, failed_files),
+                args=(executable, args, options, tmpdir, task_queue, lock, failed_files),
             )
             t.daemon = True
             t.start()
@@ -262,12 +251,17 @@ def main():
         # Wait for all threads to be done.
         task_queue.join()
 
+    except FileNotFoundError as ex:
+        return 1
     except KeyboardInterrupt:
         print()
         print("Ctrl-C detected, goodbye.")
         if tmpdir:
             shutil.rmtree(tmpdir)
+        # Kill subprocesses (and ourselves!)
+        # No simple, clean alternative appears to be available.
         os.kill(0, 9)
+        return 2    # Will not execute.
 
     if args.fix and failed_files:
         print("Applying fixes ...")
@@ -277,8 +271,8 @@ def main():
             print("Error applying fixes.\n", file=sys.stderr)
             raise
 
-    sys.exit(len(failed_files))
+    return len(failed_files)
 
 
 if __name__ == "__main__":
-    main()
+    sys.exit(main())

script/helpers.py

@@ -153,3 +153,39 @@ def load_idedata(environment):
 
     temp_idedata.write_text(json.dumps(data, indent=2) + "\n")
     return data
+
+
+def get_binary(name: str, version: str) -> str:
+    binary_file = f"{name}-{version}"
+    try:
+        result = subprocess.check_output([binary_file, "-version"])
+        if result.returncode == 0:
+            return binary_file
+    except Exception:
+        pass
+    binary_file = name
+    try:
+        result = subprocess.run(
+            [binary_file, "-version"], text=True, capture_output=True
+        )
+        if result.returncode == 0 and (f"version {version}") in result.stdout:
+            return binary_file
+        raise FileNotFoundError(f"{name} not found")
+
+    except FileNotFoundError as ex:
+        print(
+            f"""
+            Oops. It looks like {name} is not installed. It should be available under venv/bin
+            and in PATH after running in turn:
+              script/setup
+              source venv/bin/activate.
+
+            Please confirm you can run "{name} -version" or "{name}-{version} -version"
+            in your terminal and install
+            {name} (v{version}) if necessary.
+
+            Note you can also upload your code as a pull request on GitHub and see the CI check
+            output to apply {name}
+            """
+        )
+        raise

script/setup

@@ -15,10 +15,14 @@ if [ -n "$DEVCONTAINER" ];then
   git config --global --add safe.directory "$PWD"
 fi
 
-pip3 install -r requirements.txt -r requirements_optional.txt -r requirements_test.txt
+pip3 install -r requirements.txt -r requirements_optional.txt -r requirements_test.txt -r requirements_dev.txt
 pip3 install setuptools wheel
 pip3 install --no-use-pep517 -e .
 
 pre-commit install
 
 script/platformio_install_deps.py platformio.ini --libraries --tools --platforms
+
+echo
+echo
+echo "Virtual environment created; source venv/bin/activate to use it"