Merge pull request #1876 from fpistm/astyle_review

ci: review

Author: fpistm

.github/workflows/CodeSpell.yml

@@ -14,7 +14,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@main
 
       # See: https://github.com/codespell-project/actions-codespell/blob/master/README.md
       - name: Spell check

.github/workflows/Continuous-Integration.yml

@@ -27,27 +27,6 @@ on:
   # Allows you to run this workflow manually from the Actions tab
   workflow_dispatch:
 jobs:
-  astyle_check:
-    runs-on: ubuntu-latest
-    name: AStyle check
-    steps:
-    # First of all, clone the repo using the checkout action.
-    - name: Checkout
-      uses: actions/checkout@main
-
-    - name: Astyle check
-      id: Astyle
-      uses: stm32duino/actions/astyle-check@main
-      with:
-        astyle-definition: 'CI/astyle/.astylerc'
-        ignore-path-list: 'CI/astyle/.astyleignore'
-
-    # Use the output from the `Astyle` step
-    - name: Astyle Errors
-      if: failure()
-      run: |
-        cat ${{ steps.Astyle.outputs.astyle-result }}
-        exit 1
   core_build:
     runs-on: ubuntu-latest
     name: Core compilation

.github/workflows/astyle.yml

@@ -0,0 +1,48 @@
+name: Check code formatting with astyle
+on:
+  push:
+    branches:
+      - main
+    paths-ignore:
+      - '*.json'
+      - '**.md'
+      - keywords.txt
+      - CI/**
+      - '!CI/astyle/.astyleignore'
+      - '!CI/astyle/.astylerc'
+      - '!CI/astyle/astyle.py'
+      - tools/**
+  pull_request:
+    paths-ignore:
+      - '*.json'
+      - '**.md'
+      - keywords.txt
+      - CI/**
+      - '!CI/astyle/.astyleignore'
+      - '!CI/astyle/.astylerc'
+      - '!CI/astyle/astyle.py'
+      - tools/**
+  # Allows you to run this workflow manually from the Actions tab
+  workflow_dispatch:
+jobs:
+  astyle_check:
+    runs-on: ubuntu-latest
+    name: Check for astyle errors
+    steps:
+    # First of all, clone the repo using the checkout action.
+    - name: Checkout
+      uses: actions/checkout@main
+
+    - name: Astyle check
+      id: Astyle
+      uses: stm32duino/actions/astyle-check@main
+      with:
+        astyle-definition: 'CI/astyle/.astylerc'
+        ignore-path-list: 'CI/astyle/.astyleignore'
+
+    # Use the output from the `Astyle` step
+    - name: Astyle Errors
+      if: failure()
+      run: |
+        cat ${{ steps.Astyle.outputs.astyle-result }}
+        exit 1

CI/astyle/astyle.py

@@ -1,39 +1,30 @@
-# -*- coding: utf-8 -*-
-
-# File name            : astyle.py
-# Author               : Frederic PILLON <frederic.pillon@st.com>
-# Created              : 11/16/2018
-# Python Version       :
-# Requirements         : Artistic Style Version 3.1
-# Description          : Launch astyle on found source files
-
 import argparse
-import os
+from pathlib import Path
 import re
 import subprocess
 import sys
 
-script_path = os.path.dirname(os.path.abspath(__file__))
+script_path = Path(__file__).parent.resolve()
 ignore_filename = ".astyleignore"
 def_filename = ".astylerc"
 astyle_out_filename = "astyle.out"
-if os.getcwd() != script_path:
-    src_path = os.getcwd()
+if Path.cwd() != script_path:
+    src_path = Path.cwd()
 else:
-    src_path = os.path.realpath(os.path.join("..", ".."))
-ignore_path = os.path.join(script_path, ignore_filename)
-def_path = os.path.join(script_path, def_filename)
-astyle_out_path = astyle_out_filename
+    src_path = script_path.parent.parent
+ignore_path = script_path / ignore_filename
+def_path = script_path / def_filename
+astyle_out_path = Path(astyle_out_filename)
 git_branch = "remotes/origin/main"
 
+script_version = "1.0.0"
 astyle_major = 3
 astyle_minor = 1
-astyle_path = ""
+astyle_path = Path()
+astyle_cmd = "astyle"
 if sys.platform.startswith("win32"):
-    astyle_cmd = "astyle.exe"
-elif sys.platform.startswith("linux") or sys.platform.startswith("darwin"):
-    astyle_cmd = "astyle"
-else:
+    astyle_cmd += ".exe"
+elif not sys.platform.startswith("linux") and not sys.platform.startswith("darwin"):
     print("Platform unknown.")
     sys.exit(1)
 
@@ -43,8 +34,8 @@
 
 
 # Check if path exists
-def checkPath(path, msg):
-    if not os.path.exists(path):
+def checkPath(p, msg):
+    if not p.exists():
         print(msg)
         sys.exit(1)
 
@@ -53,7 +44,7 @@ def checkPath(path, msg):
 def checkAstyle():
     try:
         output = subprocess.check_output(
-            [os.path.join(astyle_path, astyle_cmd), "--version"],
+            [astyle_path, "--version"],
             stderr=subprocess.STDOUT,
         )
 
@@ -64,11 +55,8 @@ def checkAstyle():
             if major >= astyle_major and minor >= astyle_minor:
                 print(output.decode("utf-8").rstrip())
             else:
-                print(
-                    "Required Astyle version {}.{} (Current: {}.{})".format(
-                        astyle_major, astyle_minor, major, minor
-                    )
-                )
+                print(f"Astyle minimum version required {astyle_major}.{astyle_minor}.")
+                print(f"Current is {major}.{minor}.")
                 sys.exit(1)
         else:
             raise subprocess.CalledProcessError(1, "No version found")
@@ -91,16 +79,21 @@ def gitdiff_files():
         print(e.output)
         sys.exit(1)
 
-    gitroot = output.decode("utf-8").rstrip()
-    rel_src = re.sub("^[/\\]+]", "", re.sub(gitroot, "", src_path))
+    gitroot = Path(output.decode("utf-8").rstrip())
+    try:
+        rel_src = src_path.relative_to(gitroot)
+    except ValueError:
+        print(f"{src_path} not in git repository.")
+        sys.exit(1)
 
-    cmd = []
-    cmd.append("git")
+    cmd = ["git"]
     cmd.append("diff")
     cmd.append("--name-only")
     cmd.append("--diff-filter=d")
     cmd.append(git_branch)
-    cmd.append("--relative=" + rel_src)
+    # Relative only if source root path specified
+    if args.root:
+        cmd.append(f"--relative={rel_src}")
 
     proc = subprocess.Popen(cmd, stdout=subprocess.PIPE)
     while True:
@@ -109,18 +102,22 @@ def gitdiff_files():
             break
         if line:
             if line.endswith((".h", ".c", ".hpp", ".cpp")):
-                source_list.append(os.path.join(gitroot, line.rstrip()))
+                source_list.append(gitroot / line.rstrip())
     if proc.poll() != 0:
         sys.exit(1)
     source_list.sort()
 
 
 # Find all files in source root path
 def find_files():
-    for root, dirs, files in os.walk(src_path, followlinks=True):
-        for f in files:
-            if f.endswith((".h", ".c", ".hpp", ".cpp")):
-                source_list.append(os.path.join(root, f))
+    for spath_object in src_path.glob("**/*"):
+        if spath_object.is_file() and spath_object.suffix in [
+            ".h",
+            ".c",
+            ".hpp",
+            ".cpp",
+        ]:
+            source_list.append(spath_object)
     source_list.sort()
 
 
@@ -132,22 +129,17 @@ def manage_exclude_list():
                 exclude_list.append(line.rstrip())
     if exclude_list:
         for pattern in exclude_list:
-            if sys.platform.startswith("win32"):
-                winpattern = os.path.join(src_path, pattern.replace("/","\\")) .replace("\\","\\\\")
-                exclude_pattern = re.compile(winpattern + ".*")
-            else:
-                exclude_pattern = re.compile(os.path.join(src_path, pattern) + ".*")
+            exclude_path = src_path / pattern
             for s in reversed(source_list):
-                if exclude_pattern.search(s):
+                if s.is_relative_to(exclude_path):
                     source_list.remove(s)
 
 
 # Launch Astyle on all source files
 def astyle():
-    cmd = []
-    cmd.append(os.path.join(astyle_path, astyle_cmd))
+    cmd = [astyle_path]
     cmd.append("-n")
-    cmd.append("--options=" + def_path)
+    cmd.append(f"--options={def_path}")
     cmd.append("dummy_file")
 
     stddout_name = astyle_out_path
@@ -161,21 +153,19 @@ def astyle():
 
 
 # Parser
-parser = argparse.ArgumentParser(
-    description="Launch astyle on source files found at specified root path."
-)
+parser = argparse.ArgumentParser(description="Launch astyle on source files.")
 
 parser.add_argument(
     "-d",
     "--definition",
     metavar="<code style definition file>",
-    help="Code style definition file for Astyle. Default: " + def_path,
+    help=f"Code style definition file for Astyle. Default: {def_path}",
 )
 g0 = parser.add_mutually_exclusive_group()
 g0.add_argument(
     "-g",
     "--gitdiff",
-    help="Use changes files from git default branch. Default: " + git_branch,
+    help="Use changes files from git default branch. Default: {git_branch}",
     action="store_true",
 )
 g0.add_argument(
@@ -188,7 +178,7 @@ def astyle():
     "-i",
     "--ignore",
     metavar="<ignore file>",
-    help="File containing path to ignore. Default: " + ignore_path,
+    help="File containing path to ignore. Default: {ignore_path}",
 )
 parser.add_argument(
     "-p", "--path", metavar="<astyle install path>", help="Astyle installation path"
@@ -197,7 +187,7 @@ def astyle():
     "-r",
     "--root",
     metavar="<source root path>",
-    help="Source root path to use. Default: " + src_path,
+    help="Source root path to use. Default: {src_path}",
 )
 args = parser.parse_args()
 
@@ -209,27 +199,28 @@ def main():
     global astyle_path
     global git_branch
 
+    print(f"Script {Path(__file__).name} version {script_version}")
     if args.root:
-        src_path = os.path.realpath(args.root)
+        src_path = Path(args.root).resolve()
 
     if args.definition:
-        def_path = os.path.realpath(args.definition)
+        def_path = Path(args.definition).resolve()
 
     if args.ignore:
-        ignore_path = os.path.realpath(args.ignore)
+        ignore_path = Path(args.ignore).resolve()
 
     if args.path:
-        astyle_path = os.path.realpath(args.path)
-        checkPath(
-            os.path.join(astyle_path, astyle_cmd), "Could not find Astyle binary!"
-        )
+        astyle_path = Path(args.path).resolve() / astyle_cmd
+        checkPath(astyle_path, "Could not find Astyle binary!")
+    else:
+        astyle_path = Path(astyle_cmd)
 
     checkPath(src_path, "Source root path does not exist!")
     checkPath(def_path, "Code style definition file does not exist!")
     checkPath(ignore_path, "Ignore file does not exist!")
     checkAstyle()
     try:
-        os.remove(astyle_out_path)
+        astyle_out_path.unlink()
     except OSError:
         pass