Skip to content

Commit aab66d7

Browse files
AlexeySachkovvmaksimo
authored andcommitted
Move clang-format and cland-tidy to GitHub Actions (#770)
Switched to GitHub Actions for performing code style checks: new `check-code-style` workflow which is responsible for launching `clang-format` and `clang-tidy` on incoming PRs. Features: - workflow is skipped for docs-/test-only changes - as a result, patch with fixes for clang-format is uploaded to artifacts - workflow is optimized, i.e. it won't even install clang-format package if there are no changes to format (i.e. only workflow file was changed) Notable changes: - both tidy and format were enabled on `lib/Mangler/` and `lib/runtime/` subdirectories - tidy was enabled on the translator header files - both tidy and format were enabled on `OpenCL.std.h` and `spirv.hpp`, which are "external" files and it is not necessary that they conform with our coding standards. clang-format was disabled in those files in place, but clang-tidy will still be launched for them Also removed `utils/` folder: those scripts are not usable from local environment out-of-the box and new coding style check with GitHub Actions won't use them as well.
1 parent f7be667 commit aab66d7

File tree

7 files changed

+136
-316
lines changed

7 files changed

+136
-316
lines changed
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
# This workflow is intended to check if PR conforms with coding standards used
2+
# in the project.
3+
#
4+
# Documentation for GitHub Actions:
5+
# [workflow-syntax]: https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions
6+
# [context-and-expression-syntax]: https://docs.github.com/en/free-pro-team@latest/actions/reference/context-and-expression-syntax-for-github-actions
7+
# [workflow-commands]: https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions
8+
9+
name: Check code style
10+
11+
on:
12+
pull_request:
13+
branches:
14+
- master
15+
- llvm_release_*
16+
paths-ignore: # no need to check formatting for documentation and tests
17+
- 'docs/**'
18+
- 'test/**'
19+
20+
env:
21+
# We need compile command database in order to perform clang-tidy check. So,
22+
# in order to perform configure step we need to setup llvm-dev package. This
23+
# env variable used to specify desired version of it
24+
LLVM_VERSION: 12
25+
26+
jobs:
27+
clang-format-and-tidy:
28+
name: Run clang-format & clang-tidy
29+
runs-on: ubuntu-latest
30+
steps:
31+
- name: Checkout sources
32+
uses: actions/checkout@v2
33+
with:
34+
# In order to gather diff from PR we need to fetch not only the latest
35+
# commit. Depth of 2 is enough, because GitHub Actions supply us with
36+
# merge commit as {{ github.sha }}, i.e. the second commit is a merge
37+
# base between target branch and PR
38+
fetch-depth: 2
39+
40+
- name: Gather list of changes
41+
id: gather-list-of-changes
42+
run: |
43+
git diff -U0 --no-color ${{ github.sha }}^ > diff-to-inspect.txt
44+
if [ -s diff-to-inspect.txt ]; then
45+
# Here we set an output of our step, which is used later to either
46+
# perform or skip further steps, i.e. there is no sense to install
47+
# clang-format if PR hasn't changed .cpp files at all
48+
# See [workflow-commands] for reference
49+
echo '::set-output name=HAS_CHANGES::true'
50+
fi
51+
52+
- name: Install dependencies
53+
if: ${{ steps.gather-list-of-changes.outputs.HAS_CHANGES }}
54+
run: |
55+
# clang-tidy requires compile command database in order to be properly
56+
# launched, so, we need to setup llvm package to perform cmake
57+
# configuration step to generate that database
58+
curl -L "https://apt.llvm.org/llvm-snapshot.gpg.key" | sudo apt-key add -
59+
echo "deb https://apt.llvm.org/bionic/ llvm-toolchain-bionic main" | sudo tee -a /etc/apt/sources.list
60+
sudo apt-get update
61+
sudo apt-get install -yqq clang-format-9 clang-tidy-9 llvm-${{ env.LLVM_VERSION }}-dev
62+
63+
- name: Generate compile command database
64+
if: ${{ steps.gather-list-of-changes.outputs.HAS_CHANGES }}
65+
run: |
66+
mkdir build && cd build
67+
cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Release -G "Unix Makefiles" ${{ github.workspace }}
68+
69+
- name: Run clang-format
70+
if: ${{ steps.gather-list-of-changes.outputs.HAS_CHANGES }}
71+
id: run-clang-format
72+
run: |
73+
cat diff-to-inspect.txt | /usr/share/clang/clang-format-9/clang-format-diff.py -p1 > clang-format.patch
74+
if [ -s clang-format.patch ]; then
75+
echo "clang-format found incorrectly formatted code:"
76+
cat clang-format.patch;
77+
exit 1;
78+
else
79+
rm clang-format.patch # to avoid uploading empty file
80+
fi
81+
82+
- name: Run clang-tidy
83+
# By some reason, GitHub Actions automatically include "success()"
84+
# expression into an "if" statement if it doesn't contain any of job
85+
# status check functions. This is why this and following steps has
86+
# "always()" and "failure()" in "if" conditions.
87+
# See "Job status check functions" in [context-and-expression-syntax]
88+
if: ${{ always() && steps.gather-list-of-changes.outputs.HAS_CHANGES }}
89+
id: run-clang-tidy
90+
run: |
91+
cat diff-to-inspect.txt | /usr/lib/llvm-9/share/clang/clang-tidy-diff.py \
92+
-p1 -clang-tidy-binary clang-tidy-9 -quiet \
93+
-path ${{ github.workspace}}/build > clang-tidy.log 2>/dev/null
94+
# By some reason, clang-tidy log contains tons of extra empty lines,
95+
# that confuse the check below
96+
sed -i '/^$/d' clang-tidy.log
97+
if [ -s clang-tidy.log ]; then
98+
if ! grep -q "No relevant changes found." clang-tidy.log; then
99+
echo "clang-tidy found incorrectly written code:"
100+
cat clang-tidy.log
101+
exit 1
102+
else
103+
rm clang-tidy.log # to avoid uploading empty file
104+
fi
105+
fi
106+
107+
- name: Upload patch with clang-format fixes
108+
uses: actions/upload-artifact@v2
109+
if: ${{ failure() && steps.run-clang-format.outcome == 'failure' }}
110+
with:
111+
name: clang-format.patch
112+
path: clang-format.patch
113+
if-no-files-found: ignore
114+
115+
- name: Upload clang-tidy log
116+
uses: actions/upload-artifact@v2
117+
if: ${{ failure() && steps.run-clang-tidy.outcome == 'failure' }}
118+
with:
119+
name: clang-tidy.log
120+
path: clang-tidy.log
121+
if-no-files-found: ignore

llvm-spirv/.travis.yml

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ before_install:
3232
sudo apt-get -yq --no-install-suggests --no-install-recommends install \
3333
llvm-${LLVM_VERSION}-dev \
3434
clang-${LLVM_VERSION} \
35-
clang-format-${LLVM_VERSION} \
36-
clang-tidy-${LLVM_VERSION} \
3735
spirv-tools \
3836
cmake
3937
fi
@@ -44,8 +42,6 @@ compiler:
4442
env:
4543
global:
4644
- MAKEFLAGS="-j2"
47-
- CHECK_FORMAT=0
48-
- CHECK_TIDY=0
4945
- LLVM_VERSION=12
5046
matrix:
5147
- BUILD_TYPE=Release BUILD_EXTERNAL=1 SHARED_LIBS=ON MAKE_TARGETS="" MAKE_TEST_TARGET="test"
@@ -75,10 +71,6 @@ matrix:
7571

7672
- compiler: clang
7773
env: BUILD_TYPE=Release BUILD_EXTERNAL=0 SHARED_LIBS=ON MAKE_TARGETS="llvm-spirv" MAKE_TEST_TARGET="check-llvm-spirv"
78-
79-
- env: BUILD_EXTERNAL=1 CHECK_FORMAT=1
80-
81-
- env: BUILD_EXTERNAL=1 CHECK_TIDY=1
8274
allow_failures:
8375
- os: osx
8476
fast_finish: true
@@ -103,17 +95,15 @@ script:
10395
- PATH=/usr/bin:$PATH
10496
- |
10597
if [ $BUILD_EXTERNAL == "1" ]; then
106-
if [ $CHECK_FORMAT != "1" ]; then
107-
cmake .. \
108-
-DCMAKE_BUILD_TYPE=${BUILD_TYPE} \
109-
-DBUILD_SHARED_LIBS=${SHARED_LIBS} \
110-
-DLLVM_BUILD_TOOLS=ON \
111-
-DLLVM_EXTERNAL_LIT="/usr/lib/llvm-${LLVM_VERSION}/build/utils/lit/lit.py" \
112-
-DLLVM_INCLUDE_TESTS=ON \
113-
-DCMAKE_INSTALL_PREFIX=../install/ \
114-
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
115-
-G "Unix Makefiles"
116-
fi
98+
cmake .. \
99+
-DCMAKE_BUILD_TYPE=${BUILD_TYPE} \
100+
-DBUILD_SHARED_LIBS=${SHARED_LIBS} \
101+
-DLLVM_BUILD_TOOLS=ON \
102+
-DLLVM_EXTERNAL_LIT="/usr/lib/llvm-${LLVM_VERSION}/build/utils/lit/lit.py" \
103+
-DLLVM_INCLUDE_TESTS=ON \
104+
-DCMAKE_INSTALL_PREFIX=../install/ \
105+
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
106+
-G "Unix Makefiles"
117107
else
118108
BUILDDIR=$(pwd)
119109
cmake ../llvm-project/llvm/ \
@@ -131,17 +121,8 @@ script:
131121
-G "Unix Makefiles"
132122
ln -s /usr/lib/llvm-${LLVM_VERSION}/bin/clang bin/
133123
fi
134-
- if [ $CHECK_FORMAT == "1" ]; then
135-
cd ..;
136-
ln -s /usr/share/clang/clang-format-${LLVM_VERSION}/clang-format-diff.py utils/;
137-
./utils/check_code_format.sh;
138-
elif [ $CHECK_TIDY == "1" ]; then
139-
cd ..;
140-
ln -s build/compile_commands.json;
141-
./utils/check_code_tidyness.sh;
142-
else
143-
make $MAKE_TARGETS && make $MAKE_TEST_TARGET && if [ $BUILD_EXTERNAL == "1" ]; then make install; fi
144-
fi
124+
125+
- make $MAKE_TARGETS && make $MAKE_TEST_TARGET && if [ $BUILD_EXTERNAL == "1" ]; then make install; fi
145126

146127
before_deploy:
147128
# Travis CI relies on the tag name to push to the correct release.

llvm-spirv/lib/SPIRV/libSPIRV/OpenCL.std.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// clang-format off
12
/*
23
** Copyright (c) 2015 The Khronos Group Inc.
34
**
@@ -229,3 +230,4 @@ enum Entrypoints {
229230
};
230231

231232
} // end namespace OpenCL20
233+
// clang-format on

llvm-spirv/lib/SPIRV/libSPIRV/spirv.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// clang-format off
12
// Copyright (c) 2014-2019 The Khronos Group Inc.
23
//
34
// Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -2137,3 +2138,4 @@ inline KernelProfilingInfoMask operator|(KernelProfilingInfoMask a, KernelProfil
21372138
} // end namespace spv
21382139

21392140
#endif // #ifndef spirv_HPP
2141+
// clang-format on

llvm-spirv/utils/check_code_format.sh

Lines changed: 0 additions & 62 deletions
This file was deleted.

llvm-spirv/utils/check_code_tidyness.sh

Lines changed: 0 additions & 75 deletions
This file was deleted.

0 commit comments

Comments
 (0)