Skip to content

checkpatch: remove and use clang-format instead #10619

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.

Sign up for GitHub

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

Open
lgirdwood wants to merge 1 commit into
base: main
Choose a base branch
from
+365 −14,557
Open

checkpatch: remove and use clang-format instead #10619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
181 changes: 181 additions & 0 deletions .clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
# SPDX-License-Identifier: GPL-2.0
#
# clang-format configuration file. Intended for clang-format >= 11.
#
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This clang-format configuration appears to be duplicated verbatim in two places (.clang-format and tools/rimage/.clang-format). Keeping identical configs in multiple locations is likely to diverge over time and create inconsistent formatting depending on where clang-format is invoked from. Consider consolidating to a single source of truth (preferably the repo root), or add an explicit comment/guardrail explaining why tools/rimage/ needs its own copy and how it should be kept in sync.

Suggested change
#
#
# NOTE: This file is intentionally duplicated in tools/rimage/.clang-format.
# Both copies must remain identical. If you change formatting options here,
# make the same change in tools/rimage/.clang-format to keep them in sync.
#

Copilot uses AI. Check for mistakes.
# For more information, see:
#
# Documentation/dev-tools/clang-format.rst
# https://clang.llvm.org/docs/ClangFormat.html
# https://clang.llvm.org/docs/ClangFormatStyleOptions.html
#
Copy link
Collaborator

@kv2019i kv2019i Mar 13, 2026

Choose a reason for hiding this comment

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

@lgirdwood Should we have a link to the Zephyr version as this is including many definitions from it (and we should probably take in updates from Zephyr if they change converntions)?

---
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This clang-format configuration appears to be duplicated verbatim in two places (.clang-format and tools/rimage/.clang-format). Keeping identical configs in multiple locations is likely to diverge over time and create inconsistent formatting depending on where clang-format is invoked from. Consider consolidating to a single source of truth (preferably the repo root), or add an explicit comment/guardrail explaining why tools/rimage/ needs its own copy and how it should be kept in sync.

Copilot uses AI. Check for mistakes.
AccessModifierOffset: -4
AlignAfterOpenBracket: Align
AlignConsecutiveAssignments: false
AlignConsecutiveDeclarations: false
AlignEscapedNewlines: Left
AlignOperands: true
AlignTrailingComments: false
AllowAllParametersOfDeclarationOnNextLine: false
AllowShortBlocksOnASingleLine: false
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: None
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: false
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
AfterClass: false
AfterControlStatement: false
AfterEnum: false
AfterFunction: true
AfterNamespace: true
AfterObjCDeclaration: false
AfterStruct: false
AfterUnion: false
AfterExternBlock: false
BeforeCatch: false
BeforeElse: false
IndentBraces: false
SplitEmptyFunction: true
SplitEmptyRecord: true
SplitEmptyNamespace: true
BreakBeforeBinaryOperators: None
BreakBeforeBraces: Custom
BreakBeforeInheritanceComma: false
BreakBeforeTernaryOperators: false
BreakConstructorInitializersBeforeComma: false
BreakConstructorInitializers: BeforeComma
BreakAfterJavaFieldAnnotations: false
BreakStringLiterals: false
ColumnLimit: 120
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This clang-format configuration appears to be duplicated verbatim in two places (.clang-format and tools/rimage/.clang-format). Keeping identical configs in multiple locations is likely to diverge over time and create inconsistent formatting depending on where clang-format is invoked from. Consider consolidating to a single source of truth (preferably the repo root), or add an explicit comment/guardrail explaining why tools/rimage/ needs its own copy and how it should be kept in sync.

Copilot uses AI. Check for mistakes.
CommentPragmas: '^ IWYU pragma:'
CompactNamespaces: false
ConstructorInitializerAllOnOneLineOrOnePerLine: false
ConstructorInitializerIndentWidth: 8
ContinuationIndentWidth: 8
Cpp11BracedListStyle: false
DerivePointerAlignment: false
DisableFormat: false
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: false

# Taken from:
# git grep -h '^#define [^[:space:]]*for_each[^[:space:]]*(' include/ tools/ \
# | sed "s,^#define \([^[:space:]]*for_each[^[:space:]]*\)(.*$, - '\1'," \
# | LC_ALL=C sort -u
ForEachMacros:
- 'ARRAY_FOR_EACH'
- 'ARRAY_FOR_EACH_PTR'
- 'FOR_EACH'
- 'FOR_EACH_FIXED_ARG'
- 'FOR_EACH_IDX'
- 'FOR_EACH_IDX_FIXED_ARG'
- 'FOR_EACH_NONEMPTY_TERM'
- 'FOR_EACH_FIXED_ARG_NONEMPTY_TERM'
- 'RB_FOR_EACH'
- 'RB_FOR_EACH_CONTAINER'
- 'SYS_DLIST_FOR_EACH_CONTAINER'
- 'SYS_DLIST_FOR_EACH_CONTAINER_SAFE'
- 'SYS_DLIST_FOR_EACH_NODE'
- 'SYS_DLIST_FOR_EACH_NODE_SAFE'
- 'SYS_SEM_LOCK'
- 'SYS_SFLIST_FOR_EACH_CONTAINER'
- 'SYS_SFLIST_FOR_EACH_CONTAINER_SAFE'
- 'SYS_SFLIST_FOR_EACH_NODE'
- 'SYS_SFLIST_FOR_EACH_NODE_SAFE'
- 'SYS_SLIST_FOR_EACH_CONTAINER'
- 'SYS_SLIST_FOR_EACH_CONTAINER_SAFE'
- 'SYS_SLIST_FOR_EACH_NODE'
- 'SYS_SLIST_FOR_EACH_NODE_SAFE'
- '_WAIT_Q_FOR_EACH'
- '_WAIT_Q_FOR_EACH_SAFE'
- 'Z_FOR_EACH'
- 'Z_FOR_EACH_ENGINE'
- 'Z_FOR_EACH_EXEC'
- 'Z_FOR_EACH_FIXED_ARG'
- 'Z_FOR_EACH_FIXED_ARG_EXEC'
- 'Z_FOR_EACH_IDX'
- 'Z_FOR_EACH_IDX_EXEC'
- 'Z_FOR_EACH_IDX_FIXED_ARG'
- 'Z_FOR_EACH_IDX_FIXED_ARG_EXEC'
- 'Z_GENLIST_FOR_EACH_CONTAINER'
- 'Z_GENLIST_FOR_EACH_CONTAINER_SAFE'
- 'Z_GENLIST_FOR_EACH_NODE'
- 'Z_GENLIST_FOR_EACH_NODE_SAFE'
- 'STRUCT_SECTION_FOREACH'
- 'STRUCT_SECTION_FOREACH_ALTERNATE'
- 'TYPE_SECTION_FOREACH'
- 'K_SPINLOCK'
- 'COAP_RESOURCE_FOREACH'
- 'COAP_SERVICE_FOREACH'
- 'COAP_SERVICE_FOREACH_RESOURCE'
- 'HTTP_RESOURCE_FOREACH'
- 'HTTP_SERVER_CONTENT_TYPE_FOREACH'
- 'HTTP_SERVICE_FOREACH'
- 'HTTP_SERVICE_FOREACH_RESOURCE'
- 'I3C_BUS_FOR_EACH_I3CDEV'
- 'I3C_BUS_FOR_EACH_I2CDEV'
- 'MIN_HEAP_FOREACH'
- 'comp_dev_for_each_consumer'
- 'comp_dev_for_each_consumer_safe'
- 'comp_dev_for_each_producer'
- 'comp_dev_for_each_producer_safe'

IncludeBlocks: Preserve
IncludeCategories:
- Regex: '.*'
Priority: 1
IncludeIsMainRegex: '(Test)?$'
IndentCaseLabels: false
IndentGotoLabels: false
IndentPPDirectives: None
IndentWidth: 4
IndentWrappedFunctionNames: false
JavaScriptQuotes: Leave
JavaScriptWrapImports: true
KeepEmptyLinesAtTheStartOfBlocks: false
MacroBlockBegin: ''
MacroBlockEnd: ''
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBinPackProtocolList: Auto
ObjCBlockIndentWidth: 8
ObjCSpaceAfterProperty: true
ObjCSpaceBeforeProtocolList: true

# Taken from git's rules
PenaltyBreakAssignment: 10
PenaltyBreakBeforeFirstCallParameter: 30
PenaltyBreakComment: 10
PenaltyBreakFirstLessLess: 0
PenaltyBreakString: 10
PenaltyExcessCharacter: 100
PenaltyReturnTypeOnItsOwnLine: 60

PointerAlignment: Right
ReflowComments: false
SortIncludes: false
SortUsingDeclarations: false
SpaceAfterCStyleCast: false
SpaceAfterTemplateKeyword: true
SpaceBeforeAssignmentOperators: true
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: true
SpaceBeforeParens: ControlStatementsExceptForEachMacros
SpaceBeforeRangeBasedForLoopColon: true
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles: false
SpacesInContainerLiterals: false
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: Cpp03
TabWidth: 4
UseTab: Never
...
Comment on lines +180 to +181
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This clang-format configuration appears to be duplicated verbatim in two places (.clang-format and tools/rimage/.clang-format). Keeping identical configs in multiple locations is likely to diverge over time and create inconsistent formatting depending on where clang-format is invoked from. Consider consolidating to a single source of truth (preferably the repo root), or add an explicit comment/guardrail explaining why tools/rimage/ needs its own copy and how it should be kept in sync.

Copilot uses AI. Check for mistakes.
4 changes: 2 additions & 2 deletions .github/workflows/SPDX-README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Read this section if there are some SPDX warnings above.
Pleasing checkpatch is hard when adding new files because:
Adding correct SPDX headers to new files can be tricky because:

- checkpatch requests a different SPDX style for .c versus .h files.
- a different SPDX style is expected for .c versus .h files.
This is because some .h files are included in linker scripts or
assembly code.
- Some SOF reviewers reject C99 comments starting with //
Expand Down
27 changes: 0 additions & 27 deletions .github/workflows/checkpatch_list.sh
View file
Open in desktop

This file was deleted.

38 changes: 1 addition & 37 deletions .github/workflows/codestyle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,45 +14,9 @@ name: codestyle
# yamllint disable-line rule:truthy
on: [pull_request, workflow_call, workflow_dispatch]

jobs:
checkpatch:
runs-on: ubuntu-22.04

strategy:
fail-fast: false
matrix:
strictness: [null, --subjective]

env:
PR_NUM: ${{github.event.number}}
# TODO: reduce duplication with scripts/sof-*-commit-hook.sh
# thanks to either some .conf file or some wrapper script
CHK_CMD_OPTS: --ignore UNKNOWN_COMMIT_ID --codespell

steps:
# depth 2 so:
# ^1. we can show the Subject of the current target branch tip
# ^2. we reconnect/graft to the later fetch pull/1234/head,
- uses: actions/checkout@v4
with: {fetch-depth: 2}

- name: install codespell
run: sudo apt-get -y install codespell && dpkg -L codespell | grep dict

# See shallowness issue https://github.com/thesofproject/linux/issues/2556
- name: fetch PR commits
run: |
.github/workflows/shallowfetchPRcommits.sh \
${GITHUB_REPOSITORY} "$PR_NUM"
# show what we got
git --no-pager log --oneline --graph --decorate --max-count=50

- name: checkpatch
env:
STRICTNESS: ${{ matrix.strictness }}
run: .github/workflows/checkpatch_list.sh ${CHK_CMD_OPTS}
${STRICTNESS} < PR_SHAs.txt

jobs:
yamllint:
runs-on: ubuntu-latest
Comment on lines 15 to 21
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

With the checkpatch CI job removed (and the checkpatch-based git hooks removed elsewhere in this PR), contributors lose the previously documented/implicit way to validate style locally and in CI. Add or update developer documentation to describe the new expected workflow (how to run clang-format, which paths it applies to, and any recommended editor/hook integration), ideally linking to the new .clang-format setup.

Copilot uses AI. Check for mistakes.
steps:
Comment on lines 14 to 22
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The PR description says checkpatch is removed and clang-format is used instead, but the workflow change removes the checkpatch job without adding any clang-format enforcement. This means CI no longer checks C/C++ formatting at all (only yamllint remains). Add a new GitHub Actions job that runs clang-format in verify mode (e.g., formatting check / diff check) using the committed .clang-format so the intended replacement is actually applied in CI.

Copilot uses AI. Check for mistakes.
Expand Down
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ tools/test/audio/reports/*
tools/test/audio/zeros_in.raw

__pycache__
.checkpatch-camelcase.git.*

build*/

Expand Down
Loading
Loading