Skip to content

Commit 4a760fa

Browse files
aperezdcaperezdc
committed
CI: Add a job that runs clang-format on PR changes
Introduce a script to check the coding style, and a CI worflow which runs it only on the changes introduced by a PR. The goal of checking only the changed code is to avoid having to run a full reformatting of the tree: instead, changes that modified existing code will slowly make the tree drift towards the desired status without introducing noise in the repository history. There are is some trickery involved in getting this to work reliably: - The "clang-format-diff" helper script is installed at different locations depending on the distribution. The "check-style" script checks a few locations that cover popular GNU/Linux distributions (Ubuntu, Debian, Arch Linux, Fedora); it can be improved as we see fit in the future. - The "actions/checkout" step defaults to shallow clones of the single commit which triggered a PR, which is enough to build, but not to find the differences between it and the base commit. To avoid forcing a full fetch of the history, the job uses a second instance of the action to fetch only the base tree in a separate directory. - Even with both trees checked out, Git will refuse to calculate a diff because the commits are not linked to one another. But both trees are now checked out, so instead it is possible to use a plain "diff" and feed its output to "check-style --diff". As a bonus, the "check-style" script can be run by developers locally from the command line in order to verify their changes prior to filing merge requests, e.g: % git co -b my-feature % $EDITOR cog.c % git ci -m'Awesome improvements' % scripts/check-style master The output from the script is an unified diff, which can be piped back into "patch" or "git apply" to get the suggested changes applied to the tree: % scripts/check-style master | git apply -p0 - % git add -p && git commit --amend
1 parent 064bd78 commit 4a760fa

File tree

3 files changed

+310
-0
lines changed

3 files changed

+310
-0
lines changed

‎.clang-format

+148
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
---
2+
Language: Cpp
3+
BasedOnStyle: LLVM
4+
AccessModifierOffset: -2
5+
AlignAfterOpenBracket: Align
6+
AlignConsecutiveMacros: true
7+
AlignConsecutiveAssignments: false
8+
AlignConsecutiveBitFields: true
9+
AlignConsecutiveDeclarations: true
10+
AlignEscapedNewlines: Left
11+
AlignTrailingComments: true
12+
AllowAllArgumentsOnNextLine: false
13+
AllowAllConstructorInitializersOnNextLine: false
14+
AllowAllParametersOfDeclarationOnNextLine: false
15+
AllowShortEnumsOnASingleLine: true
16+
AllowShortBlocksOnASingleLine: Never
17+
AllowShortCaseLabelsOnASingleLine: false
18+
AllowShortFunctionsOnASingleLine: All
19+
AllowShortLambdasOnASingleLine: All
20+
AllowShortIfStatementsOnASingleLine: Never
21+
AllowShortLoopsOnASingleLine: false
22+
AlwaysBreakAfterDefinitionReturnType: None
23+
AlwaysBreakAfterReturnType: TopLevelDefinitions
24+
AlwaysBreakBeforeMultilineStrings: false
25+
AlwaysBreakTemplateDeclarations: MultiLine
26+
BinPackArguments: true
27+
BinPackParameters: false
28+
BraceWrapping:
29+
AfterCaseLabel: false
30+
AfterClass: false
31+
AfterControlStatement: Never
32+
AfterEnum: false
33+
AfterFunction: false
34+
AfterNamespace: false
35+
AfterObjCDeclaration: false
36+
AfterStruct: false
37+
AfterUnion: false
38+
AfterExternBlock: false
39+
BeforeCatch: false
40+
BeforeElse: false
41+
BeforeLambdaBody: false
42+
BeforeWhile: false
43+
IndentBraces: false
44+
SplitEmptyFunction: true
45+
SplitEmptyRecord: true
46+
SplitEmptyNamespace: true
47+
BreakBeforeBinaryOperators: None
48+
BreakBeforeBraces: WebKit
49+
BreakBeforeInheritanceComma: false
50+
BreakInheritanceList: BeforeColon
51+
BreakBeforeTernaryOperators: true
52+
BreakConstructorInitializersBeforeComma: false
53+
BreakConstructorInitializers: BeforeColon
54+
BreakAfterJavaFieldAnnotations: false
55+
BreakStringLiterals: true
56+
ColumnLimit: 120
57+
CommentPragmas: '^ IWYU pragma:'
58+
CompactNamespaces: false
59+
ConstructorInitializerAllOnOneLineOrOnePerLine: false
60+
ConstructorInitializerIndentWidth: 4
61+
ContinuationIndentWidth: 4
62+
Cpp11BracedListStyle: true
63+
DeriveLineEnding: true
64+
DerivePointerAlignment: false
65+
DisableFormat: false
66+
ExperimentalAutoDetectBinPacking: true
67+
FixNamespaceComments: true
68+
ForEachMacros:
69+
- wl_list_for_each
70+
- wl_list_for_each_safe
71+
IncludeBlocks: Preserve
72+
IncludeCategories:
73+
- Regex: '^"(llvm|llvm-c|clang|clang-c)/'
74+
Priority: 2
75+
SortPriority: 0
76+
- Regex: '^(<|"(gtest|gmock|isl|json)/)'
77+
Priority: 3
78+
SortPriority: 0
79+
- Regex: '.*'
80+
Priority: 1
81+
SortPriority: 0
82+
IncludeIsMainRegex: '(Test)?$'
83+
IncludeIsMainSourceRegex: ''
84+
IndentCaseLabels: false
85+
IndentCaseBlocks: false
86+
IndentGotoLabels: true
87+
IndentPPDirectives: None
88+
IndentExternBlock: AfterExternBlock
89+
IndentWidth: 4
90+
IndentWrappedFunctionNames: false
91+
InsertTrailingCommas: None
92+
JavaScriptQuotes: Double
93+
JavaScriptWrapImports: true
94+
KeepEmptyLinesAtTheStartOfBlocks: true
95+
MacroBlockBegin: ''
96+
MacroBlockEnd: ''
97+
MaxEmptyLinesToKeep: 1
98+
NamespaceIndentation: None
99+
ObjCBinPackProtocolList: Auto
100+
ObjCBlockIndentWidth: 2
101+
ObjCBreakBeforeNestedBlockParam: true
102+
ObjCSpaceAfterProperty: false
103+
ObjCSpaceBeforeProtocolList: true
104+
PenaltyBreakAssignment: 2
105+
PenaltyBreakBeforeFirstCallParameter: 19
106+
PenaltyBreakComment: 300
107+
PenaltyBreakFirstLessLess: 120
108+
PenaltyBreakString: 1000
109+
PenaltyBreakTemplateDeclaration: 10
110+
PenaltyExcessCharacter: 1000000
111+
PenaltyReturnTypeOnItsOwnLine: 60
112+
PointerAlignment: Left
113+
ReflowComments: false
114+
SortIncludes: true
115+
SortUsingDeclarations: true
116+
SpaceAfterCStyleCast: true
117+
SpaceAfterLogicalNot: false
118+
SpaceAfterTemplateKeyword: true
119+
SpaceBeforeAssignmentOperators: true
120+
SpaceBeforeCpp11BracedList: false
121+
SpaceBeforeCtorInitializerColon: true
122+
SpaceBeforeInheritanceColon: true
123+
SpaceBeforeParens: ControlStatementsExceptForEachMacros
124+
SpaceBeforeRangeBasedForLoopColon: true
125+
SpaceInEmptyBlock: false
126+
SpaceInEmptyParentheses: false
127+
SpacesBeforeTrailingComments: 1
128+
SpacesInAngles: false
129+
SpacesInConditionalStatement: false
130+
SpacesInContainerLiterals: true
131+
SpacesInCStyleCastParentheses: false
132+
SpacesInParentheses: false
133+
SpacesInSquareBrackets: false
134+
SpaceBeforeSquareBrackets: false
135+
Standard: Latest
136+
StatementMacros:
137+
- Q_UNUSED
138+
- QT_REQUIRE_VERSION
139+
TabWidth: 4
140+
UseCRLF: false
141+
UseTab: Never
142+
WhitespaceSensitiveMacros:
143+
- STRINGIZE
144+
- PP_STRINGIZE
145+
TypenameMacros:
146+
- g_auto
147+
- g_autoptr
148+
...

‎.github/workflows/codestyle.yml

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
name: Code Style
3+
on: [pull_request]
4+
5+
jobs:
6+
check:
7+
runs-on: ubuntu-20.04
8+
steps:
9+
- name: Checkout Head
10+
uses: actions/checkout@v2
11+
- name: Fetch Git History
12+
run: |
13+
git fetch --no-tags --prune --depth=1 \
14+
origin +refs/heads/master:refs/remotes/origin/master
15+
- name: Install Tools
16+
run: |
17+
curl -sL https://apt.llvm.org/llvm-snapshot.gpg.key | \
18+
sudo apt-key add -
19+
sudo add-apt-repository \
20+
'deb http://apt.llvm.org/focal/ llvm-toolchain-focal-13 main'
21+
sudo apt update
22+
sudo apt install -y clang-format-13 diffutils
23+
- name: Check
24+
run: |
25+
scripts/check-style -ocode-style.diff origin/master
26+
- name: Archive Diff
27+
uses: actions/upload-artifact@v2
28+
if: failure()
29+
with:
30+
name: diff
31+
path: code-style.diff

‎scripts/check-style

+131
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
#! /usr/bin/env bash
2+
set -eu -o pipefail
3+
4+
FILE=
5+
BASE=
6+
HEAD=HEAD
7+
8+
declare -a args=()
9+
diff_input=false
10+
show_help=false
11+
12+
for opt in "$@" ; do
13+
case "${opt}" in
14+
-h | --help)
15+
show_help=true
16+
;;
17+
-d | --diff)
18+
diff_input=true
19+
;;
20+
-o*)
21+
FILE=${opt#-o}
22+
;;
23+
*)
24+
args+=("${opt}")
25+
;;
26+
esac
27+
done
28+
declare -r diff_input show_help
29+
30+
show_help ()
31+
{
32+
echo "Usage: $0 [-o<file>] [-d|--diff] [<committish> [<committish>]]"
33+
}
34+
35+
if ${show_help} ; then
36+
show_help
37+
exit
38+
fi
39+
40+
if [[ ${#args[@]} -eq 1 ]] ; then
41+
BASE=${args[0]}
42+
elif [[ ${#args[@]} -eq 2 ]] ; then
43+
BASE=${args[0]}
44+
HEAD=${args[1]}
45+
elif [[ ${#args[@]} -eq 0 ]] && ${diff_input} ; then
46+
: no-op
47+
else
48+
show_help 1>&2
49+
exit 1
50+
fi
51+
declare -r BASE HEAD
52+
53+
declare -ra clang_format_diff_candidates=(
54+
# Search in $PATH (Ubuntu, Debian, maybe others)
55+
clang-format-diff-13
56+
clang-format-diff-13.py
57+
clang-format-diff
58+
clang-format-diff.py
59+
# Arch Linux, Fedora.
60+
/usr/share/clang/clang-format-diff.py
61+
)
62+
63+
find_program ()
64+
{
65+
local candidate path
66+
for candidate in "$@" ; do
67+
path=$(type -P "${candidate}")
68+
if [[ -x ${path} ]] ; then
69+
echo "${path}"
70+
return
71+
fi
72+
done
73+
74+
# Not found.
75+
return 1
76+
}
77+
78+
clang_format_diff=$(find_program "${clang_format_diff_candidates[@]}")
79+
clang_format=$(find_program clang-format-11 clang-format)
80+
declare -r clang_format_diff clang_format
81+
82+
if [[ ! -x ${clang_format_diff} ]] ; then
83+
echo "$0: clang-format-diff does not seem to be installed" 1>&2
84+
exit 1
85+
fi
86+
87+
if [[ ! -x ${clang_format} ]] ; then
88+
echo "$0: clang-format does not seem to be installed" 1>&2
89+
exit 1
90+
fi
91+
92+
#
93+
# Check the version of clang-format, to make sure that it will support the
94+
# configuration file. The output of "clang-format --version" is formatted
95+
# as follows:
96+
#
97+
# clang-format version X.Y.Z
98+
#
99+
declare -r clang_format_min_version=11
100+
101+
clang_format_version=$("${clang_format}" --version)
102+
if [[ ${clang_format_version} =~ clang-format[[:space:]]+version[[:space:]]+([[:digit:]]+)\. ]] ; then
103+
clang_format_version=${BASH_REMATCH[1]}
104+
if [[ ${clang_format_version} -lt ${clang_format_min_version} ]] ; then
105+
echo "$0: clang-format ${clang_format_min_version}+ is needed, ${clang_format_version} detected" 1>&2
106+
exit 1
107+
fi
108+
else
109+
echo "$0: clang-format did not report its version number"
110+
exit 1
111+
fi
112+
113+
if [[ -z ${FILE} ]] ; then
114+
FILE=$(mktemp)
115+
trap 'rm -f "${FILE}"' EXIT
116+
else
117+
trap '[[ -s ${FILE} ]] || rm -f "${FILE}"' EXIT
118+
fi
119+
120+
declare -a input_cmd
121+
if ${diff_input} ; then
122+
input_cmd=(cat)
123+
else
124+
input_cmd=(git diff --no-color -U0 "${BASE}" "${HEAD}")
125+
fi
126+
127+
"${input_cmd[@]}" \
128+
| "${clang_format_diff}" -p1 -style=file -regex='.+\.(c|h)' \
129+
| tee "${FILE}"
130+
131+
[[ ! -s ${FILE} ]]

0 commit comments

Comments
 (0)