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

Format native code using clang-format #2101

Merged
merged 16 commits into from
Feb 2, 2023
82 changes: 80 additions & 2 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -1,3 +1,81 @@
---
# We'll use defaults from the LLVM style, but with 4 columns indentation.
BasedOnStyle: Google
# copied from: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/.clang-format
Language: Cpp
AccessModifierOffset: -4
AlignAfterOpenBracket: Align
AlignConsecutiveAssignments: true
AlignConsecutiveDeclarations: true
AlignEscapedNewlinesLeft: false
AlignOperands: true
AlignTrailingComments: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: false
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: Empty
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: true
BinPackArguments: true
BinPackParameters: false
BraceWrapping:
AfterClass: true
AfterControlStatement: true
AfterEnum: false
AfterFunction: true
AfterNamespace: false
AfterObjCDeclaration: false
AfterStruct: true
AfterUnion: true
BeforeCatch: true
BeforeElse: true
IndentBraces: false
BreakBeforeBinaryOperators: None
BreakBeforeBraces: Allman
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: true
ColumnLimit: 120
CommentPragmas: '^ IWYU pragma:'
ConstructorInitializerAllOnOneLineOrOnePerLine: true
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 4
Cpp11BracedListStyle: true
DerivePointerAlignment: false
DisableFormat: false
ExperimentalAutoDetectBinPacking: false
ForEachMacros: [ ]
IndentCaseLabels: true
IndentWidth: 4
IndentWrappedFunctionNames: false
KeepEmptyLinesAtTheStartOfBlocks: true
MacroBlockBegin: ''
MacroBlockEnd: ''
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBlockIndentWidth: 2
ObjCSpaceAfterProperty: false
ObjCSpaceBeforeProtocolList: true
PenaltyBreakBeforeFirstCallParameter: 400
PenaltyBreakComment: 50
PenaltyBreakFirstLessLess: 500
PenaltyBreakString: 1000
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 100000
PointerAlignment: Left
ReflowComments: true
SortIncludes: false
SpaceAfterCStyleCast: false
SpaceBeforeAssignmentOperators: true
SpaceBeforeParens: ControlStatements
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles: false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: Cpp11
TabWidth: 4
UseTab: Never
...
1 change: 1 addition & 0 deletions .github/workflows/dotnet-format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ on:
paths:
- '**.cs'
- '.editorconfig'
workflow_dispatch:

jobs:
check-format:
Expand Down
26 changes: 26 additions & 0 deletions .github/workflows/format-native.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: format native

on:
push:
branches: [ main ]
pull_request:
workflow_dispatch:

jobs:
check-native-format:
strategy:
fail-fast: false
matrix:
machine: [ windows-2022, ubuntu-20.04, macos-11 ]
runs-on: ${{ matrix.machine }}

steps:
- uses: actions/checkout@v3.3.0

- name: Install Clang tools
shell: bash
run: ./scripts/download-clang-tools.sh

- name: Format native code
shell: bash
run: ./scripts/format-native.sh
35 changes: 35 additions & 0 deletions docs/developing.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,41 @@ nuke MarkdownLintFix

All MarkdownLint tasks require [Node.js](https://nodejs.org/) installed locally.

### Managed code formatting

The .NET code formatting is based on the
[OpenTelemetry .NET repository](https://github.com/open-telemetry/opentelemetry-dotnet).

Installing formatter:

```sh
dotnet tool install -g dotnet-format
```

Formatting (Bash):

```sh
dotnet-format --folder
```

### Native code formatting

The C++ code formatting is based on the
[.NET Runtime repository](https://github.com/dotnet/runtime)
and [.NET JIT utils repository](https://github.com/dotnet/jitutils).

Installing formatter (Bash):

```sh
./scripts/download-clang-tools.sh
```

Formatting (Bash):

```sh
./scripts/format-native.sh
```

## Manual testing

### Test environment
Expand Down
58 changes: 58 additions & 0 deletions scripts/download-clang-tools.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
#!/bin/bash

set -eo pipefail

SCRIPT_DIR=$(cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd)

# guess OS_TYPE if not provided
if [ -z "$OS_TYPE" ]; then
case "$(uname -s | tr '[:upper:]' '[:lower:]')" in
cygwin_nt*|mingw*|msys_nt*)
OS_TYPE="windows"
;;
linux*)
if [ "$(ldd /bin/ls | grep -m1 'musl')" ]; then
OS_TYPE="linux-musl"
else
OS_TYPE="linux-glibc"
fi
;;
darwin*)
OS_TYPE="macos"
;;
esac
fi

function DownloadClangTool {
if [ "$OS_TYPE" == "windows" ]; then
FILENAME=$1.exe
else
FILENAME=$1
fi

case "$OS_TYPE" in
"linux-glibc")
TOOLS_URL=https://clrjit.blob.core.windows.net/clang-tools/ubuntu.18.04-x64/$FILENAME
;;
"macos")
TOOLS_URL=https://clrjit.blob.core.windows.net/clang-tools/osx.10.15-x64/$FILENAME
;;
"windows")
TOOLS_URL=https://clrjit.blob.core.windows.net/clang-tools/windows/$FILENAME
;;
*)
echo "Set the operating system type using the OS_TYPE environment variable. Supported values: linux-glibc, macos, windows." >&2
exit 1
;;
esac

cd $SCRIPT_DIR/..
mkdir -p bin/artifacts
cd bin/artifacts

curl --retry 5 -o "${FILENAME}" "$TOOLS_URL"
chmod +x $FILENAME
}

DownloadClangTool "clang-format"
# DownloadClangTool "clang-tidy" # unused
42 changes: 42 additions & 0 deletions scripts/format-native.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/bin/sh

SCRIPT_DIR=$(cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd)
cd $SCRIPT_DIR/..

LC_ALL=C

# guess OS_TYPE if not provided
if [ -z "$OS_TYPE" ]; then
case "$(uname -s | tr '[:upper:]' '[:lower:]')" in
cygwin_nt*|mingw*|msys_nt*)
OS_TYPE="windows"
;;
linux*)
if [ "$(ldd /bin/ls | grep -m1 'musl')" ]; then
OS_TYPE="linux-musl"
else
OS_TYPE="linux-glibc"
fi
;;
darwin*)
OS_TYPE="macos"
;;
esac
fi

if [ "$OS_TYPE" == "windows" ]; then
EXT=.exe
else
EXT=
fi

# files to format
NATIVE_FILES=$(find . -iname *.cpp -o -iname *.hpp -iname *.h -iname *.inc | grep -v ./packages | grep -v ./src/OpenTelemetry.AutoInstrumentation.Native/lib)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is the main reason why it is not yet implemented in Nuke. I think we can create an issue to move the formatting logic to Nuke targets. Let me know what you think if we should address it ASAP or can we postpone it.


# clang-format
echo "$NATIVE_FILES" | xargs "./bin/artifacts/clang-format$EXT" -style=file -i

## check if anything changed
git status
git diff
test -z "$(git status --porcelain)"
Loading