Skip to content

Commit

Permalink
Merge pull request #25 from ZedThree/cmake-command
Browse files Browse the repository at this point in the history
Add argument to pass in CMake command
  • Loading branch information
ZedThree authored Feb 2, 2022
2 parents 4544136 + 4fb70b8 commit 7c3cca0
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 14 deletions.
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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"]
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'

Expand Down
5 changes: 5 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -62,3 +66,4 @@ runs:
- --include='${{ inputs.include }}'
- --exclude='${{ inputs.exclude }}'
- --apt-packages=${{ inputs.apt_packages }}
- --cmake-command='${{ inputs.cmake_command }}'
59 changes: 47 additions & 12 deletions review.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
# See LICENSE for more information

import argparse
import contextlib
import datetime
import itertools
import fnmatch
Expand All @@ -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
Expand Down Expand Up @@ -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}"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand All @@ -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]
Expand Down

0 comments on commit 7c3cca0

Please sign in to comment.