diff --git a/Dockerfile b/Dockerfile index bfa4b1e..f8b72ed 100644 --- a/Dockerfile +++ b/Dockerfile @@ -5,6 +5,7 @@ COPY requirements.txt /requirements.txt RUN apt update && \ DEBIAN_FRONTEND=noninteractive \ apt-get install -y --no-install-recommends\ + build-essential cmake git \ tzdata \ clang-tidy-6.0 \ clang-tidy-7 \ @@ -18,6 +19,5 @@ RUN apt update && \ pip3 install -r requirements.txt COPY review.py /review.py -COPY entrypoint.sh /entrypoint.sh -ENTRYPOINT ["/entrypoint.sh"] +ENTRYPOINT ["/review.py"] diff --git a/README.md b/README.md index d51a94b..8b24748 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,11 @@ start-up in order to build the Docker container. If you need to install some additional packages you can pass them via the `apt_packages` argument. +Except for very simple projects, a `compile_commands.json` file is necessary for +clang-tidy to find headers, set preprocessor macros, and so on. You can generate +one as part of this Action by setting `cmake_command` to something like `cmake +. -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=on`. + GitHub only mounts the `GITHUB_WORKSPACE` directory (that is, the default place where it clones your repository) on the container. If you install additional libraries/packages yourself, you'll need to @@ -71,12 +76,18 @@ at once, so `clang-tidy-review` will only attempt to post the first - default: '11' - `clang_tidy_checks`: List of checks - default: `'-*,performance-*,readability-*,bugprone-*,clang-analyzer-*,cppcoreguidelines-*,mpi-*,misc-*'` +- `config_file`: Path to clang-tidy config file. If set, the config file is used + instead of `clang_tidy_checks` + - default: '' - `include`: Comma-separated list of files or patterns to include - default: `"*.[ch],*.[ch]xx,*.[ch]pp,*.[ch]++,*.cc,*.hh"` - `exclude`: Comma-separated list of files or patterns to exclude - default: '' - `apt_packages`: Comma-separated list of apt packages to install - default: '' +- `cmake_command`: A CMake command to configure your project and generate + `compile_commands.json` in `build_dir` + - default: '' - `max_comments`: Maximum number of comments to post at once - default: '25' diff --git a/action.yml b/action.yml index b5ad342..8afde12 100644 --- a/action.yml +++ b/action.yml @@ -37,6 +37,10 @@ inputs: description: 'Comma-separated list of apt packages to install' required: false default: '' + cmake_command: + description: 'If set, run CMake as part of the action using this command' + required: false + default: '' max_comments: description: 'Maximum number of comments to post at once' required: false @@ -62,3 +66,4 @@ runs: - --include='${{ inputs.include }}' - --exclude='${{ inputs.exclude }}' - --apt-packages=${{ inputs.apt_packages }} + - --cmake-command='${{ inputs.cmake_command }}' diff --git a/review.py b/review.py index 782aa83..81978a7 100755 --- a/review.py +++ b/review.py @@ -6,6 +6,7 @@ # See LICENSE for more information import argparse +import contextlib import datetime import itertools import fnmatch @@ -24,6 +25,15 @@ DIFF_HEADER_LINE_LENGTH = 5 +@contextlib.contextmanager +def message_group(title: str): + print(f"::group::{title}", flush=True) + try: + yield + finally: + print("::endgroup::", flush=True) + + def make_file_line_lookup(diff): """Get a lookup table for each file in diff, to convert between source line number to line number in the diff @@ -157,13 +167,13 @@ def get_clang_tidy_warnings( print(f"Using config: {config}") command = f"{clang_tidy_binary} -p={build_dir} {config} -line-filter={line_filter} {files}" - print(f"Running:\n\t{command}") start = datetime.datetime.now() try: - output = subprocess.run( - command, capture_output=True, shell=True, check=True, encoding="utf-8" - ) + with message_group(f"Running:\n\t{command}"): + output = subprocess.run( + command, capture_output=True, shell=True, check=True, encoding="utf-8" + ) except subprocess.CalledProcessError as e: print( f"\n\nclang-tidy failed with return code {e.returncode} and error:\n{e.stderr}\nOutput was:\n{e.stdout}" @@ -301,6 +311,18 @@ def main( pull_request.create_review(**trimmed_review) +def strip_enclosing_quotes(string: str) -> str: + """Strip leading/trailing whitespace and remove any enclosing quotes""" + stripped = string.strip() + + # Need to check double quotes again in case they're nested inside + # single quotes + for quote in ['"', "'", '"']: + if stripped.startswith(quote) and stripped.endswith(quote): + stripped = stripped[1:-1] + return stripped + + if __name__ == "__main__": parser = argparse.ArgumentParser( description="Create a review from clang-tidy warnings" @@ -342,6 +364,12 @@ def main( type=str, default="", ) + parser.add_argument( + "--cmake-command", + help="If set, run CMake as part of the action with this command", + type=str, + default="", + ) parser.add_argument( "--max-comments", help="Maximum number of comments to post at once", @@ -356,22 +384,29 @@ def main( args = parser.parse_args() # Remove any enclosing quotes and extra whitespace - exclude = args.exclude.strip(""" "'""").split(",") - include = args.include.strip(""" "'""").split(",") + exclude = strip_enclosing_quotes(args.exclude).split(",") + include = strip_enclosing_quotes(args.include).split(",") if args.apt_packages: # Try to make sure only 'apt install' is run apt_packages = re.split(BAD_CHARS_APT_PACKAGES_PATTERN, args.apt_packages)[ 0 ].split(",") - print("Installing additional packages:", apt_packages) - subprocess.run( - ["apt", "install", "-y", "--no-install-recommends"] + apt_packages - ) + with message_group(f"Installing additional packages: {apt_packages}"): + subprocess.run( + ["apt", "install", "-y", "--no-install-recommends"] + apt_packages + ) build_compile_commands = f"{args.build_dir}/compile_commands.json" - if os.path.exists(build_compile_commands): + # If we run CMake as part of the action, then we know the paths in + # the compile_commands.json file are going to be correct + if args.cmake_command: + cmake_command = strip_enclosing_quotes(args.cmake_command) + with message_group(f"Running cmake: {cmake_command}"): + subprocess.run(cmake_command, shell=True, check=True) + + elif os.path.exists(build_compile_commands): print(f"Found '{build_compile_commands}', updating absolute paths") # We might need to change some absolute paths if we're inside # a docker container @@ -381,7 +416,7 @@ def main( original_directory = compile_commands[0]["directory"] # directory should either end with the build directory, - # unless it's '.', in which case use all of directory + # unless it's '.', in which case use original directory if original_directory.endswith(args.build_dir): build_dir_index = -(len(args.build_dir) + 1) basedir = original_directory[:build_dir_index]