Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang format proposal v4.test - new pull request #4

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Suricata settings as per
# https://redmine.openinfosecfoundation.org/projects/suricata/wiki/Coding_Style
#
# Some discussions to be had. See companion sample_formatted_code.do_not_merge.c
# for what this looks like.
---
BasedOnStyle: LLVM
AlignAfterOpenBracket: DontAlign
AlignConsecutiveMacros: true
AlignEscapedNewlines: Left
# clang 10: AllowShortBlocksOnASingleLine: Never
# clang 11: AllowShortEnumsOnASingleLine: false
AllowShortFunctionsOnASingleLine: None
# BreakBeforeBraces: Mozilla is closest, but does not split empty functions/structs
BraceWrapping:
AfterClass: true
AfterControlStatement: false
AfterEnum: false
AfterFunction: true
AfterStruct: false
AfterUnion: false
AfterExternBlock: true
BeforeElse: false
IndentBraces: false
SplitEmptyFunction: true
SplitEmptyRecord: true
BreakBeforeBraces: Custom
Cpp11BracedListStyle: true
ConstructorInitializerIndentWidth: 8
ContinuationIndentWidth: 8
ForEachMacros: ['json_array_foreach', 'json_object_foreach']
IndentCaseLabels: true
IndentWidth: 4
ReflowComments: true
SortIncludes: false

# implicit by LLVM style
#BreakBeforeTernaryOperators: true
#ColumnLimit: 80
#UseTab: Never
#TabWidth: 8

...
146 changes: 146 additions & 0 deletions .github/workflows/formatting.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
name: formatting-check

on:
push:
# Only run on pushes to pull request branches
branches-ignore:
- 'master'
- 'master-*'
pull_request:

jobs:

# Checking for correct formatting of branch for C code changes
check-formatting:
name: Formatting Check (clang 9)
runs-on: ubuntu-18.04
container: ubuntu:18.04
steps:

# Cache Rust stuff.
- name: Cache cargo registry
uses: actions/cache@v1
with:
path: ~/.cargo/registry
key: cargo-registry

- name: Install dependencies
run: |
apt update
apt -y install \
libpcre3 \
libpcre3-dev \
build-essential \
autoconf \
automake \
git \
libtool \
libpcap-dev \
libnet1-dev \
libyaml-0-2 \
libyaml-dev \
libcap-ng-dev \
libcap-ng0 \
libmagic-dev \
libnetfilter-queue-dev \
libnetfilter-queue1 \
libnfnetlink-dev \
libnfnetlink0 \
libhiredis-dev \
libjansson-dev \
libpython2.7 \
make \
python \
rustc \
software-properties-common \
wget \
zlib1g \
zlib1g-dev
- name: Install packages for clang-format 9
run: |
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
# no need to install full clang using https://apt.llvm.org/llvm.sh
# using clang 9
add-apt-repository "deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-9 main"
apt-get update
apt-get install -y clang-format-9
- name: Install cbindgen
run: cargo install --force --debug --version 0.14.1 cbindgen
- run: echo "::add-path::$HOME/.cargo/bin"
# Checking out the branch is not as simple as "checking out".
#
# In case master has any new commits since we branched off, github will
# automatically add a "merge commit" from our branch to master and check
# this out instead of the original last commit on the branch.
#
# This screws up git clang-format as it'll also format the "merge commit"
# and essentially report any formatting issues in the "merge commit".
# However, we really don't care about any of the new commits on master.
#
# There are supposed to be ways to use with/ref to fix that and while
# they work perfectly well for pull requests within the forked repo, none
# of the ones tried worked for pull requests from forks to the OISF repo.
#
# My patience simply ran too short to keep on looking. See follow-on
# action to manually fix this up.
- name: Checkout - might be merge commit!
uses: actions/checkout@v1
# Use last commit of branch, not potential merge commit!
#
# This works perfectly well on pull requests within forked repos, but
# not for pull requests from forks to the OISF repo as the latter one
# does not know the branch (from the forked repo). Argh.
# with:
# ref: ${{ github.head_ref }} # check out branch

# Manually ignore the merge commit as none of the with/ref things tried
# with actions/checkout seemed to work for pull requests from forks into
# the OISF repo.
- name: Peel off potential merge request
run: |
# The "merge commit" has a distinct subject that we can look for.
# If we find it, ignore it by checking out the "real last commit".
#
# Note, github uses the non-abbreviated sha for the commit subject.
#
# Commit history example in case github added merge, i.e. if you did
# git log --pretty=oneline -2:
# sha_1 Merge sha_2 into latest_sha_on_master
# sha_2 This is the real last commit on branch
echo "Last two commits on checkout:"
git log --pretty=oneline -2
last_commit_subject=$(git log --pretty=%s -1)
second_last_commit_sha=$(git log --pretty=%H -2 |tail -1)
echo "$last_commit_subject" | grep -e "^Merge $second_last_commit_sha into [0-9a-fA-F]*$" > /dev/null 2>&1
if [ $? -eq 0 ]; then
# Last commit was a merge to master - ignore
echo "Found github merge commit - checking out real last commit instead..."
git checkout $second_last_commit_sha
else
echo "No github merge commit found"
fi
shell: bash {0}
- run: git clone https://github.com/OISF/libhtp -b 0.5.x
- run: ./autogen.sh
- run: ./configure --enable-unittests
- name: Check formatting
run: |
# Do not use make as it hides the script's exit code
# make diffstat-style-branch >> check_formatting_log.txt 2>&1
./scripts/clang-format.sh check-branch --make --diffstat --show-commits >> check_formatting_log.txt 2>&1
rc=$?
if [ $rc -eq 0 ]; then
cat check_formatting_log.txt
echo "Formatting is following code style guide"
elif [ $rc -eq 1 ]; then
# limit output as it might be a lot in the worst case
tail -n 100 check_formatting_log.txt
echo "::warning ::Formatting is not following code style guide!"
else
cat check_formatting_log.txt
# use last output line as error
last_line=$(tail -n 1 check_formatting_log.txt)
echo "::error ::$last_line"
exit 1
fi
shell: bash {0}
34 changes: 34 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,37 @@ endif
@echo "For more information please see:"
@echo " https://suricata.readthedocs.io/en/latest/rule-management/index.html"
@echo ""

#--- clang-formatting ---
# Check if branch changes formatting is correct
.PHONY: check-style-branch
check-style-branch:
./scripts/clang-format.sh check-branch --make

# Check if branch changes formatting is correct and print diff
.PHONY: diff-style-branch
diff-style-branch:
./scripts/clang-format.sh check-branch --make --diff

# Check if branch changes formatting is correct and print diffstat, i.e. files
.PHONY: diffstat-style-branch
diffstat-style-branch:
./scripts/clang-format.sh check-branch --make --diffstat --show-commits

# Format changes in git staging
.PHONY: style
style:
./scripts/clang-format.sh cached

# Reformat all changes in branch. Allows you to add the formatting as a separate
# commit "at the end".
.PHONY: style-branch
style-branch:
./scripts/clang-format.sh branch

# Reformat all commits in branch off master and rewrite history using the
# existing commit metadata.
.PHONY: style-rewrite-branch
style-rewrite-branch:
./scripts/clang-format.sh rewrite-branch

135 changes: 135 additions & 0 deletions clang-format.full.do_not_merge
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# To use minimalistic .clang-format with BasedOnStyle: or the full shebang like
# this?
#
# This probably needs some more pruning if we wanted the full thing, such as
# cleaning up IncludeCategories (that is not used as SortIncludes:false).
#
# Reproduce from minimalistic .clang-format with:
# clang-format --dump-config --style=file >clang-format.full.do_not_merge
#
---
Language: Cpp
AccessModifierOffset: -2
AlignAfterOpenBracket: DontAlign
AlignConsecutiveMacros: true
AlignConsecutiveAssignments: false
AlignConsecutiveDeclarations: false
AlignEscapedNewlines: Left
AlignOperands: true
AlignTrailingComments: true
AllowAllArgumentsOnNextLine: true
AllowAllConstructorInitializersOnNextLine: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: false
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: None
AllowShortLambdasOnASingleLine: All
AllowShortIfStatementsOnASingleLine: Never
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: MultiLine
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
AfterCaseLabel: false
AfterClass: true
AfterControlStatement: false
AfterEnum: false
AfterFunction: true
AfterNamespace: false
AfterObjCDeclaration: false
AfterStruct: false
AfterUnion: false
AfterExternBlock: true
BeforeCatch: false
BeforeElse: false
IndentBraces: false
SplitEmptyFunction: true
SplitEmptyRecord: true
SplitEmptyNamespace: true
BreakBeforeBinaryOperators: None
BreakBeforeBraces: Custom
BreakBeforeInheritanceComma: false
BreakInheritanceList: BeforeColon
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: false
BreakConstructorInitializers: BeforeColon
BreakAfterJavaFieldAnnotations: false
BreakStringLiterals: true
ColumnLimit: 80
CommentPragmas: '^ IWYU pragma:'
CompactNamespaces: false
ConstructorInitializerAllOnOneLineOrOnePerLine: false
ConstructorInitializerIndentWidth: 8
ContinuationIndentWidth: 8
Cpp11BracedListStyle: true
DerivePointerAlignment: false
DisableFormat: false
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: true
ForEachMacros:
- json_array_foreach
- json_object_foreach
- BOOST_FOREACH
IncludeBlocks: Preserve
IncludeCategories:
- Regex: '^"(llvm|llvm-c|clang|clang-c)/'
Priority: 2
- Regex: '^(<|"(gtest|gmock|isl|json)/)'
Priority: 3
- Regex: '.*'
Priority: 1
IncludeIsMainRegex: '(Test)?$'
IndentCaseLabels: true
IndentPPDirectives: None
IndentWidth: 4
IndentWrappedFunctionNames: false
JavaScriptQuotes: Leave
JavaScriptWrapImports: true
KeepEmptyLinesAtTheStartOfBlocks: true
MacroBlockBegin: ''
MacroBlockEnd: ''
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBinPackProtocolList: Auto
ObjCBlockIndentWidth: 2
ObjCSpaceAfterProperty: false
ObjCSpaceBeforeProtocolList: true
PenaltyBreakAssignment: 2
PenaltyBreakBeforeFirstCallParameter: 19
PenaltyBreakComment: 300
PenaltyBreakFirstLessLess: 120
PenaltyBreakString: 1000
PenaltyBreakTemplateDeclaration: 10
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 60
PointerAlignment: Right
ReflowComments: true
SortIncludes: false
SortUsingDeclarations: true
SpaceAfterCStyleCast: false
SpaceAfterLogicalNot: false
SpaceAfterTemplateKeyword: true
SpaceBeforeAssignmentOperators: true
SpaceBeforeCpp11BracedList: false
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: true
SpaceBeforeParens: ControlStatements
SpaceBeforeRangeBasedForLoopColon: true
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles: false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: Cpp11
StatementMacros:
- Q_UNUSED
- QT_REQUIRE_VERSION
TabWidth: 8
UseTab: Never
...

Loading